As check_deps is run in a subshell, exit had the same meaning as return. Since the intention is to halt makepkg when pacman throws an error other than 127, the enclosing function has to handle error control instead. Fixes FS#19840 Signed-off-by: Andres P --- scripts/makepkg.sh.in | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 07fc6d5..02d9df8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -399,7 +399,7 @@ check_deps() { echo "$pmout" elif (( ret )); then error "$(gettext "'%s' returned a fatal error (%i): %s")" "$PACMAN" "$ret" "$pmout" - exit 1 + return "$ret" fi } @@ -439,14 +439,15 @@ resolve_deps() { local R_DEPS_SATISFIED=0 local R_DEPS_MISSING=1 - local deplist="$(check_deps $*)" - if [[ -z $deplist ]]; then - return $R_DEPS_SATISFIED - fi + # 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 + [[ -z $deplist ]] && return $R_DEPS_SATISFIED if handle_deps $deplist; then # check deps again to make sure they were resolved - deplist="$(check_deps $*)" + deplist="$(set +E; check_deps $*)" || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED elif (( DEP_BIN )); then error "$(gettext "Failed to install all missing dependencies.")" -- 1.7.1

ERR trap was being toggled on and off in 3 places in confusingly different ways. One of these ocurrances went untouched after this commit. Since disabling the ERR trap is unintuitive enough, at least it should be done in a way that is uniform across makepkg. Also, in check_deps the ERR trap was being disabled before needed. If the assignments included in the range were unsuccesful because the variables had the readonly attribute, makepkg would've exited without "unknown error" thanks to errexit. Signed-off-by: Andres P --- scripts/makepkg.sh.in | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2237cfe..bbc28d9 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -389,11 +389,9 @@ check_deps() { # Disable error trap in pacman subshell call as this breaks bash-3.2 compatibility # Also, a non-zero return value is not unexpected and we are manually dealing them - set +E local ret=0 local pmout - pmout=$(run_pacman -T "$@") || ret=$? - set -E + pmout=$(set +E; run_pacman -T "$@") || ret=$? if (( ret == 127 )); then #unresolved deps echo "$pmout" @@ -427,10 +425,9 @@ handle_deps() { # we might need the new system environment # avoid triggering the ERR trap - local restoretrap=$(trap -p ERR) - trap - ERR + set +E source /etc/profile &>/dev/null - eval $restoretrap + set -E return $R_DEPS_SATISFIED } -- 1.7.1

During check_sanity, use regex and abstract the series of variable checks into a list. Also add descriptive error message to exceptions involving backup array members, given that "invalid backup entry" isn't all that communicative. Signed-off-by: Andres P --- scripts/makepkg.sh.in | 55 +++++++++++++++++------------------------------- 1 files changed, 20 insertions(+), 35 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 81a842e..0958797 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1170,6 +1170,19 @@ install_package() { fi } +var_lint() { + local pattern="$1" + local message="$2" + shift 2 + + local i + for i; do + [[ $i =~ $pattern ]] || continue + error "$(gettext "%s")" "$message" + return 1 + done +} + check_sanity() { # check for no-no's in the build script local i @@ -1179,27 +1192,15 @@ check_sanity() { return 1 fi done + + var_lint '-' "pkgver is not allowed to contain hyphens" "$pkgver" + var_lint '-' "pkgrel is not allowed to contain hyphens" "$pkgrel" - local name - for name in "${pkgname[@]}"; do - if [[ ${name:0:1} = "-" ]]; then - error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname" - return 1 - fi - done + var_lint '^-' "pkgname entries are not allowed to start with a hyphen" "${pkgname[@]}" + var_lint '^-' "pkgbase is not allowed to start with a hyphen" "$pkgbase" - if [[ ${pkgbase:0:1} = "-" ]]; then - error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgbase" - return 1 - fi - if [[ $pkgver != ${pkgver//-/} ]]; then - error "$(gettext "%s is not allowed to contain hyphens.")" "pkgver" - return 1 - fi - if [[ $pkgrel != ${pkgrel//-/} ]]; then - error "$(gettext "%s is not allowed to contain hyphens.")" "pkgrel" - return 1 - fi + var_lint '[<>]' "provides entries are not allowed to contain comparison (< or >) characters" "${provides[@]}" + var_lint '^/' "backup entries are not allowed to start with a slash" "${backup[@]}" if [[ $arch != 'any' ]]; then if ! in_array $CARCH ${arch[@]}; then @@ -1212,22 +1213,6 @@ check_sanity() { fi fi - local provide - for provide in ${provides[@]}; do - if [[ $provide != ${provide///} ]]; then - error "$(gettext "Provides array cannot contain comparison (< or >) operators.")" - return 1 - fi - done - - local file - for file in "${backup[@]}"; do - if [[ ${file:0:1} = "/" ]]; then - error "$(gettext "Invalid backup entry : %s")" "$file" - return 1 - fi - done - local optdepend for optdepend in "${optdepends[@]}"; do local pkg=${optdepend%%:*} -- 1.7.1

If optdepends was defined with empty members; optdepends=('' '' ''), the behaviour would've been exit later rather than now, defeating the whole point of the aptly named check_sanity. Fixing this required changing the regex from <atom>* to <atom>+. Also, move the regex into a var so that it doesn't need to be escaped and use the standard i index to avoid having a destinct local assignment for every for loop, which are numerous in this function. Signed-off-by: Andres P --- scripts/makepkg.sh.in | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 0958797..50cdae7 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1213,15 +1213,14 @@ check_sanity() { fi fi - local optdepend - for optdepend in "${optdepends[@]}"; do - local pkg=${optdepend%%:*} - if [[ ! $pkg =~ ^[[:alnum:]\>\<\=\.\+\_\-]*$ ]]; then - error "$(gettext "Invalid syntax for optdepend : '%s'")" "$optdepend" + local regex='^[[:alnum:]><=.+_-]+$' + for i in "${optdepends[@]}"; do + i=${i%%:*} + if [[ ! $i =~ $regex ]]; then + error "$(gettext "Invalid syntax for optdepend: '%s'")" "$i" fi done - local i for i in 'changelog' 'install'; do local filelist=$(sed -n "s/^[[:space:]]*$i=//p" "$BUILDFILE") local file -- 1.7.1