[pacman-dev] [PATCH] libmakepkg/executable: don't rely on scoped value of $ret to flag outcomes
Elsewhere, we return 1 if a library dropin fails, and when running functions in a loop, we use `|| ret=1` to preserve scope. This ensures the return value of the function remains useful in isolation. Do the same thing here as well. Drop trivial function which wraps a dropin that also uses $ret, since it's no longer needed. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- scripts/libmakepkg/executable.sh.in | 2 +- scripts/libmakepkg/executable/ccache.sh.in | 2 +- scripts/libmakepkg/executable/distcc.sh.in | 2 +- scripts/libmakepkg/executable/fakeroot.sh.in | 2 +- scripts/libmakepkg/executable/gpg.sh.in | 4 ++++ scripts/libmakepkg/executable/gzip.sh.in | 2 +- scripts/libmakepkg/executable/pacman.sh.in | 2 +- scripts/libmakepkg/executable/strip.sh.in | 2 +- scripts/libmakepkg/executable/vcs.sh.in | 8 +------- 9 files changed, 12 insertions(+), 14 deletions(-) diff --git a/scripts/libmakepkg/executable.sh.in b/scripts/libmakepkg/executable.sh.in index 68c48038..92031d43 100644 --- a/scripts/libmakepkg/executable.sh.in +++ b/scripts/libmakepkg/executable.sh.in @@ -38,7 +38,7 @@ check_software() { local ret=0 for func in ${executable_functions[@]}; do - $func + $func || ret=1 done return $ret diff --git a/scripts/libmakepkg/executable/ccache.sh.in b/scripts/libmakepkg/executable/ccache.sh.in index dafde076..6143fee2 100644 --- a/scripts/libmakepkg/executable/ccache.sh.in +++ b/scripts/libmakepkg/executable/ccache.sh.in @@ -31,7 +31,7 @@ executable_ccache() { if check_buildoption "ccache" "y"; then if ! type -p ccache >/dev/null; then error "$(gettext "Cannot find the %s binary required for compiler cache usage.")" "ccache" - ret=1 + return 1 fi fi } diff --git a/scripts/libmakepkg/executable/distcc.sh.in b/scripts/libmakepkg/executable/distcc.sh.in index b9883f6b..d3a8cc25 100644 --- a/scripts/libmakepkg/executable/distcc.sh.in +++ b/scripts/libmakepkg/executable/distcc.sh.in @@ -31,7 +31,7 @@ executable_distcc() { if check_buildoption "distcc" "y"; then if ! type -p distcc >/dev/null; then error "$(gettext "Cannot find the %s binary required for distributed compilation.")" "distcc" - ret=1 + return 1 fi fi } diff --git a/scripts/libmakepkg/executable/fakeroot.sh.in b/scripts/libmakepkg/executable/fakeroot.sh.in index e22d9a96..56c1b3fd 100644 --- a/scripts/libmakepkg/executable/fakeroot.sh.in +++ b/scripts/libmakepkg/executable/fakeroot.sh.in @@ -31,7 +31,7 @@ executable_fakeroot() { if check_buildenv "fakeroot" "y" && (( EUID > 0 )); then if ! type -p fakeroot >/dev/null; then error "$(gettext "Cannot find the %s binary.")" "fakeroot" - ret=1 + return 1 fi fi } diff --git a/scripts/libmakepkg/executable/gpg.sh.in b/scripts/libmakepkg/executable/gpg.sh.in index c0f57013..139773ef 100644 --- a/scripts/libmakepkg/executable/gpg.sh.in +++ b/scripts/libmakepkg/executable/gpg.sh.in @@ -28,6 +28,8 @@ source "$LIBRARY/util/option.sh" executable_functions+=('executable_gpg') executable_gpg() { + local ret=0 + if [[ $SIGNPKG == 'y' ]] || { [[ -z $SIGNPKG ]] && check_buildenv "sign" "y"; }; then if ! type -p gpg >/dev/null; then error "$(gettext "Cannot find the %s binary required for signing packages.")" "gpg" @@ -41,4 +43,6 @@ executable_gpg() { ret=1 fi fi + + return $ret } diff --git a/scripts/libmakepkg/executable/gzip.sh.in b/scripts/libmakepkg/executable/gzip.sh.in index 66748320..bb1626bc 100644 --- a/scripts/libmakepkg/executable/gzip.sh.in +++ b/scripts/libmakepkg/executable/gzip.sh.in @@ -31,7 +31,7 @@ executable_gzip() { if check_option "zipman" "y"; then if ! type -p gzip >/dev/null; then error "$(gettext "Cannot find the %s binary required for compressing man and info pages.")" "gzip" - ret=1 + return 1 fi fi } diff --git a/scripts/libmakepkg/executable/pacman.sh.in b/scripts/libmakepkg/executable/pacman.sh.in index d9967f45..d1433ffd 100644 --- a/scripts/libmakepkg/executable/pacman.sh.in +++ b/scripts/libmakepkg/executable/pacman.sh.in @@ -31,7 +31,7 @@ executable_pacman() { 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 + return 1 fi fi } diff --git a/scripts/libmakepkg/executable/strip.sh.in b/scripts/libmakepkg/executable/strip.sh.in index a6b81db6..867dc928 100644 --- a/scripts/libmakepkg/executable/strip.sh.in +++ b/scripts/libmakepkg/executable/strip.sh.in @@ -31,7 +31,7 @@ executable_strip() { if check_option "strip" "y"; then if ! type -p strip >/dev/null; then error "$(gettext "Cannot find the %s binary required for object file stripping.")" "strip" - ret=1 + return 1 fi fi } diff --git a/scripts/libmakepkg/executable/vcs.sh.in b/scripts/libmakepkg/executable/vcs.sh.in index 527d6f22..46631f39 100644 --- a/scripts/libmakepkg/executable/vcs.sh.in +++ b/scripts/libmakepkg/executable/vcs.sh.in @@ -49,7 +49,7 @@ get_vcsclient() { printf "%s\n" "$client" } -check_vcs_software() { +executable_vcs() { local netfile all_sources all_deps deps ret=0 if (( SOURCEONLY == 1 )); then @@ -101,9 +101,3 @@ check_vcs_software() { return $ret } - -executable_vcs() { - if ! check_vcs_software; then - ret=1 - fi -} -- 2.19.2
On 28/11/18 2:00 pm, Eli Schwartz wrote:
Elsewhere, we return 1 if a library dropin fails, and when running functions in a loop, we use `|| ret=1` to preserve scope. This ensures the return value of the function remains useful in isolation. Do the same thing here as well.
Drop trivial function which wraps a dropin that also uses $ret, since it's no longer needed.
Thanks - I keep forgetting that the point of libmakepkg is reuseability in addition to splitting up the massive script... Allan
participants (2)
-
Allan McRae
-
Eli Schwartz