Re: [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 <luke...@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