[devtools] Expand check_root keepenv variables
diff mbox

Message ID 20180723221527.12071-1-foxboron@archlinux.org
State New
Headers show

Commit Message

Eli Schwartz via arch-projects July 23, 2018, 10:15 p.m. UTC
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.

Signed-off-by: Morten Linderud <foxboron@archlinux.org>
---
 archbuild.in    | 2 +-
 lib/archroot.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Eli Schwartz via arch-projects July 24, 2018, 2:08 a.m. UTC | #1
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.

Patch
diff mbox

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