Re: Fix testsuite errors due to shell quoted parameter expansion issue.

2010-08-03 Thread Mikael Magnusson
On 3 August 2010 22:55, Eric Blake  wrote:
> [adding autoconf to document some shell bugs]
>
> On 08/03/2010 02:32 PM, Ralf Wildenhues wrote:
>> Interesting shell unportability:
>>
>> $ bash -c 'f=" val" e=; echo "$e"$f'
>> val
>> $ ksh -c 'f=" val" e=; echo "$e"$f'
>>  val
>>
>> ksh93, dash, zsh all do it like ksh.  Is that a bug in bash?
>
> Yes; adding bug-bash accordingly.  According to POSIX:
>
> http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_05
>
> "After parameter expansion ( Parameter Expansion ), command substitution
> ( Command Substitution ), and arithmetic expansion ( Arithmetic
> Expansion  ), the shell shall scan the results of expansions and
> substitutions that did not occur in double-quotes for field splitting
> and multiple fields can result."
>
> Since $f is not quoted, its expansion must undergo field splitting.  But
> since "$e" is quoted, it must not be elided even though empty.  The
> result must be _two_ fields, as if you had done "echo '' 'val'".
>
> But it is _also_ a bug in zsh; adding zsh-workers accordingly.
>
> $ zsh -cvx 'f=" val" e=; echo "$e"$f'
> +zsh:1> f=' val' e=''
> +zsh:1> echo ' val'
>  val
>
> Oops - zsh only passed one argument to echo, with a leading space,
> instead of passing an empty argument and letting echo supply the space.
>  ksh93, pdksh, and dash get it right (although dash doesn't use quotes
> in -vx output, hence my use of n() to force things to tell; n() is
> another way to expose the bash and zsh bugs).

zsh doesn't do word splitting by default, you can enable it with the = modifier:
% zsh -fcvx 'f=" val" e=; echo "$e"$=f'
+zsh:1> f=' val' e=''
+zsh:1> echo '' val
 val

does what you want

