Re: help-version failure on cygwin

[Jim Meyering]

Jim Meyering wrote:
> 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_.

I did that for diffutils.
The new rule spotted 5 tests that were inadequate:

>From 55cf264a2bac21ed6b8729bc175df421e35facf4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Apr 2010 10:56:06 +0200
Subject: [PATCH 1/3] tests: use path_prepend_ consistently; remove unnecessary 
VERBOSE check

* tests/basic: Likewise.
* tests/binary: Likewise.
* tests/function-line-vs-leading-space: Likewise.
* tests/label-vs-func: Likewise.
* tests/stdin: Likewise.
---
 tests/basic                          |    3 +--
 tests/binary                         |    3 +--
 tests/function-line-vs-leading-space |    3 +--
 tests/label-vs-func                  |    3 +--
 tests/stdin                          |    3 +--
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/tests/basic b/tests/basic
index 87bb7d8..bfee8bd 100755
--- a/tests/basic
+++ b/tests/basic
@@ -1,9 +1,8 @@
 #!/bin/sh
 # small examples

-test "$VERBOSE" = yes && set -x
 : ${srcdir=.}
-. "$srcdir/init.sh"
+. "$srcdir/init.sh"; path_prepend_ ../src

 fail=0

diff --git a/tests/binary b/tests/binary
index 9c31bc6..c66ea04 100644
--- a/tests/binary
+++ b/tests/binary
@@ -1,9 +1,8 @@
 #!/bin/sh
 # small examples

-test "$VERBOSE" = yes && set -x
 : ${srcdir=.}
-. "$srcdir/init.sh"
+. "$srcdir/init.sh"; path_prepend_ ../src

 printf 'Binary files - and /dev/null differ\n' > out-exp || fail_ setup

diff --git a/tests/function-line-vs-leading-space 
b/tests/function-line-vs-leading-space
index 320452c..62fd021 100755
--- a/tests/function-line-vs-leading-space
+++ b/tests/function-line-vs-leading-space
@@ -1,9 +1,8 @@
 #!/bin/sh
 # Demonstrate how -F RE behavior changed after diff-2.9.

-test "$VERBOSE" = yes && set -x
 : ${srcdir=.}
-. "$srcdir/init.sh"
+. "$srcdir/init.sh"; path_prepend_ ../src

 cat <<EOF > in || fail_ "failed to create temporary file"
 procedure AdaCode is
diff --git a/tests/label-vs-func b/tests/label-vs-func
index c862bed..2026b0a 100755
--- a/tests/label-vs-func
+++ b/tests/label-vs-func
@@ -1,9 +1,8 @@
 #!/bin/sh
 # Show how diff's -p option can mistakenly match a label: in column 1.

-test "$VERBOSE" = yes && set -x
 : ${srcdir=.}
-. "$srcdir/init.sh"
+. "$srcdir/init.sh"; path_prepend_ ../src

 fail=0
 cat <<EOF > exp || fail=1
diff --git a/tests/stdin b/tests/stdin
index 3c114bd..66ef2bb 100755
--- a/tests/stdin
+++ b/tests/stdin
@@ -1,9 +1,8 @@
 #!/bin/sh
 # Ensure that "-" means "standard input".

-test "$VERBOSE" = yes && set -x
 : ${srcdir=.}
-. "$srcdir/init.sh"
+. "$srcdir/init.sh"; path_prepend_ ../src

 fail=0

--
1.7.1.rc0.212.gbd88f


>From 46773b6abbfd75432387a6e28579a11c62e897d9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Apr 2010 10:51:19 +0200
Subject: [PATCH 2/3] tests: add syntax-check rule to verify that tests use 
proper PATH

* cfg.mk (sc_cross_check_PATH_usage_in_tests): New rule, that is
useful only in conjunction with the help-version script.
---
 cfg.mk |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index fcebfab..83db3a1 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -62,3 +62,23 @@ config-save:
        mkdir -p $(_cf_state_dir)/$(_date_time)
        ln -nsf $(date_time) $(_cf_state_dir)/latest
        cp lib/config.h config.status $(_cf_state_dir)/latest
