Re-roll of GPG commit signing #94
Conversation
GPG-sign commits if option -S/--gpg-sign is given, or if configurations revise.gpgSign or commit.gpgSign are set to true. Unlike Git, we do not support passing the key ID as argument. Git's way of accepting this optional argument is a bit weird. It would be awkward to try to support the same with argparse. So it's probably best to use a separate argument for the key ID in future. For now, the key ID is taken from Git configuration (user.signingKey) or the committer signature as fallback. A call of `git-revise COMMIT --gpg-sign` w/o any changes in the index causes creation of GPG commit signatures for all commits since COMMIT. Conversely, if GPG signing is disabled, git-revise will remove the signature, even if the commits were not modified otherwise. So adding/removing a signature is just another modification, like rewording/rebasing. This logic is applied to all commits in the todo-list. The test is a bit large, but the pieces are mostly independen. Maybe we should share the GPG initialization somehow? Closes mystor#46 Closes mystor#73 Co-authored-by: Johannes Altmanninger <aclopte@gmail.com> Signed-off-by: Nicolas Schier <nicolas@fjasle.eu>
krobelus
force-pushed
the
gpgsign-at-last
branch
from
941b470
to
c2f30a8
Compare
| check=True, | ||
| ) | ||
| except CalledProcessError as gpg: | ||
| print(gpg.stderr.decode(), file=sys.stderr, end="") |
There was a problem hiding this comment.
This access to gpg.stderr will never work: if run() throws, gpg will not have been assigned a value other than the None above.
There was a problem hiding this comment.
Nevermind, I see now that gpg is aliased to the caught exception itself!
There was a problem hiding this comment.
heh, that's pretty clever 😮
| @pytest.fixture | ||
| def short_tmpdir(): | ||
| with tempfile.TemporaryDirectory() as tdir: | ||
| yield py.path.local(tdir) |
There was a problem hiding this comment.
py.path is deprecated. Should this just use from pathlib import Path which is used elsewhere in this project?
There was a problem hiding this comment.
Thanks, I didn't know! I remember I was just glad to get the macOS tests working somehow..
| main(["HEAD~~", "--gpg-sign"]) | ||
| assert repo.get_commit("HEAD~~").gpgsig is not None | ||
| assert repo.get_commit("HEAD~").gpgsig is not None | ||
| assert repo.get_commit("HEAD").gpgsig is not None |
There was a problem hiding this comment.
Something concerning about these tests: they only check round-tripping of gitrevise's signature parsing/generation. There are no tests with git verify-commit, for example, to demonstrate that the signature is formatted as git itself expects.
There was a problem hiding this comment.
Yeah, we should definitely have at least one test that shows that we produce/consume the same signatures as Git.
This is #73 with review comments applied and two behavior changes.
(the broken Mac tests were pytest-dev/pytest#5802)