[pacman-dev] [PATCH 01/10] makepkg: undeclared local variables
Variables that are only meaningful within the function they are declared in are now prefixed by "local". Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 29 +++++++++++++++++++++-------- 1 files changed, 21 insertions(+), 8 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index cbc344d..07fc6d5 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -135,6 +135,9 @@ clean_up() { fi if (( ! EXIT_CODE && CLEANUP )); then + local pkg + local file + # If it's a clean exit and -c/--clean has been passed... msg "$(gettext "Cleaning up...")" rm -rf "$pkgdir" "$srcdir" @@ -308,7 +311,7 @@ get_downloadclient() { for i in "${DLAGENTS[@]}"; do local handler="${i%%::*}" if [[ $proto = $handler ]]; then - agent="${i##*::}" + local agent="${i##*::}" break fi done @@ -388,6 +391,7 @@ check_deps() { # Also, a non-zero return value is not unexpected and we are manually dealing them set +E local ret=0 + local pmout pmout=$(run_pacman -T "$@") || ret=$? set -E @@ -655,7 +659,7 @@ extract_sources() { msg "$(gettext "Extracting Sources...")" local netfile for netfile in "${source[@]}"; do - file=$(get_filename "$netfile") + local file=$(get_filename "$netfile") if in_array "$file" ${noextract[@]}; then #skip source files in the noextract=() array # these are marked explicitly to NOT be extracted @@ -731,7 +735,7 @@ run_function() { if [[ -z $1 ]]; then return 1 fi - pkgfunc="$1" + local pkgfunc="$1" # clear user-specified makeflags if requested if [[ $(check_option makeflags) = "n" ]]; then @@ -747,8 +751,9 @@ run_function() { local shellopts=$(shopt -p) local ret=0 + local restoretrap if (( LOGGING )); then - BUILDLOG="${startdir}/${pkgbase}-${pkgver}-${pkgrel}-${CARCH}-$pkgfunc.log" + local BUILDLOG="${startdir}/${pkgbase}-${pkgver}-${pkgrel}-${CARCH}-$pkgfunc.log" if [[ -f $BUILDLOG ]]; then local i=1 while true; do @@ -803,6 +808,7 @@ run_build() { } run_package() { + local pkgfunc if [[ -z $1 ]]; then pkgfunc="package" else @@ -936,6 +942,7 @@ write_pkginfo() { [[ $provides ]] && printf "provides = %s\n" "${provides[@]}" [[ $backup ]] && printf "backup = %s\n" "${backup[@]}" + local it for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then @@ -984,6 +991,7 @@ create_package() { cd "$pkgdir" msg "$(gettext "Creating package...")" + local nameofpkg if [[ -z $1 ]]; then nameofpkg="$pkgname" else @@ -1019,6 +1027,7 @@ create_package() { # tar it up msg2 "$(gettext "Compressing package...")" + local EXT case "$PKGEXT" in *tar.gz) EXT=${PKGEXT%.gz} ;; *tar.bz2) EXT=${PKGEXT%.bz2} ;; @@ -1153,7 +1162,7 @@ install_package() { msg "$(gettext "Installing %s package group with %s -U...")" "$pkgbase" "$PACMAN" fi - local pkglist + local pkg pkglist for pkg in ${pkgname[@]}; do if [[ -f $PKGDEST/${pkg}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT} ]]; then pkglist+=" $PKGDEST/${pkg}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}" @@ -1228,7 +1237,7 @@ check_sanity() { local optdepend for optdepend in "${optdepends[@]}"; do - pkg=${optdepend%%:*} + local pkg=${optdepend%%:*} if [[ ! $pkg =~ ^[[:alnum:]\>\<\=\.\+\_\-]*$ ]]; then error "$(gettext "Invalid syntax for optdepend : '%s'")" "$optdepend" fi @@ -1267,6 +1276,7 @@ check_sanity() { return 1 fi + local pkg if (( ${#pkgname[@]} > 1 )); then for pkg in ${pkgname[@]}; do if ! declare -f package_${pkg} >/dev/null; then @@ -1373,15 +1383,17 @@ devel_update() { } backup_package_variables() { + local var for var in ${splitpkg_overrides[@]}; do - indirect="${var}_backup" + local indirect="${var}_backup" eval "${indirect}=(\"\${$var[@]}\")" done } restore_package_variables() { + local var for var in ${splitpkg_overrides[@]}; do - indirect="${var}_backup" + local indirect="${var}_backup" if [[ -n ${!indirect} ]]; then eval "${var}=(\"\${$indirect[@]}\")" else @@ -1410,6 +1422,7 @@ parse_options() { local long_options=$1; shift; local ret=0; local unused_options="" + local i while [[ -n $1 ]]; do if [[ ${1:0:2} = '--' ]]; then -- 1.7.1
As check_deps is run in a subshell, exit had the same meaning as return. Since the intention is to halt makepkg when pacman throws an error other than 127, the enclosing function has to handle error control instead. Fixes FS#19840 Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 07fc6d5..02d9df8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -399,7 +399,7 @@ check_deps() { echo "$pmout" elif (( ret )); then error "$(gettext "'%s' returned a fatal error (%i): %s")" "$PACMAN" "$ret" "$pmout" - exit 1 + return "$ret" fi } @@ -439,14 +439,15 @@ resolve_deps() { local R_DEPS_SATISFIED=0 local R_DEPS_MISSING=1 - local deplist="$(check_deps $*)" - if [[ -z $deplist ]]; then - return $R_DEPS_SATISFIED - fi + # deplist cannot be declared like this: local deplist=$(foo) + # Otherwise, the return value will depend on the assignment. + local deplist + deplist="$(set +E; check_deps $*)" || exit 1 + [[ -z $deplist ]] && return $R_DEPS_SATISFIED if handle_deps $deplist; then # check deps again to make sure they were resolved - deplist="$(check_deps $*)" + deplist="$(set +E; check_deps $*)" || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED elif (( DEP_BIN )); then error "$(gettext "Failed to install all missing dependencies.")" -- 1.7.1
The error message that has been removed never gets to print because, given the same condition, handle_deps throws the same error and then immediately exits makepkg. Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 02d9df8..2237cfe 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -449,8 +449,6 @@ resolve_deps() { # check deps again to make sure they were resolved deplist="$(set +E; check_deps $*)" || exit 1 [[ -z $deplist ]] && return $R_DEPS_SATISFIED - elif (( DEP_BIN )); then - error "$(gettext "Failed to install all missing dependencies.")" fi msg "$(gettext "Missing Dependencies:")" -- 1.7.1
ERR trap was being toggled on and off in 3 places in confusingly different ways. One of these ocurrances went untouched after this commit. Since disabling the ERR trap is unintuitive enough, at least it should be done in a way that is uniform across makepkg. Also, in check_deps the ERR trap was being disabled before needed. If the assignments included in the range were unsuccesful because the variables had the readonly attribute, makepkg would've exited without "unknown error" thanks to errexit. Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2237cfe..bbc28d9 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -389,11 +389,9 @@ check_deps() { # Disable error trap in pacman subshell call as this breaks bash-3.2 compatibility # Also, a non-zero return value is not unexpected and we are manually dealing them - set +E local ret=0 local pmout - pmout=$(run_pacman -T "$@") || ret=$? - set -E + pmout=$(set +E; run_pacman -T "$@") || ret=$? if (( ret == 127 )); then #unresolved deps echo "$pmout" @@ -427,10 +425,9 @@ handle_deps() { # we might need the new system environment # avoid triggering the ERR trap - local restoretrap=$(trap -p ERR) - trap - ERR + set +E source /etc/profile &>/dev/null - eval $restoretrap + set -E return $R_DEPS_SATISFIED } -- 1.7.1
Combine changelog and install file creation as in previous commits. Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 27 ++++++++++++--------------- 1 files changed, 12 insertions(+), 15 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index bbc28d9..81a842e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1004,21 +1004,18 @@ create_package() { local comp_files=".PKGINFO" - # check for an install script - if [[ -n $install ]]; then - msg2 "$(gettext "Adding install script...")" - cp "$startdir/$install" .INSTALL - chmod 644 .INSTALL - comp_files="$comp_files .INSTALL" - fi - - # do we have a changelog? - if [[ -n $changelog ]]; then - msg2 "$(gettext "Adding package changelog...")" - cp "$startdir/$changelog" .CHANGELOG - chmod 644 .CHANGELOG - comp_files="$comp_files .CHANGELOG" - fi + # check for changelog/install files + for i in 'changelog' 'install'; do + orig=${!i} + dest=$(tr '[:lower:]' '[:upper:]' <<<".$i") + + if [[ -n $orig ]]; then + msg2 "$(gettext "Adding %s file...")" "$i" + cp "$startdir/$orig" "$dest" + chmod 644 "$dest" + comp_files+=" $dest" + fi + done # tar it up msg2 "$(gettext "Compressing package...")" -- 1.7.1
During check_sanity, use regex and abstract the series of variable checks into a list. Also add descriptive error message to exceptions involving backup array members, given that "invalid backup entry" isn't all that communicative. Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 55 +++++++++++++++++------------------------------- 1 files changed, 20 insertions(+), 35 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 81a842e..0958797 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1170,6 +1170,19 @@ install_package() { fi } +var_lint() { + local pattern="$1" + local message="$2" + shift 2 + + local i + for i; do + [[ $i =~ $pattern ]] || continue + error "$(gettext "%s")" "$message" + return 1 + done +} + check_sanity() { # check for no-no's in the build script local i @@ -1179,27 +1192,15 @@ check_sanity() { return 1 fi done + + var_lint '-' "pkgver is not allowed to contain hyphens" "$pkgver" + var_lint '-' "pkgrel is not allowed to contain hyphens" "$pkgrel" - local name - for name in "${pkgname[@]}"; do - if [[ ${name:0:1} = "-" ]]; then - error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname" - return 1 - fi - done + var_lint '^-' "pkgname entries are not allowed to start with a hyphen" "${pkgname[@]}" + var_lint '^-' "pkgbase is not allowed to start with a hyphen" "$pkgbase" - if [[ ${pkgbase:0:1} = "-" ]]; then - error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgbase" - return 1 - fi - if [[ $pkgver != ${pkgver//-/} ]]; then - error "$(gettext "%s is not allowed to contain hyphens.")" "pkgver" - return 1 - fi - if [[ $pkgrel != ${pkgrel//-/} ]]; then - error "$(gettext "%s is not allowed to contain hyphens.")" "pkgrel" - return 1 - fi + var_lint '[<>]' "provides entries are not allowed to contain comparison (< or >) characters" "${provides[@]}" + var_lint '^/' "backup entries are not allowed to start with a slash" "${backup[@]}" if [[ $arch != 'any' ]]; then if ! in_array $CARCH ${arch[@]}; then @@ -1212,22 +1213,6 @@ check_sanity() { fi fi - local provide - for provide in ${provides[@]}; do - if [[ $provide != ${provide//</} || $provide != ${provide//>/} ]]; then - error "$(gettext "Provides array cannot contain comparison (< or >) operators.")" - return 1 - fi - done - - local file - for file in "${backup[@]}"; do - if [[ ${file:0:1} = "/" ]]; then - error "$(gettext "Invalid backup entry : %s")" "$file" - return 1 - fi - done - local optdepend for optdepend in "${optdepends[@]}"; do local pkg=${optdepend%%:*} -- 1.7.1
Signed-off-by: Andres P <aepd87@gmail.com> --- doc/PKGBUILD.5.txt | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 2f49ca5..684800e 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -38,18 +38,21 @@ This will prevent any possible name clashes with internal makepkg variables. For example, to store the base kernel version in a variable, use something similar to `$_basekernver`. -*pkgname*:: +*pkgname (array)*:: The name of the package. This has be a unix-friendly name as it will be - used in the package filename. + used in the package filename. Members of the array are not allowed to start + with hyphens. *pkgver*:: The version of the software as released from the author (e.g. '2.7.1'). + The variable is not allowed to contain hyphens. *pkgrel*:: This is the release number specific to the Arch Linux release. This allows package maintainers to make updates to the package's configure flags, for example. A pkgrel of 1 is typically used for each upstream - software release and is incremented for intermediate PKGBUILD updates. + software release and is incremented for intermediate PKGBUILD updates. The + variable is not allowed to contain hyphens. *pkgdesc*:: This should be a brief description of the package and its functionality. @@ -282,7 +285,8 @@ An optional global directive is available when building a split package: *pkgbase*:: The name used to refer to the group of packages in the output of makepkg and in the naming of source-only tarballs. If not specified, the first - element in the `pkgname` array is used. + element in the `pkgname` array is used. The variable is not allowed to + begin with a hyphen. Install/Upgrade/Remove Scripting -------------------------------- -- 1.7.1
If optdepends was defined with empty members; optdepends=('' '' ''), the behaviour would've been exit later rather than now, defeating the whole point of the aptly named check_sanity. Fixing this required changing the regex from <atom>* to <atom>+. Also, move the regex into a var so that it doesn't need to be escaped and use the standard i index to avoid having a destinct local assignment for every for loop, which are numerous in this function. Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 0958797..50cdae7 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1213,15 +1213,14 @@ check_sanity() { fi fi - local optdepend - for optdepend in "${optdepends[@]}"; do - local pkg=${optdepend%%:*} - if [[ ! $pkg =~ ^[[:alnum:]\>\<\=\.\+\_\-]*$ ]]; then - error "$(gettext "Invalid syntax for optdepend : '%s'")" "$optdepend" + local regex='^[[:alnum:]><=.+_-]+$' + for i in "${optdepends[@]}"; do + i=${i%%:*} + if [[ ! $i =~ $regex ]]; then + error "$(gettext "Invalid syntax for optdepend: '%s'")" "$i" fi done - local i for i in 'changelog' 'install'; do local filelist=$(sed -n "s/^[[:space:]]*$i=//p" "$BUILDFILE") local file -- 1.7.1
Here's how for loops work in bash and just about any language out there. If $var expands to nothing, `for i in $var` does *not* iterate. Taking this in mind, wrapping the whole loop in a "[[ -n $var ]]" conditional is unnecesary, expands the variable twice, indents the whole block for no good reason, and shows that there's no fluency. Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 29 ++++++++++++----------------- 1 files changed, 12 insertions(+), 17 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 50cdae7..59519a6 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1253,24 +1253,19 @@ check_sanity() { return 1 fi - local pkg - if (( ${#pkgname[@]} > 1 )); then - for pkg in ${pkgname[@]}; do - if ! declare -f package_${pkg} >/dev/null; then - error "$(gettext "missing package function for split package '%s'")" "$pkg" - return 1 - fi - done - fi + for i in ${pkgname[@]}; do + if ! declare -f package_${i} >/dev/null; then + error "$(gettext "missing package function for split package '%s'")" "$i" + return 1 + fi + done - if [[ -n "${PKGLIST[@]}" ]]; then - for pkg in ${PKGLIST[@]}; do - if ! in_array $pkg ${pkgname[@]}; then - error "$(gettext "requested package %s is not provided in %s")" "$pkg" "$BUILDFILE" - return 1 - fi - done - fi + for i in ${PKGLIST[@]}; do + if ! in_array $i ${pkgname[@]}; then + error "$(gettext "requested package %s is not provided in %s")" "$i" "$BUILDFILE" + return 1 + fi + done return 0 } -- 1.7.1
Make package_$pkgname a pointer to package() if it's not a split package. Signed-off-by: Andres P <aepd87@gmail.com> --- Note that '[PATCH 09/10] makepkg: do not expand for loop variables twice' [1] depends on this. [1] Message-Id: 1277507805-22149-9-git-send-email-aepd87@gmail.com scripts/makepkg.sh.in | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index c770c7f..100ed6f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1762,6 +1762,12 @@ if (( GENINTEG )); then exit 0 # $E_OK fi +if (( ${#pkgname[@]} > 1 )); then + SPLITPKG=1 +else + eval 'package_'$pkgname'() { package; }' +fi + # check the PKGBUILD for some basic requirements check_sanity || exit 1 @@ -1772,10 +1778,6 @@ check_sanity || exit 1 devel_check devel_update -if (( ${#pkgname[@]} > 1 )); then - SPLITPKG=1 -fi - # test for available PKGBUILD functions if declare -f build >/dev/null; then BUILDFUNC=1 -- 1.7.1
Signed-off-by: Andres P <aepd87@gmail.com> --- scripts/makepkg.sh.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 59519a6..c770c7f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1228,7 +1228,7 @@ check_sanity() { # evaluate any bash variables used eval file=${file} if [[ ! -f $file ]]; then - error "$(gettext "%s file (%s) does not exist.")" "${i^}" "$file" + error "$(gettext "%s file (%s) does not exist.")" "$i" "$file" return 1 fi done -- 1.7.1
On 26/06/10 09:16, Andres P wrote:
Signed-off-by: Andres P<aepd87@gmail.com> --- scripts/makepkg.sh.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 59519a6..c770c7f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1228,7 +1228,7 @@ check_sanity() { # evaluate any bash variables used eval file=${file} if [[ ! -f $file ]]; then - error "$(gettext "%s file (%s) does not exist.")" "${i^}" "$file" + error "$(gettext "%s file (%s) does not exist.")" "$i" "$file" return 1 fi done
Signed-off-by: Allan. On my working-maint branch.
participants (2)
-
Allan McRae
-
Andres P