Hallo, Dan McGee:
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@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