[pacman-dev] [PATCH 2/2] Save path to Pacman, which could be lost during --syncdeps operation

[Dave Reisner]

On Wed, Oct 31, 2012 at 07:06:13AM +0000, Martin Panter wrote:
> Unmangled version: https://github.com/vadmium/pacman-arch/commit/b984a8b.patch
> 
> Turns out it wasn’t too hard to allow spaces in the path returned from
> “command -v”, while handling any extra arguments.
> 
> From b984a8b2cb3daa05177420d1a9d83648d6c0aa0d Mon Sep 17 00:00:00 2001
> From: Martin Panter <vadmium à gmail·com>
> Date: Wed, 31 Oct 2012 02:45:36 +0000
> Subject: [PATCH] Save path to Pacman, which could be lost during --syncdeps
>  operation
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Access to the Pacman command could be lost when /etc/profile is invoked and
> $PATH is reset. Tested to make sure the following scenarios behave sensibly,
> where “roopwn” is in ~/bin/:
> 
> PACMAN="pacman --config /etc/pacman.conf" makepkg --syncdeps --install
> PACMAN="nonexistent --dummy" makepkg --syncdeps --install
> PACMAN="nonexistent --dummy" makepkg
> PACMAN="roopwn --verbose" makepkg --syncdeps --install

NAK'ing this as well, based on feedback from the prior patch. This is
the wrong approach to extending invocations to pacman.

> ---
>  scripts/makepkg.sh.in | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 3b4f27f..2b224b8 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -873,9 +873,9 @@ source_has_signatures() {
>  run_pacman() {
>  	local cmd
>  	if [[ ! $1 = -@(T|Qq) ]]; then
> -		cmd=($PACMAN $PACMAN_OPTS "$@")
> +		cmd=("${PACMAN[@]}" $PACMAN_OPTS "$@")
>  	else
> -		cmd=($PACMAN "$@")
> +		cmd=("${PACMAN[@]}" "$@")
>  	fi
>  	if (( ! ASROOT )) && [[ ! $1 = -@(T|Qq) ]]; then
>  		if type -p sudo >/dev/null; then
> @@ -898,7 +898,7 @@ check_deps() {
>  	if (( ret == 127 )); then #unresolved deps
>  		printf "%s\n" "$pmout"
>  	elif (( ret )); then
> -		error "$(gettext "'%s' returned a fatal error (%i): %s")" "$PACMAN"
> "$ret" "$pmout"
> +		error "$(gettext "'%s' returned a fatal error (%i): %s")"
> "${PACMAN[*]}" "$ret" "$pmout"
>  		return "$ret"
>  	fi
>  }
> @@ -920,7 +920,7 @@ handle_deps() {
>  		msg "$(gettext "Installing missing dependencies...")"
> 
>  		if ! run_pacman -S --asdeps $deplist; then
> -			error "$(gettext "'%s' failed to install missing dependencies.")" "$PACMAN"
> +			error "$(gettext "'%s' failed to install missing dependencies.")"
> "${PACMAN[*]}"
>  			exit 1 # TODO: error code
>  		fi
>  	fi
> @@ -1899,9 +1899,9 @@ install_package() {
>  	(( ! INSTALL )) && return
> 
>  	if (( ! SPLITPKG )); then
> -		msg "$(gettext "Installing package %s with %s...")" "$pkgname" "$PACMAN -U"
> +		msg "$(gettext "Installing package %s with %s...")" "$pkgname"
> "${PACMAN[*]} -U"
>  	else
> -		msg "$(gettext "Installing %s package group with %s...")"
> "$pkgbase" "$PACMAN -U"
> +		msg "$(gettext "Installing %s package group with %s...")"
> "$pkgbase" "${PACMAN[*]} -U"
>  	fi
> 
>  	local fullver pkgarch pkg pkglist
> @@ -2457,8 +2457,9 @@ if [[ $MAKEPKG_CONF = "$confdir/makepkg.conf" &&
> -r ~/.makepkg.conf ]]; then
>  	source_safe ~/.makepkg.conf
>  fi
> 
> -# set pacman command if not already defined
> -PACMAN=${PACMAN:-pacman}
> +# Set pacman command if not already defined. Convert to Bash array so that
> +# spaces can be included in the path while any arguments are retained.
> +PACMAN=(${PACMAN:-pacman})
> 
>  # check if messages are to be printed using color
>  unset ALL_OFF BOLD BLUE GREEN RED YELLOW
> @@ -2728,7 +2729,11 @@ if (( NODEPS || (NOBUILD && !DEP_BIN ) )); then
>  	if (( NODEPS )); then
>  		warning "$(gettext "Skipping dependency checks.")"
>  	fi
> -elif type -p "${PACMAN%% *}" >/dev/null; then
> +elif output="$(command -v "$PACMAN")"; then
> +	# Save full path to Pacman in case the --syncdeps operation modifies
> +	# the $PATH and makes it inaccessible
> +	PACMAN="$output"
> +	
>  	if (( RMDEPS && ! INSTALL )); then
>  		original_pkglist=($(run_pacman -Qq))    # required by remove_dep
>  	fi
> @@ -2757,7 +2762,7 @@ elif type -p "${PACMAN%% *}" >/dev/null; then
>  		exit 1
>  	fi
>  else
> -	warning "$(gettext "%s was not found in %s; skipping dependency
> checks.")" "${PACMAN%% *}" "PATH"
> +	warning "$(gettext "%s was not found in %s; skipping dependency
> checks.")" "$PACMAN" "PATH"
>  fi
> 
>  # ensure we have a sane umask set
> -- 
> 1.7.12
>