[arch-projects] [devtools] [PATCH] Expand check_root keepenv variables

[Eli Schwartz]

On 07/23/2018 06:15 PM, Morten Linderud via arch-projects wrote:
> From: Morten Linderud <morten at 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 at 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.

-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/arch-projects/attachments/20180723/98db5642/attachment.asc>