Display commit signatures #4853
Draft
+472
−37
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pylbrecht
force-pushed
the
jj-sign/display
branch
15 times, most recently
from
760fda2
to
b000597
Compare
pylbrecht
force-pushed
the
jj-sign/display
branch
from
b000597
to
30ef3c4
Compare
We need to make `TestSigningBackend` available for cli tests, as we will add cli tests for signing related functionality (templates for displaying commit signatures, `jj sign`) in upcoming commits.
pylbrecht
force-pushed
the
jj-sign/display
branch
2 times, most recently
from
8f52d58
to
d1a1a72
Compare
Disclaimer: this is the work of @necauqua and @julienvincent (see martinvonz#3141). I simply materialized the changes by rebasing them on latest `main` and making the necessary adjustments to pass CI. --- I had to fix an issue in `TestSignatureBackend::sign()`. The following test was failing: ``` ---- test_signature_templates::test_signature_templates stdout ---- ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot: signature_templates Source: cli/tests/test_signature_templates.rs:28 ──────────────────────────────────────────────────────────────────────────────── Expression: stdout ──────────────────────────────────────────────────────────────────────────────── -old snapshot +new results ────────────┬─────────────────────────────────────────────────────────────────── 0 0 │ @ Commit ID: 05ac066d05701071af20e77506a0f2195194cbc9 1 1 │ │ Change ID: qpvuntsmwlqtpsluzzsnyyzlmlwvmlnu 2 2 │ │ Author: Test User <[email protected]> (2001-02-03 08:05:07) 3 3 │ │ Committer: Test User <[email protected]> (2001-02-03 08:05:07) 4 │-│ Signature: Good test signature 4 │+│ Signature: Bad test signature 5 5 │ │ 6 6 │ │ (no description set) 7 7 │ │ 8 8 │ ◆ Commit ID: 0000000000000000000000000000000000000000 ────────────┴─────────────────────────────────────────────────────────────────── ``` Print debugging revealed that the signature was bad, because of a missing trailing `\n` in `TestSignatureBackend::sign()`. ```diff diff --git a/lib/src/test_signing_backend.rs b/lib/src/test_signing_backend.rs index d47fef1086..0ba249e358 100644 --- a/lib/src/test_signing_backend.rs +++ b/lib/src/test_signing_backend.rs @@ -59,6 +59,8 @@ let key = (!key.is_empty()).then_some(std::str::from_utf8(key).unwrap().to_owned()); let sig = self.sign(data, key.as_deref())?; + dbg!(&std::str::from_utf8(&signature).unwrap()); + dbg!(&std::str::from_utf8(&sig).unwrap()); if sig == signature { Ok(Verification::new( SigStatus::Good, ``` ``` [lib/src/test_signing_backend.rs:62:9] &std::str::from_utf8(&signature).unwrap() = \"--- JJ-TEST-SIGNATURE ---\\nKEY: \\n5300977ff3ecda4555bd86d383b070afac7b7459c07f762af918943975394a8261d244629e430c8554258904f16dd9c18d737f8969f2e7d849246db0d93cc004\\n\" [lib/src/test_signing_backend.rs:63:9] &std::str::from_utf8(&sig).unwrap() = \"--- JJ-TEST-SIGNATURE ---\\nKEY: \\n5300977ff3ecda4555bd86d383b070afac7b7459c07f762af918943975394a8261d244629e430c8554258904f16dd9c18d737f8969f2e7d849246db0d93cc004\" ``` I still have no idea, where in the call chain that trailing `\n` is added to `signature`. I tried to retrace `signature`'s steps. However, it seemed to be returned from `TestSignatureBackend::sign()`, which was even more mind-boggling to me since `sig` is also returned from `TestSignatureBackend::sign()`. How can they be different?
pylbrecht
force-pushed
the
jj-sign/display
branch
from
d1a1a72
to
4e6ba53
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes are continuing the work of #3141.
I took over its changes and rebased them on latest
main.This work is required for #4747, as it lays the foundation for writing CLI tests for
jj sign.Checklist
If applicable:
CHANGELOG.md