Re: [SCM] GNU Libtool branch, master, updated. v2.2.10-35-gee2dde8

[Gary V. Vaughan]

Hallo Ralf,

On 27 Jun 2010, at 15:49, Ralf Wildenhues wrote:
> * Gary V. Vaughan wrote on Sun, Jun 27, 2010 at 08:57:58AM CEST:
>> commit ee2dde86ba2f1bdc0638726c6580b96800ad4b39
>> Author: Gary V. Vaughan <address@hidden>
>> Date:   Sun Jun 27 13:57:50 2010 +0700
>> 
>>    Fix portability regressions in today's earlier changeset.
>> 
>>    * libltdl/m4/libtool.m4 (_LT_PROG_XSI_REPLACE): `sed -i' is not
>>    portable; use `mv -f ...|| cp -f ...' instead.
>>    Add an initial backslash to sed `c' substitutions with leading
>>    whitespace, so that indentation is not removed by some non-POSIX
>>    compliant sed implementations.
>>    (_LT_PROG_XSI_SHELLFNS): Display a diagnostic warning message if
>>    one or more XSI function replacements failed.
> 
>> --- a/libltdl/m4/libtool.m4
>> +++ b/libltdl/m4/libtool.m4
>> @@ -7257,13 +7257,15 @@ _LT_DECL([NL2SP], [lt_NL2SP], [1], [turn newlines 
>> into spaces])dnl
>> # '^} FUNCNAME ', and replace its body with REPLACEMENT-BODY.
>> m4_defun([_LT_PROG_XSI_REPLACE],
>> [dnl {
>> -sed -i .tmp -e '/^$1 ()$/,/^} # $1 /c\
>> +sed -e '/^$1 ()$/,/^} # $1 /c\
>> $1 ()\
>> {\
>> -m4_bpatsubst([$2], [$], [\\])
>> -} # XSI $1 implementation' "$cfgfile" \
>> -  || (mv -f "$cfgfile.tmp" "$cfgfile"; exit 1)
>> -rm -f "$cfgfile.tmp"])
>> +m4_bpatsubsts([$2], [$], [\\], [^\([        ]\)], [\\\1])
> 
> Please avoid space before TAB, eager editor replace that; TAB before
> space is safer.

Not something I've encountered, but good point.  I'll correct presently.

>> +} # XSI $1 implementation' "$cfgfile" > $cfgfile.tmp \
>> +  && mv -f "$cfgfile.tmp" "$cfgfile" \
>> +    || (rm -f "$cfgfile" && cp "$cfgfile.tmp" "$cfgfile" && rm -f 
>> "$cfgfile.tmp")
> 
> FWIW, I don't think this line is needed, but it shouldn't hurt either.
> However, if the cp fails, it might have done half of its work before
> finding out that the disk is full, so the script might actually be
> garbage after that, so ...

Just copying the style of the surrounding code.  Feel free to remove the last
line if you'd prefer.

>> +test 0 -eq $? || _lt_xsi_replace_fail=:
>> +])
>> 
>> 
>> # _LT_PROG_XSI_SHELLFNS
>> @@ -7315,4 +7317,8 @@ fi
>> if test x"$lt_shell_append" = xyes; then
>>   _LT_PROG_XSI_REPLACE([func_append], [    eval "${1}+=\\${2}"])
>> fi
>> +
>> +if test x"$_lt_xsi_replace_fail" = x":"; then
>> +  AC_MSG_WARN([Unable to substitute faster XSI functions in $ofile]) 
> 
> ... I think this should indeed be a hard error.  Also, I don't see where
> _lt_xsi_replace_fail is initialized in the good case; it should be.

That would prevent me from checking that the warning works with:

  ../configure _lt_xsi_replace_fail=:

The absolute worst case scenario for leaving it as I've done it, is
that somehow the environment gets _lt_xsi_replace_fail set to ":",
and then the user will see a spurious warning even though the
substitutions did work in actuality.

Why do you want to force a hard failure?  The warning is the very
last line a user will see after a non-substituting configure run,
and even if they ignore it, libtool will still work correctly.  I'm
not strongly opposed to your changing it if that's what you would
prefer, but I find the current implementation more convenient for
developers and users.

> Does the tree pass make check with this patch?

Yes, a complete 'make check' takes less than 10 minutes on my current
laptop, so I run it before every commit and push.

Cheers,
-- 
Gary V. Vaughan (address@hidden)