Re: help-version failure on cygwin

[Jim Meyering]

Eric Blake wrote:
> A relevant portion of the log of the failure on cygwin:
>
> + mkdir dir-1088
> + test gzip.exe = '['
> + prog=gzip.exe
> + eval 'args=$gzip.exe_args'
> ++ args=.exe_args
> + env gzip.exe .exe_args
> gzip: .exe_args: No such file or directory
> + echo FAIL: gzip.exe
> FAIL: gzip.exe
> + fail=1
>
> It looks like the help-version test needs to take $EXEEXT into account.  I
> don't have time to look into this right now, but I'll get to it in the
> next week or so if no one beats me to it.  Actually, updating this to use
> the help-version used by grep may be good enough, since that uses shell
> functions rather than eval to add arguments.

Thanks for testing and raising the issue.
help-version is another one of those shared-multi-project files
that is still updated manually, mostly on an as-needed basis.

I've just updated it and added a useful cross-check.
Note that to make this new cross-check pass, I had to switch
from using the VERSION macro (in its --version-printing function)
to printing the value of a global variable that is updated more
reliably, from a makefile dependency standpoint.

I've had enough trouble with getting path_prepend_ right
recently that I want to use this sort of PATH cross check
more widely, but without incurring the cost of running a prog --version
in every init.sh-using test.  Here's what I'm considering:

    add a cfg.mk/maint.mk rule that looks for help-version, and if found,
    looks for a use of init.sh within that file.  If found, it would then
    verify that every other test that uses init.sh has the same use of
    path_prepend_.

>From af63e88f7765e45fee3f4c1a5ad7171f306e1dcb Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Apr 2010 08:52:09 +0200
Subject: [PATCH 1/4] build: keep --version strictly up to date

Before this change, in development, gzip's --version output could lag
behind reality by a couple deltas or by a "-dirty" suffix.  That would
lead to spurious failure of the new --version-$VERSION PATH cross-check.
* Makefile.am (version.c, version.h): New rules.
(BUILT_SOURCES): Set/append.
(noinst_LIBRARIES, noinst_libver_a_SOURCES): Define.
(gzip_LDADD): Add libver.a.
* gzip.c (license): Use Version, not VERSION.
---
 Makefile.am |   21 ++++++++++++++++++++-
 gzip.c      |    3 ++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 129c453..b26490b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -18,6 +18,7 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

 ALL_RECURSIVE_TARGETS =
+BUILT_SOURCES =

 SUBDIRS = lib doc . tests
 ACLOCAL_AMFLAGS = -I m4
@@ -27,6 +28,9 @@ AM_CFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS)
 # Tell the linker to omit references to unused shared libraries.
 AM_LDFLAGS = $(IGNORE_UNUSED_LIBRARIES_CFLAGS)

+noinst_LIBRARIES = libver.a
+nodist_libver_a_SOURCES = version.c version.h
+
 man_MANS = gunzip.1 gzexe.1 gzip.1 \
   zcat.1 zcmp.1 zdiff.1 zforce.1 zgrep.1 zless.1 zmore.1 znew.1

@@ -53,9 +57,24 @@ bin_SCRIPTS = gunzip gzexe zcat zcmp zdiff \
 gzip_SOURCES = \
   bits.c crypt.c deflate.c gzip.c inflate.c lzw.c \
   trees.c unlzh.c unlzw.c unpack.c unzip.c util.c zip.c
-gzip_LDADD = lib/libgzip.a
+gzip_LDADD = libver.a lib/libgzip.a
 gzip_LDADD += $(LIB_CLOCK_GETTIME)

+BUILT_SOURCES += version.c
+version.c: Makefile
+       $(AM_V_GEN)rm -f $@
+       $(AM_V_at)printf '#include <config.h>\n' > address@hidden
+       $(AM_V_at)printf 'char const *Version = "$(PACKAGE_VERSION)";\n' >> 
address@hidden
+       $(AM_V_at)chmod a-w address@hidden
+       $(AM_V_at)mv address@hidden $@
+
+BUILT_SOURCES += version.h
+version.h: Makefile
+       $(AM_V_GEN)rm -f $@
+       $(AM_V_at)printf 'extern char const *Version;\n' > address@hidden
+       $(AM_V_at)chmod a-w address@hidden
+       $(AM_V_at)mv address@hidden $@
+
 gzip.doc: gzip.1
        $(AM_V_GEN)groff -man -Tascii $(srcdir)/gzip.1 | col -b | uniq > 
