Hi Ralf,
On Fri, Aug 26, 2005 at 10:38:46AM +0200, Ralf Wildenhues wrote:
> * 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:
> > 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'
ACK.
> 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 agree with your analysis. In the links I posted, there was no actual bug,
only the appearance of one. I do not know libtool well enough to guess whether
it has bugs of this nature.
This is orthogonal to the bug yielding the output problem you reported.
> > 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?
Yes.
> It's how things have always been in the libtool script.
Okay; I studied branch-2-0, and you are right. The `eval' is essentially safe,
because the foo_cmds that would otherwise break are written to expect two levels
of `eval'uation. Er, sort of; most commands don't care. finish_cmds does care
and appears to work by near-accident. Cygwin postinstall_cmds does care and
seems to work by design. You seem to be saying that this dual-`eval'uation,
though longstanding, is harmful.
> > > 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.
ACK.
> > > > 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.
ACK.
> > > 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?
Even if one could write such a thing, it seems better to remove the extra level
of evaluation, for that also makes libtool.m4 easier to write and read.
> > 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. :-/
Here, I refer to my lack of `sed' expertise. I would surely write it in shell.
> > 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.
I was fooled. At least 1/3 of the call sites are affected.
---
Let me try to summarize the situation. In branch-2-0, we use this idiom:
cmd=\"$cmd\"
$show "$cmd"
eval "$cmd" || exit $?
In HEAD, we use this idiom:
cmd=\"$cmd\"
func_show_eval "$cmd" "${2-:}"
Both run the same commands for a given $cmd. The level of evaluation did not
change. What did change is the output we produce for a given $cmd. The HEAD
output code is certainly dissatisfying, particularly because it actively
misleads when it mistreats a `$' as starting a parameter expansion.
I understand you to say that we should change everything to evaluate each
command once and selectively evaluate other variables to keep things correct.
Where we currently do something like this,
# libtool.m4
foo_cmd='\$tool \$flag'
flag='${wl}-whatever'
wl=-Wl,
# ltmain.m4sh
eval cmd=\"$foo_cmd\"
$show "$cmd"
$run eval "$cmd" || exit $?
we should instead write this:
# libtool.m4
foo_cmd='$tool $flag'
flag='${wl}-whatever'
wl=-Wl,
# ltmain.m4sh
eval flag=\"$flag\"
...
cmd="$foo_cmd"
$show "$cmd"
$run eval "$cmd" || exit $?
The example is in the style of branch-2-0, but the conversion to HEAD-style is
obvious and purely cosmetic. Also, the example is entirely fictitious, because
the good real example, the Cygwin postinstall_cmds, is too hard on my brain.
How fully does that summary intersect your understanding?
