Re: Implementing notrans_man_MANS

[Ralf Wildenhues]

Hello Peter,

* Peter Breitenlohner wrote on Wed, Feb 20, 2008 at 02:20:04PM CET:
> 
> attached are five patches (the diffs are actually against the result of
> applying your patch for PR 516 from 2008-01-22):

Thanks for your work in this, and sorry for the delay.  I saw that your
paperwork is now complete, thanks!

> 2. patch-03-notrans_MANS-impl:
> 
> The actual implementation of `notrans_'. The patch looks awful but the
> logic in the resulting patched file should be reasonably clear.

Indeed.  ;-)

> At the same time the dependencies for `install-man%SECTION%' have been
> revised. All make variables not used in Makefile.am are omitted from the
> install/uninstall rules. All those used in the rules also appear as
> dependencies.
> 
> One consequence: "make dist_man_MANS='bar.1 baz.5' install-man" will only
> work well if `dist_man_MANS' is used in Makefile.am and Automake has
> generated the relevant (!notrans_) part of the install-man1 and
> install-man5 rules. So ... ?

Yeah, that kind of thing is to be expected from automake-active
variables.  Not new in this patch.

> Please feel free to revise the wording in texinfo, comments, and/or
> ChangeLog.

Well, here's a review of your patches.  If you have time to address the
comments, that would be great, otherwise I will eventually do it.

> diff -ur -N -x 'automake.info*' -x version.texi -x stamp-vti 
> automake-1.10.1.orig/automake.in automake-1.10.1/automake.in
> --- automake-1.10.1.orig/automake.in  2008-02-19 18:07:41.000000000 +0100
> +++ automake-1.10.1/automake.in       2008-02-20 01:52:26.000000000 +0100
> @@ -3363,27 +3363,38 @@
>    # Find all the sections in use.  We do this by first looking for
>    # "standard" sections, and then looking for any additional
>    # sections used in man_MANS.
> -  my (%sections, %vlist);
> +  my (%sections, %nsections, %tsections, %nvlist, %tvlist, %nsvlist, 
> %tsvlist);

Variable naming seems, uhm, a bit hard to read, starting from ugly names
and putting more on top.  ;-)

>    # We handle nodist_ for uniformity.  man pages aren't distributed
>    # by default so it isn't actually very important.
> +  foreach my $npfx ('', 'notrans_')
> +    {
>        foreach my $pfx ('', 'dist_', 'nodist_')
>       {
>         # Add more sections as needed.
>         foreach my $section ('0'..'9', 'n', 'l')

This triple loop is already at 72 iterations.  Not good for performance.
If this grows further, we may have to access variables in a different
manner here.

>           {
> -           my $varname = $pfx . 'man' . $section . '_MANS';
> +           my $varname = $npfx . $pfx . 'man' . $section . '_MANS';
[...]

> --- automake-1.10.1.orig/lib/am/mans.am       2008-02-20 09:38:10.000000000 
> +0100
> +++ automake-1.10.1/lib/am/mans.am    2008-02-20 00:50:53.000000000 +0100
> @@ -16,6 +16,20 @@
>  ## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>  ## 02110-1301, USA.
>  
> +## Parameters:
> +##   SECTION string  man section
> +##   DEPS    string  dependencies
> +##
> +##   NOTR    bool    have notrans_ MANS
> +##   NOTRX   string  notrans_ man%SECTION%_MANS (if any)
> +##   NOTRY   bool    have notrans_ man_MANS
> +##   NOTRZ   string  notrans_ man_MANS
> +##
> +##   DOTR    bool    same
> +##   DOTRX   string    as above
> +##   DOTRY   bool        without
> +##   DOTRZ   string        notrans_

What is the rationale for NOTRY, do the X, Y, Z suffixes have any
meaning?  I understand NO and DO prefixes.  So what about using
   NOTR_SECT_MANS     instead of  NOTRX
   NOTR_NOSECT_MANS   instead of  NOTRZ
consequently
   NOTR_NS            instead of  NOTRY

and likewise DOTR*?  For NOTR and DOTR see below.

>  man%SECTION%dir = $(mandir)/man%SECTION%
>  
>  ## ------------ ##
> @@ -29,37 +43,65 @@
>  ?INSTALL-MAN?install-data-am: install-man
>  ?INSTALL-MAN?am__installdirs += "$(DESTDIR)$(man%SECTION%dir)"
>  .PHONY install-man: install-man%SECTION%
> -install-man%SECTION%: $(man%SECTION%_MANS) $(man_MANS)
> +install-man%SECTION%: %DEPS%
>       @$(NORMAL_INSTALL)
>       test -z "$(man%SECTION%dir)" || $(MKDIR_P) 
> "$(DESTDIR)$(man%SECTION%dir)"
> -     @list='$(man%SECTION%_MANS) $(dist_man%SECTION%_MANS) 
> $(nodist_man%SECTION%_MANS)'; \
> +## Handle MANS with notrans_ prefix
> +?NOTR?       @list='%NOTRX%'; \
> +## Extract all items from notrans_man_MANS that should go in this section.
> +## This must be done dynamically to support conditionals.
> +?NOTRY?      l2='%NOTRZ%'; \
> +?NOTRY?      for i in $$l2; do \
> +?NOTRY?        case "$$i" in \
> +## Have to accept files like `foo.1c'.
> +?NOTRY?          *.%SECTION%*) list="$$list $$i" ;; \
> +?NOTRY?        esac; \
> +?NOTRY?      done; \
> +?NOTR?       for i in $$list; do \
> +## Find the file.
> +?NOTR?         if test -f $$i; then file=$$i; \
> +?NOTR?         else file=$(srcdir)/$$i; fi; \
> +## Change the extension if needed.
> +?NOTR?         ext=`echo $$i | sed -e 's/^.*\\.//'`; \
> +?NOTR?         case "$$ext" in \
> +?NOTR?           %SECTION%*) ;; \
> +?NOTR?           *) ext='%SECTION%' ;; \
> +?NOTR?         esac; \
> +## Extract basename of man page and append extension.
> +?NOTR?         inst=`echo $$i | sed -e 's/\\.[0-9a-z]*$$//'`; \
> +?NOTR?         inst=`echo $$inst | sed -e 's/^.*\///'`.$$ext; \
> +?NOTR?         echo " $(INSTALL_DATA) '$$file' 
> '$(DESTDIR)$(man%SECTION%dir)/$$inst'"; \
> +?NOTR?         $(INSTALL_DATA) "$$file" 
> "$(DESTDIR)$(man%SECTION%dir)/$$inst"; \
> +?NOTR?       done

