Wolfgang Müller
2019-May-04 17:31 UTC
[PATCH] configure.ac: Add mandoc as valid formatter
Hi, On systems that have mandoc installed but are missing an nroff binary, the configure script will fall back to pre-formatted manual pages despite the fact that mandoc could be used. The proposed patch adds mandoc as a valid formatter to configure.ac. As mandoc supports the -mdoc flag, it can simply be added to the list of nroff-like binaries. Wolfgang -------------- next part -------------- A non-text attachment was scrubbed... Name: mandoc-configure-ac.patch Type: text/x-diff Size: 504 bytes Desc: not available URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20190504/63cb9d3f/attachment.bin>
Hi, Wolfgang Mueller wrote on Sat, May 04, 2019 at 07:31:23PM +0200:> On systems that have mandoc installed but are missing an nroff binary, > the configure script will fall back to pre-formatted manual pages > despite the fact that mandoc could be used. > > The proposed patch adds mandoc as a valid formatter to configure.ac. As > mandoc supports the -mdoc flag, it can simply be added to the list of > nroff-like binaries.Thanks for reporting. I agree very much with the principle. Strange no one from Void Linux or Alpine Linux reported this issue before...> diff --git a/configure.ac b/configure.ac > index 9022ee9c..813b7fa2 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -4627,7 +4627,7 @@ AC_ARG_WITH([mantype], > ) > if test -z "$MANTYPE"; then > TestPath="/usr/bin${PATH_SEPARATOR}/usr/ucb" > - AC_PATH_PROGS([NROFF], [nroff awf], [/bin/false], [$TestPath]) > + AC_PATH_PROGS([NROFF], [mandoc nroff awf], [/bin/false], [$TestPath]) > if ${NROFF} -mdoc ${srcdir}/ssh.1 >/dev/null 2>&1; then > MANTYPE=doc > elif ${NROFF} -man ${srcdir}/ssh.1 >/dev/null 2>&1; thenHowever, i'd suggest AC_PATH_PROGS([NROFF], [nroff mandoc awf], ... Otherwise, you get this strange output from ./configure when both are installed, needlessly confusing users: checking for nroff... /usr/local/bin/nroff checking for mandoc... /usr/bin/mandoc [...] checking for mandoc... (cached) /usr/local/bin/nroff The reason for this confusing output is that the first and the last of these lines both use the variable NROFF, so they should also better log "checking for nroff...", hence let's keep nroff first. The final value of the NROFF variable doesn't matter, it isn't used for anything in the build. Now arguably, it's a mess that the NROFF variable is used for two different purposes with two different definitions - the first while figuring out MANFMT which is only used in a maintainer target, the second while figuring out MANTYPE, and that, even though the definitions are different, the second uses the cached value from the first if the first succeeded. But i guess when people the sell their soul to the devil of autohell, the result is always a mess. Someone should clean it up, but that is *very hard* to do because autohell is such incredibly stupid software. I'm certainly not volunteering. For the immediate bugfix, the above seems sufficient. Yours, Ingo
Hi, Ingo Schwarze <schwarze at usta.de> wrote:> Thanks for reporting. I agree very much with the principle. Strange > no one from Void Linux or Alpine Linux reported this issue before...I had a look at Void's and Alpine's build scripts, and they both explicitly set --with-mantype=doc. I'm assuming the issue was known but never reported upstream since it'd imply more work and friction than committing a quick local change. I'm planning to make this fix known to the maintainers of the scripts once it's merged.> However, i'd suggest > > AC_PATH_PROGS([NROFF], [nroff mandoc awf], ... > > Otherwise, you get this strange output from ./configure > when both are installed, needlessly confusing users: > > checking for nroff... /usr/local/bin/nroff > checking for mandoc... /usr/bin/mandoc > [...] > checking for mandoc... (cached) /usr/local/bin/nroff > > The reason for this confusing output is that the first and the last of > these lines both use the variable NROFF, so they should also better > log "checking for nroff...", hence let's keep nroff first. > The final value of the NROFF variable doesn't matter, it isn't used > for anything in the build.Had not spotted this in my tests and wasn't aware of this behaviour at all - I agree that we should keep nroff first in this case. Wolfgang
On Sat, May 04, 2019 at 07:31:23PM +0200, Wolfgang M?ller wrote:> On systems that have mandoc installed but are missing an nroff binary, > the configure script will fall back to pre-formatted manual pages > despite the fact that mandoc could be used. > > The proposed patch adds mandoc as a valid formatter to configure.ac. As > mandoc supports the -mdoc flag, it can simply be added to the list of > nroff-like binaries.Having read this and Ingo's feedback in addition to performing some git archaelogy (and subsequently learning what "awf" is) I have come up with the following which I think should resolve this by removing one of the AC_PATH_PROG checks. Thanks. $ ./configure | egrep -i 'roff|mandoc|manpage format' checking for groff... no checking for nroff... no checking for mandoc... /usr/bin/mandoc Manpage format: doc diff --git a/configure.ac b/configure.ac index 9022ee9c..17a11dee 100644 --- a/configure.ac +++ b/configure.ac @@ -41,11 +41,11 @@ AC_PATH_PROG([TEST_MINUS_S_SH], [ksh]) AC_PATH_PROG([TEST_MINUS_S_SH], [sh]) AC_PATH_PROG([SH], [sh]) AC_PATH_PROG([GROFF], [groff]) -AC_PATH_PROG([NROFF], [nroff]) +AC_PATH_PROG([NROFF], [nroff awf]) AC_PATH_PROG([MANDOC], [mandoc]) AC_SUBST([TEST_SHELL], [sh]) -dnl select manpage formatter +dnl select manpage formatter to be used to build "cat" format pages. if test "x$MANDOC" != "x" ; then MANFMT="$MANDOC" elif test "x$NROFF" != "x" ; then @@ -4626,9 +4626,9 @@ AC_ARG_WITH([mantype], ] ) if test -z "$MANTYPE"; then - TestPath="/usr/bin${PATH_SEPARATOR}/usr/ucb" - AC_PATH_PROGS([NROFF], [nroff awf], [/bin/false], [$TestPath]) - if ${NROFF} -mdoc ${srcdir}/ssh.1 >/dev/null 2>&1; then + if ${MANDOC} ${srcdir}/ssh.1 >/dev/null 2>&1; then + MANTYPE=doc + elif ${NROFF} -mdoc ${srcdir}/ssh.1 >/dev/null 2>&1; then MANTYPE=doc elif ${NROFF} -man ${srcdir}/ssh.1 >/dev/null 2>&1; then MANTYPE=man -- Darren Tucker (dtucker at dtucker.net) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
Hi Darren, Darren Tucker wrote on Fri, May 10, 2019 at 03:03:58PM +1000:> Having read this and Ingo's feedback in addition to performing some git > archaelogy (and subsequently learning what "awf" is)https://manpages.bsd.lv/history.html> I have come up with the following which I think should resolve this > by removing one of the AC_PATH_PROG checks.Your commit looks good and works for me. Besides, it is surprisingly clean. Thanks, Ingo> diff --git a/configure.ac b/configure.ac > index 9022ee9c..17a11dee 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -41,11 +41,11 @@ AC_PATH_PROG([TEST_MINUS_S_SH], [ksh]) > AC_PATH_PROG([TEST_MINUS_S_SH], [sh]) > AC_PATH_PROG([SH], [sh]) > AC_PATH_PROG([GROFF], [groff]) > -AC_PATH_PROG([NROFF], [nroff]) > +AC_PATH_PROG([NROFF], [nroff awf]) > AC_PATH_PROG([MANDOC], [mandoc]) > AC_SUBST([TEST_SHELL], [sh]) > > -dnl select manpage formatter > +dnl select manpage formatter to be used to build "cat" format pages. > if test "x$MANDOC" != "x" ; then > MANFMT="$MANDOC" > elif test "x$NROFF" != "x" ; then > @@ -4626,9 +4626,9 @@ AC_ARG_WITH([mantype], > ] > ) > if test -z "$MANTYPE"; then > - TestPath="/usr/bin${PATH_SEPARATOR}/usr/ucb" > - AC_PATH_PROGS([NROFF], [nroff awf], [/bin/false], [$TestPath]) > - if ${NROFF} -mdoc ${srcdir}/ssh.1 >/dev/null 2>&1; then > + if ${MANDOC} ${srcdir}/ssh.1 >/dev/null 2>&1; then > + MANTYPE=doc > + elif ${NROFF} -mdoc ${srcdir}/ssh.1 >/dev/null 2>&1; then > MANTYPE=doc > elif ${NROFF} -man ${srcdir}/ssh.1 >/dev/null 2>&1; then > MANTYPE=man