Signature verification #3141
Closed
Signature verification #3141
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
julienvincent
force-pushed
the
jv/signature-verification
branch
3 times, most recently
from
90f7c4b to
3b07dbe
Compare
necauqua
reviewed
Feb 25, 2024
cli/src/cli_util.rs
Outdated
Comment on lines
2755
to
2362
| if args.show_signature.unwrap_or_default() { | ||
| args.config_toml | ||
| .push(r#"signing.show-signatures=true"#.to_owned()); | ||
| } |
There was a problem hiding this comment.
I think this should not be a global flag - it only applies to log and status, maybe something else that's using the template, and that's it
There was a problem hiding this comment.
I agree, but it's really not clear to me how to link command local arguments to the config as it is done here. How would I go about doing that?
There was a problem hiding this comment.
I think you will have to make the flag a member of CommitTemplateLanguage.
There was a problem hiding this comment.
Is it common to temporarily turn on/off this flag?
Can't we simply let the user select -Tbuiltin_log_compact_without_signature or -Tbuiltin_log_compact_with_signature? (Or something like template-aliases.format_cryptographic_signature(signature) to override?)
There was a problem hiding this comment.
Personally I believe this does belong as a feature flag. Signing is a fairly broad operation that has 'first-class' support in other commands and should I think have similar 'first-class' support in commands that render commits.
It seems a bit clunky to have to maintain separate (otherwise identical) templates just for rendering a signature - and similarly clunky from a users perspective to then need to select the correct template variation. This is made even worse if the user is then making their own templates that need to show this information.
Being able to build templates that support rendering a signature based on whether the information is present or not seems to me like a better all-round approach.
Is it common to temporarily turn on/off this flag?
I don't know if I would claim this is a commonly used option, but I do use this fairly often to check (for example) that I am not overriding someones signatures during a rebase.
By default I want it off unless I want to quickly view this information. If this was a template I would need to google the template name every time I wanted to use it as apposed to just checking help text.
Lastly, and probably not a very strong point, but git does also exposes a flag with this name. Doing the same would help ease the experience of users coming from git as things will all work in a familiar way.
There was a problem hiding this comment.
yeah I agree with everything said, templates seem a bit clunky for this
There was a problem hiding this comment.
I'm not against adding a flag to disable verification process (which I think @necauqua suggested somewhere), but I think the visibility should be controlled by template. It's confusing if -Tsignature needed additional flag to turn it into non-empty string.
I don't know if I would claim this is a commonly used option, but I do use this fairly often to check (for example) that I am not overriding someones signatures during a rebase.
By default I want it off unless I want to quickly view this information. If this was a template I would need to google the template name every time I wanted to use it as apposed to just checking help text.
It's not uncommon to switch between a few sets of preferred templates. Git provides various flags to control outputs (--oneline, --pretty=, etc.) In jj (and hg), it's controlled by -T/--template. I don't think --show-signature is special, and I would probably want to enable the signature only in detailed output (e.g. only in -Tbuiltin_log_detailed.)
Regarding discoverability, I agree. (related: #3175)
There was a problem hiding this comment.
Yeah, I guess this all makes sense too.
Since the verification is only invoked when the templater asks for it, you can just not have it in the default template and have a non-default template that shows the sigs.
With that, to show the signature once you'd do jj log .. -T show-signature or similar, and to have it shown always you'd edit the template in your config - eliminating the need for the signing.show-signatures config option.
We could add a hidden --show-signature flag to the log command the sole purpose of which would be to print a hint that "probably you want -T builtin_log_with_signature_or_something_shorter_than_that_lol" - this would be great for learning when moving from git
@julienvincent what do you think about this
Some other thoughts:
I'd keep the template alias function thing that makes pretty checkmarks from the signature variable that I had there - just don't use it by default, but keep it for me so that the default template override is simple - and such an override could go into the docs like "if you want to see the checkmars like on github".
Probably would be great to go the format_short_id route - have a template verify_commit_signature that resolves to nothing and is used in the default templates, and then people can do something like verify_commit_signature = render_the_checkmark_thing in their config.
Another thing is I'd refactor the verification so that it happens only on concrete signature.valid/invalid/etc template calls, but not on signature.present(), which would just check for the gpgsig header presence which is real fast, to support the usecase I've described where someone (me again) might want to just always have a symbol in the log showing that there's some signature without paying the performance cost of full verification on every log.
There was a problem hiding this comment.
I'd keep the template alias function thing that makes pretty checkmarks from the signature variable that I had there - just don't use it by default, but keep it
for meso that the default template override is simple - and such an override could go into the docs like "if you want to see the checkmars like on github".Probably would be great to go the format_short_id route - have a template
verify_commit_signaturethat resolves to nothing and is used in the default templates, and then people can do something likeverify_commit_signature = render_the_checkmark_thingin their config.
That could be other way around, or both?
builtin_log_compact(whatever_template_to_insert_after_commit_id) = ...
then
my_log_sig = 'builtin_log_compact(fancy_sig_status)'
There was a problem hiding this comment.
Yes I think I am also in agreement with what has been discussed here. I had already started to implement the discussed changes, starting by adding a way to test the signature templates and then removing the changes to the default templates and the --show-signature flag.
I haven't had a lot of time in the last couple weeks so sorry this is dragging on a bit. I will probably only get time to complete the work here on the upcoming weekend.
necauqua
reviewed
Feb 25, 2024
Comment on lines
+583
to
+591
| Commit verification uses the configured [signing backends](#commit-signing) to attempt to verify | ||
| any signatures found on commits when displaying them with `jj log` or `jj show`. |
There was a problem hiding this comment.
"configured signing backends" here sounds like "enabled signing backends" - which is not the case, the verification uses "all the backends" - and well while you have to configure allowed signers for ssh, for gpg it actually just works™ if it's installed and the keys are imported (and even for unknown keys it'll show key fingerprint), no jj configuration needed
yuja
reviewed
Feb 26, 2024
| * `parents: List<Commit>` | ||
| * `author: Signature` | ||
| * `committer: Signature` | ||
| * `signature: CommitSignature`: The information about a cryptographic signature of the commit. |
There was a problem hiding this comment.
I've moved the whole section under the "Commit type", so signature: .. will have to be moved there.
I'm also doing some structural changes in the template engine. It should be relatively easy to migrate to the new form, but auto-rebase will fail. Just fyi.
cli/src/config/templates.toml
Outdated
There was a problem hiding this comment.
Maybe better to split the changes in the default templates? I think this would be a controversial part, and we'll have to be careful because these changes affect all users.
| ui: &Ui, | ||
| old_commit: &Commit, | ||
| rewritten: &Commit, | ||
| ) -> Result<(), std::io::Error> { |
There was a problem hiding this comment.
nit: maybe this can be inlined? It's not obvious at the call site whether the arguments are (old, new) or (new, old).
CHANGELOG.md
Outdated
| @@ -30,6 +30,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| ### New features | |||
|
|
|||
| + Add templater support for rendering commit signatures and update default | |||
| templates. | |||
|
|
|||
There was a problem hiding this comment.
nit: can be squashed to the templating commit.
We usually include trivial test and doc changes in the implementation patch.
yuja
reviewed
Feb 28, 2024
julienvincent
force-pushed
the
jv/signature-verification
branch
6 times, most recently
from
ad2bfd4 to
bf1620e
Compare
Currently the TestSigningBackend can only be used from the test cradle. This makes it hard to build tests for signing commands as they are tested against the CLI interface directly. This commit renames the TestSigningBackend struct to MockSigningBackend and moves it to the lib module, allowing us to configure it via config and therefore use it from command based tests.
Add it to default templates as well, it only shows anything when there's a signature.
julienvincent
force-pushed
the
jv/signature-verification
branch
from
bf1620e to
739ddf2
Compare
4 tasks
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Nov 23, 2024
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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? --- Co-authored-by: julienvincent Co-authored-by: pylbrecht
pylbrecht
added a commit
to pylbrecht/jj
that referenced
this pull request
Nov 23, 2024
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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? --- Co-authored-by: julienvincent <[email protected]> Co-authored-by: pylbrecht <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Nov 23, 2024
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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?
bnjmnt4n
pushed a commit
that referenced
this pull request
Dec 20, 2024
Disclaimer: this is the work of @necauqua and @julienvincent (see #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
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Dec 24, 2024
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Dec 24, 2024
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Dec 24, 2024
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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? Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 1, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 1, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 1, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 1, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 2, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
|
Superseded by #4853 |
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 3, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 4, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 4, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 4, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 4, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
to pylbrecht/jj
that referenced
this pull request
Jan 4, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see jj-vcs#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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
pylbrecht
pushed a commit
that referenced
this pull request
Jan 4, 2025
Disclaimer: this is the work of @necauqua and @julienvincent (see #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\" ``` Thankfully, @yuja pointed out that libgit2 appends a trailing newline (see bfb7613). Co-authored-by: necauqua <[email protected]> Co-authored-by: julienvincent <[email protected]>
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.
This is a continuation from #3007 and #2728 which adds in the templater changes for verifying and displaying commit signatures.
There were some outstanding comments on #3007 which I am listing below:
Checklist
If applicable:
CHANGELOG.md