Re: ``install -C'' / unnecessarily updating time stamps

[Paul Eggert]

Benoit Sigoure <address@hidden> writes:

> http://lists.gnu.org/archive/html/automake-patches/2006-10/msg00070.html

That patch does two things: it cleans up and refactors the code, and
it adds -C to install-sh.  I split it into two.  Here's the first
part, which is just the cleanup part.  It shouldn't change any
behavior.  I installed this.  I'll look into the -C part shortly.

2006-12-24  Paul Eggert  <address@hidden>

        * lib/install-sh: Fix typo in previous patch for handling --.
        Use more-consistent style for ';;'.
        Prefer || to if-then-else-:.
        * tests/install2.test: Rework to avoid set -e problems.

2006-12-24  Akim Demaille  <address@hidden>

        Simplify install-sh and its test.  This shouldn't change any behavior.
        (This patch is a subset of the patch proposed in
        
<http://lists.gnu.org/archive/html/automake-patches/2006-10/msg00077.html>.)
        * lib/install-sh (usage): Use usual GNU style.
        (dstarg): Rename as...
        (dst_arg): this for consistency.
        Simplify quoting of assignments.
        Sort them.
        Don't use '\' to continue commands: && suffices.
        Remove useless "continue" in the argument processing,
        and factor the shifts.
        * tests/defs.in: Some improvements to make it set -e clean.
        Use the traditional ":" trick to protect loops from being empty.
        Remove an empty straightforward piece of code prepared to define
        additional variables.
        Use test instead of [], for consistency.
        * tests/install2.test: Use set -e, to simplify code.

Index: lib/install-sh
===================================================================
RCS file: /cvs/automake/automake/lib/install-sh,v
retrieving revision 1.37
diff -u -p -r1.37 install-sh
--- lib/install-sh      14 Oct 2006 13:37:32 -0000      1.37
+++ lib/install-sh      25 Dec 2006 06:30:26 -0000
@@ -1,7 +1,7 @@
 #!/bin/sh
 # install - install a program, script, or datafile

-scriptversion=2006-10-14.15
+scriptversion=2006-12-24.16

 # This originates from X11R5 (mit/util/scripts/install.sh), which was
 # later released in X11R6 (xc/config/util/install.sh) with the
@@ -48,7 +48,7 @@ IFS=" ""      $nl"
 # set DOITPROG to echo to test this script

 # Don't use :- since 4.3BSD and earlier shells don't like it.
-doit="${DOITPROG-}"
+doit=${DOITPROG-}
 if test -z "$doit"; then
   doit_exec=exec
 else
@@ -58,14 +58,14 @@ fi
 # Put in absolute file names if you don't have them in your path;
 # or use environment vars.

-mvprog="${MVPROG-mv}"
-cpprog="${CPPROG-cp}"
-chmodprog="${CHMODPROG-chmod}"
-chownprog="${CHOWNPROG-chown}"
-chgrpprog="${CHGRPPROG-chgrp}"
-stripprog="${STRIPPROG-strip}"
-rmprog="${RMPROG-rm}"
-mkdirprog="${MKDIRPROG-mkdir}"
+chgrpprog=${CHGRPPROG-chgrp}
+chmodprog=${CHMODPROG-chmod}
+chownprog=${CHOWNPROG-chown}
+cpprog=${CPPROG-cp}
+mkdirprog=${MKDIRPROG-mkdir}
+mvprog=${MVPROG-mv}
+rmprog=${RMPROG-rm}
+stripprog=${STRIPPROG-strip}

 posix_glob=
 posix_mkdir=
@@ -73,19 +73,22 @@ posix_mkdir=
 # Desired mode of installed file.
 mode=0755

+chgrpcmd=
 chmodcmd=$chmodprog
 chowncmd=
-chgrpcmd=
-stripcmd=
+mvcmd=$mvprog
 rmcmd="$rmprog -f"
-mvcmd="$mvprog"
+stripcmd=
+
 src=
 dst=
 dir_arg=
-dstarg=
+dst_arg=
+
 no_target_directory=

-usage="Usage: $0 [OPTION]... [-T] SRCFILE DSTFILE
+usage="\
+Usage: $0 [OPTION]... [-T] SRCFILE DSTFILE
    or: $0 [OPTION]... SRCFILES... DIRECTORY
    or: $0 [OPTION]... -t DIRECTORY SRCFILES...
    or: $0 [OPTION]... -d DIRECTORIES...
@@ -95,16 +98,17 @@ In the 2nd and 3rd, copy all SRCFILES to
 In the 4th, create DIRECTORIES.

 Options:
--c         (ignored)
--d         create directories instead of installing files.
--g GROUP   $chgrpprog installed files to GROUP.
--m MODE    $chmodprog installed files to MODE.
--o USER    $chownprog installed files to USER.
--s         $stripprog installed files.
--t DIRECTORY  install into DIRECTORY.
--T         report an error if DSTFILE is a directory.
---help     display this help and exit.
---version  display version info and exit.
+     --help     display this help and exit.
+     --version  display version info and exit.
+
+  -c            (ignored)
+  -d            create directories instead of installing files.
+  -g GROUP      $chgrpprog installed files to GROUP.
+  -m MODE       $chmodprog installed files to MODE.
+  -o USER       $chownprog installed files to USER.
+  -s            $stripprog installed files.
+  -t DIRECTORY  install into DIRECTORY.
+  -T            report an error if DSTFILE is a directory.

 Environment variables override the default commands:
   CHGRPPROG CHMODPROG CHOWNPROG CPPROG MKDIRPROG MVPROG RMPROG STRIPPROG
@@ -112,48 +116,33 @@ Environment variables override the defau

 while test $# -ne 0; do
   case $1 in
-    -c) shift
-        continue;;
+    -c) ;;

