Re: Feature request: setting env vars for binary wrappers

[Gary V. Vaughan]

Hallo Ralf,

[adding bug-m4, and quoting generously to remind us to address the
 issues you raise]

Ralf Wildenhues wrote:
> * Gary V. Vaughan wrote on Thu, Jun 15, 2006 at 12:57:42PM CEST:
>> Ralf Wildenhues wrote:
>>> So could you please list the technical problems you see with this patch,
>>> so I can fix them, if any?
>> Starting to add new features on this side of the 2.0 release is like
>> the road to hell (paved with good intentions, yet doesn't lead to
>> somewhere we want to be).
> 
> Yes.  You already mentioned that, and I did not object.  But you
> mentioned that *in addition to* some idiosyncracies, and I wanted to
> know what they were.

ACK.

>> I don't strongly object to the patch.  However, CVS M4 achieves exactly
>> this in a much more elegant fashion by simply including its own wrapper
>> script to set the module load path for uninstalled m4 to load modules
>> from the build tree.  It also sanitises the error output to make writing
>> tests easier (something libtool wrapper scripts shouldn't do), and can
>> be easily extended without adding complexity to the libtool code.
> 
> So you think having a wrapper for a wrapper is a better general solution
> to the problem?  Well, if I can tell correctly, this is what Behdad and
> Bob and everyone has been using so far.
> 
>>   #! /bin/sh
>>   # @configure_input@
>>   # Wrapper around a non installed m4 to make it work as an installed one.
>>
>>   "@abs_top_builddir@/src/m4" \
>>       --module-directory="@abs_top_builddir@/modules" \
> 
> This part is fine, and can be achieved by my patch.

Indeed.

>>       ${1+"$@"} 2>/tmp/m4-$$
> 
> This is very very unfortunate for debugging.  I won't see the output
> when it appears, which is sometimes crucial for a debug session.
> Besides the fact that this line is a rampant security hazard
> (I do `ln -s /tmp/m4-1234 /home/gary/some/important/file' for a few
> probably PIDs, and you'll be sorry).

Good point.  However, the intent of this wrapper was to centralize
and simplify the code in the autotest suite to work equally with
installed and uninstalled m4, so timeliness of stderr isn't an issue.

I don't suppose many people will go to the effort of breaking in to
my machine just to overwrite a few files in my home directory.  However,
now that we've advertised the security hole... best to close it :-D

>>   status=$?
> 
> Using the variable status is not portable, as is using ${1+"$@"} without
> taking care of zsh.

I wasn't aware of the former,  I need to read up more on zsh.  Thanks.

>>   sed 's,^[^:]*[lt-]*m4[.ex]*:,m4:,' /tmp/m4-$$ >&2
> 
> I intend to fix the changed naming in libtool eventually, if possible.
> (Using an executable wrapper is one possibility that would work for many
> systems; having the relinked binary with the same name, but in another
> subdirectory helps, too; for bash, we could cheaply use "exec -a"; but
> all this is work still to do...)
> 
> So all I see with this wrapper is a definite advantage for fixing things
> *inside* the libtool wrapper, and not moving this work to third parties.

I'm still somewhat ambivalent.   My only reservation (and it isn't
a strong one admittedly) is that there will always be cases where
libtool doesn't (yet) cover various useful things one might like
to do in a shell wrapper... and this simple m4 wrapper alone has
shown us a few more we haven't tackled yet.  Surely there will be
many others...

Perhaps putting the common ones into libtool, but also documenting
the wrapper script technique for users who want to do uncommon things
that lend themselves to this approach?

Cheers,
        Gary.
-- 
Gary V. Vaughan      ())_.  address@hidden,gnu.org}
Research Scientist   ( '/   http://blog.azazil.net
GNU Hacker           / )=   http://trac.azazil.net/projects/libtool
Technical Author   `(_~)_   http://sources.redhat.com/autobook

signature.asc
Description: OpenPGP digital signature