[pacman-dev] [PATCH 3/3] makepkg: Introduce validpgpkeys array

[Dave Reisner]

On Sat, Mar 08, 2014 at 06:34:53PM +0100, Thomas Bächler wrote:
> Am 08.03.2014 18:20, schrieb Dave Reisner:
> >> +	pubkey=$(grep VALIDSIG "$statusfile" | sed -nr 's/.* VALIDSIG ([A-Z0-9]*) .*/\1/p;')
> > 
> > I think you just want:
> > 
> >   pubkey=$(sed -n '/VALIDSIG/ s/.* VALIDSIG \([[:alnum:]]*\) .*/\1/p' "$statusfile")
> > 
> > sed's -r flag isn't portable.
> 
> I took that from another place in makepkg:
> 
> scripts/makepkg.sh.in:1740:             for sofile in $(LC_ALL=C readelf
> -d "$filename" 2>/dev/null | sed -nr 's/.*Shared library: \[(.*)\].*/\1/p')
> 
> If it's not portable, it should be fixed there, too.

Hrmm. It seems that, at least, more recent freebsd now suppots this:

     -r      Same as -E for compatibility with GNU sed.

I'd prefer that we avoid it, though. We can absolutely get by without it
in both cases.

> >> +	echo "$pubkey"
> > 
> > Don't you only want to echo this if the check that follows succeeds?
> 
> Actually, I only need it in the failure case.
> 
> >> +	in_array "$pubkey" ${validpgpkeys[@]}
> > 
> > The array needs quoting.
> 
> Not according to the documentation, but sure, I'll add it.
> 
> >> +	return $?
> > 
> > Wholly redundant for this function in its current form.
> 
> You are right, although I hate that implicit return value stuff in bash.
> 
> >> +}
> >> +
> >>  check_pgpsigs() {
> >>  	(( SKIPPGPCHECK )) && return 0
> >>  	! source_has_signatures && return 0
> >> @@ -1303,9 +1312,12 @@ check_pgpsigs() {
> >>  			if grep -q "REVKEYSIG" "$statusfile"; then
> >>  				printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key has been revoked.")" >&2
> >>  				errors=1
> >> -			elif grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then
> >> +			elif (( ${#validpgpkeys[@]} == 0 )) && grep -q -e "TRUST_UNDEFINED" -e "TRUST_NEVER" "$statusfile"; then
> >>  				printf '%s (%s)' "$(gettext "FAILED")" "$(gettext "the key is not trusted")" >&2
> >>  				errors=1
> >> +			elif (( ${#validpgpkeys[@]} > 0 )) && ! pubkey=$(is_valid_pgpkey "$statusfile"); then
> >> +				printf "%s (%s $pubkey)" "$(gettext "FAILED")" "$(gettext "invalid key")"
> >> +				errors=1
> > 
> > Is there a decent way to extract the real status from the file once and
> > then do string comparisons in bash, rather than forking to grep all the
> > time?
> 
> You just used the word 'decent' in a sentence that talks about gnupg.

You're right, I must apologize for that faux pas.

> We could use read to parse the file, set some variables and test those.
> Is that desirable?

I forget what the format is of the status files, exactly. I don't mind
calling external tools to parse once, but calling them repeatedly to
operate on the same section of a file seems wasteful.