Alternatively you can make zsh try to be closer to sh by setting
argv[0] to sh when executing it, or running 'emulate sh' as the first
command (and possibly other ways I don't know about):
% zsh -fcvx 'emulate sh;f=" val" e=; echo "$e"$f'
+zsh:1> emulate sh
+zsh:1> f=' val' e=''
+zsh:1> echo '' val
 val

There's also --shwordsplit for this specific case:
% zsh --shwordsplit -fcvx 'f=" val" e=; echo "$e"$f'
+zsh:1> f=' val' e=''
+zsh:1> echo '' val
 val

(the -f above only avoids loading my .zshenv which would spam my output)

-- 
Mikael Magnusson



Re: Fix testsuite errors due to shell quoted parameter expansion issue.

2010-08-03 Thread Eric Blake
[adding autoconf to document some shell bugs]

On 08/03/2010 02:32 PM, Ralf Wildenhues wrote:
> Interesting shell unportability:
> 
> $ bash -c 'f=" val" e=; echo "$e"$f'
> val
> $ ksh -c 'f=" val" e=; echo "$e"$f'
>  val
> 
> ksh93, dash, zsh all do it like ksh.  Is that a bug in bash?

Yes; adding bug-bash accordingly.  According to POSIX:

http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_05

"After parameter expansion ( Parameter Expansion ), command substitution
( Command Substitution ), and arithmetic expansion ( Arithmetic
Expansion  ), the shell shall scan the results of expansions and
substitutions that did not occur in double-quotes for field splitting
and multiple fields can result."

Since $f is not quoted, its expansion must undergo field splitting.  But
since "$e" is quoted, it must not be elided even though empty.  The
result must be _two_ fields, as if you had done "echo '' 'val'".

But it is _also_ a bug in zsh; adding zsh-workers accordingly.

$ zsh -cvx 'f=" val" e=; echo "$e"$f'
+zsh:1> f=' val' e=''
+zsh:1> echo ' val'
 val

Oops - zsh only passed one argument to echo, with a leading space,
instead of passing an empty argument and letting echo supply the space.
 ksh93, pdksh, and dash get it right (although dash doesn't use quotes
in -vx output, hence my use of n() to force things to tell; n() is
another way to expose the bash and zsh bugs).

$ ksh -cvx 'n() { echo $#; }; f=" val" e=; n "$e"$f'
n() { echo $#; }; f=" val" e=; n ""$f+ f=' val'
+ e=''
+ n '' val
+ echo 2
2

And meanwhile, I found a ksh93 parsing bug (don't know where to report
that):

$ ksh -c 'a(){ echo hi; }; a'
ksh: syntax error at line 1: `}' unexpected
$ ksh -c 'a() { echo hi; }; a'
hi
$ bash -c 'a(){ echo hi; }; a'
hi
$ /bin/sh -c 'a(){ echo hi; }; a'
hi

ksh is the only shell that requires a space between the ) and {, even
though the ) is a metacharacter and should not need trailing space (even
Solaris /bin/sh got that right).

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Fix testsuite errors due to shell quoted parameter expansion issue.

2010-08-03 Thread Ralf Wildenhues
Interesting shell unportability:

$ bash -c 'f=" val" e=; echo "$e"$f'
val
$ ksh -c 'f=" val" e=; echo "$e"$f'
 val

ksh93, dash, zsh all do it like ksh.  Is that a bug in bash?

Anyway, the patch below seems to fix it, but I may want to fix the log
entry to pinpoing the culprit correctly.

Thanks,
Ralf

Fix testsuite errors due to shell quoted parameter expansion issue.

* tests/getopt-m4sh.at (_LT_AT_GETOPT_M4SH_SETUP): Insert space
between double-quoted and unquoted shell parameter, as ksh may
add one in its output anyway if the expansion of the second one
starts with a space.
(short option splitting, enhanced shell short option splitting)
(long option splitting, XSI long option splitting): Add space in
expected output.
Fixes testsuite failures on AIX, FreeBSD, etc.

diff --git a/tests/getopt-m4sh.at b/tests/getopt-m4sh.at
index ca5d5b8..768d595 100644
--- a/tests/getopt-m4sh.at
+++ b/tests/getopt-m4sh.at
@@ -44,7 +44,7 @@ M4SH_GETOPTS(
   [i], [--install],[], [options="$options install"],
   [v], [--verbose],[], [options="$options verbose"],
   [!], [--ltdl],   [false],[options="$options ltdl=$optarg"],
-[echo "$list"$options])
+[echo "$list" $options])
 ]])
 m4_pattern_forbid([m4_include])
 m4_pattern_forbid([AS_INIT])
@@ -87,7 +87,7 @@ rm -f options && mv options.tmp options])
 AT_SETUP([short option splitting])
 
 AT_DATA(expout,
-[[force verbose install
+[[ force verbose install
 ]])
 
 _LT_AT_GETOPT_M4SH_SETUP
@@ -103,7 +103,7 @@ AT_SETUP([enhanced shell short option splitting])
 AT_CHECK([fgrep '# Extended-shell func_split_short_opt' 
$abs_top_builddir/libtool >/dev/null 2>&1 || (exit 77)])
 
 AT_DATA(expout,
-[[force verbose install
+[[ force verbose install
 ]])
 
 _LT_AT_GETOPT_M4SH_SETUP
@@ -121,7 +121,7 @@ AT_CLEANUP
 AT_SETUP([long option splitting])
 
 AT_DATA(expout,
-[[ltdl=long
+[[ ltdl=long
 ]])
 
 _LT_AT_GETOPT_M4SH_SETUP
@@ -137,7 +137,7 @@ AT_SETUP([XSI long option splitting])
 AT_CHECK([fgrep '# Extended-shell func_split_long_opt' 
$abs_top_builddir/libtool >/dev/null 2>&1 || (exit 77)])
 
 AT_DATA(expout,
-[[ltdl=long
+[[ ltdl=long
 ]])
 
 _LT_AT_GETOPT_M4SH_SETUP



Re: [PATCH] Add func_append_quoted and do inline func_append substitutions.

2010-08-03 Thread Ralf Wildenhues
* Peter Rosin wrote on Thu, Jul 01, 2010 at 07:38:45PM CEST:
> Den 2010-06-28 01:24 skrev Gary V. Vaughan:
> >@@ -704,15 +710,14 @@ func_mode_compile ()
> >   save_ifs="$IFS"; IFS=','
> >   for arg in $args; do
> > IFS="$save_ifs"
> >-func_quote_for_eval "$arg"
> >-lastarg="$lastarg $func_quote_for_eval_result"
> >+func_append lastarg " $arg"
> >   done
> >   IFS="$save_ifs"
> >   func_stripname ' ' '' "$lastarg"
> 
> Oh, and this look suspicious...
> 
> s/func_append/func_append_quoted/ ?

A change like you suggested was apparently done in v2.2.10-49-gc13532a,
but it is not right.  The quoting may not extend to the added space.

This causes the compile failure reported in
.

I'm fixing the issue as follows.  I'm seeing lots more testsuite
failures in the quoting section part with AIX and FreeBSD shells
though ...

Cheers,
Ralf

Fix build failure with AIX sh due to shell quoting error.

* libltdl/config/ltmain.m4sh (func_append_quoted): Document
that this function inserts a separator space.
(func_mode_compile): Do not pass extra space here.
Fixes regression introduced in v2.2.10-49-gc13532a.
Report by Rainer Tammer.

diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 897a5e8..193ff1a 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -157,7 +157,8 @@ func_append ()
 } # func_append may be replaced by extended shell implementation
 
 # func_append_quoted var value
-# Quote VALUE and append to the end of shell variable VAR.
+# Quote VALUE and append to the end of shell variable VAR, separated
+# by a space.
 func_append_quoted ()
 {
 func_quote_for_eval "${2}"
@@ -710,7 +711,7 @@ func_mode_compile ()
  save_ifs="$IFS"; IFS=','
  for arg in $args; do
IFS="$save_ifs"
-   func_append_quoted lastarg " $arg"
+   func_append_quoted lastarg "$arg"
  done
  IFS="$save_ifs"
  func_stripname ' ' '' "$lastarg"



Re: [RFT PATCH v3 3/9] add --with-sysroot

2010-08-03 Thread Ralf Wildenhues
* Paolo Bonzini wrote on Tue, Aug 03, 2010 at 10:04:08AM CEST:
> On 08/01/2010 12:48 PM, Ralf Wildenhues wrote:
> 
[ other packages that already use --with-sysroot ]

> >I'm not sure how to address these (not even sure
> >they use Libtool) except for a big sign in NEWS, and documentation in
> >the manual.
> 
> I can think of two ways:
> 
> 1) ignore --with-sysroot if not cross-compiling.  This however has a
> problem: if we test $cross_compiling = yes, sysroot tests will fail
> under Wine.

Yes, both that, and: I thought this patch series was meant to be usable
also for native compilation?

Distro packages much like to do things like that.

> 2) support both --with-sysroot and --with-host-sysroot, with the
> latter overriding the first.  Make GCC's toplevel always pass
> --without-host-sysroot to build modules, always pass
> --without-host-sysroot unless it was specified for host modules
> (this serves Canadian crosses), and skip --with-host-sysroot for
> target modules.  This is a hack, but one that can work reliably.

At this point you have me lost, simply because I can't claim to
understand all the details here.  So I can't give good advice.
My best bet would be to either trust you, or require some way to
set things up with GCC to try them out.  Doesn't have to be a
testsuite addition, just some recipe that works.


Also, I just noticed that recent GCC can just be passed --sysroot $D
and it will print $D upon --print-sysroot, so maybe if this is supported
it can be used for wider test exposure?  If you need GCC to support the
sysroot, that is.  And maybe symlink-tree will come in handy ... ?

Thanks,
Ralf



Re: [RFT PATCH v4 6/8] emit sysrooted paths when installing .la files

2010-08-03 Thread Ralf Wildenhues
* Paolo Bonzini wrote on Tue, Aug 03, 2010 at 09:52:54AM CEST:
> On 08/01/2010 08:31 PM, Ralf Wildenhues wrote:
> >Although it is fairly unlikely, I prefer not relying on :+ support, so
> >please replace with 'if test -z "$lt_sysroot"...'.  Two instances.
> 
> Is this still true in the light of the recent discussion on autoconf
> mailing lists?

No, I guess that's fine then.

Thanks,
Ralf



Re: [RFT PATCH v3 3/9] add --with-sysroot

2010-08-03 Thread Paolo Bonzini

On 08/01/2010 12:48 PM, Ralf Wildenhues wrote:

Actually, I don't see any other uses of target in the whole patch
series, so why not just omit that here?


All of this is dead code actually...


> +   AC_MSG_RESULT([$with_sysroot])
> +   lt_sysroot=`echo "$with_sysroot" | sed -e "$sed_quote_subst"`
> +   ;; #(
> + no)

Normalization of "no" to empty?  The rest of the series doesn't
special-case "no", and I think empty would please GCC configury, too
(untested).



I'm not sure how to address these (not even sure
they use Libtool) except for a big sign in NEWS, and documentation in
the manual.


I can think of two ways:

1) ignore --with-sysroot if not cross-compiling.  This however has a 
problem: if we test $cross_compiling = yes, sysroot tests will fail 
under Wine.


2) support both --with-sysroot and --with-host-sysroot, with the latter 
overriding the first.  Make GCC's toplevel always pass 
--without-host-sysroot to build modules, always pass 
--without-host-sysroot unless it was specified for host modules (this 
serves Canadian crosses), and skip --with-host-sysroot for target 
modules.  This is a hack, but one that can work reliably.


Paolo



Re: [RFT PATCH v4 6/8] emit sysrooted paths when installing .la files

2010-08-03 Thread Paolo Bonzini

On 08/01/2010 08:31 PM, Ralf Wildenhues wrote:

Although it is fairly unlikely, I prefer not relying on :+ support, so
please replace with 'if test -z "$lt_sysroot"...'.  Two instances.


Is this still true in the light of the recent discussion on autoconf 
mailing lists?


Paolo



Re: [RFT PATCH v4 0/8] Sysroot series

2010-08-03 Thread Paolo Bonzini

On 08/02/2010 07:55 PM, Ralf Wildenhues wrote:

* Charles Wilson wrote on Sun, Aug 01, 2010 at 10:04:16PM CEST:

support, use --prefix=/mingw + DESTDIR=/the-sysroot.


What about the w32 users that use --prefix=C:/mingw and then cannot use
DESTDIR because that will not concatenate?

Also, in 'ln -sf' in the testsuite addition, the -f is non-portable
to Solaris, and for DJGPP (cough) it would even need $(LN_S) but let's
ignore that for now.


I'm squashing this:

diff --git a/tests/sysroot.at b/tests/sysroot.at
index 0d98990..850e0cd 100644
--- a/tests/sysroot.at
+++ b/tests/sysroot.at
@@ -58,7 +58,9 @@ AT_CHECK([
 while read file; do
   dir=`local_dirname "$sysroot/$file"`
   test -d "$dir" || mkdir -p "$dir"
-  ln -sf "$gcc_sysroot/$file" "$sysroot/$file"
+  rm -f "$sysroot/$file"
+  ln -s "$gcc_sysroot/$file" "$sysroot/$file" || \
+cp "$gcc_sysroot/$file" "$sysroot/$file"
 done])

 LDFLAGS="$LDFLAGS --sysroot=$sysroot -no-undefined"




Re: [RFT PATCH v4 0/8] Sysroot series

2010-08-03 Thread Paolo Bonzini

On 08/03/2010 07:52 AM, Charles Wilson wrote:

On 8/2/2010 1:53 PM, Ralf Wildenhues wrote:

This should be fixed:

| ++ make all
| stderr:
| lib2.c: In function 'g':
| lib2.c:6:13: warning: initialization makes pointer from integer without a cast
| stdout:


This addendum appears to do it (Tested on cygwin->mingw cross. Note that
this implementation assumes for PE/COFF that auto-import support is
present.  This is safe since 2000.  No PE/COFF support for runtime
pseudo-relocs is necessary in these examples.  This is just a
long-winded way of saying we don't need to do the Dance Of The Declspecs.)

Paolo?


Wquashed, thanks.

Paolo