| Message ID | 20230526160824.655279-5-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/12] tests/docker: simplify HOST_ARCH definition | expand |
26.05.2023 19:08, Paolo Bonzini wrote: .. > # Unset some variables known to interfere with behavior of common tools, > -# just as autoconf does. > -CLICOLOR_FORCE= GREP_OPTIONS= > -unset CLICOLOR_FORCE GREP_OPTIONS > +# just as autoconf does. Unlike autoconf, we assume that unset exists. > +unset CLICOLOR_FORCE GREP_OPTIONS BASH_ENV ENV MAIL MAILPATH CDPATH Wonder how relevant all this is. gnu grep (from coreutils) does not document and does not use $GREP_OPTIONS. While $BASH_ENV is relevant for non-interactive mode (ie, when running as a script), but this variable a) is not used when it is invoked as sh (as opposed to bash), and b) it is a way to pass actual additional configuration to the shell, -- we do not override $PATH, do we? So why we override $BASH_ENV? For example, with $BASH_ENV, one can turn on tracing of shell functions, which is nearly impossible now when everything is run from within meson. Ditto for $ENV. Others - MAIL and MAILPATH? - those are only relevant for interactive usage, and only when mail actually goes to /var/mail/$user (or equivalent), it does not matter for scripts at all. CLICOLOR_FORCE is interesting, and it was there before already. It looks like whomever set that, don't really care about things like ./configure failing due to grep et al trying to color-paint its output. This variable shouldn't be used normally, it smells like a single-use thing - eg, to force color when output is displayed within less(1), or when grepping output but keeping colors. If it is set in environment before ./configure is run, it's not our fault. Now we come to CDPATH. But even there, it should not contain something else besides "." (current dir) as the first element, it's kinda interesting when CDPATH has something else in there. We've been here for like decades, and this is the first time we've hit this. Do we _really_ need to reset all this? Especially the $ENV and $BASH_ENV thing, which are useful.. But oh well :) /mjt
Michael Tokarev <mjt@tls.msk.ru> wrote: > 26.05.2023 19:08, Paolo Bonzini wrote: > .. >> # Unset some variables known to interfere with behavior of common tools, >> -# just as autoconf does. >> -CLICOLOR_FORCE= GREP_OPTIONS= >> -unset CLICOLOR_FORCE GREP_OPTIONS >> +# just as autoconf does. Unlike autoconf, we assume that unset exists. >> +unset CLICOLOR_FORCE GREP_OPTIONS BASH_ENV ENV MAIL MAILPATH CDPATH > > Wonder how relevant all this is. > > gnu grep (from coreutils) does not document and does not use $GREP_OPTIONS. > > While $BASH_ENV is relevant for non-interactive mode (ie, when running as > a script), but this variable a) is not used when it is invoked as sh (as > opposed to bash), and b) it is a way to pass actual additional configuration > to the shell, -- we do not override $PATH, do we? So why we override $BASH_ENV? > For example, with $BASH_ENV, one can turn on tracing of shell functions, which > is nearly impossible now when everything is run from within meson. > Ditto for $ENV. > > Others - MAIL and MAILPATH? - those are only relevant for interactive usage, > and only when mail actually goes to /var/mail/$user (or equivalent), it does > not matter for scripts at all. > > CLICOLOR_FORCE is interesting, and it was there before already. It looks like > whomever set that, don't really care about things like ./configure failing due > to grep et al trying to color-paint its output. This variable shouldn't be used > normally, it smells like a single-use thing - eg, to force color when output is > displayed within less(1), or when grepping output but keeping colors. If it > is set in environment before ./configure is run, it's not our fault. > > Now we come to CDPATH. But even there, it should not contain something else besides > "." (current dir) as the first element, it's kinda interesting when CDPATH has > something else in there. We've been here for like decades, and this is the > first time we've hit this. > > Do we _really_ need to reset all this? Especially the $ENV and $BASH_ENV thing, > which are useful.. CDPATH -> It broke my setup, I have had this on my .bashrc since the 90's: export CDPATH=.:~/work:/scratch/ For the rest, I don't know. But if autoconf disables them, some weird system, somewhere in the world makes this fail. Later, Juan.
diff --git a/configure b/configure index 80ca1c922151..9cdce69b7852 100755 --- a/configure +++ b/configure @@ -4,9 +4,8 @@ # # Unset some variables known to interfere with behavior of common tools, -# just as autoconf does. -CLICOLOR_FORCE= GREP_OPTIONS= -unset CLICOLOR_FORCE GREP_OPTIONS +# just as autoconf does. Unlike autoconf, we assume that unset exists. +unset CLICOLOR_FORCE GREP_OPTIONS BASH_ENV ENV MAIL MAILPATH CDPATH # Don't allow CCACHE, if present, to use cached results of compile tests! export CCACHE_RECACHE=yes
Apart from CLICOLOR_FORCE and GREP_OPTIONS, there are other variables that are listed in the Autoconf manual. While Autoconf neutralizes them very early, and assumes it does not (yet) run in a shell that has "unset", QEMU assumes that the user invoked configure under a POSIX shell, and therefore can simply use "unset" to clear them. CDPATH is particularly nasty because it messes up "cd ... && pwd". Reported-by: Juan Quintela <quintela@redhat.com> Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- configure | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)