[pacman-dev] [PATCH 1/2] Add support for verifying pgp signatures to makepkg

[Wieland Hoffmann]

Hallo, Dan McGee:
> On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann
> <themineo at googlemail.com> wrote:
> 
> First, thanks for giving this a try.
> 
> Commit descriptions are nice to have in permanent history, but all
> that stuff you wrote in the cover letter won't show up. Can you
> instead include some of that right here in the patch in commit
> message-style writing?

Will do (looks like I have to resubmit these anyway).

> > +
> > +       msg "$(gettext "Validating source files with gpg...")"
> > +
> > +       local file
> > +       local errors=0
> > +
> > +       for file in "${pgpsigs[@]}"; do
> > +               local valid
> > +               local found=1
> > +
> > +               file="$(get_filename "$file")"
> > +               echo -n "    ${file%.sig} ... " >&2
> > +
> > +               if ! file="$(get_filepath "$file")"; then
> What are you doing here? Assignment or comparison? Do this in two
> statements rather than trying to be cute, and use an actual [[ -z ]]
> block please.

That's just a copy of
http://projects.archlinux.org/pacman.git/tree/scripts/makepkg.sh.in#n640
get_filepath() already checks if the file exists.

> > +                       echo "$(gettext "NOT FOUND")" >&2
> > +                       errors=1
> > +                       found=0
> > +               fi
> > +
> > +               if (( found )); then
> > +                       if ! gpg --quiet --batch --verify "$file" 2> /dev/null; then
> > +                               echo "$(gettext "Verification failed")" >&2
> Any need to eat stderr? If things only show up in exceptional cases,
> I'd rather it come through.

Oh, that's supposed to be a 1.

> > @@ -2129,6 +2174,9 @@ else
> >        download_sources
> >        if (( ! SKIPINTEG )); then
> >                check_checksums
> > +               if (( PGPSIGS )); then
> > +                       check_pgpsigs
> > +               fi
> This check should probably match how we do it elsewhere; e.g.,
> check_signature() the first two lines.

Hm, check_signature()? Either grep is lying to me or that doesn't exist
anywhere.

> I'd move it inside check_pgpsigs itself. Also declare it upfront where
> we set the rest of these.

I'm not exactly sure how I missed that, thanks!

-- 
Wieland
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://mailman.archlinux.org/pipermail/pacman-dev/attachments/20110624/7d8d5edc/attachment-0001.asc>