Re: automake/535: Extract correct man section from files in MAN_MANS

[Ralf Wildenhues]

Hello Peter,

thanks for the bug reports and patches, and sorry for the delay.

* Peter Breitenlohner wrote on Mon, Feb 02, 2009 at 04:24:30PM CET:
> recently I have reported two automake problems #533 and #535 (#534
> duplicates #533, my mistake) with patch and test case.
>
> They have been assigned to Alexandre Duret-Lutz <...>, but according
> to the git log his last activity was in June 2006.  Maybe you can have a
> look.

Yes; don't bother with the default assignment of GNATS entries.  I read
them.  I do prefer bug reports to the mailing list a wee bit, but don't
mind the GNATS enough to do something about it.

> 533: applies equally to git master and branch 1.10, but may need some
> refinement.

Yes, that one is rather tricky.  I hope to address it separately.

> 535: applies to git master, but an analogous change ought to be applied to
> branch 1.10.  I promised a second patch for that but couldn't convince
> automake-gnats to accept another attachment.  Thus I attach it here.

Thanks.

* Peter Breitenlohner wrote on Mon, Feb 02, 2009 at 03:54:04PM CET:
> >Synopsis:       Extract correct man section from files in MAN_MANS

> man_MANS = foo-1.4.5/foo.2 foo-1.4.5/bar.4 baz-1.4.2
> 
> Automake correctly installs man2/foo.2, man4/bar.4, and man2/baz-1.4.2
> (the latter notation occasionally used for a section 2 page related to
> a program baz-1.4) but in addition also - wrongly - man4/foo4 and
> man4/baz-1.4.4

Ugh.

> Attached is a small patch (current git) correcting this problem.

> @@ -39,8 +39,8 @@ if %?NOTRANS_MANS%
>  ## Extract all items from notrans_man_MANS that should go in this section.
>  ## This must be done dynamically to support conditionals.
>  ?HAVE_NOTRANS? l2='%NOTRANS_LIST%'; for i in $$l2; do \
> -## Accept files like `foo.1c'.
> -?HAVE_NOTRANS?   case $$i in *.%SECTION%*) echo "$$i";; esac; \
> +## Accept for `man1' files like `foo.1c' but not `sub.1/foo.2' or 
> `foo-2.1.4'.
> +?HAVE_NOTRANS?   case `echo $$i | sed -e 's,.*\.,,'` in %SECTION%*) echo 
> "$$i";; esac; \
>  ?HAVE_NOTRANS? done; \
>  ## Extract basename of manpage, change the extension if needed.
>         } | while read p; do \

This has the disadvantage that we're now again forking at least twice
per installed file.  The point of the install rule changes in master
was to make them faster!  :-)

I don't know a good way to fix this without adding any forks at all, but
the patch below fixes the issue for master with adding two forks per set
of man pages installed.  It uses the same regex for matching the section
as is used in automake.in.

> +# Let's play with $DESTDIR too, it shouldn't hurt.
> +./configure --prefix='' --mandir=/man

Let's just omit --prefix here, rather than passing an empty one.
Also, let's add a 'make uninstall' as well as the notrans man pages,
so all changed code paths are exercised; and rename a bit, so we can
better test that no wrong directories are created.

Lemme see if I like your proposed patch for branch-1-10 well enough;
since there is a simple enough workaround by using the man<X>_MANS
variables with suitable <X>, I'm yet not sure we need this in 1.10.3.

Cheers, and thanks again,
Ralf

2009-03-07  Peter Breitenlohner  <address@hidden>
            Ralf Wildenhues  <address@hidden>

        Extract correct man section from files in man_MANS.
        * lib/am/mans.am: Extract correct man section in the presence of
        multiple dots in the file base name and/or directory components.
        * tests/man5.test: New test case.
        * tests/Makefile.am: Update.

iff --git a/lib/am/mans.am b/lib/am/mans.am
index 803a892..d389145 100644
--- a/lib/am/mans.am
+++ b/lib/am/mans.am
@@ -1,5 +1,5 @@
 ## automake - create Makefile.in from Makefile.am
-## Copyright (C) 1998, 2001, 2003, 2004, 2006, 2008 Free Software
+## Copyright (C) 1998, 2001, 2003, 2004, 2006, 2008, 2009 Free Software
 ## Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
@@ -38,10 +38,9 @@ if %?NOTRANS_MANS%
        { for i in $$list; do echo "$$i"; done;  \
 ## Extract all items from notrans_man_MANS that should go in this section.
 ## This must be done dynamically to support conditionals.
-?HAVE_NOTRANS? l2='%NOTRANS_LIST%'; for i in $$l2; do \
-## Accept files like `foo.1c'.
-?HAVE_NOTRANS?   case $$i in *.%SECTION%*) echo "$$i";; esac; \
-?HAVE_NOTRANS? done; \
+?HAVE_NOTRANS? l2='%NOTRANS_LIST%'; for i in $$l2; do echo "$$i"; done | \
+## Accept for `man1' files like `foo.1c' but not `sub.1/foo.2' or `foo-2.1.4'.
+?HAVE_NOTRANS?   sed -n '/\.%SECTION%[a-z]*$$/p'; \
 ## Extract basename of manpage, change the extension if needed.
        } | while read p; do \
 ## Find the file.
@@ -71,10 +70,9 @@ if %?TRANS_MANS%
        { for i in $$list; do echo "$$i"; done; \
 ## Extract all items from notrans_man_MANS that should go in this section.
 ## This must be done dynamically to support conditionals.
-?HAVE_TRANS?   l2='%TRANS_LIST%'; for i in $$l2; do \
-## Accept files like `foo.1c'.
-?HAVE_TRANS?     case $$i in *.%SECTION%*) echo "$$i";; esac; \
-?HAVE_TRANS?   done; \
+?HAVE_TRANS?   l2='%TRANS_LIST%'; for i in $$l2; do echo "$$i"; done | \
+## Accept for `man1' files like `foo.1c' but not `sub.1/foo.2' or `foo-2.1.4'.
+?HAVE_TRANS?     sed -n '/\.%SECTION%[a-z]*$$/p'; \
 ## Extract basename of manpage, change the extension if needed.
        } | while read p; do \
 ## Find the file.
@@ -116,10 +114,9 @@ if %?NOTRANS_MANS%
        files=`{ for i in $$list; do echo "$$i"; done; \
 ## Extract all items from notrans_man_MANS that should go in this section.
 ## This must be done dynamically to support conditionals.
-?HAVE_NOTRANS? l2='%NOTRANS_LIST%'; for i in $$l2; do \
-## Accept files like `foo.1c'.
-?HAVE_NOTRANS?   case $$i in *.%SECTION%*) echo "$$i";; esac; \
-?HAVE_NOTRANS? done; \
+?HAVE_NOTRANS? l2='%NOTRANS_LIST%'; for i in $$l2; do echo "$$i"; done | \
+## Accept for `man1' files like `foo.1c' but not `sub.1/foo.2' or `foo-2.1.4'.
+?HAVE_NOTRANS?   sed -n '/\.%SECTION%[a-z]*$$/p'; \
 ## Extract basename of manpage, change the extension if needed.
        } | sed 's,.*/,,;s,\.[^%SECTION%][0-9a-z]*$$,.%SECTION%,'`; \
        test -n "$$files" || exit 0; \
@@ -132,10 +129,9 @@ if %?TRANS_MANS%
        files=`{ for i in $$list; do echo "$$i"; done; \
 ## Extract all items from man_MANS that should go in this section.
 ## This must be done dynamically to support conditionals.
-?HAVE_TRANS?   l2='%TRANS_LIST%'; for i in $$l2; do \
-## Accept files like `foo.1c'.
-?HAVE_TRANS?     case $$i in *.%SECTION%*) echo "$$i";; esac; \
-?HAVE_TRANS?   done; \
+?HAVE_TRANS?   l2='%TRANS_LIST%'; for i in $$l2; do echo "$$i"; done | \
+## Accept for `man1' files like `foo.1c' but not `sub.1/foo.2' or `foo-2.1.4'.
+?HAVE_TRANS?     sed -n '/\.%SECTION%[a-z]*$$/p'; \
 ## Extract basename of manpage, run it through the program rename
 ## transform, and change the extension if needed.
        } | sed -e 's,.*/,,;h;s,.*\.,,;s,^[^%SECTION%][0-9a-z]*$$,%SECTION%,;x' 
\
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2d8a974..d4687f4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -398,6 +398,7 @@ man.test \
 man2.test \
 man3.test \
 man4.test \
+man5.test \
 mclean.test \
 mdate.test \
 mdate2.test \
diff --git a/tests/man5.test b/tests/man5.test
new file mode 100755
index 0000000..626602a
--- /dev/null
+++ b/tests/man5.test
@@ -0,0 +1,67 @@
+#! /bin/sh
+# Copyright (C) 2009  Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make sure to extract the correct mansection from files in man_MANS.
+
+. ./defs || Exit 1
+
+set -e
+
+echo AC_OUTPUT >> configure.in
+
+cat > Makefile.am << 'END'
+man_MANS = foo-1.4.5/foo.2 foo-1.4.5/bar.3 baz-1.4.2
+notrans_man_MANS = foo-1.4.5/nfoo.2 foo-1.4.5/nbar.3 nbaz-1.4.2
+END
+
+mkdir foo-1.4.5
+
+: > foo-1.4.5/foo.2
+: > foo-1.4.5/nfoo.2
+: > foo-1.4.5/bar.3
+: > foo-1.4.5/nbar.3
+: > baz-1.4.2
+: > nbaz-1.4.2
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+# Let's play with $DESTDIR too, it shouldn't hurt.
+./configure --mandir=/man
+DESTDIR="`pwd`/_inst" $MAKE -e install
+
+test -f ./_inst/man/man2/foo.2
+test -f ./_inst/man/man2/nfoo.2
+test -f ./_inst/man/man2/baz-1.4.2
+test -f ./_inst/man/man2/nbaz-1.4.2
+test -f ./_inst/man/man3/bar.3
+test -f ./_inst/man/man3/nbar.3
+
+test ! -d ./_inst/man/man1
+test ! -d ./_inst/man/man4
+test ! -d ./_inst/man/man5
+
+DESTDIR="`pwd`/_inst" $MAKE -e uninstall
+
+test ! -f ./_inst/man/man2/foo.2
+test ! -f ./_inst/man/man2/nfoo.2
+test ! -f ./_inst/man/man2/baz-1.4.2
+test ! -f ./_inst/man/man2/nbaz-1.4.2
+test ! -f ./_inst/man/man3/bar.3
+test ! -f ./_inst/man/man3/nbar.3
+
+: