Re: [pacman-dev] [PATCH] libmakepkg/executable: don't rely on scoped value of $ret to flag outcomes

[Allan McRae]

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

[pacman-dev] [PATCH] libmakepkg/executable: don't rely on scoped value of $ret to flag outcomes

[Eli Schwartz]

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 
---
 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