Radish alpha
h
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
radicle/patch: Add Change ID
Draft lorenz opened 11 months ago

Radicle patches have IDs, which are the commit IDs of the COB.

Other tools, like Jujutusu, Gerrit, and GitButler use Change IDs to refer to commits consistently as they are rebased.

COB

In order for Radicle to integrate with these tools more easily in the future, add an action that allows linking a patch with a Change ID.

This naturally requires an optional struct member and some boilerplate.

CLI

rad patch show will print the Change ID if linked.

CLI Tests

Most of this is taken from Jujutsu’s own testing setup, see: https://github.com/jj-vcs/jj/blob/98d884827e2b9db72d480bb0c3610e51f103b7d4/cli/tests/common/test_environment.rs#L106-L150

Not all features are preserved, but this is good enough.


See also: https://lore.kernel.org/git/CAESOdVAspxUJKGAA58i0tvks4ZOfoGf1Aa5gPr0FXzdcywqUUw@mail.gmail.com/T

9 files changed +289 -20 5b4cbc2c 0dde04a9
modified flake.nix
@@ -102,6 +102,7 @@
          ]);
          nativeCheckInputs = with pkgs; [
            jq
+
            jujutsu
          ];

          env =
modified radicle-cli-test/src/lib.rs
@@ -107,6 +107,9 @@ pub struct TestRunner<'a> {
    cwd: Option<PathBuf>,
    homes: HashMap<String, Home>,
    formula: &'a TestFormula,
+

+
    /// For seeding randomness in Jujutsu.
+
    jj_seed: usize,
}

impl<'a> TestRunner<'a> {
@@ -115,6 +118,7 @@ impl<'a> TestRunner<'a> {
            cwd: None,
            homes: formula.homes.clone(),
            formula,
+
            jj_seed: 0,
        }
    }

