From: Tomi Ollila <tomi.ollila@iki.fi>
To: "W. Trevor King" <wking@tremily.us>, notmuch@notmuchmail.org
Subject: Re: [PATCH 5/7] doc: Allow rst2man.py as an alternative to rst2man
In-Reply-To:
 <adce76bb9a0ca728d856da4ecaf6b282e22e7440.1396718720.git.wking@tremily.us>
References: <cover.1396718720.git.wking@tremily.us>
	<adce76bb9a0ca728d856da4ecaf6b282e22e7440.1396718720.git.wking@tremily.us>
User-Agent: Notmuch/0.17+174~gef82849 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Sat, 05 Apr 2014 22:05:31 +0300
Message-ID: <m2a9bzd2bo.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Sat, Apr 05 2014, "W. Trevor King" <wking@tremily.us> wrote:

> Gentoo's dev-python/docutils-0.10 installs Docutils scripts with a
> *.py extension, so I have /usr/bin/rst2man.py and no rst2man script.
> This patch supports users with both types of systems by checking for
> rst2man, falling back on rst2man.py, and giving up only if neither is
> found.  Users can also set the new RST2MAN path variable explicitly
> when they call Make:
>
>   make RST2MAN=/my/custom/rst_to_man_converter build-man
>
> I use POSIX's 'command -v' [1] to find the path to rst2man or
> rst2man.py, and save that as RST2MAN in Makefile.config.  We can use a
> non-empty RST2MAN to check for the availability of an rst2man program,
> so there's no need for a separate HAVE_RST2MAN.  Then pass the
> configured RST2MAN path through to prerst2man.py to use in its system
> call.
>
> [1]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html

This series looks good to me. 

Except the reference to _POSIX_ page. One knows how consistent these
specifications are; alternative:

http://pubs.opengroup.org/onlinepubs/009695399/utilities/command.html

mentions additionally that -v flag
"(On systems supporting the User Portability Utilities option.)" 

Also, we don't give such a treatment to other command either; I'd rather
see RST2MAN=rst2man, RST2MAN=rst2man.py *and* RST2MAN= lines used
instead -- the last to set RST2MAN to empty string instead of being unset.

Tomi


> ---
>  configure          | 12 +++++++-----
>  doc/Makefile.local |  6 +++---
>  doc/prerst2man.py  |  9 +++++----
>  3 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/configure b/configure
> index 1d430b9..81c286b 100755
> --- a/configure
> +++ b/configure
> @@ -400,7 +400,6 @@ printf "Checking if sphinx is available and supports nroff output... "
>  if hash sphinx-build > /dev/null 2>&1 && python -m sphinx.writers.manpage > /dev/null 2>&1 ; then
>      printf "Yes.\n"
>      have_sphinx=1
> -    have_rst2man=0
>  else
>      printf "No (falling back to rst2man).\n"
>      have_sphinx=0
> @@ -408,10 +407,12 @@ else
>      printf "Checking if rst2man is available... "
>      if rst2man -V > /dev/null 2>&1; then
>         printf "Yes.\n"
> -       have_rst2man=1
> +       RST2MAN=$(command -v rst2man)
> +    elif rst2man.py -V > /dev/null 2>&1; then
> +       printf "Yes.\n"
> +       RST2MAN=$(command -v rst2man.py)
>      else
>         printf "No (so will not install man pages).\n"
> -       have_rst2man=0
>      fi
>  fi
>  
> @@ -788,8 +789,9 @@ HAVE_EMACS = ${have_emacs}
>  # Whether there's a sphinx-build binary available for building documentation
>  HAVE_SPHINX=${have_sphinx}
>  
> -# Whether there's a rst2man binary available for building documentation
> -HAVE_RST2MAN=${have_rst2man}
> +# The path to the rst2man program for building documentation.  Set to
> +# an empty string if no such program is available.
> +RST2MAN=${RST2MAN}
>  
>  # The directory to which desktop files should be installed
>  desktop_dir = \$(prefix)/share/applications
> diff --git a/doc/Makefile.local b/doc/Makefile.local
> index 0980c71..e08fc99 100644
> --- a/doc/Makefile.local
> +++ b/doc/Makefile.local
> @@ -42,8 +42,8 @@ ifeq ($(HAVE_SPHINX),1)
>  	    mkdir -p $(DOCBUILDDIR)/man/man$${section}; \
>  	    mv $(DOCBUILDDIR)/man/*.$${section} $(DOCBUILDDIR)/man/man$${section}; \
>  	done
> -else ifeq ($(HAVE_RST2MAN),1)
> -	$(prerst2man) $(srcdir)/doc $(DOCBUILDDIR)/man
> +else ifdef RST2MAN
> +	$(prerst2man) "$(RST2MAN)" $(srcdir)/doc $(DOCBUILDDIR)/man
>  else
>  	@echo "Fatal: build dependency fail."
>  	@false
> @@ -51,7 +51,7 @@ endif
>  
>  # Do not try to build or install man pages if a man page converter is
>  # not available.
> -ifeq ($(HAVE_SPHINX)$(HAVE_RST2MAN),00)
> +ifeq ($(HAVE_SPHINX)$(RST2MAN),0)
>  build-man:
>  install-man:
>  	@echo "No sphinx or rst2man, will not install man pages."
> diff --git a/doc/prerst2man.py b/doc/prerst2man.py
> index 437dea9..81ce817 100644
> --- a/doc/prerst2man.py
> +++ b/doc/prerst2man.py
> @@ -4,8 +4,9 @@ from os.path import dirname, isdir
>  from os import makedirs, system
>  import re
>  
> -sourcedir = argv[1]
> -outdir = argv[2]
> +rst2man = argv[1]
> +sourcedir = argv[2]
> +outdir = argv[3]
>  
>  if not isdir(outdir):
>      makedirs(outdir, 0o755)
> @@ -59,5 +60,5 @@ for page in man_pages:
>      outfile.write("".join(lines))
>      outfile.close()
>  
> -    system('set -x; rst2man {0} {1}/{2}.{3}'
> -           .format(filename, outdir, page[0], page[4]))
> +    system('set -x; {0} {1} {2}/{3}.{4}'
> +           .format(rst2man, filename, outdir, page[0], page[4]))
> -- 
> 1.9.1.353.gc66d89d
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch