Re: bug#9773: "make install-info" with DESTDIR writes unwanted usr/share/info/dir.gz

[Stefano Lattarini]

On Sunday 23 October 2011, Jonathan Nieder wrote:
> Stefano Lattarini wrote:
> > On Monday 17 October 2011, Jonathan Nieder wrote:
> 
> >> +++ b/ChangeLog
> >> @@ -1,3 +1,16 @@
> >> +2011-10-17  Jonathan Nieder  <address@hidden>
> >> +
> >> +  For Debian Bug#543992:
> >> +  * lib/am/texinfos.am (install-info-am): When deciding whether to
> >> +  add to an info/dir file, instead of checking for Debian
> >> +  install-info, check if DESTDIR is nonempty.
> [...]
> > I must say that I don't like this very much.  A better fix IMHO would
> > be to have an environment variable to inhibit the creation/updating of
> > the `${infodir}/dir' file
> 
> Yes, that sounds sensible.  It would be better than checking for
> Debian install-info (because that was insane) and better than the
> current state (because it gives packagers on all distros an easy way
> to request inhibiting creation of the ${infodir}/dir file).
> 
> Keying behavior on $(DESTDIR) left an unpleasant taste in my mouth,
> too.  Thanks for your thoughtfulness.
> 
Glad we agree :-)

Here is my proposed patch (for maint).  I will push in 72 hours if there
is no objection or review by then.

Thanks,
  Stefano

From 8b0bb8bbda8ac1e859c577cba1549ffbc0ae4d52 Mon Sep 17 00:00:00 2001
Message-Id: <address@hidden>
From: Stefano Lattarini <address@hidden>
Date: Sun, 23 Oct 2011 13:19:21 +0200
Subject: [PATCH] info: allow user to inhibit creation/update of '${infodir}/dir'

With this change, we allow the user to request the install-info
rules not to update the `${infodir}/dir' file, by exporting the
environment variable `AM_UPDATE_INFO_DIR' to the value "no".
This is especially useful to distro packagers, and is a definite
improvement over our previous hack of looking whether the
`install-info' program was the Debian or GNU version -- hack
which had been silently broken with recent versions of debian
install-info BTW (probably since dpkg 1.15.4, 2009-09-06).
This change fixes automake bug#9773.  See also Debian Bug#543992.

* lib/am/texinfos.am: Don't look anymore at the output of
`install-info --version' to decide whether to use it to update
the `${infodir}/dir' or not; instead, honour the environment
variable `AM_UPDATE_INFO_DIR'.
* tests/install-info-dir.test: New test.
* tests/Makefile.am (TESTS): Add it.
* tests/defs: Also unset `AM_UPDATE_INFO_DIR', to avoid unwanted
interferences from the environment.
* doc/automake.texi (Texinfo): Update.
* NEWS: Likewise.
* THANKS: Likewise.
Report by Jonathan Nieder.
---
 ChangeLog                   |   25 ++++++++
 NEWS                        |    8 +++
 THANKS                      |    1 +
 doc/automake.texi           |    7 ++
 lib/am/texinfos.am          |   22 +++----
 tests/Makefile.am           |    1 +
 tests/Makefile.in           |    1 +
 tests/install-info-dir.test |  142 +++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 195 insertions(+), 12 deletions(-)
 create mode 100755 tests/install-info-dir.test

diff --git a/ChangeLog b/ChangeLog
index 0836c77..44d77f8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,28 @@
+2011-10-23  Stefano Lattarini  <address@hidden>
+
+       info: allow user to inhibit creation/update of '${infodir}/dir'
+       With this change, we allow the user to request the install-info
+       rules not to update the `${infodir}/dir' file, by exporting the
+       environment variable `AM_UPDATE_INFO_DIR' to the value "no".
+       This is especially useful to distro packagers, and is a definite
+       improvement over our previous hack of looking whether the
+       `install-info' program was the Debian or GNU version -- hack
+       which had been silently broken with recent versions of debian
+       install-info BTW (probably since dpkg 1.15.4, 2009-09-06).
+       This change fixes automake bug#9773.  See also Debian Bug#543992.
+       * lib/am/texinfos.am: Don't look anymore at the output of
+       `install-info --version' to decide whether to use it to update
+       the `${infodir}/dir' or not; instead, honour the environment
+       variable `AM_UPDATE_INFO_DIR'.
+       * tests/install-info-dir.test: New test.
+       * tests/Makefile.am (TESTS): Add it.
+       * tests/defs: Also unset `AM_UPDATE_INFO_DIR', to avoid unwanted
+       interferences from the environment.
+       * doc/automake.texi (Texinfo): Update.
+       * NEWS: Likewise.
+       * THANKS: Likewise.
+       Report by Jonathan Nieder.
+
 2011-10-19  Stefano Lattarini  <address@hidden>
 
        dejagnu: allow the package developer to extend site.exp
diff --git a/NEWS b/NEWS
index 92c7735..fe1dc97 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,10 @@ New in 1.11.0a:
     file generated by automake-provided rules by defining the special make
     variable `$(EXTRA_DEJAGNU_SITE_CONFIG)'.
 
