| Message ID | b7b079f559a17b6d6cef037afd6ce023df8f90b0.1585832270.git.congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Travis + Azure jobs for linux with musl libc | expand |
On Thu, Apr 02, 2020 at 08:04:01PM +0700, Đoàn Trần Công Danh wrote: > We're using "su -m" to preserve environment variables in the shell run > by "su". But, that options will be ignored while "-l" (aka "--login") is > specified. This is not true. See any previous runs of the 32 bit Linux job, which worked as expected, because none of these environment variables were cleared. > Since we don't have interest in all environment variables, > pass only those necessary variables to the inner script. > > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > ci/run-linux32-build.sh | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh > index e3a193adbc..7f985615c2 100755 > --- a/ci/run-linux32-build.sh > +++ b/ci/run-linux32-build.sh > @@ -51,10 +51,17 @@ else > fi > > # Build and test > -linux32 --32bit i386 su -m -l $CI_USER -c ' > +linux32 --32bit i386 su -m -l $CI_USER -c " > set -ex > + export DEVELOPER='$DEVELOPER' > + export DEFAULT_TEST_TARGET='$DEFAULT_TEST_TARGET' > + export GIT_PROVE_OPTS='$GIT_PROVE_OPTS' > + export GIT_TEST_OPTS='$GIT_TEST_OPTS' > + export GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB' > + export MAKEFLAGS='$MAKEFLAGS' > + export cache_dir='$cache_dir' > cd /usr/src/git > - test -n "$cache_dir" && ln -s "$cache_dir/.prove" t/.prove > + test -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove > make > make test > -' > +" > -- > 2.26.0.334.g6536db25bb >
On 2020-04-03 10:22:54+0200, SZEDER Gábor <szeder.dev@gmail.com> wrote: > On Thu, Apr 02, 2020 at 08:04:01PM +0700, Đoàn Trần Công Danh wrote: > > We're using "su -m" to preserve environment variables in the shell run > > by "su". But, that options will be ignored while "-l" (aka "--login") is > > specified. > > This is not true. See any previous runs of the 32 bit Linux job, > which worked as expected, because none of these environment variables > were cleared. Different su have different behavior when combine "-m" and "-l" util-linux's su has this as far as 60541961f, (docs: improve grammar, wording and formatting of su man page, 2013-10-12) -m, -p, --preserve-environment Preserve the entire environment, i.e., it does not set HOME, SHELL, USER nor LOGNAME. This option is ignored if the option --login is specified. https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/login-utils/su.1#n120 Ubuntu (our Linux32 builder), ships su by shadow-utils: Note that the default behavior for the environment is the following: The $HOME, $SHELL, $USER, $LOGNAME, $PATH, and $IFS environment variables are reset. If --login is not used, the environment is copied, except for the variables above. If --login is used, the $TERM, $COLORTERM, $DISPLAY, and $XAUTHORITY environment variables are copied if they were set. There're no mentions of other variables, I _think_ our Linux32 works by accident. Alpine ships su from busybox, this su ignores "-m" if "-l" is set.
On Fri, Apr 03, 2020 at 05:09:31PM +0700, Danh Doan wrote: > On 2020-04-03 10:22:54+0200, SZEDER Gábor <szeder.dev@gmail.com> wrote: > > On Thu, Apr 02, 2020 at 08:04:01PM +0700, Đoàn Trần Công Danh wrote: > > > We're using "su -m" to preserve environment variables in the shell run > > > by "su". But, that options will be ignored while "-l" (aka "--login") is > > > specified. > > > > This is not true. See any previous runs of the 32 bit Linux job, > > which worked as expected, because none of these environment variables > > were cleared. > > Different su have different behavior when combine "-m" and "-l" > > util-linux's su has this as far as 60541961f, (docs: improve grammar, > wording and formatting of su man page, 2013-10-12) > > -m, -p, --preserve-environment > Preserve the entire environment, i.e., it does not set HOME, > SHELL, USER nor LOGNAME. This option is ignored if the option > --login is specified. > > https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/login-utils/su.1#n120 > > Ubuntu (our Linux32 builder), ships su by shadow-utils: > > Note that the default behavior for the environment is the following: > The $HOME, $SHELL, $USER, $LOGNAME, $PATH, > and $IFS environment variables are reset. > > If --login is not used, the environment is copied, > except for the variables above. > > If --login is used, the $TERM, $COLORTERM, $DISPLAY, and > $XAUTHORITY environment variables are copied if they were set. > > There're no mentions of other variables, I _think_ our Linux32 works > by accident. We do know which image we use, and we do know how its 'su' behaves. I think relying on that is fine, and it's not just "works by accident". > Alpine ships su from busybox, this su ignores "-m" if "-l" is set. Then the commit message should specifically mention these behavior differences between different 'su' variants.
diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh index e3a193adbc..7f985615c2 100755 --- a/ci/run-linux32-build.sh +++ b/ci/run-linux32-build.sh @@ -51,10 +51,17 @@ else fi # Build and test -linux32 --32bit i386 su -m -l $CI_USER -c ' +linux32 --32bit i386 su -m -l $CI_USER -c " set -ex + export DEVELOPER='$DEVELOPER' + export DEFAULT_TEST_TARGET='$DEFAULT_TEST_TARGET' + export GIT_PROVE_OPTS='$GIT_PROVE_OPTS' + export GIT_TEST_OPTS='$GIT_TEST_OPTS' + export GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB' + export MAKEFLAGS='$MAKEFLAGS' + export cache_dir='$cache_dir' cd /usr/src/git - test -n "$cache_dir" && ln -s "$cache_dir/.prove" t/.prove + test -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove make make test -' +"
We're using "su -m" to preserve environment variables in the shell run by "su". But, that options will be ignored while "-l" (aka "--login") is specified. Since we don't have interest in all environment variables, pass only those necessary variables to the inner script. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- ci/run-linux32-build.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)