[pacman-dev] [PATCH] makepkg: save path to PACMAN and test availability

[Martin Panter]

On 13 November 2012 03:14, Allan McRae <allan at archlinux.org> wrote:
> After we install dependencies, we source /etc/profile so that new
> elements get added to the path. As this can override any local setting
> of PATH, we store the full path of the PACMAN variable passed to makepkg.
>
> Also, add a check for PACMAN availability if it is needed to deal with any
> dependency operations.
>
> Signed-off-by: Allan McRae <allan at archlinux.org>
> ---
>
> This is a replacement for the patch provided by Martin Panter.  While dealing
> with patches today, I decided that the check for the full pacman path was
> being done in the wrong place and we should do the usual check of software
> availability if "pacman" is needed.
>
>
>  scripts/makepkg.sh.in | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index f650b1b..16e421b 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -875,9 +875,9 @@ source_has_signatures() {
>  run_pacman() {
>         local cmd
>         if [[ ! $1 = -@(T|Qq) ]]; then
> -               cmd=("$PACMAN" $PACMAN_OPTS "$@")
> +               cmd=("$PACMAN_PATH" $PACMAN_OPTS "$@")
>         else
> -               cmd=("$PACMAN" "$@")
> +               cmd=("$PACMAN_PATH" "$@")
>         fi
>         if (( ! ASROOT )) && [[ ! $1 = -@(T|Qq) ]]; then
>                 if type -p sudo >/dev/null; then
> @@ -2191,6 +2191,14 @@ check_software() {
>         # check for needed software
>         local ret=0
>
> +       # check for PACMAN if we need it
> +       if (( ! NODEPS || DEP_BIN || RMDEPS || INSTALL )); then
> +               if [[ -z $PACMAN_PATH ]]; then
> +                       error "$(gettext "Cannot find the %s binary required for dependency operations.")" "$PACMAN"
> +                       ret=1
> +               fi
> +       fi
> +
>         # check for sudo if we will need it during makepkg execution
>         if (( ! ( ASROOT || INFAKEROOT ) && ( DEP_BIN || RMDEPS || INSTALL ) )); then
>                 if ! type -p sudo >/dev/null; then
> @@ -2548,6 +2556,8 @@ fi
>
>  # set pacman command if not already defined
>  PACMAN=${PACMAN:-pacman}
> +# save full path to command as PATH may change when sourcing /etc/profile
> +PACMAN_PATH=$(type -P $PACMAN) || true

Thanks for cleaning this up for me. The trouble with this version is
that $PACMAN_PATH gets recalculated in the fake root recursion, after
$PATH has been modified.

What do you think about overwriting $PACMAN rather than using
$PACMAN_PATH? It could make some of the error messages a bit more
verbose though (including the full path). Another idea is to pass the
original path through to the fake root environment, but I think that
might be too complex and not worth it.

Sample messages with extra debugging added:

$ PACMAN=roopwn makepkg -f --syncdeps
PACMAN=roopwn; PATH=/home/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/bin/vendor_perl:/usr/bin/core_perl:/opt/qt/bin
Path: [/home/bin/roopwn]
==> Making package: pacaur 3.2.6-1 (Tue Nov 13 04:01:30 UTC 2012)
==> WARNING: Using a PKGBUILD without a package() function is deprecated.
==> Checking runtime dependencies...
==> Installing missing dependencies...
. . .
==> Entering fakeroot environment...
PACMAN=roopwn; PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/bin/vendor_perl:/usr/bin/core_perl:/opt/qt/bin
Path: []
==> ERROR: Cannot find the roopwn binary required for dependency operations.
[Exit 1]

>  # check if messages are to be printed using color
>  unset ALL_OFF BOLD BLUE GREEN RED YELLOW
> @@ -2825,7 +2835,7 @@ if (( NODEPS || (NOBUILD && !DEP_BIN ) )); then
>         if (( NODEPS )); then
>                 warning "$(gettext "Skipping dependency checks.")"
>         fi
> -elif type -p "$PACMAN" >/dev/null; then
> +else
>         if (( RMDEPS && ! INSTALL )); then
>                 original_pkglist=($(run_pacman -Qq))    # required by remove_dep
>         fi
> @@ -2853,8 +2863,6 @@ elif type -p "$PACMAN" >/dev/null; then
>                 error "$(gettext "Could not resolve all dependencies.")"
>                 exit 1
>         fi
> -else
> -       warning "$(gettext "%s was not found in %s; skipping dependency checks.")" "$PACMAN" "PATH"
>  fi
>
>  # ensure we have a sane umask set
> --
> 1.8.0