Note to self: should rewrite the above when applying the multi-file
install.

Also, the whole thing would probably be a lot more readable if the
?NOTR?/?DOTR? prefixes vanished and were replaced by full rules, wrapped
in `if %?NOTRANS_MANS%'.  That would be a better name for NOTR, and
since it would then only appear a couple of times, it could be longer
without pain, too.

[...]
> diff -ur -N automake-1.10.1.orig/doc/automake.texi 
> automake-1.10.1/doc/automake.texi
> --- automake-1.10.1.orig/doc/automake.texi    2008-01-21 23:41:02.000000000 
> +0100
> +++ automake-1.10.1/doc/automake.texi 2008-02-19 20:52:29.000000000 +0100
> @@ -1962,8 +1962,9 @@
>  
>  Some primaries also allow additional prefixes that control other
>  aspects of @command{automake}'s behavior.  The currently defined prefixes
> -are @samp{dist_}, @samp{nodist_}, and @samp{nobase_}.  These prefixes
> -are explained later (@pxref{Program and Library Variables}).
> +are @samp{dist_}, @samp{nodist_}, @samp{nobase_}, and @samp{notrans_}.
> +These prefixes are explained later (@pxref{Program and Library Variables})
> +(@pxref{Man pages}).
>  
>  
>  @node Canonicalization
> @@ -7633,6 +7634,34 @@
>  The @code{nobase_} prefix is meaningless for man pages and is
>  disallowed.
>  
> address@hidden notrans_
> address@hidden @code{notrans_} prefix
> address@hidden Man page renaming, avoiding
> address@hidden Avoiding man page renaming
> +
> +The GNU Build System provides means to automatically rename executables
> +before they are installed.

This is duplication from the Renaming node.  I'd prefer something like
  Executables and manpages may be renamed upon installation
  (@pxref{Renaming}).

and then continue with

> [T]his can be avoided by use
> +of the @code{notrans_} prefix.  For instance here is how to install two man
> +pages for an executable @samp{foo} (to be renamed) allowing to access a
> +library function @samp{foo()} (not to be renamed) from the command line.

and the Renaming node should be fixed to state:
  The GNU Build System provides means to automatically rename
  executables and manpages before they are installed.

and cross-ref back.

> address@hidden must be specified first when used in conjunction with
> +either @samp{dist_} or @samp{nodist_} (@pxref{Dist}).  For instance:

Can this limitation be lifted (without making the code much slower)?

> --- automake-1.10.1.orig/tests/man4.test      1970-01-01 01:00:00.000000000 
> +0100
> +++ automake-1.10.1/tests/man4.test   2008-02-20 10:07:12.000000000 +0100
[...]
> +
> +# Check that dist_man1_MANS appears as dependency for install-man1.

This comment does not match the code below.

> +
> +. ./defs || exit 1
> +
> +set -e
> +
> +cat > Makefile.am << 'END'
> +dist_man_MANS = foo.1
> +END
> +
> +cat >>configure.in <<'END'
> +AC_OUTPUT
> +END
> +
> +: > foo.1
> +
> +$ACLOCAL
> +$AUTOMAKE
> +
> +grep '^install-man1:' Makefile.in | grep '\$(dist_man_MANS)'
> diff -ur -N -x 'automake.info*' -x version.texi -x stamp-vti 
> automake-1.10.1.orig/tests/notrans.test automake-1.10.1/tests/notrans.test
> --- automake-1.10.1.orig/tests/notrans.test   1970-01-01 01:00:00.000000000 
> +0100
> +++ automake-1.10.1/tests/notrans.test        2008-02-20 11:25:40.000000000 
> +0100
> @@ -0,0 +1,53 @@
[...]
> +
> +# Check notrans_ prefix for MANS primary.
> +
> +. ./defs || exit 1
> +
> +set -e
> +
> +cat >>configure.in <<'END'
> +AC_OUTPUT
> +END
> +
> +cat > Makefile.am << 'EOF'
> +dist_man_MANS = foo.1
> +notrans_man_MANS = foo.5

Let's have these as well:
  man3_MANS = bar.man
  notrans_man3_MANS = baz.man

(including testing for them further below), and also the
combinations of notrans_ with dist_ and nodist_.  Ideally,
we want to exercise every code path in mans.am and automake.in.

> +
> +test-install: install
> +     test -f inst/man/man1/gnu-foo.1
> +     test -f inst/man/man5/foo.5
> +EOF
> +
> +: > foo.1
> +: > foo.5
> +
> +$ACLOCAL
> +$AUTOCONF
> +$AUTOMAKE
> +
> +./configure --program-prefix=gnu- --prefix `pwd`/inst --mandir `pwd`/inst/man

Please quote instances of `pwd' for the master testsuite.

> +$MAKE
> +$MAKE test-install
> +$MAKE uninstall
> +test `find inst/man -type f -print | wc -l` = 0
> +
> +# Opportunistically test for installdirs.
> +rm -rf inst
> +$MAKE installdirs
> +test -d inst/man/man1
> +test -d inst/man/man5

> 2008-02-20  Peter Breitenlohner  <address@hidden>
> 
>       * automake.in (handle_man_pages), lib/am/mans.am: Implement
>       notrans_ prefix for MANS primary and rework dependencies for
>       install-man%SECTION%; use only vars defined in Makefile.am.
>       * tests/man4.test: New test (install-man%SECTION% dependencies).
>       * tests/notrans.test: New test (notrans_ prefix).
>       * tests/Makefile.am: Update.
>       * doc/automake.texi: Document notrans_ prefix.