Re: [lmi] [lmi-commits] odd/multiarch adf0725 04/10: Validate $LMI_COMPILER as well as $LMI_TRIPLET

[Vadim Zeitlin]

On Wed, 15 May 2019 23:55:58 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2019-05-15 22:44, Vadim Zeitlin wrote:
GC> > On Wed, 15 May 2019 17:59:55 -0400 (EDT) Greg Chicares <address@hidden> 
wrote:
[...]
GC> > GC> +foo
GC> > GC> +
GC> > GC>  unset -f foo
GC> > 
GC> >  And while I'm asking questions, why do we use this foo function? This is
GC> > actually 2 questions:
GC> > 
GC> > 1. Why do we use a function at all?
GC> 
GC> A comment says:
GC> 
GC> # Implemented as a function that runs and then erases itself, so that
GC> # sourcing this script changes the environment only as intended.

 I've somehow missed this comment, sorry!

GC> I can do local stuff inside the function, and the function's {} wrapper
GC> contains it, so that it won't leak out and pollute the environment.

 This is an interesting technique. I'm more used to the approach used by
various xxx-agent programs that output the variables they set on stdout and
then advise you to invoke them as "eval $(gpg-agent)" which ensures that
they don't modify the callers environment accidentally, but in practice I
don't see any problems with this one neither.

GC> > 2. Why does this function use this meaningless name?
GC> 
GC> I was thinking of changing it to 'f'. I like names that lack any
GC> possible lexical significance, where there's only one possible
GC> meaning: "actually do the thing this script is supposed to do".

 I still think the name is bad and "f" would be even worse because it can
easily conflict with a function with the same name defined in the caller
and while this should be pretty unlikely, the potential for confusion and
hair-pulling when it does happen IMO justifies taking some extra care to
reduce the disk of this happening even further (and FWIW, I did already
define a function called "foo" doing something temporary in my shell in the
past and I'd be pretty upset if it stopped working after sourcing some lmi
script -- especially considering the time it would take me to understand
that this was what happened).

 I.e. I'd rename this function to lmi_actually_export_vars() or something
even more unique.

 Regards,
VZ