@@ -124,6 +128,7 @@ impl<'a> TestRunner<'a> {

        if let Some(ref h) = test.home {
            if let Some(home) = self.homes.get(h) {
+
                env.insert("USER".to_owned(), h.to_owned());
                return TestRun {
                    home: home.clone(),
                    env,
@@ -460,6 +465,17 @@ impl TestFormula {
                    fs::create_dir_all(run.path())?;
                }

+
                let jj_envs = if assertion.command == "jj" {
+
                    runner.jj_seed += 1;
+
                    vec![
+
                        ("JJ_RANDOMNESS_SEED", runner.jj_seed.to_string()),
+
                        ("JJ_TIMESTAMP", "2001-02-03T04:05:06+07:00".to_string()),
+
                        ("JJ_OP_TIMESTAMP", "2001-02-03T04:05:06+07:00".to_string()),
+
                    ]
+
                } else {
+
                    vec![]
+
                };
+

                let bins = self
                    .bins
                    .iter()
@@ -471,6 +487,7 @@ impl TestFormula {
                    .env("PATH", &bins)
                    .env("RUST_BACKTRACE", "1")
                    .envs(run.envs())
+
                    .envs(jj_envs)
                    .current_dir(run.path())
                    .args(args)
                    .with_assert(assert.clone())
added radicle-cli/examples/jj-init.md
@@ -0,0 +1,51 @@
+
Let's make sure that the config is exactly what we expect.
+

+
```
+
$ jj config list
+
ui.editor = "true"
+
user.name = "Test User"
+
user.email = "test.user@example.com"
+
debug.commit-timestamp = "2001-02-03T04:05:06+07:00"
+
debug.randomness-seed = 1
+
debug.operation-timestamp = "2001-02-03T04:05:06+07:00"
+
operation.hostname = "host.example.com"
+
operation.username = "test-username"
+
```
+

+
We enable writing Change ID headers to our commits.
+

+
```
+
$ jj config set --user git.write-change-id-header true
+
```
+

+
We initialize Jujutusu for our repository.
+

+
```(stderr)
+
$ jj git init --colocate
+
Done importing changes from the underlying Git repo.
+
Hint: The following remote bookmarks aren't associated with the existing local bookmarks:
+
  master@rad
+
Hint: Run `jj bookmark track master@rad` to keep local bookmarks updated on future pulls.
+
Initialized repo in "."
+
```
+

+
Off we go!
+

+
```
+
$ jj status
+
The working copy has no changes.
+
Working copy  (@) : pmmvwywv ed64a0f3 (empty) (no description set)
+
Parent commit (@-): xpnzuzwn f2de534b master master@rad | Second commit
+
```
+

+
Just making sure that Git sees the Change ID…
+

+
```
+
$ git cat-file commit ed64a0f3
+
tree b4eecafa9be2f2006ce1b709d6857b07069b4608
+
parent f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354
+
author Test User <test.user@example.com> 981147906 +0700
+
committer Test User <test.user@example.com> 981147906 +0700
+
change-id pmmvwywvzvvnvnzntqnqknuzpwttyvkr
+

+
```

\ No newline at end of file
added radicle-cli/examples/rad-patch-jj.md
@@ -0,0 +1,78 @@
+
The scenario in this file is a variation of the one in `rad-patch.md`,
+
but uses Change IDs generated by Jujutsu.
+

+
When contributing to another's project, it is common for the contribution to be
+
of many commits and involve a discussion with the project's maintainer.  This is supported
+
via Radicle's patches.
+

+
Here we give a brief overview for using patches in our hypothetical car
+
scenario.  It turns out instructions containing the power requirements were
+
missing from the project.
+

+
```
+
$ jj new
+
$ touch REQUIREMENTS
+
```
+

+
Here the instructions are added to the project's README for 1.21 gigawatts and
+
commit the changes to git.
+

+
```
+
$ jj describe --message "Define power requirements"
+
```
+

+
Once the code is ready, we open (or create) a patch with our changes for the project.
+

+
As of 2025-05 we can't use `jj` to do this directly, see:
+

+
 - <https://github.com/jj-vcs/jj/issues/4075>
+
 - <https://github.com/jj-vcs/jj/pull/2098>
+

+
However, since we initialized Jujutusu to colocate with Git, we can just use
+
Git to push.
+

+
``` (stderr)
+
$ git push rad -o patch.message="Define power requirements" -o patch.message="See details." HEAD:refs/patches
+
✓ Patch 0a47ee4cd4a976a021df5acf60eee5eb74b0c62d opened
+
✓ Change ID pmmvwywvzvvnvnzntqnqknuzpwttyvkr detected and linked
+
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
+
 * [new reference]   HEAD -> refs/patches
+
```
+

+
It will now be listed as one of the project's open patches.
+

+
```
+
$ rad patch
+
╭─────────────────────────────────────────────────────────────────────────────────────────╮
+
│ ●  ID       Title                      Author         Reviews  Head     +   -   Updated │
+
├─────────────────────────────────────────────────────────────────────────────────────────┤
+
│ ●  0a47ee4  Define power requirements  alice   (you)  -        ed64a0f  +0  -0  now     │
+
╰─────────────────────────────────────────────────────────────────────────────────────────╯
+
```
+

+
Let's also create a bookmark for it.
+

+
```
+
$ jj bookmark create flux-capacitor-power
+
```
+

+
```
+
$ rad patch show 0a47ee4 -p
+
╭─────────────────────────────────────────────────────╮
+
│ Title      Define power requirements                │
+
│ Patch      0a47ee4cd4a976a021df5acf60eee5eb74b0c62d │
+
│ Author     alice (you)                              │
+
│ Change ID  pmmvwywvzvvnvnzntqnqknuzpwttyvkr         │
+
│ Head       ed64a0f39d9929f5945d47e07de109fdf1dabc57 │
+
│ Head       3e674d1a1df90807e934f9ae5da2591dd6848a33 │
+
│ Branches   flux-capacitor-power                     │
+
│ Commits    ahead 1, behind 0                        │
+
│ Status     open                                     │
+
│                                                     │
+
│ See details.                                        │
+
├─────────────────────────────────────────────────────┤
+
│ 3e674d1 Define power requirements                   │
+
├─────────────────────────────────────────────────────┤
+
│ ● opened by alice (you) (3e674d1) now               │
+
╰─────────────────────────────────────────────────────╯
+
```

\ No newline at end of file
modified radicle-cli/src/terminal/patch.rs
@@ -388,6 +388,17 @@ pub fn show(
            term::format::secondary(labels.join(", ")).into(),
        ]);
    }
+
    if let Some(change_id) = patch.change_id() {
+
        attrs.push([
+
            term::format::tertiary("Change ID".to_owned()).into(),
+
            term::format::secondary(change_id).into(),
+
        ]);
+
    } else if verbose {
+
        attrs.push([
+
            term::format::tertiary("Change ID".to_owned()).into(),
+
            term::format::dim("None".to_owned()).into(),
+
        ]);
+
    }
    attrs.push([
        term::format::tertiary("Head".to_owned()).into(),
        term::format::secondary(revision.head().to_string()).into(),
modified radicle-cli/tests/commands.rs
@@ -99,6 +99,7 @@ fn test<'a>(

    formula(cwd.as_ref(), test)?
        .env("RAD_HOME", home.to_string_lossy())
+
        .env("JJ_CONFIG", home.join(".jjconfig.toml").to_string_lossy())
        .envs(envs)
        .run()?;

@@ -116,6 +117,11 @@ fn formula(root: &Path, test: impl AsRef<Path>) -> Result<TestFormula, Box<dyn s
        .env("GIT_COMMITTER_DATE", "1671125284")
        .env("GIT_COMMITTER_EMAIL", "radicle@localhost")
        .env("GIT_COMMITTER_NAME", "radicle")
+
        .env("JJ_USER", "Test User")
+
        .env("JJ_EMAIL", "test.user@example.com")
+
        .env("JJ_OP_HOSTNAME", "host.example.com")
+
        .env("JJ_OP_USERNAME", "test-username")
+
        .env("JJ_TZ_OFFSET_MINS", "660")
        .env("EDITOR", "true")
        .env("TZ", "UTC")
        .env("LANG", "C")
@@ -134,6 +140,24 @@ fn formula(root: &Path, test: impl AsRef<Path>) -> Result<TestFormula, Box<dyn s
    Ok(formula)
}

+
fn program_reports_version(program: &str) -> bool {
+
    use std::io::ErrorKind;
+
    use std::process::{Command, Stdio};
+

+
    match Command::new(program)
+
        .arg("--version")
+
        .stdout(Stdio::null())
+
        .status()
+
    {
+
        Err(e) if e.kind() == ErrorKind::NotFound => {
+
            log::warn!(target: "test", "`{program}` not found.");
+
            false
+
        }
+
        Err(e) => panic!("while checking for program {program}: {}", e),
+
        Ok(_) => true,
+
    }
+
}
+

#[test]
fn rad_auth() {
    test("examples/rad-auth.md", Path::new("."), None, []).unwrap();
@@ -204,21 +228,11 @@ fn rad_cob_update_identity() {

#[test]
fn rad_cob_multiset() {
-
    {
-
        // `rad-cob-multiset` is a `jq` script, which requires `jq` to be installed.
-
        // We test whether `jq` is installed, and have this test succeed if it is not.
-
        // Programmatic skipping of tests is not supported as of 2024-08.
-
        use std::io::ErrorKind;
-
        use std::process::{Command, Stdio};
-

-
        match Command::new("jq").arg("-V").stdout(Stdio::null()).status() {
-
            Err(e) if e.kind() == ErrorKind::NotFound => {
-
                log::warn!(target: "test", "`jq` not found. Succeeding prematurely.");
-
                return;
-
            }
-
            Err(e) => panic!("while checking for jq: {}", e),
-
            Ok(_) => {}
-
        }
+
    // `rad-cob-multiset` is a `jq` script, which requires `jq` to be installed.
+
    // We test whether `jq` is installed, and have this test succeed if it is not.
+
    // Programmatic skipping of tests is not supported as of 2024-08.
+
    if !program_reports_version("jq") {
+
        return;
    }

    let mut environment = Environment::new();
@@ -992,6 +1006,44 @@ fn rad_patch() {
}

#[test]
+
fn rad_patch_jj() {
+
    // We test whether `jj` is installed, and have this test succeed if it is not.
+
    // Programmatic skipping of tests is not supported as of 2024-08.
+
    if !program_reports_version("jj") {
+
        return;
+
    }
+

+
    let mut environment = Environment::new();
+
    let profile = environment.profile(config::profile("alice"));
+
    let working = tempfile::tempdir().unwrap();
+

+
    // Setup a test repository.
+
    fixtures::repository(working.path());
+

+
    test(
+
        "examples/rad-init.md",
+
        working.path(),
+
        Some(&profile.home),
+
        [],
+
    )
+
    .unwrap();
+
    test(
+
        "examples/jj-init.md",
+
        working.path(),
+
        Some(&profile.home),
+
        [],
+
    )
+
    .unwrap();
+
    test(
+
        "examples/rad-patch-jj.md",
+
        working.path(),
+
        Some(&profile.home),
+
        [],
+
    )
+
    .unwrap();
+
}
+

+
#[test]
fn rad_patch_diff() {
    let mut environment = Environment::new();
    let profile = environment.profile(config::profile("alice"));
modified radicle-remote-helper/src/push.rs
@@ -409,6 +409,12 @@ where
{
    let reference = working.find_reference(src.as_str())?;
    let commit = reference.peel_to_commit()?;
+

+
    let change_id = commit
+
        .header_field_bytes("change-id")
+
        .ok()
+
        .and_then(|buf| buf.as_str().map(String::from));
+

    let dst = git::refs::storage::staging::patch(nid, commit.id());

    // Before creating the patch, we must push the associated git objects to storage.
@@ -440,6 +446,7 @@ where
            patch::MergeTarget::default(),
            base,
            commit.id(),
+
            change_id,
            &[],
            signer,
        )
@@ -450,6 +457,7 @@ where
            patch::MergeTarget::default(),
            base,
            commit.id(),
+
            change_id,
            &[],
            signer,
        )
@@ -461,19 +469,27 @@ where
            } else {
                "opened"
            };
-
            let patch = patch.id;
+
            let patch_id = patch.id;

            eprintln!(
                "{} Patch {} {action}",
                term::format::positive("✓"),
-
                term::format::tertiary(patch),
+
                term::format::tertiary(patch_id),
            );

+
            if let Some(change_id) = patch.change_id() {
+
                eprintln!(
+
                    "{} Change ID {} detected and linked",
+
                    term::format::positive("✓"),
+
                    term::format::tertiary(change_id),
+
                );
+
            }
+

            // Create long-lived patch head reference, now that we know the Patch ID.
            //
            //  refs/namespaces/<nid>/refs/heads/patches/<patch-id>
            //
-
            let refname = git::refs::patch(&patch).with_namespace(nid.into());
+
            let refname = git::refs::patch(&patch_id).with_namespace(nid.into());
            let _ = stored.raw().reference(
                refname.as_str(),
                commit.id(),
@@ -483,7 +499,7 @@ where

            // Setup current branch so that pushing updates the patch.
            if let Some(branch) =
-
                rad::setup_patch_upstream(&patch, commit.id().into(), working, upstream, false)?
+
                rad::setup_patch_upstream(&patch_id, commit.id().into(), working, upstream, false)?
            {
                if let Some(name) = branch.name()? {
                    if profile.hints() {
@@ -496,7 +512,7 @@ where
                    }
                }
            }
-
            Ok(Some(ExplorerResource::Patch { id: patch }))
+
            Ok(Some(ExplorerResource::Patch { id: patch_id }))
        }
        Err(e) => Err(e),
    };
modified radicle/src/cob/patch.rs
@@ -173,6 +173,8 @@ pub enum Action {
        revision: RevisionId,
        commit: git::Oid,
    },
+
    #[serde(rename = "linkChangeId")]
+
    LinkChangeId { change_id: String },

    //
    // Review actions
@@ -434,6 +436,8 @@ pub struct Patch {
    pub(super) timeline: Vec<EntryId>,
    /// Reviews index. Keeps track of reviews for better performance.
    pub(super) reviews: BTreeMap<ReviewId, Option<(RevisionId, ActorId)>>,
+
    /// Change ID associated with the patch. This is emitted by tools like Jujutsu.
+
    pub(super) change_id: Option<String>,
}

impl Patch {
@@ -450,6 +454,7 @@ impl Patch {
            assignees: BTreeSet::default(),
            timeline: vec![id.into_inner()],
            reviews: BTreeMap::default(),
+
            change_id: None,
        }
    }

@@ -499,6 +504,11 @@ impl Patch {
        &self.author
    }

+
    /// Change ID linked to the patch.
+
    pub fn change_id(&self) -> &Option<String> {
+
        &self.change_id
+
    }
+

    /// All revision authors.
    pub fn authors(&self) -> BTreeSet<&Author> {
        self.revisions
@@ -672,6 +682,7 @@ impl Patch {
            Action::Merge { .. } => match self.target() {
                MergeTarget::Delegates => Authorization::Deny,
            },
+
            Action::LinkChangeId { .. } => Authorization::from(actor == author),
            // Anyone can submit a review.
            Action::Review { .. } => Authorization::Allow,
            Action::ReviewRedact { review, .. } | Action::ReviewEdit { review, .. } => {
@@ -818,6 +829,9 @@ impl Patch {
            Action::Assign { assignees } => {
                self.assignees = BTreeSet::from_iter(assignees.into_iter().map(ActorId::from));
            }
+
            Action::LinkChangeId { change_id } => {
+
                self.change_id = Some(change_id);
+
            }
            Action::RevisionEdit {
                revision,
                description,
@@ -1963,6 +1977,11 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {
            labels: labels.into_iter().collect(),
        })
    }
+

+
    /// Links this patch to a Change ID.
+
    pub fn link_change_id(&mut self, change_id: String) -> Result<(), store::Error> {
+
        self.push(Action::LinkChangeId { change_id })
+
    }
}

pub struct PatchMut<'a, 'g, R, C> {
@@ -2600,6 +2619,7 @@ where
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        change_id: Option<String>,
        labels: &[Label],
        cache: &'g mut C,
        signer: &Device<G>,
@@ -2614,6 +2634,7 @@ where
            target,
            base,
            oid,
+
            change_id,
            labels,
            Lifecycle::default(),
            cache,
@@ -2629,6 +2650,7 @@ where
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        change_id: Option<String>,
        labels: &[Label],
        cache: &'g mut C,
        signer: &Device<G>,
@@ -2643,6 +2665,7 @@ where
            target,
            base,
            oid,
+
            change_id,
            labels,
            Lifecycle::Draft,
            cache,
@@ -2677,6 +2700,7 @@ where
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        change_id: Option<String>,
        labels: &[Label],
        state: Lifecycle,
        cache: &'g mut C,
@@ -2696,6 +2720,9 @@ where
            if state != Lifecycle::default() {
                tx.lifecycle(state)?;
            }
+
            if let Some(change_id) = change_id {
+
                tx.link_change_id(change_id)?;
+
            }
            Ok(())
        })?;
        cache
@@ -2982,6 +3009,7 @@ mod test {
                target,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3022,6 +3050,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3055,6 +3084,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3086,6 +3116,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3138,6 +3169,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3326,6 +3358,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3370,6 +3403,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3401,6 +3435,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3445,6 +3480,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3492,6 +3528,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3540,6 +3577,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
@@ -3589,6 +3627,7 @@ mod test {
                MergeTarget::Delegates,
                branch.base,
                branch.oid,
+
                None,
                &[],
                &alice.signer,
            )
modified radicle/src/cob/patch/cache.rs
@@ -115,6 +115,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        change_id: Option<String>,
        labels: &[Label],
        signer: &Device<G>,
    ) -> Result<PatchMut<'a, 'g, R, C>, super::Error>
@@ -129,6 +130,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
            target,
            base,
            oid,
+
            change_id,
            labels,
            &mut self.cache,
            signer,
@@ -145,6 +147,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
        target: MergeTarget,
        base: impl Into<git::Oid>,
        oid: impl Into<git::Oid>,
+
        change_id: Option<String>,
        labels: &[Label],
        signer: &Device<G>,
    ) -> Result<PatchMut<'a, 'g, R, C>, super::Error>
@@ -159,6 +162,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> {
            target,
            base,
            oid,
+
            change_id,
            labels,
            &mut self.cache,
            signer,