[arch-projects] [devtools] [PATCH 2/3] makechrootpkg: properly handle makepkg args

[Luke Shumaker]

At Sat, 10 May 2014 09:44:33 -0400,
Dave Reisner wrote:
> This allows handling of args with whitespace and other nonsense to be
> passed properly to makepkg. Contrived example:
> 
>   makechrootpkg -r /path/to/chroot -- --config "/path/to/some config"

Unless I'm missing something, this patch won't successfully do that.

> @@ -100,7 +100,7 @@ else
>  fi
>  
>  # Pass all arguments after -- right to makepkg
> -makepkg_args="$makepkg_args ${*:$OPTIND}"
> +makepkg_args=("${@:OPTIND}")

What about the previous value of makepkg_args?

> @@ -256,8 +256,9 @@ EOF
>  
>  	# This is a little gross, but this way the script is recreated every time in the
>  	# working copy
> +	printf -v extra_args ' %q' "${makepkg_args[@]}"
>  	printf $'#!/bin/bash\n%s\n_chrootbuild %q %q' "$(declare -f _chrootbuild)" \
> -		"$makepkg_args" "$run_namcap" >"$copydir/chrootbuild"
> +		"$extra_args" "$run_namcap" >"$copydir/chrootbuild"
>  	chmod +x "$copydir/chrootbuild"
>  }

I believe that the arguments are getting double-escaped; once by %q in
the first printf, then again in the second printf.  Also, this leaks
$extra_args as a global.

> @@ -320,7 +321,7 @@ _chrootbuild() {
>  		exit 1
>  	fi
>  
> -	sudo -u nobody makepkg $makepkg_args || exit 1
> +	sudo -u nobody makepkg "$makepkg_args" || exit 1

However extra_args was escaped, doesn't this basically ignore that,
and shove it all into one argument?

----

Interestingly, I was just getting ready to submit a patch also does
this:

From 35b2638d44de2b9033b8c71163c997c588ded6fa Mon Sep 17 00:00:00 2001
From: Luke Shumaker <LukeShu at sbcglobal.net>
Date: Sat, 10 May 2014 19:21:44 -0400
Subject: [PATCH] makechrootpkg: Store makepkg_args as an array.

To do this simply, instead of embedding makepkg_args in /chrootbuild, it
passes them as arguments to /chrootbuild.
---
 makechrootpkg.in | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/makechrootpkg.in b/makechrootpkg.in
index 6a99408..f646117 100644
--- a/makechrootpkg.in
+++ b/makechrootpkg.in
@@ -12,7 +12,8 @@ m4_include(lib/common.sh)
 
 shopt -s nullglob
 
-makepkg_args='-s --noconfirm -L --holdver'
+default_makepkg_args=(-s --noconfirm -L --holdver)
+makepkg_args=("${default_makepkg_args[@]}")
 repack=false
 update_first=false
 clean_first=false
@@ -46,7 +47,7 @@ usage() {
 	echo 'command:'
 	echo '    mkarchroot <chrootdir>/root base-devel'
 	echo ''
-	echo "Default makepkg args: $makepkg_args"
+	echo "Default makepkg args: ${default_makepkg_args[*]}"
 	echo ''
 	echo 'Flags:'
 	echo '-h         This help'
@@ -76,7 +77,7 @@ while getopts 'hcur:I:l:nTD:d:' arg; do
 		r) passeddir="$OPTARG" ;;
 		I) install_pkgs+=("$OPTARG") ;;
 		l) copy="$OPTARG" ;;
-		n) run_namcap=true; makepkg_args="$makepkg_args -i" ;;
+		n) run_namcap=true; makepkg_args+=('-i') ;;
 		T) temp_chroot=true; copy+="-$$" ;;
 	esac
 done
@@ -100,7 +101,7 @@ else
 fi
 
 # Pass all arguments after -- right to makepkg
-makepkg_args="$makepkg_args ${*:$OPTIND}"
+makepkg_args+=("${@:OPTIND}")
 
 # See if -R was passed to makepkg
 for arg in "${@:OPTIND}"; do
@@ -256,8 +257,8 @@ EOF
 
 	# This is a little gross, but this way the script is recreated every time in the
 	# working copy
-	printf $'#!/bin/bash\n%s\n_chrootbuild %q %q' "$(declare -f _chrootbuild)" \
-		"$makepkg_args" "$run_namcap" >"$copydir/chrootbuild"
+	printf $'#!/bin/bash\n%s\n_chrootbuild %q "$@"' "$(declare -f _chrootbuild)" \
+		"$run_namcap" >"$copydir/chrootbuild"
 	chmod +x "$copydir/chrootbuild"
 }
 
@@ -283,8 +284,8 @@ download_sources() {
 _chrootbuild() {
 	# This function isn't run in makechrootpkg,
 	# so no global variables
-	local makepkg_args="$1"
-	local run_namcap="$2"
+	local run_namcap="$1"; shift
+	local makepkg_args=("$@")
 
 	. /etc/profile
 	export HOME=/build
@@ -320,7 +321,7 @@ _chrootbuild() {
 		exit 1
 	fi
 
-	sudo -u nobody makepkg $makepkg_args || exit 1
+	sudo -u nobody makepkg "${makepkg_args[@]}" || exit 1
 
 	if $run_namcap; then
 		pacman -S --needed --noconfirm namcap
@@ -379,7 +380,7 @@ if arch-nspawn "$copydir" \
 	--bind-ro="$PWD:/startdir_host" \
 	--bind-ro="$SRCDEST:/srcdest_host" \
 	"${bindmounts_ro[@]}" "${bindmounts_rw[@]}" \
-	/chrootbuild
+	/chrootbuild "${makepkg_args[@]}"
 then
 	move_products
 else
-- 
1.9.2