Re: install-sh -C

[Ralf Wildenhues]

Hello Akim,

A quick review of your patch, completely untested.

> from  Akim Demaille  <address@hidden>
[...]

>       Don't use '\' to continue commands: && suffices.

I think using \ then newline then && was done to mimic GCS style for C
code.  FWIW, I prefer your style as well.

> --- NEWS 16 Oct 2006 05:24:17 -0000 1.322
> +++ NEWS 22 Oct 2006 09:00:25 -0000
> @@ -11,6 +11,10 @@
>  * Miscellaneous changes:
>  
>    - New shorthand `$(pkglibexecdir)' for `$(libexecdir)/@PACKAGE@'.
> +
> +  - install-sh supports -C, which does not updates the installed file

s/updates/update/

> +    (and its time stamps) if it is the same.

> --- lib/install-sh 14 Oct 2006 13:37:32 -0000 1.37
> +++ lib/install-sh 22 Oct 2006 09:00:26 -0000

> @@ -459,10 +458,21 @@
>      # 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"; } &&
> +
> +    # Maybe we don't need to install the file.  Use diff, not cmp,
> +    # to be robust to end-of-line encoding.
> +    { if $copy_on_change &&
> +      "$diffprog" "$dsttmp" "$dst" >/dev/null 2>&1 &&

Wouldn't using $diffprog without double quotes be more consistent,
so the user could pass options to diff as well?

> +      test x"`ls -l \"$dsttmp\" | awk '{print $1\":\"$3 \":\"$4}'`" = \
> +           x"`ls -l \"$dst\"    | awk '{print $1\":\"$3 \":\"$4}'`"

This is not portable: "...`...\"...\"...`...".  Write like this:
  new=`ls -l "$dsttmp" | awk '{print $1":"$3 ":"$4}'`
  old=`ls -l "$dst"    | awk '{print $1":"$3 ":"$4}'`
  test x"$old" = x"$new"

(untested).

> +      then
> +     # No need to copy, that's the same file.
> +     continue
> +      else :; fi } &&

There has to be a semicolon before the }, I think.

>  
>      # Now rename the file to the real destination.
>      { $doit $mvcmd -f "$dsttmp" "$dst" 2>/dev/null \

> --- tests/defs.in 6 Jul 2006 18:13:01 -0000 1.39
> +++ tests/defs.in 22 Oct 2006 09:00:29 -0000

> @@ -322,6 +322,24 @@
>  testsrcdir=$srcdir
>  unset srcdir
>  
> +# youngest_file FILES
> +# -------------------
> +# Return the youngest member of FILES.
> +youngest_file ()
> +{
> +  ls -1t "$@" | sed 1q
> +}
> +
> +# is_younger FILE FILES
> +# ---------------------
> +# Check that FILE is younger than all the FILES.
> +is_younger ()
> +{
> +  test `youngest_file "$@"` = "$1"
> +  return $?
> +}

I have some problems with these definitions.  If there are several files
that have the youngest time stamp, then the order depends on the locale.
Yuck.  Stepan suggested `find -newer' as one way out in the case we ran
into this, I think.

Otherwise there need to be even more "$sleep"s in the test suite over
the time, and they already slow it down noticeably.

Cheers,
Ralf