address@hidden \
          && mv address@hidden $@
diff --git a/gzip.c b/gzip.c
index 41322ec..4762e88 100644
--- a/gzip.c
+++ b/gzip.c
@@ -73,6 +73,7 @@ static char const *const license_msg[] = {
 #include "getopt.h"
 #include "ignore-value.h"
 #include "stat-time.h"
+#include "version.h"

                 /* configuration */

@@ -383,7 +384,7 @@ local void license()
 {
     char const *const *p = license_msg;

-    printf ("%s %s\n", program_name, VERSION);
+    printf ("%s %s\n", program_name, Version);
     while (*p) printf ("%s\n", *p++);
 }

--
1.7.0.4.552.gc303


>From c6cf1bdde3a3652d21cac17448f626243fd22e55 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Apr 2010 08:16:51 +0200
Subject: [PATCH 2/4] tests: pull help-version from grep

---
 tests/help-version |  170 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 95 insertions(+), 75 deletions(-)

diff --git a/tests/help-version b/tests/help-version
index 79f4b57..3c865fb 100755
--- a/tests/help-version
+++ b/tests/help-version
@@ -17,8 +17,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.

-test "$VERBOSE" = yes && set -x
-
 # Ensure that $SHELL is set to *some* value and exported.
 # This is required for dircolors, which would fail e.g., when
 # invoked via debuild (which removes SHELL from the environment).
@@ -54,6 +52,10 @@ expected_failure_status_zgrep=2
 expected_failure_status_zegrep=2
 expected_failure_status_zfgrep=2

+expected_failure_status_grep=2
+expected_failure_status_egrep=2
+expected_failure_status_fgrep=2
+
 test "$built_programs" \
   || { echo "$this_test: no programs built!?!" 1>&2; Exit 1; }

@@ -112,102 +114,111 @@ tmp_out=out-$$
 mkdir $tmp || fail=1
 cd $tmp || fail=1

-comm_args="$tmp_in $tmp_in"
-csplit_args="$tmp_in //"
-cut_args='-f 1'
-join_args="$tmp_in $tmp_in"
-tr_args='a a'
+comm_setup () { args="$tmp_in $tmp_in"; }
+csplit_setup () { args="$tmp_in //"; }
+cut_setup () { args='-f 1'; }
+join_setup () { args="$tmp_in $tmp_in"; }
+tr_setup () { args='a a'; }

-chmod_args="a+x $tmp_in"
+chmod_setup () { args="a+x $tmp_in"; }
 # Punt on these.
-chgrp_args=--version
-chown_args=--version
-mkfifo_args=--version
-mknod_args=--version
+chgrp_setup () { args=--version; }
+chown_setup () { args=--version; }
+mkfifo_setup () { args=--version; }
+mknod_setup () { args=--version; }
 # Punt on uptime, since it fails (e.g., failing to get boot time)
 # on some systems, and we shouldn't let that stop `make check'.
-uptime_args=--version
+uptime_setup () { args=--version; }

 # Create a file in the current directory, not in $TMPDIR.
-mktemp_args=mktemp.XXXX
+mktemp_setup () { args=mktemp.XXXX; }

-cmp_args="$tmp_in $tmp_in2"
+cmp_setup () { args="$tmp_in $tmp_in2"; }

 # Tell dd not to print the line with transfer rate and total.
 # The transfer rate would vary between runs.
-dd_args=status=noxfer
-
-zdiff_args="$zin $zin2"
-zcmp_args="$zin $zin2"
-zcat_args=$zin
-gunzip_args=$zin
-zmore_args=$zin
-zless_args=$zin
-znew_args=$bigZ_in
-zforce_args=$zin
-zgrep_args="z $zin"
-zegrep_args="z $zin"
-zfgrep_args="z $zin"
-gzexe_args=$tmp_in
-
-diff_args="$tmp_in $tmp_in2"
-sdiff_args="$tmp_in $tmp_in2"
-diff3_args="$tmp_in $tmp_in2 $tmp_in2"
-cp_args="$tmp_in $tmp_in2"
-ln_args="$tmp_in ln-target"
-ginstall_args="$tmp_in $tmp_in2"
-mv_args="$tmp_in $tmp_in2"
-mkdir_args=$tmp_dir/subdir
-rmdir_args=$tmp_dir
-rm_args=$tmp_in
-shred_args=$tmp_in
-touch_args=$tmp_in2
-truncate_args="--reference=$tmp_in $tmp_in2"
-
-basename_args=$tmp_in
-dirname_args=$tmp_in
-expr_args=foo
+dd_setup () { args=status=noxfer; }
+
+zdiff_setup () { args="$zin $zin2"; }
+zcmp_setup () { args="$zin $zin2"; }
+zcat_setup () { args=$zin; }
+gunzip_setup () { args=$zin; }
+zmore_setup () { args=$zin; }
+zless_setup () { args=$zin; }
+znew_setup () { args=$bigZ_in; }
+zforce_setup () { args=$zin; }
+zgrep_setup () { args="z $zin"; }
+zegrep_setup () { args="z $zin"; }
+zfgrep_setup () { args="z $zin"; }
+gzexe_setup () { args=$tmp_in; }
+
+# We know that $tmp_in contains a "0"
+grep_setup () { args="0 $tmp_in"; }
+egrep_setup () { args="0 $tmp_in"; }
+fgrep_setup () { args="0 $tmp_in"; }
+
+diff_setup () { args="$tmp_in $tmp_in2"; }
+sdiff_setup () { args="$tmp_in $tmp_in2"; }
+diff3_setup () { args="$tmp_in $tmp_in2 $tmp_in2"; }
+cp_setup () { args="$tmp_in $tmp_in2"; }
+ln_setup () { args="$tmp_in ln-target"; }
+ginstall_setup () { args="$tmp_in $tmp_in2"; }
+mv_setup () { args="$tmp_in $tmp_in2"; }
+mkdir_setup () { args=$tmp_dir/subdir; }
+rmdir_setup () { args=$tmp_dir; }
+rm_setup () { args=$tmp_in; }
+shred_setup () { args=$tmp_in; }
+touch_setup () { args=$tmp_in2; }
+truncate_setup () { args="--reference=$tmp_in $tmp_in2"; }
+
+basename_setup () { args=$tmp_in; }
+dirname_setup () { args=$tmp_in; }
+expr_setup () { args=foo; }

 # Punt, in case GNU `id' hasn't been installed yet.
-groups_args=--version
-
-pathchk_args=$tmp_in
-yes_args=--version
-logname_args=--version
-nohup_args=--version
-printf_args=foo
-seq_args=10
-sleep_args=0
-su_args=--version
-stdbuf_args="-oL true"
-timeout_args=--version
+groups_setup () { args=--version; }
+
+pathchk_setup () { args=$tmp_in; }
+yes_setup () { args=--version; }
+logname_setup () { args=--version; }
+nohup_setup () { args=--version; }
+printf_setup () { args=foo; }
+seq_setup () { args=10; }
+sleep_setup () { args=0; }
+su_setup () { args=--version; }
+stdbuf_setup () { args="-oL true"; }
+timeout_setup () { args=--version; }

 # I'd rather not run sync, since it spins up disks that I've
 # deliberately caused to spin down (but not unmounted).
-sync_args=--version
+sync_setup () { args=--version; }

-test_args=foo
+test_setup () { args=foo; }

 # This is necessary in the unusual event that there is
 # no valid entry in /etc/mtab.
-df_args=/
+df_setup () { args=/; }

 # This is necessary in the unusual event that getpwuid (getuid ()) fails.
-id_args=-u
+id_setup () { args=-u; }

 # Use env to avoid invoking built-in sleep of Solaris 11's /bin/sh.
-env sleep 10m &
-kill_args=$!
+kill_setup () {
+  env sleep 10m &
+  args=$!
+}

-link_args="$tmp_in link-target"
-unlink_args=$tmp_in
+link_setup () { args="$tmp_in link-target"; }
+unlink_setup () { args=$tmp_in; }

-ln -s . slink
-readlink_args=slink
+readlink_setup () {
+  ln -s . slink
+  args=slink;
+}

-stat_args=$tmp_in
-unlink_args=$tmp_in
-lbracket_args=": ]"
+stat_setup () { args=$tmp_in; }
+unlink_setup () { args=$tmp_in; }
+lbracket_setup () { args=": ]"; }

 # Ensure that each program "works" (exits successfully) when doing
 # something more than --help or --version.
@@ -219,12 +230,21 @@ for i in $built_programs; do
   echo z |gzip > $zin
   cp $zin $zin2
   cp $zin $bigZ_in
-  echo > $tmp_in
-  echo > $tmp_in2
+
+  # This is sort of kludgey: use numbers so this is valid input for factor,
+  # and two tokens so it's valid input for tsort.
+  echo 2147483647 0 > $tmp_in
+  # Make $tmp_in2 identical. Then, using $tmp_in and $tmp_in2 as arguments
+  # to the likes of cmp and diff makes them exit successfully.
+  cp $tmp_in $tmp_in2
   mkdir $tmp_dir
   # echo ================== $i
   test $i = [ && prog=lbracket || prog=$i
-  eval "args=\$${prog}_args"
+  if type ${prog}_setup > /dev/null 2>&1; then
+    ${prog}_setup
+  else
+    args=
+  fi
   if env $i $args < $tmp_in > $tmp_out; then
     : # ok
   else
--
1.7.0.4.552.gc303


>From a237633667839895bf7be5c410cce50ff8e2cc6b Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Apr 2010 08:19:02 +0200
Subject: [PATCH 3/4] tests: improve help-version

* tests/help-version: Use fail_, rather than echo+Exit.
---
 tests/help-version |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/help-version b/tests/help-version
index 3c865fb..063ad30 100755
--- a/tests/help-version
+++ b/tests/help-version
@@ -57,7 +57,7 @@ expected_failure_status_egrep=2
 expected_failure_status_fgrep=2

 test "$built_programs" \
-  || { echo "$this_test: no programs built!?!" 1>&2; Exit 1; }
+  || fail_ "built_programs not specified!?!"

 for lang in C fr da; do
   for i in $built_programs; do
--
1.7.0.4.552.gc303


>From b1933f23c12bf6cebb8947ac38fcb343a0be6f0a Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Apr 2010 08:30:51 +0200
Subject: [PATCH 4/4] tests: help-version: cross-check PATH in tests

* tests/help-version: Cross-check $VERSION and --version output.
* tests/Makefile.am (TESTS_ENVIRONMENT): Export VERSION=$(VERSION).
---
 tests/Makefile.am  |    1 +
 tests/help-version |   13 +++++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 59ea2f2..03eb4a3 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -70,6 +70,7 @@ TESTS_ENVIRONMENT =                           \
   };                                           \
   export                                       \
   LC_ALL=C                                     \
+  VERSION=$(VERSION)                           \
   abs_top_builddir='$(abs_top_builddir)'       \
   abs_top_srcdir='$(abs_top_srcdir)'           \
   abs_srcdir='$(abs_srcdir)'                   \
diff --git a/tests/help-version b/tests/help-version
index 063ad30..5d73f5d 100755
--- a/tests/help-version
+++ b/tests/help-version
@@ -59,6 +59,19 @@ expected_failure_status_fgrep=2
 test "$built_programs" \
   || fail_ "built_programs not specified!?!"

+test "$VERSION" \
+  || fail_ "set envvar VERSION; it is required for a PATH sanity-check"
+
+# Extract version from --version output of the first program
+for i in $built_programs; do
+  v=$(env $i --version | sed -n '1s/.* //p;q')
+  break
+done
+
+# Ensure that it matches $VERSION.
+test "x$v" = "x$VERSION" \
+  || fail_ "--version-\$VERSION mismatch"
+
 for lang in C fr da; do
   for i in $built_programs; do

--
1.7.0.4.552.gc303