This causes package_$pkgname() to be preferred over package() in the non-split case, but the behavior if both functions exist was undocumented anyway. --- scripts/makepkg.sh.in | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index e9080a70..748481e4 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1481,30 +1481,28 @@ fi # check we have the software required to process the PKGBUILD check_software || exit $E_MISSING_MAKEPKG_DEPS -if (( ${#pkgname[@]} > 1 )); then +if (( ${#pkgname[@]} > 1 )) || have_function package_${pkgname}; then SPLITPKG=1 fi # test for available PKGBUILD functions if have_function prepare; then # "Hide" prepare() function if not going to be run if [[ $RUN_PREPARE != "n" ]]; then PREPAREFUNC=1 fi fi if have_function build; then BUILDFUNC=1 fi if have_function check; then # "Hide" check() function if not going to be run if [[ $RUN_CHECK = 'y' ]] || { ! check_buildenv "check" "n" && [[ $RUN_CHECK != "n" ]]; }; then CHECKFUNC=1 fi fi if have_function package; then PKGFUNC=1 -elif [[ $SPLITPKG -eq 0 ]] && have_function package_${pkgname}; then - SPLITPKG=1 fi # check if gpg signature is to be created and if signing key is valid -- 2.17.0

Put this into a new function close to run_split_packaging so similar code is nearby. --- scripts/makepkg.sh.in | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4684e444..f8f43540 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1121,6 +1121,18 @@ backup_package_variables() { done } +run_solo_packaging() { + pkgdir="$pkgdirbase/$pkgname" + mkdir "$pkgdir" + if (( PKGFUNC )); then + run_package + fi + tidy_install + lint_package || exit $E_PACKAGE_FAILED + create_package + create_debug_package +} + run_split_packaging() { local pkgname_backup=("${pkgname[@]}") local restore_package_variables @@ -1537,15 +1549,7 @@ if (( INFAKEROOT )); then chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then - pkgdir="$pkgdirbase/$pkgname" - mkdir "$pkgdir" - if (( PKGFUNC )); then - run_package - fi - tidy_install - lint_package || exit $E_PACKAGE_FAILED - create_package - create_debug_package + run_solo_packaging else run_split_packaging fi -- 2.17.0

We don't need to re-backup the variables we restored on the previous iteration. --- scripts/makepkg.sh.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 86af4675..84ae08db 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1134,9 +1134,8 @@ run_solo_packaging() { run_split_packaging() { local pkgname_backup=("${pkgname[@]}") - local restore_package_variables + local restore_package_variables="$(backup_package_variables)" for pkgname in ${pkgname_backup[@]}; do - restore_package_variables="$(backup_package_variables)" run_solo_packaging $pkgname eval "$restore_package_variables" done -- 2.17.0

--- scripts/makepkg.sh.in | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 27f627ac..ed0ceaec 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -499,23 +499,23 @@ run_package() { } find_libdepends() { - local d sodepends; + local d sodepends - sodepends=0; + sodepends=0 for d in "${depends[@]}"; do if [[ $d = *.so ]]; then - sodepends=1; - break; + sodepends=1 + break fi done if (( sodepends == 0 )); then (( ${#depends[@]} )) && printf '%s\n' "${depends[@]}" - return; + return fi - local libdeps filename soarch sofile soname soversion; - declare -A libdeps; + local libdeps filename soarch sofile soname soversion + declare -A libdeps while read -r filename; do # get architecture of the file; if soarch is empty it's not an ELF binary @@ -1231,7 +1231,7 @@ OPT_LONG=('allsource' 'check' 'clean' 'cleanbuild' 'config:' 'force' 'geninteg' OPT_LONG+=('asdeps' 'noconfirm' 'needed' 'noprogressbar') if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then - exit $E_INVALID_OPTION; + exit $E_INVALID_OPTION fi set -- "${OPTRET[@]}" unset OPT_SHORT OPT_LONG OPTRET -- 2.17.0

It's especially dangerous in trap handlers since the return value of the function becomes the return value of the last command before the trap, not the last command in the current function. This applies to any function executed in a trap handler, nested functions included. In one case, install_packages failed (via return 14), which was inside a conditional that then ran exit 14, which triggered the EXIT handler, which called clean_up, which called remove_deps, which had !RMDEPS and thus returned. The return value of remove_deps became the return value of install_packages, triggering the ERR handler, which (due to another problem) was still the user function handler, which then printed a misleading error message and overrode the exit code with 4. --- scripts/makepkg.sh.in | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3a3f4c30..968fab0d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -133,7 +133,7 @@ clean_up() { if (( INFAKEROOT )); then # Don't clean up when leaving fakeroot, we're not done yet. - return + return 0 fi if (( (EXIT_CODE == E_OK || EXIT_CODE == E_INSTALL_FAILED) && CLEANUP )); then @@ -313,7 +313,7 @@ resolve_deps() { } remove_deps() { - (( ! RMDEPS )) && return + (( ! RMDEPS )) && return 0 # check for packages removed during dependency install (e.g. due to conflicts) # removing all installed packages is risky in this case @@ -512,7 +512,7 @@ find_libdepends() { if (( sodepends == 0 )); then (( ${#depends[@]} )) && printf '%s\n' "${depends[@]}" - return + return 0 fi local libdeps filename soarch sofile soname soversion @@ -714,7 +714,7 @@ list_package_files() { } create_package() { - (( NOARCHIVE )) && return + (( NOARCHIVE )) && return 0 if [[ ! -d $pkgdir ]]; then error "$(gettext "Missing %s directory.")" "\$pkgdir/" @@ -779,12 +779,12 @@ create_debug_package() { # check if a debug package was requested if ! check_option "debug" "y" || ! check_option "strip" "y"; then - return + return 0 fi # check if we have any debug symbols to package if dir_is_empty "$pkgdir/usr/lib/debug"; then - return + return 0 fi unset groups depends optdepends provides conflicts replaces backup install changelog @@ -868,7 +868,7 @@ create_srcpackage() { } install_package() { - (( ! INSTALL )) && return + (( ! INSTALL )) && return 0 if (( ! SPLITPKG )); then msg "$(gettext "Installing package %s with %s...")" "$pkgname" "$PACMAN -U" -- 2.17.0

The ERR trap is not inherited by functions unless the "errtrace" option is set. So in the current situation, makepkg's internal functions are supposed to do manual error checking. Bad returns from function calls at the top level will trigger the trap, though. On Mon, Jun 4, 2018 at 9:59 AM Allan McRae wrote:

On 01/06/18 02:24, Jan Alexander Steffens (heftig) wrote:

...

$restoretrap is empty if the trap was not set. This caused the trap handler to remain and override later exit codes.

How is this ever unset? We set the error trap early in makepkg:

trap 'trap_exit USR1 "$(gettext "An unknown error has occurred. Exiting...")"' ERR

...

--- scripts/makepkg.sh.in | 1 + 1 file changed, 1 insertion(+)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed0ceaec..3a3f4c30 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -432,6 +432,7 @@ run_function_safe() {

run_function "$1"

+ trap - ERR eval "$restoretrap" eval "$restoreset" eval "$restoreshopt"

On 01/06/18 02:24, Jan Alexander Steffens (heftig) wrote:

Pull out the expected=y/n check into a separate function and make use of the fact we can just concatenate the fallback arrays to get the same result. ---

Breaks using options=(!strip) in a PKGBUILD.

scripts/libmakepkg/util/option.sh.in | 85 +++++++++------------------- 1 file changed, 28 insertions(+), 57 deletions(-)

diff --git a/scripts/libmakepkg/util/option.sh.in b/scripts/libmakepkg/util/option.sh.in index 46e0568d..28357e27 100644 --- a/scripts/libmakepkg/util/option.sh.in +++ b/scripts/libmakepkg/util/option.sh.in @@ -48,95 +48,66 @@ in_opt_array() { }

+## +# usage : check_opt_array( $option, $expected_val, $haystack ) +# return : 0 - matches expected +# 1 - does not match expected +# 127 - not found +## +check_opt_array() { + local option=$1 expected=$2; shift 2 + + in_opt_array "$option" "$@" + case $? in + 0) # assert enabled + [[ $expected = y ]] + return ;; + 1) # assert disabled + [[ $expected = n ]] + return ;; + esac + + # not found + return 127 +} + + ## # Checks to see if options are present in makepkg.conf or PKGBUILD; # PKGBUILD options always take precedence. # # usage : check_option( $option, $expected_val ) # return : 0 - matches expected # 1 - does not match expected # 127 - not found ## check_option() { - in_opt_array "$1" ${options[@]} - case $? in - 0) # assert enabled - [[ $2 = y ]] - return ;; - 1) # assert disabled - [[ $2 = n ]] - return - esac - - # fall back to makepkg.conf options - in_opt_array "$1" ${OPTIONS[@]} - case $? in - 0) # assert enabled - [[ $2 = y ]] - return ;; - 1) # assert disabled - [[ $2 = n ]] - return - esac - - # not found - return 127 + check_opt_array "$@" "${options[@]}" "${OPTIONS[@]}"

These need switched.

}

## # Check if option is present in BUILDENV # # usage : check_buildenv( $option, $expected_val ) # return : 0 - matches expected # 1 - does not match expected # 127 - not found ## check_buildenv() { - in_opt_array "$1" ${BUILDENV[@]} - case $? in - 0) # assert enabled - [[ $2 = "y" ]] - return ;; - 1) # assert disabled - [[ $2 = "n" ]] - return ;; - esac - - # not found - return 127 + check_opt_array "$@" "${BUILDENV[@]}" }

+ ## # Checks to see if options are present in BUILDENV or PKGBUILD; # PKGBUILD options always take precedence. # # usage : check_buildoption( $option, $expected_val ) # return : 0 - matches expected # 1 - does not match expected # 127 - not found ## check_buildoption() { - in_opt_array "$1" ${options[@]} - case $? in - 0) # assert enabled - [[ $2 = y ]] - return ;; - 1) # assert disabled - [[ $2 = n ]] - return - esac - - in_opt_array "$1" ${BUILDENV[@]} - case $? in - 0) # assert enabled - [[ $2 = y ]] - return ;; - 1) # assert disabled - [[ $2 = n ]] - return - esac - - # not found - return 127 + check_opt_array "$@" "${options[@]}" "${BUILDENV[@]}"

These need switched.

}