-    -d) dir_arg=true
-        shift
-        continue;;
+    -d) dir_arg=true;;

     -g) chgrpcmd="$chgrpprog $2"
-        shift
-        shift
-        continue;;
+       shift;;

     --help) echo "$usage"; exit $?;;

     -m) mode=$2
-        shift
-        shift
        case $mode in
          *' '* | *'    '* | *'
 '*       | *'*'* | *'?'* | *'['*)
            echo "$0: invalid mode: $mode" >&2
            exit 1;;
        esac
-        continue;;
+       shift;;

     -o) chowncmd="$chownprog $2"
-        shift
-        shift
-        continue;;
-
-    -s) stripcmd=$stripprog
-        shift
-        continue;;
-
-    -t) dstarg=$2
-       shift
-       shift
-       continue;;
-
-    -T) no_target_directory=true
-       shift
-       continue;;
+       shift;;
+
+    -s) stripcmd=$stripprog;;
+
+    -t) dst_arg=$2
+       shift;;
+
+    -T) no_target_directory=true;;

     --version) echo "$0 $scriptversion"; exit $?;;

@@ -165,21 +154,22 @@ while test $# -ne 0; do

     *)  break;;
   esac
+  shift
 done

-if test $# -ne 0 && test -z "$dir_arg$dstarg"; then
+if test $# -ne 0 && test -z "$dir_arg$dst_arg"; then
   # When -d is used, all remaining arguments are directories to create.
   # When -t is used, the destination is already specified.
   # Otherwise, the last argument is the destination.  Remove it from 
address@hidden
   for arg
   do
-    if test -n "$dstarg"; then
+    if test -n "$dst_arg"; then
       # $@ is not empty: it contains at least $arg.
-      set fnord "$@" "$dstarg"
+      set fnord "$@" "$dst_arg"
       shift # fnord
     fi
     shift # arg
-    dstarg=$arg
+    dst_arg=$arg
   done
 fi

@@ -224,7 +214,7 @@ for src
 do
   # Protect names starting with `-'.
   case $src in
-    -*) src=./$src ;;
+    -*) src=./$src;;
   esac

   if test -n "$dir_arg"; then
@@ -242,22 +232,22 @@ do
       exit 1
     fi

