[pacman-dev] [PATCH] makepkg: save path to PACMAN and test availability
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@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 # 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
On 13 November 2012 03:14, Allan McRae <allan@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@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
On 13/11/12 14:18, Martin Panter wrote:
On 13 November 2012 03:14, Allan McRae <allan@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@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
change this to: if (( ! INFAKEROOT && ( ! 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.
Ah... we never actually call PACMAN in the fakeroot, so we do not care what it is there... The test should be modified as given above. Can you test that change does all that is expected? Thanks, Allan
On 13 November 2012 05:57, Allan McRae <allan@archlinux.org> wrote:
On 13/11/12 14:18, Martin Panter wrote:
On 13 November 2012 03:14, Allan McRae <allan@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@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
change this to:
if (( ! INFAKEROOT && ( ! 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.
Ah... we never actually call PACMAN in the fakeroot, so we do not care what it is there... The test should be modified as given above.
Can you test that change does all that is expected?
I added your INFAKEROOT test and it seems to be working properly for me. Thanks.
On 13/11/12 16:07, Martin Panter wrote:
On 13 November 2012 05:57, Allan McRae <allan@archlinux.org> wrote:
On 13/11/12 14:18, Martin Panter wrote:
On 13 November 2012 03:14, Allan McRae <allan@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@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
change this to:
if (( ! INFAKEROOT && ( ! 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.
Ah... we never actually call PACMAN in the fakeroot, so we do not care what it is there... The test should be modified as given above.
Can you test that change does all that is expected?
I added your INFAKEROOT test and it seems to be working properly for me. Thanks.
Great - fixed version has been pushed to my working branch. Allan
participants (2)
-
Allan McRae
-
Martin Panter