| Message ID | 20180723221527.12071-1-foxboron@archlinux.org |
|---|---|
| State | Superseded |
| Headers | show |
On 07/23/2018 06:15 PM, Morten Linderud via arch-projects wrote: > From: Morten Linderud <morten@linderud.pw> > > makechrootpkg checks the environment for multiple variables before > overwriting them with makepkg.conf configurations. Expand check_root > with the variables makechrootpkg check for so we are capable of > overwriting them when needed. The two times we use check_root we're checking the environment for something decidedly not related to makepkg.conf at all? Not sure what this means. You're suggesting makechrootpkg does check for them, therefore we should expand check_root itself in order to check for them? How is it checking for them? ... The fact that it could use the values from the environment before makepkg.conf is technically an implementation detail due to the lazy-loaded way makechrootpkg will grep for the value (which is totally wrong and it should source the bash-compatible file), see in contrast how makepkg itself does a whole song-and-a-dance to preserve the values from the environment, source makepkg.conf, and restore the environment versions if available. I would not describe this commit message by the implementation method "expand check_root", but instead describe it as "allow makepkg.conf settings to be sourced from the environment too". > Signed-off-by: Morten Linderud <foxboron@archlinux.org> > --- > archbuild.in | 2 +- > lib/archroot.sh | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/archbuild.in b/archbuild.in > index 1e5b582..6f22b12 100644 > --- a/archbuild.in > +++ b/archbuild.in > @@ -39,7 +39,7 @@ while getopts 'hcr:' arg; do > esac > done > > -check_root SOURCE_DATE_EPOCH > +check_root SOURCE_DATE_EPOCH SRCDEST SRCPKGDEST PKGDEST LOGDEST MAKEFLAGS PACKAGER > > # Pass all arguments after -- right to makepkg > makechrootpkg_args+=("${@:$OPTIND}") > diff --git a/lib/archroot.sh b/lib/archroot.sh > index f279603..82276e2 100644 > --- a/lib/archroot.sh > +++ b/lib/archroot.sh > @@ -10,7 +10,7 @@ CHROOT_VERSION='v4' > ## > orig_argv=("$0" "$@") > check_root() { > - local keepenv=$1 > + local keepenv=$(tr " " "," <<< $@) Why is it necessary to post-process the arguments, when every other user simply passes them comma-separated in the first place? When I wrote this code, I intended it to maintain sudo-like semantics rather than require some preprocessor to fork a subshell and exec a binary, introduce word splitting, and so forth... just to fix whitespace in the arguments that should never have been added in the first place.
diff --git a/archbuild.in b/archbuild.in index 1e5b582..6f22b12 100644 --- a/archbuild.in +++ b/archbuild.in @@ -39,7 +39,7 @@ while getopts 'hcr:' arg; do esac done -check_root SOURCE_DATE_EPOCH +check_root SOURCE_DATE_EPOCH SRCDEST SRCPKGDEST PKGDEST LOGDEST MAKEFLAGS PACKAGER # Pass all arguments after -- right to makepkg makechrootpkg_args+=("${@:$OPTIND}") diff --git a/lib/archroot.sh b/lib/archroot.sh index f279603..82276e2 100644 --- a/lib/archroot.sh +++ b/lib/archroot.sh @@ -10,7 +10,7 @@ CHROOT_VERSION='v4' ## orig_argv=("$0" "$@") check_root() { - local keepenv=$1 + local keepenv=$(tr " " "," <<< $@) (( EUID == 0 )) && return if type -P sudo >/dev/null; then