[pacman-dev] [PATCH] makepkg: remove subshelling from check_option and friends
Dave Reisner
dreisner at archlinux.org
Wed Apr 25 10:54:17 EDT 2012
Instead of creating a subshell for each of these checks (of which there
are many), pass in an expected value and make the check_* function do
the comparison for us, returning 0 (match), 1, (mismatch), or 127 (not
found).
For a measureable benefit: I tested this on a fairly simple package
(perl-term-readkey) and counted the number of clone syscalls to try
and isolate those generated by makepkg itself, rather than the user
defined functions. Results as shown below:
336 before
180 after
So, roughly a 50% reduction, which makes sense given that a single
check_option() call could be up to 3 subprocesses in total.
Signed-off-by: Dave Reisner <dreisner at archlinux.org>
---
I've looked over this enough times that I'm starting to go blind looking at the
patch. It's passed my testing as well, but this is the sort of thing that's ripe
for regressions, so another set of eyes/testing would be appreciated.
scripts/makepkg.sh.in | 137 ++++++++++++++++++++++++++++---------------------
1 file changed, 79 insertions(+), 58 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 0f3c466..cfe00d3 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -264,47 +264,67 @@ get_full_version() {
# Checks to see if options are present in makepkg.conf or PKGBUILD;
# PKGBUILD options always take precedence.
#
-# usage : check_option( $option )
-# return : y - enabled
-# n - disabled
-# ? - not found
+# usage : check_option( $option, $expected_val )
+# return : 0 - matches expected
+# 1 - does not match expected
+# 127 - not found
##
check_option() {
- local ret=$(in_opt_array "$1" ${options[@]})
- if [[ $ret != '?' ]]; then
- printf "%s\n" "$ret"
- return
- fi
+ 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
- ret=$(in_opt_array "$1" ${OPTIONS[@]})
- if [[ $ret != '?' ]]; then
- printf "%s\n" "$ret"
- return
- fi
+ in_opt_array "$1" ${OPTIONS[@]}
+ case $? in
+ 0) # assert enabled
+ [[ $2 = y ]]
+ return ;;
+ 1) # assert disabled
+ [[ $2 = n ]]
+ return
+ esac
- echo '?' # Not Found
+ # not found
+ return 127
}
##
# Check if option is present in BUILDENV
#
-# usage : check_buildenv( $option )
-# return : y - enabled
-# n - disabled
-# ? - not found
+# 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
}
##
# usage : in_opt_array( $needle, $haystack )
-# return : y - enabled
-# n - disabled
-# ? - not found
+# return : 0 - enabled
+# 1 - disabled
+# 127 - not found
##
in_opt_array() {
local needle=$1; shift
@@ -312,15 +332,16 @@ in_opt_array() {
local opt
for opt in "$@"; do
if [[ $opt = "$needle" ]]; then
- echo 'y' # Enabled
- return
+ # enabled
+ return 0
elif [[ $opt = "!$needle" ]]; then
- echo 'n' # Disabled
- return
+ # disabled
+ return 1
fi
done
- echo '?' # Not Found
+ # not found
+ return 127
}
@@ -910,12 +931,12 @@ run_function() {
local pkgfunc="$1"
# clear user-specified buildflags if requested
- if [[ $(check_option buildflags) = "n" ]]; then
+ if check_option "buildflags" "n"; then
unset CFLAGS CXXFLAGS LDFLAGS
fi
# clear user-specified makeflags if requested
- if [[ $(check_option makeflags) = "n" ]]; then
+ if check_option "makeflags" "n"; then
unset MAKEFLAGS
fi
@@ -962,13 +983,13 @@ run_function() {
run_build() {
# use distcc if it is requested (check buildenv and PKGBUILD opts)
- if [[ $(check_buildenv distcc) = "y" && $(check_option distcc) != "n" ]]; then
+ if check_buildenv "distcc" "y" && ! check_option "distc" "n"; then
[[ -d /usr/lib/distcc/bin ]] && export PATH="/usr/lib/distcc/bin:$PATH"
export DISTCC_HOSTS
fi
# use ccache if it is requested (check buildenv and PKGBUILD opts)
- if [[ $(check_buildenv ccache) = "y" && $(check_option ccache) != "n" ]]; then
+ if check_buildenv "ccache" "y" && ! check_option "ccache" "n"; then
[[ -d /usr/lib/ccache/bin ]] && export PATH="/usr/lib/ccache/bin:$PATH"
fi
@@ -994,12 +1015,12 @@ tidy_install() {
cd_safe "$pkgdir"
msg "$(gettext "Tidying install...")"
- if [[ $(check_option docs) = "n" && -n ${DOC_DIRS[*]} ]]; then
+ if check_option "docs" "n" && [[ -n ${DOC_DIRS[*]} ]]; then
msg2 "$(gettext "Removing doc files...")"
rm -rf -- ${DOC_DIRS[@]}
fi
- if [[ $(check_option purge) = "y" && -n ${PURGE_TARGETS[*]} ]]; then
+ if check_option "purge" "y" && [[ -n ${PURGE_TARGETS[*]} ]]; then
msg2 "$(gettext "Purging unwanted files...")"
local pt
for pt in "${PURGE_TARGETS[@]}"; do
@@ -1011,7 +1032,7 @@ tidy_install() {
done
fi
- if [[ $(check_option zipman) = "y" && -n ${MAN_DIRS[*]} ]]; then
+ if check_option "zipman" "y" && [[ -n ${MAN_DIRS[*]} ]]; then
msg2 "$(gettext "Compressing man and info pages...")"
local manpage ext file link hardlinks hl
find ${MAN_DIRS[@]} -type f 2>/dev/null |
@@ -1047,7 +1068,7 @@ tidy_install() {
done
fi
- if [[ $(check_option strip) = "y" ]]; then
+ if check_option "strip" "y"; then
msg2 "$(gettext "Stripping unneeded symbols from binaries and libraries...")"
# make sure library stripping variables are defined to prevent excess stripping
[[ -z ${STRIP_SHARED+x} ]] && STRIP_SHARED="-S"
@@ -1065,17 +1086,17 @@ tidy_install() {
done
fi
- if [[ $(check_option libtool) = "n" ]]; then
+ if check_option "libtool" "n"; then
msg2 "$(gettext "Removing "%s" files...")" "libtool"
find . ! -type d -name "*.la" -exec rm -f -- '{}' \;
fi
- if [[ $(check_option emptydirs) = "n" ]]; then
+ if check_option "emptydirs" "n"; then
msg2 "$(gettext "Removing empty directories...")"
find . -depth -type d -empty -delete
fi
- if [[ $(check_option upx) = "y" ]]; then
+ if check_option "upx" "y"; then
msg2 "$(gettext "Compressing binaries with %s...")" "UPX"
local binary
find . -type f -perm -u+w 2>/dev/null | while read binary ; do
@@ -1234,14 +1255,15 @@ write_pkginfo() {
done
for it in "${packaging_options[@]}"; do
- local ret="$(check_option $it)"
- if [[ $ret != "?" ]]; then
- if [[ $ret = "y" ]]; then
+ check_option "$it" "y"
+ case $? in
+ 0)
printf "makepkgopt = %s\n" "$it"
- else
+ ;;
+ 1)
printf "makepkgopt = %s\n" "!$it"
- fi
- fi
+ ;;
+ esac
done
check_license
@@ -1658,7 +1680,7 @@ check_software() {
fi
# fakeroot - building as non-root user
- if [[ $(check_buildenv fakeroot) = "y" ]] && (( EUID > 0 )); then
+ if check_buildenv "fakeroot" "y" && (( EUID > 0 )); then
if ! type -p fakeroot >/dev/null; then
error "$(gettext "Cannot find the %s binary required for building as non-root user.")" "fakeroot"
ret=1
@@ -1666,7 +1688,7 @@ check_software() {
fi
# gpg - package signing
- if [[ $SIGNPKG == 'y' || (-z "$SIGNPKG" && $(check_buildenv sign) == 'y') ]]; then
+ 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"
ret=1
@@ -1690,7 +1712,7 @@ check_software() {
fi
# upx - binary compression
- if [[ $(check_option upx) == 'y' ]]; then
+ if check_option "upx" "y"; then
if ! type -p upx >/dev/null; then
error "$(gettext "Cannot find the %s binary required for compressing binaries.")" "upx"
ret=1
@@ -1698,7 +1720,7 @@ check_software() {
fi
# distcc - compilation with distcc
- if [[ $(check_buildenv distcc) = "y" && $(check_option distcc) != "n" ]]; then
+ if check_buildenv "distcc" "y" && ! check_option "distcc" "n" ]]; then
if ! type -p distcc >/dev/null; then
error "$(gettext "Cannot find the %s binary required for distributed compilation.")" "distcc"
ret=1
@@ -1706,7 +1728,7 @@ check_software() {
fi
# ccache - compilation with ccache
- if [[ $(check_buildenv ccache) = "y" && $(check_option ccache) != "n" ]]; then
+ if check_buildenv "ccache" "y" && ! check_option "ccache" "n"; then
if ! type -p ccache >/dev/null; then
error "$(gettext "Cannot find the %s binary required for compiler cache usage.")" "ccache"
ret=1
@@ -1714,7 +1736,7 @@ check_software() {
fi
# strip - strip symbols from binaries/libraries
- if [[ $(check_option strip) = "y" ]]; then
+ 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
@@ -1722,7 +1744,7 @@ check_software() {
fi
# gzip - compressig man and info pages
- if [[ $(check_option zipman) = "y" ]]; then
+ 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
@@ -2062,7 +2084,7 @@ PACMAN=${PACMAN:-pacman}
# check if messages are to be printed using color
unset ALL_OFF BOLD BLUE GREEN RED YELLOW
-if [[ -t 2 && ! $USE_COLOR = "n" && $(check_buildenv color) = "y" ]]; then
+if [[ -t 2 && ! $USE_COLOR = "n" ]] && check_buildenv "color" "y"; then
# prefer terminal safe colored and bold text when tput is supported
if tput setaf 0 &>/dev/null; then
ALL_OFF="$(tput sgr0)"
@@ -2146,7 +2168,7 @@ use the %s option.")" "makepkg" "--asroot"
error "$(gettext "The %s option is meant for the root user only. Please\n\
rerun %s without the %s flag.")" "--asroot" "makepkg" "--asroot"
exit 1 # $E_USER_ABORT
- elif (( EUID > 0 )) && [[ $(check_buildenv fakeroot) != "y" ]]; then
+ elif (( EUID > 0 )) && ! check_buildenv "fakeroot" "y"; then
warning "$(gettext "Running %s as an unprivileged user will result in non-root\n\
ownership of the packaged files. Try using the %s environment by\n\
placing %s in the %s array in %s.")" "makepkg" "fakeroot" "'fakeroot'" "BUILDENV" "$MAKEPKG_CONF"
@@ -2230,7 +2252,7 @@ if declare -f build >/dev/null; then
fi
if declare -f check >/dev/null; then
# "Hide" check() function if not going to be run
- if [[ $RUN_CHECK = 'y' || (! $(check_buildenv check) = "n" && ! $RUN_CHECK = "n") ]]; then
+ if [[ $RUN_CHECK = 'y' ]] || { ! check_buildenv "check" "n" && [[ $RUN_CHECK != "n" ]]; }; then
CHECKFUNC=1
fi
fi
@@ -2246,8 +2268,7 @@ if [[ -n "${PKGLIST[@]}" ]]; then
fi
# check if gpg signature is to be created and if signing key is valid
-[[ -z $SIGNPKG ]] && SIGNPKG=$(check_buildenv sign)
-if [[ $SIGNPKG == 'y' ]]; then
+if { [[ -z $SIGNPKG ]] && check_buildenv "sign" "y"; } || [[ $SIGNPKG == 'y' ]]; then
if ! gpg --list-key ${GPGKEY} &>/dev/null; then
if [[ ! -z $GPGKEY ]]; then
error "$(gettext "The key %s does not exist in your keyring.")" "${GPGKEY}"
@@ -2361,7 +2382,7 @@ if (( SOURCEONLY )); then
cd_safe "$startdir"
# if we are root or if fakeroot is not enabled, then we don't use it
- if [[ $(check_buildenv fakeroot) != "y" ]] || (( EUID == 0 )); then
+ if ! check_buildenv "fakeroot" "y" || (( EUID == 0 )); then
create_srcpackage
else
enter_fakeroot
@@ -2453,7 +2474,7 @@ else
cd_safe "$startdir"
# if we are root or if fakeroot is not enabled, then we don't use it
- if [[ $(check_buildenv fakeroot) != "y" ]] || (( EUID == 0 )); then
+ if ! check_buildenv "fakeroot" "y" || (( EUID == 0 )); then
if (( ! REPKG )); then
devel_update
(( BUILDFUNC )) && run_build
--
1.7.10
More information about the pacman-dev
mailing list