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