Re: HEAD: func_show_eval shell expansion issue

[Ralf Wildenhues]

* Noah Misch wrote on Fri, Aug 26, 2005 at 03:46:51AM CEST:
> On Thu, Aug 25, 2005 at 04:54:06PM +0200, Ralf Wildenhues wrote:
> > * Noah Misch wrote on Thu, Aug 25, 2005 at 03:42:34PM CEST:
> > > On Thu, Aug 25, 2005 at 08:41:12AM +0200, Ralf Wildenhues wrote:
> > > 
> > > > There's one thing I still don't understand: before, we had a bunch of
> > > >   $echo "$cmd"
> > > >   $run eval "$cmd"
> > > > 
> > > > which we now replaced with func_show_eval, which does
> > > > func_quote_for_expand only to have yet another eval in the echoing path.
> > > > 
> > > > Why not just drop func_quote_for_expand and the extra eval?
> > > > (I'm pretty sure I'm overlooking something; it's that I'd like to know
> > > > what I am overlooking.)
> > > 
> > > The hope was to expand parameters in the command for the user.
> > 
> > What?  Where should this be necessary?
> 
> It improves output for code like this:
> 
> flags='-Wl,bad"char'
> foo_commands="gcc \$flags -c 'foo\$bar.c'"
> func_show_eval "$foo_commands"
> 
> If you simply echo the commands, you see `gcc $flags -c foo$bar.c'.  With a
> robust func_quote_for_expand implementation, you see `gcc -Wl,bad"char -c
> foo$bar.c', which is more descriptive.

That is not what you want to see!  Users want something
copy-and-paste-able.  In your example, they want
  gcc -Wl,bad\"char -c foo\$bar.c
or 
  gcc "-Wl,bad\"char" -c "foo\$bar.c"
or even
  gcc '-Wl,bad"char' -c 'foo$bar.c'

I am not talking about this issue!  It is closely connected, but the
issue to which you posted links is a different one:

If I have a tag variable like (quoted as in the beginning of the libtool
script):
  foo_cmds="\$CC \$some_flag -o \$output ..."

and also I have
  some_flag="\${wl}-whatever"

then, before I start any evaluation on $foo_cmds, I _need_ to eval some
variation of $some_flag once, so that pesky ${wl} is replaced.  I cannot
achieve this by evaluation of $foo_cmds once more than would otherwise
be necessary, because that might just destroy other parts of the command
line.

> I do not know how many func_show_eval call sites `eval' strings bearing
> parameter expansions, and thereby benefit from this.
> 
> > Not in all the
> >   cmds=$foobar_cmds
> >   eval cmds=\"$cmds\"
> >   ..
> >   IFS='~'
> >   func_show_eval "$cmds"
> >  ..
> > 
> > 
> > loops, they aren't.  And in fact, should libtool ever support objects
> > with dollar signs in their names, they mustn't either (surely this is
> > post next stable release).
> 
> Yes; that `eval' looks shaky.

Which one?  The one that happens logically first?
It's how things have always been in the libtool script.

> > > http://lists.gnu.org/archive/html/libtool-patches/2004-10/msg00264.html
> > > http://lists.gnu.org/archive/html/libtool/2004-10/msg00081.html
> > 
> > Well, easy!  The bug is that $allow_undefined_flag needs to be expanded
> > once, before issuing the command.  But only that flag needs to be
> > expanded, and not the whole command line!
> >   eval "allow_undefined_flag=\"$allow_undefined_flag\""
> 
> This is syntactically invalid if $allow_undefined_flag, is, say, `foo"bar'.  
> It
> silently does the wrong thing on `-Wl,'strange$flag''.  func_quote_for_expand
> aims to handle cases like those.

Heck, then we _may_ need to expand-quote $allow_undefined_flag before
this eval.  But we still need to do this evaluation on this flag alone!

And besides: we have control over the possible contents of
$allow_undefined_flag.  We can _know_ whether it may contain any fun
variables.  Similarly with most other tag variables that need this sort
of treatment.

> > > What do you mean, different levels of expanded-ness?
> > 
> > Just the above.  Having $cmds eval'ed once in the main code *and* once
> > in func_show_eval is wrong.  You can't find a regex or sed script
> > that'll undo that and keep you cozy and warm unless you make assumptions
> > about what the user gives you on the command line.  Not if you allow 
> > any kinds of fun stuff in $libobjs, e.g., `$' in object file names.
> > 
> > Would you agree with this judgement?
> 
> Yes, I bet the double eval is wrong.  No, I think someone could make a `sed'
> script that still does the right thing.

Maybe.  But the script snippet you posted as well makes me fear this
will have a significant time overhead.  :-/

> Not me, though.

Yep, I understood your other post.  :-/

> Note that only a handful of call sites have the double eval.

Don't be fooled, there are quite a bit of them, only I wrapped them
through func_execute_cmds in HEAD.

Cheers,
Ralf