+  - The `install-info' rule can now be instructed not to create/update
+    the `${infodir}/dir' file, by exporting the new environment variable
+    `AM_UPDATE_INFO_DIR' to the value "no".
+
 Bugs fixed in 1.11.0a:
 
 * Bugs introduced by 1.11:
@@ -127,6 +131,10 @@ Bugs fixed in 1.11.0a:
   - Now aclocal and automake, when they've to spawn autoconf or autom4te
     processes, honour the configure-time definitions of AUTOCONF and
     AUTOM4TE.
+
+  - The `install-info' recipe does not try anymore to guess whether the
+    `install-info' program is from Debian or from GNU, and adaptively
+    change its behaviour; this has proven to be frail and easy to regress.
 
 New in 1.11:
 
diff --git a/THANKS b/THANKS
index eae683f..7fa37c2 100644
--- a/THANKS
+++ b/THANKS
@@ -171,6 +171,7 @@ John Pierce         address@hidden
 John Ratliff           address@hidden
 John R. Cary           address@hidden
 John W. Coomes         address@hidden
+Jonathan Nieder                address@hidden
 Josh MacDonald         address@hidden
 Joshua Cowan           address@hidden
 js pendry              address@hidden
diff --git a/doc/automake.texi b/doc/automake.texi
index eac380a..f6039bb 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -7963,6 +7963,13 @@ be prevented via the @code{no-installinfo} option.  In 
this case,
 @file{.info} files are not installed by default, and user must
 request this explicitly using @samp{make install-info}.
 
address@hidden AM_UPDATE_INFO_DIR
+By default, @code{make install-info} will try to run the
address@hidden program (if available) to update (or create)
+the @address@hidden@address@hidden/dir} index.  If this is undesired, it
+can be prevented by exporting the @code{AM_UPDATE_INFO_DIR} variable
+to "@code{no}".
+
 The following variables are used by the Texinfo build rules.
 
 @vtable @code
diff --git a/lib/am/texinfos.am b/lib/am/texinfos.am
index c9dcd9d..0279bf3 100644
--- a/lib/am/texinfos.am
+++ b/lib/am/texinfos.am
@@ -208,18 +208,16 @@ install-info-am: $(INFO_DEPS)
          echo " $(INSTALL_DATA) $$files '$(DESTDIR)$(infodir)'"; \
          $(INSTALL_DATA) $$files "$(DESTDIR)$(infodir)" || exit $$?; done
        @$(POST_INSTALL)
-## Only run this code if install-info actually exists, and it is not
-## the Debian install-info.  FIXME: once Debian install-info goes
-## away, we can remove this hack.  Some versions of Debian install-info
-## print their version on stderr (e.g. 1.8.3), other do it in
-## on stdout (e.g. 1.10.15).
-##
-## Do not use
-##    install-info --version 2>&1 | sed 1q | grep -v -i debian
-## as if install-info does not exist, grep -v will be happy, and
-## therefore the code will be triggered although install-info is missing.
-       @if (install-info --version && \
-            install-info --version 2>&1 | sed 1q | grep -i -v debian) 
>/dev/null 2>&1; then \
+## Only run this code if install-info actually exists, and if the user
+## don't request it not to be run (through the `AM_UPDATE_INFO_DIR'
+## environment variable).  See automake bug#9773 and Debian Bug#543992.
+       @am__run_installinfo=yes; \
+       case $$AM_UPDATE_INFO_DIR in \
+         n|no|NO) am__run_installinfo=no;; \
+         *) (install-info --version) >/dev/null 2>&1 \
+              || am__run_installinfo=no;; \
+       esac; \
+       if test $$am__run_installinfo = yes; then \
          list='$(INFO_DEPS)'; test -n "$(infodir)" || list=; \
          for file in $$list; do \
 ## Strip directory
