Re: HEAD: func_show_eval shell expansion issue

[Ralf Wildenhues]

Hi Noah,

* Noah Misch wrote on Fri, Aug 26, 2005 at 11:59:50PM CEST:
> 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 I have a tag variable like (quoted as in the beginning of the libtool
> > script):
> >   foo_cmds="\$CC \$some_flag -o \$output ..."

> >   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'm glad we agree on both of these accounts.

> > > > Not in all the
> > > >   cmds=$foobar_cmds
> > > >   eval cmds=\"$cmds\"
> > > >   ..
> > > >   IFS='~'
> > > >   func_show_eval "$cmds"
> > > >  ..

> > > 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.

Likely harmful. One thing I'm trying to say is: the number of evals
a (one specific!) command undergoes must be fixed.  And further, any
contained parameter expansions must have the quoting/expansion level
necessary for the exact amount of remaining evaluation levels.  Or
we must know that the contents are non-problematic.

I have not checked yet whether all commands would work with, say, those
two levels of evaluation.  It would surely be good if that were uniform
in some way over all the commands passed from libtool.m4 to libtool in
the foo_cmds parameters, but if not, that is not really an issue --
it can always be made clear by an appropriate naming scheme.

Variables like $ECHO or $Xsed _rely_ on not being over-expanded on some
systems (e.g. printf '%s\n' or the fact that the shell dislikes unquoted
`^').

> > > > 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 think so, too.

> > > 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.

Oh, I'd like to avoid the need for quoting functions as much as
possible.  Like somebody who recently said here "options are easily
added, painfully substracted" one could also say: "quoting is easily
removed, painfully added".

> > > 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.

OK.

> Let me try to summarize the situation.  In branch-2-0, we use this idiom:
> 
>   cmd=\"$cmd\"

you forgot an eval here:
  eval cmd=\"$cmd\"

>   $show "$cmd"
>   eval "$cmd" || exit $?
> 
> In HEAD, we use this idiom:
> 
>   cmd=\"$cmd\"
>   func_show_eval "$cmd" "${2-:}"

ACK.

> 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.

ACK, on both counts.

> 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 don't do this currently.  We write
  foo_cmd="\$tool \$flag"
and just pray the extra eval won't kill us.

> 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 $?

Sort of.  At least in case foo_cmds contains parameters which contain
user-supplied values (for example object file names), this would be a
sensible thing to do.

> 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.

I do realize that, should this quest ever be pursued in practice, that a
decent way to measure test coverage is needed.

> How fully does that summary intersect your understanding?

Quite well, see comments above.  Thanks!

Cheers,
Ralf