-    if test -z "$dstarg"; then
+    if test -z "$dst_arg"; then
       echo "$0: no destination specified." >&2
       exit 1
     fi

-    dst=$dstarg
+    dst=$dst_arg
     # Protect names starting with `-'.
     case $dst in
-      -*) dst=./$dst ;;
+      -*) dst=./$dst;;
     esac

     # If destination is a directory, append the input filename; won't work
     # if double slashes aren't ignored.
     if test -d "$dst"; then
       if test -n "$no_target_directory"; then
-       echo "$0: $dstarg: Is a directory" >&2
+       echo "$0: $dst_arg: Is a directory" >&2
        exit 1
       fi
       dstdir=$dst
@@ -378,18 +368,18 @@ do
       # directory the slow way, step by step, checking for races as we go.

       case $dstdir in
-       /*) prefix=/ ;;
-       -*) prefix=./ ;;
-       *)  prefix= ;;
+       /*) prefix='/';;
+       -*) prefix='./';;
+       *)  prefix='';;
       esac

       case $posix_glob in
-        '')
+       '')
          if (set -f) 2>/dev/null; then
            posix_glob=true
          else
            posix_glob=false
-         fi ;;
+         fi;;
       esac

       oIFS=$IFS
@@ -459,14 +449,13 @@ do
     # ignore errors from any of these, just make sure not to ignore
     # errors from the above "$doit $cpprog $src $dsttmp" command.
     #
-    { test -z "$chowncmd" || $doit $chowncmd "$dsttmp"; } \
-      && { test -z "$chgrpcmd" || $doit $chgrpcmd "$dsttmp"; } \
-      && { test -z "$stripcmd" || $doit $stripcmd "$dsttmp"; } \
-      && { test -z "$chmodcmd" || $doit $chmodcmd $mode "$dsttmp"; } &&
+    { test -z "$chowncmd" || $doit $chowncmd "$dsttmp"; } &&
+    { test -z "$chgrpcmd" || $doit $chgrpcmd "$dsttmp"; } &&
+    { test -z "$stripcmd" || $doit $stripcmd "$dsttmp"; } &&
+    { test -z "$chmodcmd" || $doit $chmodcmd $mode "$dsttmp"; } &&

     # Now rename the file to the real destination.
-    { $doit $mvcmd -f "$dsttmp" "$dst" 2>/dev/null \
-      || {
+    { $doit $mvcmd -f "$dsttmp" "$dst" 2>/dev/null || {
           # The rename failed, perhaps because mv can't rename something else
           # to itself, or perhaps because mv is so ancient that it does not
           # support -f.
@@ -477,17 +466,14 @@ do
           # reasons.  In this case, the final cleanup might fail but the new
           # file should still install successfully.
           {
-            if test -f "$dst"; then
-              $doit $rmcmd -f "$dst" 2>/dev/null \
-              || { $doit $mvcmd -f "$dst" "$rmtmp" 2>/dev/null \
-                    && { $doit $rmcmd -f "$rmtmp" 2>/dev/null; :; }; }\
-              || {
-                echo "$0: cannot unlink or rename $dst" >&2
-                (exit 1); exit 1
-              }
-            else
-              :
-            fi
+            test ! -f "$dst" ||
+            $doit $rmcmd -f "$dst" 2>/dev/null ||
+            { $doit $mvcmd -f "$dst" "$rmtmp" 2>/dev/null &&
+              { $doit $rmcmd -f "$rmtmp" 2>/dev/null; :; }
+            } ||
+            { echo "$0: cannot unlink or rename $dst" >&2
+              (exit 1); exit 1
+            }
           } &&

           # Now rename the file to the real destination.
Index: tests/install2.test
===================================================================
RCS file: /cvs/automake/automake/tests/install2.test,v
retrieving revision 1.14
diff -u -p -r1.14 install2.test
--- tests/install2.test 14 May 2005 20:28:55 -0000      1.14
+++ tests/install2.test 25 Dec 2006 06:30:26 -0000
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 1999, 2000, 2001, 2002  Free Software Foundation, Inc.
+# Copyright (C) 1999, 2000, 2001, 2002, 2006  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -23,6 +23,7 @@

 required=gzip
 . ./defs || exit 1
+set -e

 cat > configure.in << 'END'
 AC_INIT
@@ -36,17 +37,20 @@ cat > Makefile.am << 'END'
 SUBDIRS = .
 END

-$ACLOCAL || exit 1
-$AUTOCONF || exit 1
-$AUTOMAKE -a || exit 1
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a

 chmod 000 Makefile.am

-# On some systems (like DOS and Windows), files are always readable
-test -r Makefile.am && exit 77
+# On some systems (like DOS and Windows), files are always readable.
+test ! -r Makefile.am || exit 77

-./configure || exit 1
-# `dist' should fail because we can't copy Makefile.am.
-$MAKE dist  && exit 1
+./configure

-exit 0
+# `dist' should fail because we can't copy Makefile.am.
+if $MAKE dist; then
+  exit 1
+else
+  exit 0
+fi
Index: tests/defs.in
===================================================================
RCS file: /cvs/automake/automake/tests/defs.in,v
retrieving revision 1.39
diff -u -p -r1.39 defs.in
--- tests/defs.in       6 Jul 2006 18:13:01 -0000       1.39
+++ tests/defs.in       25 Dec 2006 06:30:26 -0000
@@ -50,7 +50,7 @@ if test -z "$srcdir"; then
    # compute $srcdir.
    srcdir=`echo "$0" | sed -e 's,/[^\\/]*$,,'`
    test $srcdir = $0 && srcdir=.
-fi
+else :; fi

 # Ensure $srcdir is set correctly.
 test -f $srcdir/defs.in || {
@@ -97,131 +97,125 @@ echo $PATH
 # (See note about `export' in the Autoconf manual.)
 export PATH

-if test -n "$required"
-then
-  for tool in $required
-  do
-    # Check that each required tool is present.
-    case $tool in
-      bison)
-       # Since bison is required, we pick YACC for ./configure.
-       YACC='bison -y'
-       export YACC
-       echo "$me: running bison --version"
-       ( bison --version ) || exit 77
-       ;;
-      bzip2)
-       # Do not use --version, bzip2 still tries to compress stdin.
-       echo "$me: running bzip2 --help"
-       ( bzip2 --help ) || exit 77
-       ;;
-      etags)
-       # Exuberant Ctags will create a TAGS file even
-       # when asked for --help or --version.  (Emacs's etags
-       # does not have such problem.)  Use -o /dev/null
-       # to make sure we do not pollute the tests/ directory.
-       echo "$me: running etags --version -o /dev/null"
-       ( etags --version -o /dev/null ) || exit 77
-       ;;
-      GNUmake)
-       # Use --version AND -v, because SGI Make doesn't fail on --version.
-       # Also grep for GNU because newer versions of FreeBSD make do
-       # not complain about `--version' (they seem to silently ignore it).
-       echo "$me: running $MAKE --version -v | grep GNU"
-       ( $MAKE --version -v | grep GNU ) || exit 77
-       ;;
-      gcc)
-        # When gcc is required, export `CC=gcc' so that ./configure
-        # always use it.  This is important only when the user
-        # has defined CC in his environment, otherwise ./configure will
-        # prefer gcc to other compilers.
-        CC=gcc
-       export CC
-       echo "$me: running $CC --version"
-       ( $CC --version ) || exit 77
-       ;;
-      g++)
-        CXX=g++
-       export CXX
-       echo "$me: running $CXX --version"
-       ( $CXX --version ) || exit 77
-       ;;
-      icc)
-        CC=icc
-       export CC
-       # There is no way to ask *only* the compiler's version.
-       # This tool always want to do something (by default
-       # it will try link *nothing* and complain it cannot find
-       # main(); funny).  Use -help so it does not try linking anything.
-       echo "$me: running $CC -V -help"
-       ( $CC -V -help ) || exit 77
-       ;;
-      makedepend)
-       echo "$me: running makedepend -f-"
-       ( makedepend -f- ) || exit 77
-       ;;
-      makeinfo-html)
-       # Make sure makeinfo understands --html.
-       echo "$me: running makeinfo --html --version"
-       ( makeinfo --html --version ) || exit 77
-       ;;
-      non-root)
-       # Skip this test case if the user is root.
-       # We try to append to a read-only file to detect this.
-       priv_check_temp=priv-check.$$
-       touch $priv_check_temp || exit 1
-       chmod a-w $priv_check_temp || exit 1
-       (echo foo >> $priv_check_temp) >/dev/null 2>&1
-       overwrite_status=$?
-       rm -f $priv_check_temp
-       test $overwrite_status = 0 && exit 77
-       ;;
-      python)
-       # Python doesn't support --version, it has -V
-       echo "$me: running python -V"
-       ( python -V ) || exit 77
-       ;;
-      ro-dir)
-       # Skip this test case if read-only directories aren't supported
-       # (e.g., under DOS.)
-       ro_dir_temp=ro_dir.$$
-       mkdir $ro_dir_temp || exit 1
-       chmod a-w $ro_dir_temp || exit 1
-       (: > $ro_dir_temp/probe) >/dev/null 2>/dev/null
-       create_status=$?
-       rm -rf $ro_dir_temp
-       test $create_status = 0 && exit 77
-       ;;
-      runtest)
-       # DejaGnu's runtest program. We rely on being able to specify
-       # the program on the runtest command-line. This requires
-       # DejaGnu 1.4.3 or later.
-       echo "$me: running runtest --version"
-       (runtest SOMEPROGRAM=someprogram --version) || exit 77
-       ;;
-      tex)
-        # No all versions of Tex support `--version', so we use
-        # a configure check.
-        test -n "@TEX@" || exit 77
-       ;;
-      texi2dvi-o)
-        # Texi2dvi supports `-o' since Texinfo 4.1.
-       echo "$me: running texi2dvi -o /dev/null --version"
-       ( texi2dvi -o /dev/null --version ) || exit 77
-       ;;
-      # Generic case: the tool must support --version.
-      *)
-       echo "$me: running $tool --version"
-       ( $tool --version ) || exit 77
-       ;;
-    esac
-    # Additional variables to define if some $tool is required.
-    case $tool in
-      gcc)
-       ;;
-    esac
-  done
-fi
+for tool in : $required
+do
+  # Check that each required tool is present.
+  case $tool in
+    :) ;;
+    bison)
+      # Since bison is required, we pick YACC for ./configure.
+      YACC='bison -y'
+      export YACC
+      echo "$me: running bison --version"
+      ( bison --version ) || exit 77
+      ;;
+    bzip2)
+      # Do not use --version, bzip2 still tries to compress stdin.
+      echo "$me: running bzip2 --help"
+      ( bzip2 --help ) || exit 77
+      ;;
+    etags)
+      # Exuberant Ctags will create a TAGS file even
+      # when asked for --help or --version.  (Emacs's etags
+      # does not have such problem.)  Use -o /dev/null
+      # to make sure we do not pollute the tests/ directory.
+      echo "$me: running etags --version -o /dev/null"
+      ( etags --version -o /dev/null ) || exit 77
+      ;;
+    GNUmake)
+      # Use --version AND -v, because SGI Make doesn't fail on --version.
+      # Also grep for GNU because newer versions of FreeBSD make do
+      # not complain about `--version' (they seem to silently ignore it).
+      echo "$me: running $MAKE --version -v | grep GNU"
+      ( $MAKE --version -v | grep GNU ) || exit 77
+      ;;
+    gcc)
+      # When gcc is required, export `CC=gcc' so that ./configure
+      # always use it.  This is important only when the user
+      # has defined CC in his environment, otherwise ./configure will
+      # prefer gcc to other compilers.
+      CC=gcc
+      export CC
+      echo "$me: running $CC --version"
+      ( $CC --version ) || exit 77
+      ;;
+    g++)
+      CXX=g++
+      export CXX
+      echo "$me: running $CXX --version"
+      ( $CXX --version ) || exit 77
+      ;;
+    icc)
+      CC=icc
+      export CC
+      # There is no way to ask *only* the compiler's version.
+      # This tool always want to do something (by default
+      # it will try link *nothing* and complain it cannot find
+      # main(); funny).  Use -help so it does not try linking anything.
+      echo "$me: running $CC -V -help"
+      ( $CC -V -help ) || exit 77
+      ;;
+    makedepend)
+      echo "$me: running makedepend -f-"
+      ( makedepend -f- ) || exit 77
+      ;;
+    makeinfo-html)
+      # Make sure makeinfo understands --html.
+      echo "$me: running makeinfo --html --version"
+      ( makeinfo --html --version ) || exit 77
+      ;;
+    non-root)
+      # Skip this test case if the user is root.
+      # We try to append to a read-only file to detect this.
+      priv_check_temp=priv-check.$$
+      touch $priv_check_temp || exit 1
+      chmod a-w $priv_check_temp || exit 1
+      (echo foo >> $priv_check_temp) >/dev/null 2>&1
+      overwrite_status=$?
+      rm -f $priv_check_temp
+      test $overwrite_status = 0 && exit 77
+      ;;
+    python)
+      # Python doesn't support --version, it has -V
+      echo "$me: running python -V"
+      ( python -V ) || exit 77
+      ;;
+    ro-dir)
+      # Skip this test case if read-only directories aren't supported
+      # (e.g., under DOS.)
+      ro_dir_temp=ro_dir.$$
+      mkdir $ro_dir_temp || exit 1
+      chmod a-w $ro_dir_temp || exit 1
+      (: > $ro_dir_temp/probe) >/dev/null 2>/dev/null
+      create_status=$?
+      rm -rf $ro_dir_temp
+      test $create_status = 0 && exit 77
+      ;;
+    runtest)
+      # DejaGnu's runtest program. We rely on being able to specify
+      # the program on the runtest command-line. This requires
+      # DejaGnu 1.4.3 or later.
+      echo "$me: running runtest --version"
+      (runtest SOMEPROGRAM=someprogram --version) || exit 77
+      ;;
+    tex)
+      # No all versions of Tex support `--version', so we use
+      # a configure check.
+      test -n "@TEX@" || exit 77
+      ;;
+    texi2dvi-o)
+      # Texi2dvi supports `-o' since Texinfo 4.1.
+      echo "$me: running texi2dvi -o /dev/null --version"
+      ( texi2dvi -o /dev/null --version ) || exit 77
+      ;;
+    # Generic case: the tool must support --version.
+    *)
+      echo "$me: running $tool --version"
+      ( $tool --version ) || exit 77
+      ;;
+  esac
+done
+

 # Always use an absolute srcdir.  Otherwise symlinks made in subdirs
 # of the test dir just won't work.
@@ -278,23 +272,23 @@ case $required in
   *libtool* | *gettext* )
     aclocaldir='@prefix@/share/aclocal'
     extra_includes=""
-    if [ -f $aclocaldir/dirlist ] ; then
+    if test -f $aclocaldir/dirlist; then
        extra_includes=`(tmp_inc=""
        while read LINE ; do
          tmp_inc="$tmp_inc -I $LINE"
        done
        echo $tmp_inc) < $aclocaldir/dirlist`
-    fi
+    else :; fi

     libtool_found=no
     gettext_found=no
     for d in $extra_includes $aclocaldir ; do
-       [ "x$d" != x-I ] || continue
-       if [ -f "$d/libtool.m4" ] ; then
-          libtool_found=yes
+       test "x$d" != x-I || continue
+       if test -f "$d/libtool.m4"; then
+         libtool_found=yes
        fi
-       if [ -f "$d/gettext.m4" ] ; then
-          gettext_found=yes
+       if test -f "$d/gettext.m4"; then
+         gettext_found=yes
        fi
     done
     case $required in