diff --git a/tests/Makefile.am b/tests/Makefile.am
index efb4fea..54519b2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -437,6 +437,7 @@ init2.test \
 insh2.test \
 install2.test \
 installdir.test \
+install-info-dir.test \
 instsh.test \
 instsh2.test \
 instsh3.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index f12b81d..ea7c724 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -721,6 +721,7 @@ init2.test \
 insh2.test \
 install2.test \
 installdir.test \
+install-info-dir.test \
 instsh.test \
 instsh2.test \
 instsh3.test \
diff --git a/tests/install-info-dir.test b/tests/install-info-dir.test
new file mode 100755
index 0000000..b10a5ad
--- /dev/null
+++ b/tests/install-info-dir.test
@@ -0,0 +1,142 @@
+#! /bin/sh
+# Copyright (C) 2011 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 2, 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/>.
+
+# Checks on the `install-info' target.
+# Details about the individual checks' purposes and motivations are
+# inlined, below.
+
+# FIXME: this test is a good candidate for a conversion top TAP.
+# FIXME: and could be merged with txinfo27.test
+
+required=makeinfo
+. ./defs || Exit 1
+
+cwd=`pwd` || fatal_ "cannot get current working directory"
+
+mkdir bin
+saved_PATH=$PATH; export saved_PATH
+PATH=$cwd/bin$PATH_SEPARATOR$PATH; export PATH
+
+set -e
+
+cat >> configure.in <<'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+info_TEXINFOS = foo.texi
+END
+
+cat > foo.texi << 'END'
+\input texinfo
address@hidden foo.info
address@hidden Top
+Hello world!
address@hidden
+END
+
+: > texinfo.tex
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+
+instdir=_inst
+destdir=_dest
+
+./configure --prefix="$cwd/$instdir" --infodir="$cwd/$instdir/info"
+
+$MAKE info
+test -f foo.info
+
+if install-info --version; then
+  have_installinfo=yes
+else
+  have_installinfo=no
+fi
+
+# The `install-info' target updates the `${infodir}/dir' file
+# by default (if the `install-info' program is available).
+# This should happen in a normal as well as in a DESTDIR installation.
+if test $have_installinfo = yes; then
+  $MAKE install-info
+  test -f $instdir/info/foo.info
+  test -f $instdir/info/dir
+  $MAKE DESTDIR="$cwd/$destdir" install-info
+  test -f "$destdir/$cwd/$instdir"/info/foo.info
+  test -f "$destdir/$cwd/$instdir"/info/dir
+fi
+
+rm -rf $instdir $destdir
+
+# The `install-info' target doesn't fail if the `install-info'
+# program is not available.
+cat > bin/install-info <<'END'
+#!/bin/sh
+echo error from install-info >&2
+exit 127
+END
+chmod a+x bin/install-info
+$MAKE install-info >output 2>&1 || { cat output; Exit 1; }
+cat output
+test -f $instdir/info/foo.info
+test ! -f $instdir/info/dir
+grep 'error from install-info' output && Exit 1
+
+rm -rf $instdir output
+
+if test $have_installinfo = yes; then
+  # The `install-info' target doesn't try to guess whether the `install-info'
+  # is the GNU or debian version.
+  unindent > bin/install-info <<'END'
+    #!/bin/sh
+    set -e; set -u;
+    for fd in 1 2; do
+      for str in dpkg debian Debian; do
+        eval "echo This is $str install-info >&$fd"
+      done
+    done
+    PATH=$saved_PATH; export PATH
+    exec install-info ${1+"$@"}
+END
+  $MAKE install-info
+  test -f $instdir/info/foo.info
+  test -f $instdir/info/dir
+fi
+
+rm -rf $instdir bin/install-info
+
+# The `AM_UPDATE_INFO_DIR' environment variable can be used to
+# prevent the creation or update of the `${infodir}/dir' file,
+# if set to a "no" value.
+for val in no NO n; do
+  rm -rf $instdir
+  env AM_UPDATE_INFO_DIR="$val" $MAKE install-info
+  test -f $instdir/info/foo.info
+  test ! -f $instdir/info/dir
+done
+if test $have_installinfo = yes; then
+  for val in 'yes' 'who cares!'; do
+    rm -rf $instdir
+    env AM_UPDATE_INFO_DIR="$val" $MAKE install-info
+    test -f $instdir/info/foo.info
+    test -f $instdir/info/dir
+  done
+fi
+
+rm -rf $instdir
+
+:
-- 
1.7.2.3