Sebastian Götte <jaseg@xxxxxxxxxxxxxxxxxxx> writes: > When --verify-signatures is specified on the command-line of git-merge > or git-pull, check whether the commits being merged have good gpg > signatures and abort the merge in case they do not. This allows e.g. > auto-deployment from untrusted repo hosts. > > Signed-off-by: Sebastian Götte <jaseg@xxxxxxxxxxxxxxxxxxxxxxxx> > --- > ... > + if (verify_signatures) { > + /* Verify the commit signatures */ > + for (p = remoteheads; p; p = p->next) { > + struct commit *commit = p->item; > + struct signature signature; > + signature.check_result = 0; Even though you may happen to know all other fields are output only, it is a good practice to clear it with memset(&signature, 0, sizeof(signature); By the way, I think this variable and type should be called more like "signature_check" or something with "check" in its name, not "signature". After all it is _not_ a signature itself. > + check_commit_signature(commit, &signature); > + > + char hex[41]; builtin/merge.c: In function 'cmd_merge': builtin/merge.c:1247: error: ISO C90 forbids mixed declarations and code > + > +test_expect_success GPG 'merge unsigned commit with verification' ' > + test_must_fail git merge --ff-only --verify-signatures side-unsigned 2> mergeerror && No SP between redirection and the target filename. > + grep "does not have a GPG signature" mergeerror Do we plan to make this message localized? If so I think you would need to do this with test_i18ngrep. > +' > + > +test_expect_success GPG 'merge commit with bad signature with verification' ' > + test_must_fail git merge --ff-only --verify-signatures $(cat forged.commit) 2> mergeerror && > + grep "has a bad GPG signature" mergeerror > +' > + > +test_expect_success GPG 'merge signed commit with verification' ' > + git merge -v --ff-only --verify-signatures side-signed > mergeoutput && > + grep "has a good GPG signature" mergeoutput > +' Hmph. So the caller needs to check both the standard output and the standard error to get the whole picture? Is there a reason why we are not sending everything to the standard output consistently? > +test_expect_success GPG 'merge commit with bad signature without verification' ' > + git merge $(cat forged.commit) > +' Good to see that negative case where the new feature should _not_ trigger is properly tested. > + > +test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html