[pacman-dev] [PATCH 1/5] makepkg: break out check_checksums to reasonably sized functions

[Dave Reisner]

On Mon, Sep 29, 2014 at 01:50:08PM +1000, Allan McRae wrote:
> On 16/08/14 01:41, Dave Reisner wrote:
> > ---
> >  scripts/makepkg.sh.in | 94 +++++++++++++++++++++++++++++----------------------
> >  1 file changed, 53 insertions(+), 41 deletions(-)
> > 
> > diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> > index 8e8a64c..116b46e 100644
> > --- a/scripts/makepkg.sh.in
> > +++ b/scripts/makepkg.sh.in
> > @@ -1177,53 +1177,65 @@ generate_checksums() {
> >  	done
> >  }
> >  
> > -check_checksums() {
> > -	(( SKIPCHECKSUMS )) && return 0
> > -	(( ! ${#source[@]} )) && return 0
> 
> Because you deleted this, a PKGBUILD with no sources gives:
> 
> ==> Validating source files with md5sums...
> ==> Validating source files with sha1sums...
> ==> Validating source files with sha224sums...
> ==> Validating source files with sha256sums...
> ==> Validating source files with sha384sums...
> ==> Validating source files with sha512sums...
> 

Hrmm, I see this problem manifest differently... perhaps because you
tested it at this patch rather than at the head of my branch?

Regardless, will fix, and I'll be on the lookout for this as well.

> > +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"
> > +
> 
> So we used to do this:
> 
> local sumname="${integ}sums[@]"
> local integrity_sums=("${!sumname}")
> 
> My question is why change to array_build?  Is there an advantage that I
> am missing?
> 

The old method relies on undocumented behavior. array_build does not.

> > +	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
> > +
> > +	local correlation=0
> > +	local integ
> > +	for integ in "${known_hash_algos[@]}"; do
> > +		verify_integrity_sums "$integ" && correlation=1
> >  	done
> >  
> >  	if (( ! correlation )); then
> >