Hello Akim,
A quick review of your patch, completely untested.
> from Akim Demaille <[EMAIL PROTECTED]>
[...]
> 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
