On Fri, Jun 24, 2011 at 3:53 AM, Wieland Hoffmann <[email protected]> wrote: > 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. Aha, OK. It's fine, it just is a bit hairy in what it is doing.
> >> > + 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. I was suggesting no redirection at all. >> 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. create_signature, whoops!