+
+# If tests/help-version exists and seems to be new enough, assume that its
+# use of init.sh and path_prepend_ is correct, and ensure that every other
+# use of init.sh is identical.
+# This is useful because help-version cross-checks prog --version
+# with $(VERSION), which verifies that its path_prepend_ invocation
+# sets PATH correctly.  This is an inexpensive way to ensure that
+# the other init.sh-using tests also get it right.
+_hv_file = $(srcdir)/tests/help-version
+_hv_regex = ^ *\. [^ ]*/init\.sh
+sc_cross_check_PATH_usage_in_tests:
+       @if grep -l 'VERSION mismatch' $(_hv_file) >/dev/null           \
+           && grep -lE '$(_hv_regex)' $(_hv_file) >/dev/null; then     \
+         good=$$(grep -E '$(_hv_regex)' < $(_hv_file));                \
+         grep -LF "$$good"                                             \
+               $$(grep -lE '$(_hv_regex)' $$($(VC_LIST_EXCEPT)))       \
+             | grep . &&                                               \
+           { echo "$(ME): the above files use path_prepend_ inconsistently" \
+               1>&2; exit 1; } || :;                                   \
+       fi
--
1.7.1.rc0.212.gbd88f


>From fe2d46d4aaa79e15d2df24ae387c032858982c3f Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 7 Apr 2010 10:59:00 +0200
Subject: [PATCH 3/3] tests: pull latest help-version from gzip

* tests/help-version: Update from gzip.
* Makefile.am (TESTS_ENVIRONMENT): Export VERSION,
as required for this new help-version script.
---
 tests/Makefile.am  |    1 +
 tests/help-version |  187 ++++++++++++++++++++++++++++++++++------------------
 2 files changed, 124 insertions(+), 64 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index cf48f15..a511b0b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -33,6 +33,7 @@ TESTS_ENVIRONMENT =                           \
     fi;                                                \
   };                                           \
   export                                       \
+  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 a51ff24..9d7d11d 100755
--- a/tests/help-version
+++ b/tests/help-version
@@ -23,9 +23,8 @@
 test "x$SHELL" = x && SHELL=/bin/sh
 export SHELL

-test "$VERBOSE" = yes && set -x
 : ${srcdir=.}
-. "$srcdir/init.sh"
+. "$srcdir/init.sh"; path_prepend_ ../src

 expected_failure_status_chroot=125
 expected_failure_status_env=125
@@ -44,12 +43,34 @@ expected_failure_status_ls=2
 expected_failure_status_vdir=2

 expected_failure_status_cmp=2
+expected_failure_status_zcmp=2
 expected_failure_status_sdiff=2
 expected_failure_status_diff3=2
 expected_failure_status_diff=2
+expected_failure_status_zdiff=2
+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; }
+  || 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
@@ -94,6 +115,10 @@ for lang in C fr da; do
   done
 done

+bigZ_in=bigZ-in.Z
+zin=zin.gz
+zin2=zin2.gz
+
 tmp=tmp-$$
 tmp_in=in-$$
 tmp_in2=in2-$$
@@ -102,89 +127,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
-
-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.
@@ -192,13 +239,25 @@ for i in $built_programs; do
   # Skip these.
   case $i in chroot|stty|tty|false|chcon|runcon) continue;; esac

-  rm -rf $tmp_in $tmp_in2 $tmp_dir $tmp_out
-  echo > $tmp_in
-  echo > $tmp_in2
+  rm -rf $tmp_in $tmp_in2 $tmp_dir $tmp_out $bigZ_in $zin $zin2
+  echo z |gzip > $zin
+  cp $zin $zin2
+  cp $zin $bigZ_in
+
+  # 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.1.rc0.212.gbd88f