Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: git push: allow signing commits on push #5164

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bnjmnt4n
Copy link
Member

This adds the git.sign-on-push configuration which can be used to
automatically sign unsigned commits before pushing to a Git remote.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

pylbrecht and others added 3 commits December 21, 2024 00:41
We need to make `TestSigningBackend` available for cli tests, as we will
add cli tests for signing related functionality (templates for
displaying commit signatures, `jj sign`) in upcoming commits.
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?
This adds the `git.sign-on-push` configuration which can be used to
automatically sign unsigned commits before pushing to a Git remote.
@bnjmnt4n bnjmnt4n changed the title cli: git: allow signing commits on push cli: git push: allow signing commits on push Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants