[pacman-dev] [PATCH 1/3] makepkg: break out check_checksums to reasonably sized functions
--- scripts/makepkg.sh.in | 95 +++++++++++++++++++++++++++++---------------------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4b23a5b..cb5ded9 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1111,53 +1111,66 @@ generate_checksums() { done } -check_checksums() { - (( SKIPCHECKSUMS )) && return 0 - (( ! ${#source[@]} )) && return 0 +verify_integrity_one() { + local source_name=$1 integ=$2 expectedsum=$3 - local correlation=0 - local integ required - for integ in "${known_hash_algos[@]}"; do - local sumname="${integ}sums[@]" - local integrity_sums=("${!sumname}") - if (( ${#integrity_sums[@]} == ${#source[@]} )); then - msg "$(gettext "Validating source files with %s...")" "${integ}sums" - correlation=1 - local idx errors=0 - for (( idx = 0; idx < ${#source[*]}; idx++ )); do - local file="$(get_filename "${source[idx]}")" - printf ' %s ... ' "$file" >&2 - - if [[ ${integrity_sums[idx]} = 'SKIP' ]]; then - printf '%s\n' "$(gettext "Skipped")" >&2 - continue - fi + local file="$(get_filename "$source_name")" + printf ' %s ... ' "$file" >&2 - if ! file="$(get_filepath "$file")"; then - printf '%s\n' "$(gettext "NOT FOUND")" >&2 - errors=1 - continue - fi + if [[ $expectedsum = 'SKIP' ]]; then + printf '%s\n' "$(gettext "Skipped")" >&2 + return + fi - local expectedsum="${integrity_sums[idx],,}" - local realsum="$(openssl dgst -${integ} "$file")" - realsum="${realsum##* }" - if [[ $expectedsum = "$realsum" ]]; then - printf '%s\n' "$(gettext "Passed")" >&2 - else - printf '%s\n' "$(gettext "FAILED")" >&2 - errors=1 - fi - done + if ! file="$(get_filepath "$file")"; then + printf '%s\n' "$(gettext "NOT FOUND")" >&2 + return 1 + fi - if (( errors )); then - error "$(gettext "One or more files did not pass the validity check!")" - exit 1 # TODO: error code - fi - elif (( ${#integrity_sums[@]} )); then - error "$(gettext "Integrity checks (%s) differ in size from the source array.")" "$integ" + local realsum="$(openssl dgst -${integ} "$file")" + realsum="${realsum##* }" + if [[ ${expectedsum,,} = "$realsum" ]]; then + printf '%s\n' "$(gettext "Passed")" >&2 + else + printf '%s\n' "$(gettext "FAILED")" >&2 + return 1 + fi + + return 0 +} + +verify_integrity_sums() { + local integ=$1 integrity_sums + + array_build integrity_sums "${integ}sums" + + if (( ${#integrity_sums[@]} == ${#source[@]} )); then + msg "$(gettext "Validating source files with %s...")" "${integ}sums" + local idx errors=0 + for (( idx = 0; idx < ${#source[*]}; idx++ )); do + verify_integrity_one "${source[idx]}" "$integ" "${integrity_sums[idx]}" || errors=1 + done + + if (( errors )); then + error "$(gettext "One or more files did not pass the validity check!")" exit 1 # TODO: error code fi + elif (( ${#integrity_sums[@]} )); then + error "$(gettext "Integrity checks (%s) differ in size from the source array.")" "$integ" + exit 1 # TODO: error code + else + return 1 + fi +} + +check_checksums() { + (( SKIPCHECKSUMS )) && return 0 + (( ! ${#source[@]} )) && return 0 + + local correlation=0 + local integ + for integ in "${known_hash_algos[@]}"; do + verify_integrity_sums "$integ" && correlation=1 done if (( ! correlation )); then -- 2.0.4
This also fixes a "bug" in which a PKGBUILD without any source array would generate "md5sums=()". While not technically wrong, we can easily do better. --- scripts/makepkg.sh.in | 73 +++++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index cb5ded9..3962005 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1054,6 +1054,46 @@ get_integlist() { fi } +generate_one_checksum() { + local integ=$1 numsrc=${#source[*]} indentsz idx + + if (( numsrc == 0 )); then + return + fi + + printf "%ssums=(%n" "$integ" indentsz + + for (( idx = 0; idx < numsrc; i++ )); do + local netfile=${source[idx]} + local proto sum + proto="$(get_protocol "$netfile")" + + case $proto in + bzr*|git*|hg*|svn*) + sum="SKIP" + ;; + *) + if [[ ! $netfile = *.@(sig?(n)|asc) ]]; then + local file + file="$(get_filepath "$netfile")" || missing_source_file "$netfile" + sum="$(openssl dgst -${integ} "$file")" + sum=${sum##* } + else + sum="SKIP" + fi + ;; + esac + + # indent checksum on lines after the first + printf "%*s%s" $(( idx ? indentsz : 0 )) '' "'$sum'" + + # print a newline on lines before the last + (( ++idx < numsrc )) && echo + done + + echo ")" +} + generate_checksums() { msg "$(gettext "Generating checksums for source files...")" @@ -1076,38 +1116,7 @@ generate_checksums() { exit 1 # $E_CONFIG_ERROR fi - local indentsz idx numsrc=${#source[@]} - printf "%s%n" "${integ}sums=(" indentsz - - for (( idx = 0; idx < numsrc; i++ )); do - local netfile=${source[idx]} - local proto sum - proto="$(get_protocol "$netfile")" - - case $proto in - bzr*|git*|hg*|svn*) - sum="SKIP" - ;; - *) - if [[ ! $netfile = *.@(sig?(n)|asc) ]]; then - local file - file="$(get_filepath "$netfile")" || missing_source_file "$netfile" - sum="$(openssl dgst -${integ} "$file")" - sum=${sum##* } - else - sum="SKIP" - fi - ;; - esac - - # indent checksum on lines after the first - printf "%*s%s" $(( idx ? indentsz : 0 )) '' "'$sum'" - - # print a newline on lines before the last - (( ++idx < numsrc )) && echo - done - - echo ")" + generate_one_checksum "$integ" done } -- 2.0.4
On 08/07/14 at 08:46pm, Dave Reisner wrote:
This also fixes a "bug" in which a PKGBUILD without any source array would generate "md5sums=()". While not technically wrong, we can easily do better. --- scripts/makepkg.sh.in | 73 +++++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index cb5ded9..3962005 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1054,6 +1054,46 @@ get_integlist() { fi }
+generate_one_checksum() { + local integ=$1 numsrc=${#source[*]} indentsz idx + + if (( numsrc == 0 )); then + return + fi + + printf "%ssums=(%n" "$integ" indentsz + + for (( idx = 0; idx < numsrc; i++ )); do
Incrementing an unused variable here...
+ local netfile=${source[idx]} + local proto sum + proto="$(get_protocol "$netfile")" + + case $proto in + bzr*|git*|hg*|svn*) + sum="SKIP" + ;; + *) + if [[ ! $netfile = *.@(sig?(n)|asc) ]]; then + local file + file="$(get_filepath "$netfile")" || missing_source_file "$netfile" + sum="$(openssl dgst -${integ} "$file")" + sum=${sum##* } + else + sum="SKIP" + fi + ;; + esac + + # indent checksum on lines after the first + printf "%*s%s" $(( idx ? indentsz : 0 )) '' "'$sum'" + + # print a newline on lines before the last + (( ++idx < numsrc )) && echo
and the actual index here.
+ done + + echo ")" +} + generate_checksums() { msg "$(gettext "Generating checksums for source files...")"
This implements support for declarations such as: arch=('i686' 'x86_64') ... source=("somescript.sh") source_i686=("http://evilmonster.com/i686/ponies-9001-1.i686.bin") source_x86_64=("http://evilmonster.com/i686/ponies-9001-1.x86_64.bin") md5sums=('d41d8cd98f00b204e9800998ecf8427e') md5sums_i686=('e4ca381035a34b7a852184cc0dd89baa') md5sums_x86_64=('4019740e6998f30a3c534bac6a83f582') Just the same as the "untagged" sources, multiple integrity algorithms are supported. The manpage is updated to reflect support for these suffices. --- doc/PKGBUILD.5.txt | 4 ++ scripts/makepkg.sh.in | 104 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 90 insertions(+), 18 deletions(-) diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index c957180..a8c1fbb 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -116,6 +116,10 @@ below). Compressed files will be extracted automatically unless found in the noextract array described below. + +Additional architecture-specific sources can be added by appending an +underscore and the architecture name e.g., 'source_x86_64=()'. There must be a +corresponding integrity array with checksums, e.g. 'md5sums_x86_64=()'. ++ It is also possible to change the name of the downloaded file, which is helpful with weird URLs and for handling multiple source files with the same name. The syntax is: `source=('filename::url')`. diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3962005..639f860 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -702,7 +702,39 @@ extract_svn() { popd &>/dev/null } +get_all_sources() { + local aggregate l a + + if array_build l 'source'; then + aggregate+=("${l[@]}") + fi + + for a in "${arch[@]}"; do + if array_build l "source_$a"; then + aggregate+=("${l[@]}") + fi + done + + array_build "$1" "aggregate" +} + +get_all_sources_for_arch() { + local aggregate l + + if array_build l 'source'; then + aggregate+=("${l[@]}") + fi + + if array_build l "source_$CARCH"; then + aggregate+=("${l[@]}") + fi + + array_build "$1" "aggregate" +} + download_sources() { + local netfile all_sources + msg "$(gettext "Retrieving sources...")" local GET_VCS=1 @@ -710,8 +742,8 @@ download_sources() { GET_VCS=0 fi - local netfile - for netfile in "${source[@]}"; do + get_all_sources_for_arch 'all_sources' + for netfile in "${all_sources[@]}"; do pushd "$SRCDEST" &>/dev/null local proto=$(get_protocol "$netfile") @@ -911,8 +943,10 @@ in_array() { } source_has_signatures() { - local file - for file in "${source[@]}"; do + local file all_sources + + get_all_sources_for_arch 'all_sources' + for file in "${all_sources[@]}"; do if [[ ${file%%::*} = *.@(sig?(n)|asc) ]]; then return 0 fi @@ -1055,16 +1089,27 @@ get_integlist() { } generate_one_checksum() { - local integ=$1 numsrc=${#source[*]} indentsz idx + local integ=$1 arch=$2 sources numsrc indentsz idx + if [[ $arch ]]; then + array_build sources "source_$arch" + else + array_build sources 'source' + fi + + numsrc=${#sources[*]} if (( numsrc == 0 )); then return fi - printf "%ssums=(%n" "$integ" indentsz + if [[ $arch ]]; then + printf "%ssums_%s=(%n" "$integ" "$arch" indentsz + else + printf "%ssums=(%n" "$integ" indentsz + fi for (( idx = 0; idx < numsrc; i++ )); do - local netfile=${source[idx]} + local netfile=${sources[idx]} local proto sum proto="$(get_protocol "$netfile")" @@ -1117,6 +1162,9 @@ generate_checksums() { fi generate_one_checksum "$integ" + for a in "${arch[@]}"; do + generate_one_checksum "$integ" "$a" + done done } @@ -1149,15 +1197,25 @@ verify_integrity_one() { } verify_integrity_sums() { - local integ=$1 integrity_sums + local integ=$1 arch=$2 integrity_sums=() sources=() - array_build integrity_sums "${integ}sums" + if [[ $arch ]]; then + array_build integrity_sums "${integ}sums_$arch" + array_build sources "source_$arch" + else + array_build integrity_sums "${integ}sums" + array_build sources source + fi - if (( ${#integrity_sums[@]} == ${#source[@]} )); then + if (( ${#integrity_sums[@]} == 0 && ${#sources[@]} == 0 )); then + return 1 + fi + + if (( ${#integrity_sums[@]} == ${#sources[@]} )); then msg "$(gettext "Validating source files with %s...")" "${integ}sums" local idx errors=0 - for (( idx = 0; idx < ${#source[*]}; idx++ )); do - verify_integrity_one "${source[idx]}" "$integ" "${integrity_sums[idx]}" || errors=1 + for (( idx = 0; idx < ${#sources[*]}; idx++ )); do + verify_integrity_one "${sources[idx]}" "$integ" "${integrity_sums[idx]}" || errors=1 done if (( errors )); then @@ -1177,9 +1235,13 @@ check_checksums() { (( ! ${#source[@]} )) && return 0 local correlation=0 - local integ + local integ a for integ in "${known_hash_algos[@]}"; do verify_integrity_sums "$integ" && correlation=1 + + for a in "${arch[@]}"; do + verify_integrity_sums "$integ" "$a" && correlation=1 + done done if (( ! correlation )); then @@ -1256,8 +1318,10 @@ check_pgpsigs() { local warning=0 local errors=0 local statusfile=$(mktemp) + local all_sources - for file in "${source[@]}"; do + get_all_sources_for_arch 'all_sources' + for file in "${all_sources[@]}"; do file="$(get_filename "$file")" if [[ ! $file = *.@(sig?(n)|asc) ]]; then continue @@ -1373,8 +1437,10 @@ check_source_integrity() { extract_sources() { msg "$(gettext "Extracting sources...")" - local netfile - for netfile in "${source[@]}"; do + local netfile all_sources + + get_all_sources_for_arch 'all_sources' + for netfile in "${all_sources[@]}"; do local file=$(get_filename "$netfile") if in_array "$file" "${noextract[@]}"; then # skip source files in the noextract=() array @@ -2065,8 +2131,10 @@ create_srcpackage() { msg2 "$(gettext "Adding %s...")" "$BUILDSCRIPT" ln -s "${BUILDFILE}" "${srclinks}/${pkgbase}/${BUILDSCRIPT}" - local file - for file in "${source[@]}"; do + local file all_sources + + get_all_sources all_sources 'all_sources' + for file in "${all_sources[@]}"; do if [[ "$file" = "$(get_filename "$file")" ]] || (( SOURCEONLY == 2 )); then local absfile absfile=$(get_filepath "$file") || missing_source_file "$file" -- 2.0.4
On 08/07/14 at 08:46pm, Dave Reisner wrote:
This implements support for declarations such as:
arch=('i686' 'x86_64') ...
source=("somescript.sh") source_i686=("http://evilmonster.com/i686/ponies-9001-1.i686.bin") source_x86_64=("http://evilmonster.com/i686/ponies-9001-1.x86_64.bin")
md5sums=('d41d8cd98f00b204e9800998ecf8427e') md5sums_i686=('e4ca381035a34b7a852184cc0dd89baa') md5sums_x86_64=('4019740e6998f30a3c534bac6a83f582')
Just the same as the "untagged" sources, multiple integrity algorithms are supported. The manpage is updated to reflect support for these suffices. --- doc/PKGBUILD.5.txt | 4 ++ scripts/makepkg.sh.in | 104 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 90 insertions(+), 18 deletions(-)
...
@@ -710,8 +742,8 @@ download_sources() { GET_VCS=0 fi
- local netfile - for netfile in "${source[@]}"; do + get_all_sources_for_arch 'all_sources' + for netfile in "${all_sources[@]}"; do pushd "$SRCDEST" &>/dev/null
local proto=$(get_protocol "$netfile")
...
@@ -2065,8 +2131,10 @@ create_srcpackage() { msg2 "$(gettext "Adding %s...")" "$BUILDSCRIPT" ln -s "${BUILDFILE}" "${srclinks}/${pkgbase}/${BUILDSCRIPT}"
- local file - for file in "${source[@]}"; do + local file all_sources + + get_all_sources all_sources 'all_sources'
It looks like get_all_sources only takes one argument but you're passing two.
+ for file in "${all_sources[@]}"; do if [[ "$file" = "$(get_filename "$file")" ]] || (( SOURCEONLY == 2 )); then local absfile absfile=$(get_filepath "$file") || missing_source_file "$file"
Doesn't this break --allsource packages? This attempts to add *all* sources but only the arch-specific sources are ever downloaded. It should also probably be noted somewhere that all sources must have unique names even between different architectures, for example the following will confuse create_srcpackage: source_i686=( "http://evilmonster.com/i686/ponies-9001-1.bin") source_x86_64=("http://evilmonster.com/x64/ponies-9001-1.bin") apg
participants (2)
-
Andrew Gregory
-
Dave Reisner