Hallo, Dan McGee: > On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann > <[email protected]> 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
pgpOHXtYHzZ1b.pgp
Description: PGP signature
