lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Concinnity check and hardcoded paths


From: Vadim Zeitlin
Subject: Re: [lmi] Concinnity check and hardcoded paths
Date: Sat, 17 Oct 2020 17:48:56 +0200

On Sat, 17 Oct 2020 15:05:00 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 2020-10-16 22:31, Vadim Zeitlin wrote:
GC> > On Tue, 13 Oct 2020 01:11:44 +0000 Greg Chicares 
<gchicares@sbcglobal.net> wrote:
GC> > 
GC> > GC> The widespread assumption that 'prefix="/opt/lmi"' is probably harder
GC> > GC> to alleviate (although I think we may be getting close),
GC> > 
GC> >  I am not sure if you want to deal with this at this time, but if it could
GC> > be helpful, I can easily make a hopefully exhaustive list of places that
GC> > would need to be changed to solve it. But I suspect you have more 
important
GC> > things to do right now, so I won't sent such list unsolicited.
GC> 
GC> You're welcome to send it whenever you like. I probably won't be
GC> ready to look at it before December at the earliest.

 OK, I've added it to the "to submit" column of my TODO list (which is more
like a 3D matrix by now...), but there are currently a few items in front o
it.

GC> 
GC> > GC> Anyway, I think I should make this change everywhere:
GC> > GC> 
GC> > GC>   . ./lmi_setup_inc.sh
GC> > GC> + # shellcheck disable=SC1091
GC> > GC>   . /tmp/schroot_env
GC> > 
GC> >  Thanks for making it, this does help with not having this particular 
file.
GC> > In the interest of, dare I say it, concinnity, I wonder if this warning
GC> > should also be suppressed for the line
GC> > 
GC> >   . /opt/lmi/src/lmi/set_toolchain.sh
GC> > 
GC> > in lmi_setup_43.sh as this would solve another similar problem. Of course,
GC> 
GC> Done.

 Thanks!

GC> > IMHO a better solution would be to just use ". ./set_toolchain.sh" here,
GC> > but this might be taking us into the unsolicited territory, as mentioned
GC> > above...
GC> 
GC> If you're at that line in vim and do '?cd', you'll see
GC>   cd proprietary || { printf 'failed: cd\n'; exit 3; }
GC> about twenty lines earlier--so './' wouldn't find this script.

 Oops, sorry for missing this.

GC> 'transume_toolchain.sh' also sources another script. I've been
GC> contemplating several changes there, but can't persuade myself
GC> that any of them deserve to be committed, so let me show you
GC> what I've stashed aside and ask what you think:
GC> 
GC> --8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--
GC> diff --git a/transume_toolchain.sh b/transume_toolchain.sh
GC> index 672bda12..c61218af 100755
GC> --- a/transume_toolchain.sh
GC> +++ b/transume_toolchain.sh
GC> @@ -23,10 +23,41 @@
GC>  
GC>  # See 'GNUmakefile' for suggested use.
GC>  
GC> -. ./set_toolchain.sh
GC> +# The script here sourced, 'set_toolchain.sh', should reside in the
GC> +# same directory as this script, i.e.,
GC> +#   "$(dirname "$(readlink -f "$0")")"
GC> +# It's hard to imagine that it wouldn't, but should the possibility
GC> +# be guarded against? Is the dirname-readlink obscurity warranted?
GC> +#
GC> +# shellcheck disable=SC1090
GC> +. "$(dirname "$(readlink -f "$0")")"/set_toolchain.sh

 I think this one is worth it, but I'd define "this_script_directory"
variable at the top of the script (or maybe even in lmi_setup_inc.sh as I
think this variable might be useful elsewhere too) rather than just using
it here.

GC> -printf '%s\n' "export LMI_COMPILER := $LMI_COMPILER"
GC> -printf '%s\n' "export LMI_TRIPLET  := $LMI_TRIPLET"
GC> -printf '%s\n' "export PATH         := $PATH"
GC> -printf '%s\n' "export WINEPATH     := $WINEPATH"
GC> -printf '%s\n' "export PERFORM      := $PERFORM"
GC> +# Should $LD_LIBRARY_PATH be treated the same as $WINEPATH both
GC> +# here and in 'GNUMakefile'?

 It would make sense to me to treat them symmetrically, although this is
not terribly important in practice.

GC> +# Is it important to avoid writing
GC> +#   export FOO=bar
GC> +# tersely on a single line, which POSIX doesn't strictly support?

 IME no. I.e. I don't know of any shell in use on any current system where
this wouldn't work.

GC> +printf '%s\n' "LMI_COMPILER=$LMI_COMPILER"
GC> +printf '%s\n' "LMI_TRIPLET=$LMI_TRIPLET"
GC> +printf '%s\n' "PATH=$PATH"
GC> +printf '%s\n' "WINEPATH=$WINEPATH"
GC> +printf '%s\n' "PERFORM=$PERFORM"
GC> +
GC> +printf '%s\n' "export LMI_COMPILER"
GC> +printf '%s\n' "export LMI_TRIPLET"
GC> +printf '%s\n' "export PATH"
GC> +printf '%s\n' "export WINEPATH"
GC> +printf '%s\n' "export PERFORM"
GC> +
GC> +# More compact, but perhaps less scrutable:
GC> +#
GC> +# for z in LMI_COMPILER LMI_TRIPLET PATH WINEPATH PERFORM
GC> +#   do
GC> +#     # Avoid SC2154 with empty assignment:
GC> +#     value=
GC> +#     eval "value=\${$z}"
GC> +#     printf '%s\n' "export $z"
GC> +#     printf '%s\n' "$z=$value"
GC> +#   done

 This seems tempting. Normally the rule is to not use "eval" unless
necessary and it's not really required here, but it's nice to treat all the
variables uniformly. OTOH SC2154 workaround is ugly... Perhaps we should go
"full eval" if we use it anyhow and move "printf"s inside it too then? Of
course, not changing anything is a perfectly valid option too here.

 Thanks for looking at this,
VZ

Attachment: pgpk6lVE74ETK.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]