From c8c08f4c4857ece609205e4ff574ba719e2c781f Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Thu, 30 Jul 2026 14:05:13 -0700 Subject: [PATCH] backend: add metadata field to Commit for arbitrary extra data The new field allows servers and custom clients to attach arbitrary metadata to commits. Since it is intended mainly for shipping data from servers to clients, it is not preserved on rewrite. It is intended to impact the commit ID, however; two commits with different metadata but otherwise identical should have different IDs. We have also discussed metadata that is preserved on rewrite (could be useful e.g. for recording a bug number), as well as metadata/annotations that are attached to commits but don't influence their identity (could be useful e.g. for linking to CI results). This patch only introduces the non-sticky kind because that's the simplest kind to support. The simple backend and the test backend support reading and writing it. The Git backend errors out for now when asked to write a commit with metadata; we may later support it by writing custom commit headers (e.g. "jj:metadata:: "). Note that this changes commit ids in the simple backend and the test backend because the new field is included in the content hash. --- cli/src/commit_ref_list.rs | 1 + lib/src/backend.rs | 8 ++++ lib/src/commit.rs | 5 +++ lib/src/commit_builder.rs | 20 +++++++++ lib/src/git_backend.rs | 48 ++++++++++++++++++++ lib/src/protos/simple_store.proto | 7 +++ lib/src/protos/simple_store.rs | 12 ++++- lib/src/simple_backend.rs | 52 ++++++++++++++++++++++ lib/tests/test_id_prefix.rs | 2 +- lib/tests/test_revset_optimized.rs | 70 +++++++++++++++--------------- lib/testutils/src/lib.rs | 1 + 11 files changed, 189 insertions(+), 37 deletions(-) diff --git a/cli/src/commit_ref_list.rs b/cli/src/commit_ref_list.rs index 9408f1d8e30..22aa4f36029 100644 --- a/cli/src/commit_ref_list.rs +++ b/cli/src/commit_ref_list.rs @@ -320,6 +320,7 @@ mod tests { author, committer, secure_sig: None, + metadata: HashMap::new(), }) } diff --git a/lib/src/backend.rs b/lib/src/backend.rs index 05ccd280c3c..cd4e9526876 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -16,6 +16,7 @@ //! trait for reading and writing commits, trees, files, etc. use std::any::Any; +use std::collections::HashMap; use std::fmt::Debug; use std::pin::Pin; use std::slice; @@ -215,6 +216,12 @@ pub struct Commit { /// A cryptographic signature of this commit. #[serde(skip)] // raw data wouldn't be useful pub secure_sig: Option, + /// Additional metadata about this commit. This allows servers and custom + /// clients to attach arbitrary data to a commit. It is not preserved on + /// rewrite. It is part of the commit's identity; two commits with + /// different metadata but otherwise identical should have different IDs. + #[serde(skip)] // raw data wouldn't be useful + pub metadata: HashMap>, } /// An individual copy event, from file A -> B. @@ -530,6 +537,7 @@ pub fn make_root_commit(root_change_id: ChangeId, empty_tree_id: TreeId) -> Comm author: signature.clone(), committer: signature, secure_sig: None, + metadata: HashMap::new(), } } diff --git a/lib/src/commit.rs b/lib/src/commit.rs index 493ba510d30..f1296a08e67 100644 --- a/lib/src/commit.rs +++ b/lib/src/commit.rs @@ -15,6 +15,7 @@ #![expect(missing_docs)] use std::cmp::Ordering; +use std::collections::HashMap; use std::fmt::Debug; use std::fmt::Error; use std::fmt::Formatter; @@ -188,6 +189,10 @@ impl Commit { &self.data.committer } + pub fn metadata(&self) -> &HashMap> { + &self.data.metadata + } + /// A commit is hidden if its commit id is not in the change id index. pub fn is_hidden(&self, repo: &dyn Repo) -> IndexResult { let maybe_targets = repo.resolve_change_id(self.change_id())?; diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs index 9f4aacec1b5..e391188ffba 100644 --- a/lib/src/commit_builder.rs +++ b/lib/src/commit_builder.rs @@ -14,6 +14,7 @@ #![expect(missing_docs)] +use std::collections::HashMap; use std::sync::Arc; use pollster::FutureExt as _; @@ -136,6 +137,15 @@ impl CommitBuilder<'_> { self } + pub fn metadata(&self) -> &HashMap> { + self.inner.metadata() + } + + pub fn set_metadata(mut self, metadata: HashMap>) -> Self { + self.inner.set_metadata(metadata); + self + } + /// [`Commit::is_discardable()`] for the new commit. pub async fn is_discardable(&self) -> BackendResult { self.inner.is_discardable(self.mut_repo).await @@ -207,6 +217,7 @@ impl DetachedCommitBuilder { author: signature.clone(), committer: signature, secure_sig: None, + metadata: HashMap::new(), }; let record_predecessors_in_commit = settings .get_bool("experimental.record-predecessors-in-commit") @@ -370,6 +381,15 @@ impl DetachedCommitBuilder { self } + pub fn metadata(&self) -> &HashMap> { + &self.commit.metadata + } + + pub fn set_metadata(&mut self, metadata: HashMap>) -> &mut Self { + self.commit.metadata = metadata; + self + } + /// [`Commit::is_discardable()`] for the new commit. pub async fn is_discardable(&self, repo: &dyn Repo) -> BackendResult { Ok(self.description().is_empty() && self.is_empty(repo).await?) diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index d37b3200a32..46e35ba73d6 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -14,6 +14,7 @@ #![expect(missing_docs)] +use std::collections::HashMap; use std::collections::HashSet; use std::ffi::OsStr; use std::fmt::Debug; @@ -701,6 +702,7 @@ fn commit_from_git_without_root_parent( author, committer, secure_sig, + metadata: HashMap::new(), }) } @@ -1275,6 +1277,14 @@ impl Backend for GitBackend { ) -> BackendResult<(CommitId, Commit)> { assert!(contents.secure_sig.is_none(), "commit.secure_sig was set"); + // TODO: Support writing metadata as custom Git commit headers (e.g. + // "jj:metadata:: ")? + if !contents.metadata.is_empty() { + return Err(BackendError::Unsupported( + "The Git backend doesn't support writing commit metadata".to_owned(), + )); + } + let locked_repo = self.lock_git_repo(); let tree_ids = &contents.root_tree; let git_tree_id = match tree_ids.as_resolved() { @@ -2014,6 +2024,7 @@ mod tests { author: create_signature(), committer: create_signature(), secure_sig: None, + metadata: HashMap::new(), }; let (initial_commit_id, _init_commit) = backend.write_commit(commit, None).block_on()?; @@ -2106,6 +2117,7 @@ mod tests { author: create_signature(), committer: create_signature(), secure_sig: None, + metadata: HashMap::new(), }; let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> { @@ -2195,6 +2207,7 @@ mod tests { author: create_signature(), committer: create_signature(), secure_sig: None, + metadata: HashMap::new(), }; let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> { @@ -2276,6 +2289,38 @@ mod tests { Ok(()) } + #[test] + fn write_commit_with_metadata_fails() -> TestResult { + let settings = user_settings(); + let temp_dir = new_temp_dir(); + let backend = GitBackend::init_internal(&settings, temp_dir.path(), gix::hash::Kind::Sha1)?; + let signature = Signature { + name: "Someone".to_string(), + email: "someone@example.com".to_string(), + timestamp: Timestamp { + timestamp: MillisSinceEpoch(0), + tz_offset: 0, + }, + }; + let commit = Commit { + parents: vec![backend.root_commit_id().clone()], + predecessors: vec![], + root_tree: Merge::resolved(backend.empty_tree_id().clone()), + conflict_labels: Merge::resolved(String::new()), + change_id: ChangeId::new(vec![42; 16]), + description: "initial".to_string(), + author: signature.clone(), + committer: signature, + secure_sig: None, + metadata: HashMap::from([("foo".to_string(), b"bar".to_vec())]), + }; + assert_matches!( + backend.write_commit(commit, None).block_on(), + Err(BackendError::Unsupported(_)) + ); + Ok(()) + } + #[test_case(gix::hash::Kind::Sha1 ; "sha1")] #[test_case(gix::hash::Kind::Sha256; "sha256")] fn commit_has_ref(object_hash: gix::hash::Kind) -> TestResult { @@ -2301,6 +2346,7 @@ mod tests { author: signature.clone(), committer: signature, secure_sig: None, + metadata: HashMap::new(), }; let commit_id = backend.write_commit(commit, None).block_on()?.0; let git_refs = git_repo.references()?; @@ -2378,6 +2424,7 @@ mod tests { author: create_signature(), committer: create_signature(), secure_sig: None, + metadata: HashMap::new(), }; let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> { @@ -2474,6 +2521,7 @@ mod tests { author: create_signature(), committer: create_signature(), secure_sig: None, + metadata: HashMap::new(), }; let mut signer = |data: &_| { diff --git a/lib/src/protos/simple_store.proto b/lib/src/protos/simple_store.proto index a7a6a4e5bff..767aa0c282f 100644 --- a/lib/src/protos/simple_store.proto +++ b/lib/src/protos/simple_store.proto @@ -61,4 +61,11 @@ message Commit { Signature author = 6; Signature committer = 7; optional bytes secure_sig = 9; + + message MetadataEntry { + string key = 1; + bytes value = 2; + } + // Sorted by key so the serialized form is deterministic + repeated MetadataEntry metadata = 11; } diff --git a/lib/src/protos/simple_store.rs b/lib/src/protos/simple_store.rs index 8f1fdd5652b..b93d8b12649 100644 --- a/lib/src/protos/simple_store.rs +++ b/lib/src/protos/simple_store.rs @@ -40,7 +40,7 @@ pub mod tree { pub value: ::core::option::Option, } } -#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct Commit { #[prost(bytes = "vec", repeated, tag = "1")] pub parents: ::prost::alloc::vec::Vec<::prost::alloc::vec::Vec>, @@ -62,6 +62,9 @@ pub struct Commit { pub committer: ::core::option::Option, #[prost(bytes = "vec", optional, tag = "9")] pub secure_sig: ::core::option::Option<::prost::alloc::vec::Vec>, + /// Sorted by key so the serialized form is deterministic + #[prost(message, repeated, tag = "11")] + pub metadata: ::prost::alloc::vec::Vec, } /// Nested message and enum types in `Commit`. pub mod commit { @@ -81,4 +84,11 @@ pub mod commit { #[prost(message, optional, tag = "3")] pub timestamp: ::core::option::Option, } + #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] + pub struct MetadataEntry { + #[prost(string, tag = "1")] + pub key: ::prost::alloc::string::String, + #[prost(bytes = "vec", tag = "2")] + pub value: ::prost::alloc::vec::Vec, + } } diff --git a/lib/src/simple_backend.rs b/lib/src/simple_backend.rs index c2767691446..96bf130c6ce 100644 --- a/lib/src/simple_backend.rs +++ b/lib/src/simple_backend.rs @@ -33,6 +33,7 @@ use futures::StreamExt as _; use futures::io::Cursor; use futures::stream; use futures::stream::BoxStream; +use itertools::Itertools as _; use pollster::FutureExt as _; use prost::Message as _; use tempfile::NamedTempFile; @@ -369,6 +370,18 @@ pub fn commit_to_proto(commit: &Commit) -> crate::protos::simple_store::Commit { proto.description = commit.description.clone(); proto.author = Some(signature_to_proto(&commit.author)); proto.committer = Some(signature_to_proto(&commit.committer)); + // Sort by key to make the serialized form deterministic + proto.metadata = commit + .metadata + .iter() + .sorted_by_key(|&(key, _)| key) + .map( + |(key, value)| crate::protos::simple_store::commit::MetadataEntry { + key: key.clone(), + value: value.clone(), + }, + ) + .collect(); proto } @@ -382,6 +395,11 @@ fn commit_from_proto(mut proto: crate::protos::simple_store::Commit) -> Commit { let parents = proto.parents.into_iter().map(CommitId::new).collect(); let predecessors = proto.predecessors.into_iter().map(CommitId::new).collect(); + let metadata = proto + .metadata + .into_iter() + .map(|entry| (entry.key, entry.value)) + .collect(); let merge_builder: MergeBuilder<_> = proto.root_tree.into_iter().map(TreeId::new).collect(); let root_tree = merge_builder.build(); let conflict_labels = ConflictLabels::from_vec(proto.conflict_labels); @@ -396,6 +414,7 @@ fn commit_from_proto(mut proto: crate::protos::simple_store::Commit) -> Commit { author: signature_from_proto(proto.author.unwrap_or_default()), committer: signature_from_proto(proto.committer.unwrap_or_default()), secure_sig, + metadata, } } @@ -505,6 +524,8 @@ fn signature_from_proto(proto: crate::protos::simple_store::commit::Signature) - #[cfg(test)] mod tests { + use std::collections::HashMap; + use assert_matches::assert_matches; use pollster::FutureExt as _; @@ -530,6 +551,7 @@ mod tests { author: create_signature(), committer: create_signature(), secure_sig: None, + metadata: HashMap::new(), }; let write_commit = |commit: Commit| -> BackendResult<(CommitId, Commit)> { @@ -569,6 +591,36 @@ mod tests { Ok(()) } + /// Test that commit metadata gets written and read back + #[test] + fn write_commit_metadata() -> TestResult { + let temp_dir = new_temp_dir(); + let store_path = temp_dir.path(); + + let backend = SimpleBackend::init(store_path); + let commit = Commit { + parents: vec![backend.root_commit_id().clone()], + predecessors: vec![], + root_tree: Merge::resolved(backend.empty_tree_id().clone()), + conflict_labels: Merge::resolved(String::new()), + change_id: ChangeId::from_hex("abc123"), + description: "".to_string(), + author: create_signature(), + committer: create_signature(), + secure_sig: None, + metadata: HashMap::from([ + ("foo".to_string(), b"bar".to_vec()), + ("binary".to_string(), vec![0x00, 0xff]), + ]), + }; + + let (id, returned_commit) = backend.write_commit(commit.clone(), None).block_on()?; + assert_eq!(returned_commit, commit); + let read_commit = backend.read_commit(&id).block_on()?; + assert_eq!(read_commit, commit); + Ok(()) + } + fn create_signature() -> Signature { Signature { name: "Someone".to_string(), diff --git a/lib/tests/test_id_prefix.rs b/lib/tests/test_id_prefix.rs index ed542c14334..12ed27dbfd0 100644 --- a/lib/tests/test_id_prefix.rs +++ b/lib/tests/test_id_prefix.rs @@ -561,7 +561,7 @@ fn test_id_prefix_shadowed_by_ref() { let commit_id_sym = commit.id().to_string(); let change_id_sym = commit.change_id().to_string(); - insta::assert_snapshot!(commit_id_sym, @"b06a01f026da65ac5821"); + insta::assert_snapshot!(commit_id_sym, @"d62c40b45ee40858ea2e"); insta::assert_snapshot!(change_id_sym, @"sryyqqkqmuumyrlruupspprvnulvovzm"); let context = IdPrefixContext::default(); diff --git a/lib/tests/test_revset_optimized.rs b/lib/tests/test_revset_optimized.rs index 53d7edd0df5..ca5aba84123 100644 --- a/lib/tests/test_revset_optimized.rs +++ b/lib/tests/test_revset_optimized.rs @@ -209,15 +209,15 @@ fn test_mostly_linear() -> TestResult { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @" 00000000000000000000 - 1 78f823b31f2c4a77030b - 2 1ba216c17ef680561823 - 3 c2c719328d78654d9f8e - 4 d6b40f7dfac149c7181c - 5 c682b87d91a8940f71d5 - 6 456fe15ac6ebfdf56219 - 7 d2bba8ce1ce80751aab5 - 8 536f4a045e558c9927a5 - 9 6ab43bd6d94bdaff491f + 1 cbade58d6c3332770c1b + 2 abb5aa30a5104de798fe + 3 df46fb8d03faf508310c + 4 7f8baa22470620b27ea5 + 5 5351726ac1bd86493e30 + 6 b645e1adc90d3027a0a0 + 7 3d109bbc188776bd88b4 + 8 fa026778a99481f2b7b4 + 9 8faaa1373693018f7a68 "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); @@ -264,14 +264,14 @@ fn test_weird_merges() -> TestResult { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @" 00000000000000000000 - 1 78f823b31f2c4a77030b - 2 1ba216c17ef680561823 - 3 83a7b5b8138c9428d837 - 4 43a3ed8115915cb0ebe0 - 5 aec384ff4d34c039e4db - 6 d80cec48faa50bf2ac56 - 7 2667f762c099ffcda2f0 - 8 54feb3e8186bc4450be4 + 1 cbade58d6c3332770c1b + 2 abb5aa30a5104de798fe + 3 902435e8fb7a8a241531 + 4 0fd312be9da551a8e2a4 + 5 78a2fba150ebd1ff665c + 6 15429bb4c3a32cc1ccde + 7 c4274805e4ce7978041d + 8 23773246fc0f982cbadf "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); @@ -341,15 +341,15 @@ fn test_feature_branches() -> TestResult { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @" 00000000000000000000 - 1 78f823b31f2c4a77030b - 2 6323cf55a45bcc85315d - 3 83a7b5b8138c9428d837 - 4 93731ec1a14276206ba7 - 5 c388b47bd72fcfee9e3c - 6 da751caa45bda2e3d526 - 7 45a86fae2b51ec68f8c0 - 8 9f83496b963cbaf8cb7a - 9 998a24ecd56446732f55 + 1 cbade58d6c3332770c1b + 2 c007af652ce4661c3f60 + 3 902435e8fb7a8a241531 + 4 aa3f372d9eedc8b6398a + 5 0ff11cd350d9bd9438a2 + 6 d85647706fde704580d4 + 7 15adff54be4dc8ba84b4 + 8 d7e8d47bccd1e00dac94 + 9 aa8b1763afc27a09222d "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); @@ -411,15 +411,15 @@ fn test_rewritten() -> TestResult { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @" 00000000000000000000 - 1 78f823b31f2c4a77030b - 2 1ba216c17ef680561823 - 3 068410d7a4a5b7052c18 - 4 8ee10ec699f52df8c624 - 5 32bd65d7134884955150 - 2b a7d217f53df0908d3f7a - 3 c199d1c8c617cf15893f - 5 5fc1da61558a03f69a8d - 5 cb81e73e341e59553ff6 + 1 cbade58d6c3332770c1b + 2 abb5aa30a5104de798fe + 3 b717d1dd6cbba97f3877 + 4 e397207f71d5df7688c6 + 5 c2892ce1b6ad170939a0 + 2b 227683e4c46658c97a99 + 3 bc61df0e5d1815065bde + 5 7b14439ddb82938bc07c + 5 3bc39726df0f08ba0a69 "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index 7e779615573..55ce31423ef 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -691,6 +691,7 @@ pub fn commit_with_tree(store: &Arc, tree: MergedTree) -> Commit { author: signature.clone(), committer: signature, secure_sig: None, + metadata: HashMap::new(), }; store.write_commit(commit, None).block_on().unwrap() }