[pacman-dev] [PATCH 3/6] makepkg: refactor check_sanity, give it some sanity of its own
Dave Reisner
dreisner at archlinux.org
Thu Aug 7 19:22:23 EDT 2014
Break apart each of the blocks into their own separate functions. And,
instead of the hand crafted eval statements, reuse the logic from
pkgbuild-introspection[0] to abstract away the complexities of parsing
bash.
This commit fixes at least 3 bugs in check_sanity:
1) The wrong variable is shown for the error which would be thrown
when, e.g. pkgname=('foopkg' 'bar^pkg')
2) The "arch" variable is not sanity checked when the PKGBUILD has
an arch override, but only one output package.
3) https://bugs.archlinux.org/task/40361
Lastly, there's some string changes here which should help to clarify
a few errors emitted in the linting process.
[0] https://github.com/falconindy/pkgbuild-introspection
---
scripts/makepkg.sh.in | 406 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 299 insertions(+), 107 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 9ae1e95..e8cf60c 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -2190,18 +2190,103 @@ have_function() {
declare -f "$1" >/dev/null
}
-check_sanity() {
- # check for no-no's in the build script
- local i
- local ret=0
- for i in 'pkgname' 'pkgrel'; do
- if [[ -z ${!i} ]]; then
- error "$(gettext "%s is not allowed to be empty.")" "$i"
- ret=1
- fi
+array_build() {
+ local dest=$1 src=$2 i keys values
+
+ # it's an error to try to copy a value which doesn't exist.
+ declare -p "$2" &>/dev/null || return 1
+
+ # Build an array of the indicies of the source array.
+ eval "keys=(\"\${!$2[@]}\")"
+
+ # Read values indirectly via their index. This approach gives us support
+ # for associative arrays, sparse arrays, and empty strings as elements.
+ for i in "${keys[@]}"; do
+ values+=("printf -v '$dest[$i]' %s \"\${$src[$i]}\";")
done
+ eval "${values[*]}"
+}
+
+funcgrep() {
+ { declare -f "$1" || declare -f package; } 2>/dev/null | grep -E "$2"
+}
+
+extract_global_var() {
+ # $1: variable name
+ # $2: multivalued
+ # $3: name of output var
+
+ local attr=$1 isarray=$2 outputvar=$3
+
+ if (( isarray )); then
+ array_build ref "$attr"
+ [[ ${ref[@]} ]] && array_build "$outputvar" "$attr"
+ else
+ [[ -v $attr ]] && printf -v "$outputvar" %s "${!attr}"
+ fi
+}
+
+extract_function_var() {
+ # $1: function name
+ # $2: variable name
+ # $3: multivalued
+ # $4: name of output var
+
+ local funcname=$1 attr=$2 isarray=$3 outputvar=$4 attr_regex= decl= r=1
+
+ if (( isarray )); then
+ printf -v attr_regex '^[[:space:]]*(declare( -[[:alpha:]])*)? %q\+?=\(' "$2"
+ else
+ printf -v attr_regex '^[[:space:]]*(declare( -[[:alpha:]])*)? %q\+?=[^(]' "$2"
+ fi
+
+ while read -r; do
+ # strip leading whitespace and any usage of declare
+ decl=${REPLY##*([[:space:]])?(declare +(-+([[:alpha:]]) ))}
+ eval "${decl/#$attr/$outputvar}"
+
+ # entering this loop at all means we found a match, so notify the caller.
+ r=0
+ done < <(funcgrep "$funcname" "$attr_regex")
+
+ return $r
+}
+
+pkgbuild_get_attribute() {
+ # $1: package name
+ # $2: attribute name
+ # $3: name of output var
+ # $4: multivalued
+
+ local pkgname=$1 attrname=$2 outputvar=$3 isarray=$4
+
+ printf -v "$outputvar" %s ''
+
+ if [[ $pkgname ]]; then
+ extract_global_var "$attrname" "$isarray" "$outputvar"
+ extract_function_var "package_$pkgname" "$attrname" "$isarray" "$outputvar"
+ else
+ extract_global_var "$attrname" "$isarray" "$outputvar"
+ fi
+}
+
+lint_pkgbase() {
+ if [[ ${pkgbase:0:1} = "-" ]]; then
+ error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname"
+ return 1
+ fi
+}
+
+lint_pkgname() {
+ local ret=0 i
+
for i in "${pkgname[@]}"; do
+ if [[ -z $i ]]; then
+ error "$(gettext "%s is not allowed to be empty.")" "pkgname"
+ ret=1
+ continue
+ fi
if [[ ${i:0:1} = "-" ]]; then
error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgname"
ret=1
@@ -2212,155 +2297,262 @@ check_sanity() {
fi
if [[ $i = *[^[:alnum:]+_. at -]* ]]; then
error "$(gettext "%s contains invalid characters: '%s'")" \
- 'pkgname' "${pkgname//[[:alnum:]+_. at -]}"
+ 'pkgname' "${i//[[:alnum:]+_. at -]}"
ret=1
fi
done
- if [[ ${pkgbase:0:1} = "-" ]]; then
- error "$(gettext "%s is not allowed to start with a hyphen.")" "pkgbase"
- ret=1
+ return $ret
+}
+
+lint_pkgrel() {
+ if [[ -z $pkgrel ]]; then
+ error "$(gettext "%s is not allowed to be empty.")" "pkgrel"
+ return 1
fi
- if (( ! PKGVERFUNC )) ; then
- check_pkgver || ret=1
+ if [[ $pkgrel != +([0-9])?(.+([0-9])) ]]; then
+ error "$(gettext "%s must be a decimal, not %s.")" "pkgrel" "$pkgrel"
+ return 1
fi
+}
- awk -F'=' '$1 ~ /^[[:space:]]*pkgrel$/' "$BUILDFILE" | sed "s/[[:space:]]*#.*//" |
- while IFS='=' read -r _ i; do
- eval i=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/' <<< "${i%%+([[:space:]])}")\"
- if [[ $i != +([0-9])?(.+([0-9])) ]]; then
- error "$(gettext "%s must be a decimal.")" "pkgrel"
- return 1
- fi
- done || ret=1
+lint_pkgver() {
+ if (( PKGVERFUNC )); then
+ # defer check to after getting version from pkgver function
+ return 0
+ fi
- awk -F'=' '$1 ~ /^[[:space:]]*epoch$/' "$BUILDFILE" |
- while IFS='=' read -r _ i; do
- eval i=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/' <<< "${i%%+([[:space:]])}")\"
- if [[ $i != *([[:digit:]]) ]]; then
- error "$(gettext "%s must be an integer.")" "epoch"
- return 1
- fi
- done || ret=1
+ check_pkgver
+}
+
+lint_epoch() {
+ if [[ $epoch != *([[:digit:]]) ]]; then
+ error "$(gettext "%s must be an integer, not %s.")" "epoch" "$epoch"
+ return 1
+ fi
+}
- if [[ $arch != 'any' ]]; then
- if ! in_array $CARCH "${arch[@]}"; then
+lint_arch() {
+ local name list=("${@:-"${arch[@]}"}")
+
+ if [[ $list == 'any' ]]; then
+ return 0
+ fi
+
+ if ! in_array "$CARCH" "${list[@]}" && (( ! IGNOREARCH )); then
+ error "$(gettext "%s is not available for the '%s' architecture.")" "$pkgbase" "$CARCH"
+ plain "$(gettext "Note that many packages may need a line added to their %s")" "$BUILDSCRIPT"
+ plain "$(gettext "such as %s.")" "arch=('$CARCH')"
+ return 1
+ fi
+
+ for name in "${pkgname[@]}"; do
+ pkgbuild_get_attribute "$name" 'arch' list 1
+ if [[ $list && $list != 'any' ]] && ! in_array $CARCH "${list[@]}"; then
if (( ! IGNOREARCH )); then
- error "$(gettext "%s is not available for the '%s' architecture.")" "$pkgbase" "$CARCH"
- plain "$(gettext "Note that many packages may need a line added to their %s")" "$BUILDSCRIPT"
- plain "$(gettext "such as %s.")" "arch=('$CARCH')"
+ error "$(gettext "%s is not available for the '%s' architecture.")" "$name" "$CARCH"
ret=1
fi
fi
- fi
+ done
+}
- if (( ${#pkgname[@]} > 1 )); then
- for i in ${pkgname[@]}; do
- local arch_list=""
- eval $(declare -f package_${i} | sed -n 's/\(^[[:space:]]*arch=\)/arch_list=/p')
- if [[ ${arch_list[@]} && ${arch_list} != 'any' ]]; then
- if ! in_array $CARCH "${arch_list[@]}"; then
- if (( ! IGNOREARCH )); then
- error "$(gettext "%s is not available for the '%s' architecture.")" "$i" "$CARCH"
- ret=1
- fi
- fi
- fi
- done
- fi
+lint_provides() {
+ local list name provides_list ret=0
+
+ provides_list=("${provides[@]}")
+ for name in "${pkgname[@]}"; do
+ if extract_function_var "package_$name" provides 1 list; then
+ provides_list+=("${list[@]}")
+ fi
+ done
- local provides_list=()
- eval $(awk '/^[[:space:]]*provides=/,/\)/' "$BUILDFILE" | \
- sed -e "s/provides=/provides_list+=/" -e "s/#.*//" -e 's/\\$//')
- for i in ${provides_list[@]}; do
- if [[ $i == *['<>']* ]]; then
+ for provide in "${provides_list[@]}"; do
+ if [[ $provide == *['<>']* ]]; then
error "$(gettext "%s array cannot contain comparison (< or >) operators.")" "provides"
ret=1
fi
done
- local backup_list=()
- eval $(awk '/^[[:space:]]*backup=/,/\)/' "$BUILDFILE" | \
- sed -e "s/backup=/backup_list+=/" -e "s/#.*//" -e 's/\\$//')
- for i in "${backup_list[@]}"; do
- if [[ ${i:0:1} = "/" ]]; then
- error "$(gettext "%s entry should not contain leading slash : %s")" "backup" "$i"
+ return $ret
+}
+
+lint_backup() {
+ local list name backup_list ret=0
+
+ backup_list=("${backup[@]}")
+ for name in "${pkgname[@]}"; do
+ if extract_function_var "package_$name" backup 1 list; then
+ backup_list+=("${list[@]}")
+ fi
+ done
+
+ for name in "${backup_list[@]}"; do
+ if [[ ${name:0:1} = "/" ]]; then
+ error "$(gettext "%s entry should not contain leading slash : %s")" "backup" "$name"
ret=1
fi
done
- local optdepends_list=()
- eval $(awk '/^[[:space:]]*optdepends=\(/,/\)[[:space:]]*(#.*)?$/' "$BUILDFILE" | \
- sed -e "s/optdepends=/optdepends_list+=/" -e "s/#.*//" -e 's/\\$//')
- for i in "${optdepends_list[@]}"; do
- local pkg=${i%%:[[:space:]]*}
+ return $ret
+}
+
+lint_optdepends() {
+ local list name optdepends_list ret=0
+
+ optdepends_list=("${optdepends[@]}")
+ for name in "${pkgname[@]}"; do
+ if extract_function_var "package_$name" optdepends 1 list; then
+ optdepends_list+=("${list[@]}")
+ fi
+ done
+
+ for name in "${optdepends_list[@]}"; do
+ local pkg=${name%%:[[:space:]]*}
# the '-' character _must_ be first or last in the character range
if [[ $pkg != +([-[:alnum:]><=.+_:]) ]]; then
- error "$(gettext "Invalid syntax for %s : '%s'")" "optdepend" "$i"
+ error "$(gettext "Invalid syntax for %s: '%s'")" "optdepend" "$name"
ret=1
fi
done
- for i in 'changelog' 'install'; do
- local file
- while read -r file; do
- # evaluate any bash variables used
- eval file=\"$(sed 's/^\(['\''"]\)\(.*\)\1$/\2/' <<< "$file")\"
- if [[ $file && ! -f $file ]]; then
- error "$(gettext "%s file (%s) does not exist.")" "$i" "$file"
- ret=1
- fi
- done < <(sed -n "s/^[[:space:]]*$i=//p" "$BUILDFILE")
+ return $ret
+}
+
+check_files_exist() {
+ local kind=$1 files=("${@:2}") file ret
+
+ for file in "${files[@]}"; do
+ if [[ $file && ! -f $file ]]; then
+ error "$(gettext "%s file (%s) does not exist or is not a regular file.")" \
+ "$kind" "$file"
+ ret=1
+ fi
+ done
+
+ return $ret
+}
+
+lint_install() {
+ local list file name install_list ret=0
+
+ install_list=("${install[@]}")
+ for name in "${pkgname[@]}"; do
+ extract_function_var "package_$name" install 0 file
+ install_list+=("$file")
+ done
+
+ check_files_exist 'install' "${install_list[@]}"
+}
+
+lint_changelog() {
+ local list name file changelog_list ret=0
+
+ changelog_list=("${changelog[@]}")
+ for name in "${pkgname[@]}"; do
+ if extract_function_var "package_$name" changelog 0 file; then
+ changelog_list+=("$file")
+ fi
+ done
+
+ check_files_exist 'changelog' "${changelog_list[@]}"
+}
+
+lint_options() {
+ local ret=0 list name kopt options_list
+
+ options_list=("${options[@]}")
+ for name in "${pkgname[@]}"; do
+ if extract_function_var "package_$name" options 1 list; then
+ options_list+=("${list[@]}")
+ fi
done
- local valid_options=1
- local known kopt options_list
- eval $(awk '/^[[:space:]]*options=/,/\)/' "$BUILDFILE" | \
- sed -e "s/options=/options_list+=/" -e "s/#.*//" -e 's/\\$//')
- for i in ${options_list[@]}; do
- known=0
+ for i in "${options_list[@]}"; do
# check if option matches a known option or its inverse
- for kopt in ${packaging_options[@]} ${other_options[@]}; do
- if [[ ${i} = "${kopt}" || ${i} = "!${kopt}" ]]; then
- known=1
+ for kopt in "${packaging_options[@]}" "${other_options[@]}"; do
+ if [[ $i = "$kopt" || $i = "!$kopt" ]]; then
+ # continue to the next $i
+ continue 2
fi
done
- if (( ! known )); then
- error "$(gettext "%s array contains unknown option '%s'")" "options" "$i"
- valid_options=0
- fi
- done
- if (( ! valid_options )); then
+
+ error "$(gettext "%s array contains unknown option '%s'")" "options" "$i"
ret=1
+ done
+
+ return $ret
+}
+
+lint_source() {
+ local idx=("${!source[@]}")
+
+ if (( ${#source[*]} > 0 && (idx[-1] + 1) != ${#source[*]} )); then
+ error "$(gettext "Sparse arrays are not allowed for source")"
+ return 1
fi
+}
+
+lint_pkglist() {
+ local i ret=0
+
+ for i in "${PKGLIST[@]}"; do
+ if ! in_array "$i" "${pkgname[@]}"; then
+ error "$(gettext "Requested package %s is not provided in %s")" "$i" "$BUILDFILE"
+ ret=1
+ fi
+ done
+
+ return $ret
+}
+
+lint_packagefn() {
+ local i ret=0
if (( ${#pkgname[@]} == 1 )); then
- if have_function build && ! ( have_function package || have_function package_${pkgname}); then
+ if have_function 'build' && ! { have_function 'package' || have_function "package_$pkgname"; }; then
error "$(gettext "Missing %s function in %s")" "package()" "$BUILDFILE"
ret=1
fi
else
- for i in ${pkgname[@]}; do
- if ! have_function package_${i}; then
+ for i in "${pkgname[@]}"; do
+ if ! have_function "package_$i"; then
error "$(gettext "Missing %s function for split package '%s'")" "package_$i()" "$i"
ret=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"
- ret=1
- fi
- done
+ return $ret
+}
- local idx=("${!source[@]}")
- if (( ${#source[*]} > 0 && (idx[-1] + 1) != ${#source[*]} )); then
- error "$(gettext "Sparse arrays are not allowed for source")"
- ret=1
- fi
+check_sanity() {
+ # check for no-no's in the build script
+ local ret=0
+ local lintfn lint_functions
+
+ lint_functions=(
+ lint_pkgbase
+ lint_pkgname
+ lint_pkgver
+ lint_pkgrel
+ lint_epoch
+ lint_arch
+ lint_provides
+ lint_backup
+ lint_optdepends
+ lint_changelog
+ lint_install
+ lint_options
+ lint_packagefn
+ lint_pkglist
+ lint_source
+ )
+
+ for lintfn in "${lint_functions[@]}"; do
+ "$lintfn" || ret=1
+ done
return $ret
}
--
2.0.4
More information about the pacman-dev
mailing list