[pacman-dev] [PATCH 0/6] Assorted makepkg cleanups
Here's a bunch of patches I've had sitting on a branch for a while, plus a new one at the end which I thought would be a good idea after I found an instance of "{$pkgver}" in a PKGBUILD. These (and the other dload/curl patch) can be pulled from git://github.com/falconindy/pacman. Dave Reisner (6): makepkg.conf: add -g to default curl options makepkg: remove vestiges of global errexit makepkg: make run_function_safe more robust makepkg: fix quoting in calls to check_deps makepkg: fix quoting in calls to dependency checking makepkg: unset potentially architecture-specific vars etc/makepkg.conf.in | 6 +++--- scripts/makepkg.sh.in | 37 ++++++++++++++++++++++--------------- 2 files changed, 25 insertions(+), 18 deletions(-) -- 2.10.2
This disables globbing, which should never be used in source URL specifications as it would lead to mismatches in the checksum mapping and un-checked sources. --- etc/makepkg.conf.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in index efac16a..7129397 100644 --- a/etc/makepkg.conf.in +++ b/etc/makepkg.conf.in @@ -8,9 +8,9 @@ # #-- The download utilities that makepkg should use to acquire sources # Format: 'protocol::agent' -DLAGENTS=('ftp::/usr/bin/curl -qfC - --ftp-pasv --retry 3 --retry-delay 3 -o %o %u' - 'http::/usr/bin/curl -qb "" -fLC - --retry 3 --retry-delay 3 -o %o %u' - 'https::/usr/bin/curl -qb "" -fLC - --retry 3 --retry-delay 3 -o %o %u' +DLAGENTS=('ftp::/usr/bin/curl -gqfC - --ftp-pasv --retry 3 --retry-delay 3 -o %o %u' + 'http::/usr/bin/curl -gqb "" -fLC - --retry 3 --retry-delay 3 -o %o %u' + 'https::/usr/bin/curl -gqb "" -fLC - --retry 3 --retry-delay 3 -o %o %u' 'rsync::/usr/bin/rsync --no-motd -z %u %o' 'scp::/usr/bin/scp -C %u %o') -- 2.10.2
These 'set +E' diversions haven't been needed since global errexit was disabled in dca10b062f2 (January 2012). --- scripts/makepkg.sh.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 02398cf..a1385c1 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -276,12 +276,12 @@ resolve_deps() { # deplist cannot be declared like this: local deplist=$(foo) # Otherwise, the return value will depend on the assignment. local deplist - deplist="$(set +E; check_deps $*)" || exit 1 + deplist="$(check_deps $*)" || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED if handle_deps $deplist; then # check deps again to make sure they were resolved - deplist="$(set +E; check_deps $*)" || exit 1 + deplist="$(check_deps $*)" || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED fi @@ -959,7 +959,7 @@ check_vcs_software() { client=$(get_vcsclient "$proto") || exit $? # ensure specified program is installed local uninstalled - uninstalled="$(set +E; check_deps $client)" || exit 1 + uninstalled="$(check_deps $client)" || exit 1 # if not installed, check presence in depends or makedepends if [[ -n "$uninstalled" ]] && (( ! NODEPS || ( VERIFYSOURCE && !DEP_BIN ) )); then if ! in_array "$client" ${all_deps[@]}; then -- 2.10.2
Use shopt to set/reset errexit and errtrace, which lets us: 1) be more vigilant, resetting anything the user might do to us in PKGBUILD functions. 2) use human-readable words (errexit vs. -e) On top of this, introduce a new save/restore for the shell's other shopts. A user should not have any expectations that what happens in one function is available in another function, if it isn't explicitly defined in the PKGBUILD. While this change does not make that assertion, it gets us closer. We also replace a variable which comes from out of nowhere (pkgfunc) with the positional parameter containing the same value. Quoting is adjusted to make the expansion happen at the time the trap is set, rather than later on. --- scripts/makepkg.sh.in | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index a1385c1..a97cdc2 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -398,20 +398,23 @@ prepare_buildenv() { } run_function_safe() { - local restoretrap + local restoretrap restoreset restoreshopt - set -e - set -E + # we don't set any special shopts of our own, but we don't want the user to + # muck with our environment. + restoreshopt=$(shopt -p) + + restoreset=$(shopt -o -p) + shopt -o -s errexit errtrace restoretrap=$(trap -p ERR) - trap 'error_function $pkgfunc' ERR + trap "error_function '$1'" ERR run_function "$1" - eval $restoretrap - - set +E - set +e + eval "$restoretrap" + eval "$restoreset" + eval "$restoreshopt" } run_function() { -- 2.10.2
The inside needs quoting, and this is separate from the declartion, which does not (in these cases). --- scripts/makepkg.sh.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index a97cdc2..0aabc25 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -276,12 +276,12 @@ resolve_deps() { # deplist cannot be declared like this: local deplist=$(foo) # Otherwise, the return value will depend on the assignment. local deplist - deplist="$(check_deps $*)" || exit 1 + deplist=$(check_deps "$@") || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED if handle_deps $deplist; then # check deps again to make sure they were resolved - deplist="$(check_deps $*)" || exit 1 + deplist=$(check_deps "$@") || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED fi @@ -962,7 +962,7 @@ check_vcs_software() { client=$(get_vcsclient "$proto") || exit $? # ensure specified program is installed local uninstalled - uninstalled="$(check_deps $client)" || exit 1 + uninstalled=$(check_deps "$client") || exit 1 # if not installed, check presence in depends or makedepends if [[ -n "$uninstalled" ]] && (( ! NODEPS || ( VERIFYSOURCE && !DEP_BIN ) )); then if ! in_array "$client" ${all_deps[@]}; then -- 2.10.2
--- scripts/makepkg.sh.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 0aabc25..c212ffc 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -243,7 +243,7 @@ handle_deps() { (( $# == 0 )) && return $R_DEPS_SATISFIED - local deplist="$*" + local deplist=("$@") if (( ! DEP_BIN )); then return $R_DEPS_MISSING @@ -253,7 +253,7 @@ handle_deps() { # install missing deps from binary packages (using pacman -S) msg "$(gettext "Installing missing dependencies...")" - if ! run_pacman -S --asdeps $deplist; then + if ! run_pacman -S --asdeps "${deplist[@]}"; then error "$(gettext "'%s' failed to install missing dependencies.")" "$PACMAN" exit 1 # TODO: error code fi @@ -276,10 +276,10 @@ resolve_deps() { # deplist cannot be declared like this: local deplist=$(foo) # Otherwise, the return value will depend on the assignment. local deplist - deplist=$(check_deps "$@") || exit 1 + deplist=($(check_deps "$@")) || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED - if handle_deps $deplist; then + if handle_deps "${deplist[@]}"; then # check deps again to make sure they were resolved deplist=$(check_deps "$@") || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED -- 2.10.2
I'm not convinced this is a worthwhile goal, but let's follow suit. Since we can't know the names of all the vars that might exist, unset them by pattern. --- scripts/makepkg.sh.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index c212ffc..e7a506f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1455,7 +1455,11 @@ fi unset pkgname pkgbase pkgver pkgrel epoch pkgdesc url license groups provides unset md5sums replaces depends conflicts backup source install changelog build -unset makedepends optdepends options noextract validpgpkeys +unset sha{1,224,256,384,512}sums makedepends optdepends options noextract validpgpkeys +unset "${!makedepends_@}" "${!depends_@}" "${!source_@}" "${!checkdepends_@}" +unset "${!optdepends_@}" "${!conflicts_@}" "${!provides_@}" "${!replaces_@}" +unset "${!md5sums_@}" "${!sha1sums_@}" "${!sha224sums_@}" "${!sha256sums_@}" +unset "${!sha384sums_@}" "${!sha512sums_@}" BUILDFILE=${BUILDFILE:-$BUILDSCRIPT} if [[ ! -f $BUILDFILE ]]; then -- 2.10.2
On 08/11/16 01:23, Dave Reisner wrote:
Here's a bunch of patches I've had sitting on a branch for a while, plus a new one at the end which I thought would be a good idea after I found an instance of "{$pkgver}" in a PKGBUILD.
These (and the other dload/curl patch) can be pulled from git://github.com/falconindy/pacman.
Dave Reisner (6): makepkg.conf: add -g to default curl options makepkg: remove vestiges of global errexit makepkg: make run_function_safe more robust makepkg: fix quoting in calls to check_deps makepkg: fix quoting in calls to dependency checking makepkg: unset potentially architecture-specific vars
All look good. Thanks! A
participants (2)
-
Allan McRae
-
Dave Reisner