diff --git a/CHANGELOG.md b/CHANGELOG.md index a8d59957..ca80ecef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 will appear as a downgrade attacker. - The `rad inspect --sigrefs` command will now output the feature level of the signed references entry for each node. +- The signed references for the local node are automatically migrated when + `radicle-node` first starts up. The migration occurs for each repository in + the node's inventory, where the node has a reference + `refs/namespaces//refs/rad/sigrefs`. The migration will only take place + if the `rad/sigrefs` found were below the latest feature level, i.e. `parent`. ## 1.7.1 diff --git a/crates/radicle-cli/examples/rad-clean.md b/crates/radicle-cli/examples/rad-clean.md index 7187433b..07f81b92 100644 --- a/crates/radicle-cli/examples/rad-clean.md +++ b/crates/radicle-cli/examples/rad-clean.md @@ -18,8 +18,8 @@ Let's also inspect what remotes are in the repository: ``` ~alice $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 99c549702e2bcfe02b0e68d4a2224fb7a1524529 parent -z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk e9f48ef90fe8592e1b1c95f96c21a59ca1495300 parent +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 990bb9984b28aa09ed9f5e32f16e92b3eed57d34 parent +z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk d275a070a9ed705f44a42271f9e3c5b6c73c65b3 parent ``` Now let's clean the `heartwood` project: @@ -34,7 +34,7 @@ Inspecting the remotes again, we see that Bob is now gone: ``` ~alice $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 99c549702e2bcfe02b0e68d4a2224fb7a1524529 parent +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 990bb9984b28aa09ed9f5e32f16e92b3eed57d34 parent ``` Note that Bob will be fetched again if we do not untrack his diff --git a/crates/radicle-cli/examples/rad-id-multi-delegate.md b/crates/radicle-cli/examples/rad-id-multi-delegate.md index 882263dc..48e86e1a 100644 --- a/crates/radicle-cli/examples/rad-id-multi-delegate.md +++ b/crates/radicle-cli/examples/rad-id-multi-delegate.md @@ -1,20 +1,14 @@ ``` ~alice $ rad id update --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --title "Add Bob" --description "" --threshold 2 --delegate did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --no-confirm -q 069e7d58faa9a7473d27f5510d676af33282796f +$ rad inspect --sigrefs rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi [..] parent +z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk [..] parent +z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z 82212c030fa97709a92ead14be76759732199c61 parent ``` -A note for test authors: -> The following `rad watch` command will time out if the target given via `-t` changes. -> This happens for example when the generation of sigrefs changes. -> To recover, temporarily change from `rad watch` to something like -> -> $ sleep 5 -> $ rad inspect --sigrefs rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji -> -> And pick out the result for `z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z`. - ``` ~bob -$ rad watch --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --node z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z -r 'refs/rad/sigrefs' -t c9a828fc2fb01f893d6e6e9e17b9092dea2b3aba -i 500 --timeout 5000ms +$ rad watch --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --node z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z -r 'refs/rad/sigrefs' -t 82212c030fa97709a92ead14be76759732199c61 -i 500 --timeout 5000ms $ rad sync --fetch rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 1 potential seed(s). ✓ Target met: 1 seed(s) @@ -28,7 +22,7 @@ $ rad id --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji ╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ $ rad inspect rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --sigrefs z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi [..] parent -z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk [..] parent +z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk [..] root z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z [..] parent $ rad inspect rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --delegates did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (alice) diff --git a/crates/radicle-cli/examples/rad-id-threshold.md b/crates/radicle-cli/examples/rad-id-threshold.md index 8580c522..68ac7eaa 100644 --- a/crates/radicle-cli/examples/rad-id-threshold.md +++ b/crates/radicle-cli/examples/rad-id-threshold.md @@ -148,9 +148,10 @@ z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi Similarly, she still does not have Bob's `rad/sigrefs`: + ``` ~alice $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi e0e55994a9a234f0b1cd36d8812e2948e2672b7a parent +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 709795db0cd91451b533da83a19a4cada3f5f74f parent ``` And she can still list the project, without any worries: @@ -198,6 +199,6 @@ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 2 potential s 🌱 Fetched from z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z 🌱 Fetched from z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi e0e55994a9a234f0b1cd36d8812e2948e2672b7a parent +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 709795db0cd91451b533da83a19a4cada3f5f74f parent z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk dace6fe948548168a2bb687718949d9b5d9076ee parent ``` diff --git a/crates/radicle-cli/examples/rad-inspect.md b/crates/radicle-cli/examples/rad-inspect.md index 80761719..6ad1edb3 100644 --- a/crates/radicle-cli/examples/rad-inspect.md +++ b/crates/radicle-cli/examples/rad-inspect.md @@ -34,7 +34,7 @@ And sigrefs: ``` $ rad inspect --sigrefs -z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 99c549702e2bcfe02b0e68d4a2224fb7a1524529 parent +z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi 99c549702e2bcfe02b0e68d4a2224fb7a1524529 root ``` Or display the repository identity's payload and delegates: diff --git a/crates/radicle-cli/examples/rad-sync.md b/crates/radicle-cli/examples/rad-sync.md index 6909be9d..5ef12096 100644 --- a/crates/radicle-cli/examples/rad-sync.md +++ b/crates/radicle-cli/examples/rad-sync.md @@ -15,9 +15,9 @@ $ rad sync status --sort-by alias ╭───────────────────────────────────────────────────╮ │ Node ID Alias ? SigRefs Timestamp │ ├───────────────────────────────────────────────────┤ -│ (you) alice ! f2dfe80 [..] │ -│ z6Mkt67…v4N1tRk bob ✗ 99c5497 [..] │ -│ z6Mkux1…nVhib7Z eve ✗ 99c5497 [..] │ +│ (you) alice ! ac4a256 [..] │ +│ z6Mkt67…v4N1tRk bob ✗ 990bb99 [..] │ +│ z6Mkux1…nVhib7Z eve ✗ 990bb99 [..] │ ╰───────────────────────────────────────────────────╯ ``` @@ -37,9 +37,9 @@ $ rad sync status --sort-by alias ╭───────────────────────────────────────────────────╮ │ Node ID Alias ? SigRefs Timestamp │ ├───────────────────────────────────────────────────┤ -│ (you) alice ✓ f2dfe80 [..] │ -│ z6Mkt67…v4N1tRk bob ✓ f2dfe80 [..] │ -│ z6Mkux1…nVhib7Z eve ✓ f2dfe80 [..] │ +│ (you) alice ✓ ac4a256 [..] │ +│ z6Mkt67…v4N1tRk bob ✓ ac4a256 [..] │ +│ z6Mkux1…nVhib7Z eve ✓ ac4a256 [..] │ ╰───────────────────────────────────────────────────╯ ``` diff --git a/crates/radicle-cli/src/commands/inspect.rs b/crates/radicle-cli/src/commands/inspect.rs index c226c354..dc932450 100644 --- a/crates/radicle-cli/src/commands/inspect.rs +++ b/crates/radicle-cli/src/commands/inspect.rs @@ -76,7 +76,20 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { term::format::secondary(refs.at), match sigrefs { Ok(refs) => { - let level = refs.feature_level(); + let mut level = refs.feature_level(); + + // For their own refs, be more strict, and interpret + // `FeatureLevel::Parent` at a root commit as + // `FeatureLevel::Root`. This is so that users + // have a chance of detecting that automatic migration + // did not run or is otherwise broken. + if &remote == profile.id() + && level == FeatureLevel::Parent + && refs.parent().is_none() + { + level = FeatureLevel::Root; + } + let s = level.to_string(); match level { FeatureLevel::None => term::format::negative(s), diff --git a/crates/radicle-node/src/tests.rs b/crates/radicle-node/src/tests.rs index 503fc7f9..af6f4735 100644 --- a/crates/radicle-node/src/tests.rs +++ b/crates/radicle-node/src/tests.rs @@ -1061,16 +1061,12 @@ fn test_refs_announcement_offline() { let anns = alice .messages(bob.id()) - .filter_map(|m| { - if let Message::Announcement(Announcement { + .filter_map(|m| match m { + Message::Announcement(Announcement { message: AnnouncementMessage::Refs(ann), .. - }) = m - { - Some(ann) - } else { - None - } + }) if ann.rid == rid => Some(ann), + _ => None, }) .collect::>(); diff --git a/crates/radicle-node/src/tests/e2e.rs b/crates/radicle-node/src/tests/e2e.rs index 604b9066..27f81e0a 100644 --- a/crates/radicle-node/src/tests/e2e.rs +++ b/crates/radicle-node/src/tests/e2e.rs @@ -257,6 +257,9 @@ fn test_replication_ref_in_sigrefs() { .unwrap(); let mut alice = alice.spawn(); + + // At this point, bob will migrate sigrefs, because there only is a + // root commit in his `refs/heads/sigrefs`. let bob = bob.spawn(); alice.connect(&bob); @@ -267,15 +270,19 @@ fn test_replication_ref_in_sigrefs() { assert_matches!(result, FetchResult::Success { .. }); - // alice still sees bob's master branch since it was in his - // sigrefs. + // Before automatic migration of sigrefs was introduced, + // alice would still see bob's master branch at this point and we + // would assert `.is_ok()`. + // With automatic migration, refs are signed as bob's node starts + // up, which is after he removes his ref locally, thus we now + // assert `.is_err()`. assert!( alice .storage .repository(acme) .unwrap() .reference(&bob.id, &git::fmt::qualified!("refs/heads/master")) - .is_ok(), + .is_err(), "refs/namespaces/{}/refs/heads/master does not exist", bob.id ); diff --git a/crates/radicle-protocol/src/service.rs b/crates/radicle-protocol/src/service.rs index 492adf36..af42e538 100644 --- a/crates/radicle-protocol/src/service.rs +++ b/crates/radicle-protocol/src/service.rs @@ -528,20 +528,29 @@ where return Ok(()); }; - if refs.feature_level() >= FeatureLevel::LATEST { + if refs.feature_level() >= FeatureLevel::LATEST && refs.parent().is_some() { // Refs are at target level or above, nothing to upgrade. return Ok(()); } - log::info!( - "Migrating `rad/sigrefs` from level {} which is lower than target level {}.", - refs.feature_level(), - FeatureLevel::LATEST - ); + let rid = info.rid; + + if refs.parent().is_none() { + log::info!( + "Migrating `rad/sigrefs` of {rid} to force feature level {}, as the history currently contains only a root commit.", + FeatureLevel::LATEST + ); + } else { + log::info!( + "Migrating `rad/sigrefs` of {rid} from level {} which is lower than target level {}.", + refs.feature_level(), + FeatureLevel::LATEST + ); + } let repo = self.storage.repository_mut(info.rid)?; - // NOTE: We assume to reach `FeatureLevel::MAX` by signing refs. - repo.sign_refs(&self.signer)?; + // NOTE: We assume to reach `FeatureLevel::LATEST` by signing refs. + repo.force_sign_refs(&self.signer)?; Ok(()) } } diff --git a/crates/radicle/src/storage.rs b/crates/radicle/src/storage.rs index 47ddc04e..bd5aa619 100644 --- a/crates/radicle/src/storage.rs +++ b/crates/radicle/src/storage.rs @@ -672,6 +672,13 @@ pub trait SignRepository { fn sign_refs(&self, signer: &Device) -> Result where G: crypto::signature::Signer; + + /// Sign the repository's refs under the `refs/rad/sigrefs` branch, even if unchanged. + /// + /// Most users will prefer [`Self::sign_refs`]. + fn force_sign_refs(&self, signer: &Device) -> Result + where + G: crypto::signature::Signer; } impl ReadStorage for T diff --git a/crates/radicle/src/storage/git.rs b/crates/radicle/src/storage/git.rs index 23a9cf05..435b6624 100644 --- a/crates/radicle/src/storage/git.rs +++ b/crates/radicle/src/storage/git.rs @@ -993,6 +993,23 @@ impl SignRepository for Repository { fn sign_refs>( &self, signer: &Device, + ) -> Result { + self.sign_refs_with(signer, false) + } + + fn force_sign_refs>( + &self, + signer: &Device, + ) -> Result { + self.sign_refs_with(signer, true) + } +} + +impl Repository { + fn sign_refs_with>( + &self, + signer: &Device, + force: bool, ) -> Result { let remote = signer.public_key(); // Ensure the root reference is set, which is checked during sigref verification. @@ -1003,10 +1020,14 @@ impl SignRepository for Repository { self.set_remote_identity_root(remote)?; } - let committer = refs::sigrefs::git::committer(remote, &self.backend.signature()?)?; - let signed = self - .references_of(remote)? - .save(*remote, committer, self, signer)?; + let committer = refs::sigrefs::git::Committer::from_env_or_now(remote); + + let refs = self.references_of(remote)?; + let signed = if force { + refs.force_save(*remote, committer, self, signer)? + } else { + refs.save(*remote, committer, self, signer)? + }; Ok(signed.sigrefs) } diff --git a/crates/radicle/src/storage/git/cob.rs b/crates/radicle/src/storage/git/cob.rs index 39a29a7d..889c9c2a 100644 --- a/crates/radicle/src/storage/git/cob.rs +++ b/crates/radicle/src/storage/git/cob.rs @@ -246,6 +246,13 @@ where Ok(remote.refs) } + + fn force_sign_refs>( + &self, + signer: &Device, + ) -> Result { + self.sign_refs(signer) + } } impl RemoteRepository for DraftStore<'_, R> { diff --git a/crates/radicle/src/storage/refs.rs b/crates/radicle/src/storage/refs.rs index e0e3932d..5ecadb57 100644 --- a/crates/radicle/src/storage/refs.rs +++ b/crates/radicle/src/storage/refs.rs @@ -77,6 +77,42 @@ impl Refs { repo: &R, signer: &S, ) -> Result + where + R: sigrefs::git::object::Reader + sigrefs::git::object::Writer, + R: sigrefs::git::reference::Reader + sigrefs::git::reference::Writer, + R: HasRepoId, + S: signature::Signer, + S: signature::Verifier, + { + self.save_with(namespace, committer, repo, signer, false) + } + + /// Save the signed refs to disk, even if the refs are unchanged. + pub fn force_save( + self, + namespace: NodeId, + committer: sigrefs::git::Committer, + repo: &R, + signer: &S, + ) -> Result + where + R: sigrefs::git::object::Reader + sigrefs::git::object::Writer, + R: sigrefs::git::reference::Reader + sigrefs::git::reference::Writer, + R: HasRepoId, + S: signature::Signer, + S: signature::Verifier, + { + self.save_with(namespace, committer, repo, signer, true) + } + + fn save_with( + self, + namespace: NodeId, + committer: sigrefs::git::Committer, + repo: &R, + signer: &S, + force: bool, + ) -> Result where R: sigrefs::git::object::Reader + sigrefs::git::object::Writer, R: sigrefs::git::reference::Reader + sigrefs::git::reference::Writer, @@ -86,9 +122,13 @@ impl Refs { { let msg = "Update signed refs\n"; let reflog = format!("Save {} signed references", self.len()); - let update = - sigrefs::write::SignedRefsWriter::new(self, repo.rid(), namespace, repo, signer) - .write(committer, msg.to_string(), reflog)?; + let writer = + sigrefs::write::SignedRefsWriter::new(self, repo.rid(), namespace, repo, signer); + let update = if force { + writer.force_write(committer, msg.to_string(), reflog)? + } else { + writer.write(committer, msg.to_string(), reflog)? + }; match update { sigrefs::write::Update::Changed { entry, level } => { Ok(entry.into_sigrefs_at(namespace, level)) @@ -287,6 +327,10 @@ pub struct SignedRefs { #[serde(skip)] level: FeatureLevel, + + /// The [`Oid`] of the parent commit of the commit in which. + #[serde(skip)] + parent: Option, } impl SignedRefs { @@ -305,6 +349,12 @@ impl SignedRefs { self.level } + /// The [`Oid`] of the parent commit, or [`None`] if these signed references + /// were found at a root commit. + pub fn parent(&self) -> Option<&Oid> { + self.parent.as_ref() + } + pub fn load(remote: RemoteId, repo: &R) -> Result where R: HasRepoId, diff --git a/crates/radicle/src/storage/refs/arbitrary.rs b/crates/radicle/src/storage/refs/arbitrary.rs index 9fbd30e3..a5eaf1ed 100644 --- a/crates/radicle/src/storage/refs/arbitrary.rs +++ b/crates/radicle/src/storage/refs/arbitrary.rs @@ -27,6 +27,7 @@ where signature, id: *signer.node_id(), level: level.unwrap_or_else(|| FeatureLevel::arbitrary(g)), + parent: Arbitrary::arbitrary(g), }; SignedRefsAt { sigrefs, diff --git a/crates/radicle/src/storage/refs/sigrefs/git.rs b/crates/radicle/src/storage/refs/sigrefs/git.rs index 74d6d2ac..570c3462 100644 --- a/crates/radicle/src/storage/refs/sigrefs/git.rs +++ b/crates/radicle/src/storage/refs/sigrefs/git.rs @@ -1,21 +1,18 @@ -//! The transparency log of Radicle signed references is encoded in the Git -//! commit graph. This module provides traits for interacting with a Git -//! repository to read and write data for the transparency log process. +//! Signed References are encoded in the Git commit graph. +//! This module provides traits for interacting with a Git +//! repository to read and write data for Signed References. pub mod object; pub mod reference; -pub use git2_impls::committer; - -use crate::profile::env; use crypto::PublicKey; -use radicle_git_metadata::author; use radicle_git_metadata::author::Author; +use radicle_git_metadata::author::Time; /// Convenience type that corresponds to an [`Author`]. /// -/// If [`env::GIT_COMMITTER_DATE`] is set, then [`Committer::from_env`] can be -/// used to construct a stable [`Author`]. +/// Most users will want to instantiate this via [`Committer::from_env_or_now`], +/// which automatically constructs a stable [`Author`] for tests as well. /// /// Otherwise, an [`Author`] can be provided via [`Committer::new`]. #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -24,13 +21,73 @@ pub struct Committer { } impl Committer { - /// Construct a [`Committer`] using [`Committer::from_env`], if possible, - /// using `default` if not. - pub fn from_env_or_else(public_key: &PublicKey, default: F) -> Self - where - F: FnOnce() -> Author, - { - Self::from_env(public_key).unwrap_or_else(|| Self::new(default())) + /// Construct a [`Committer`] using the timestamp found at + /// [`GIT_COMMITTER_DATE`], + /// + /// If [`GIT_COMMITTER_DATE`] is unset, it uses the current system + /// time. + /// + /// The given [`PublicKey`] is always used for the email. + /// + /// In test code, [`Committer::stable`] is returned. + /// + /// [`GIT_COMMITTER_DATE`]: crate::profile::env::GIT_COMMITTER_DATE + pub fn from_env_or_now(public_key: &PublicKey) -> Self { + #[cfg(any(test, feature = "test"))] + return Self::stable(public_key); + + #[cfg(not(any(test, feature = "test")))] + { + use crate::profile::env::GIT_COMMITTER_DATE; + use std::env::var; + use std::env::VarError; + + let timestamp = match var(GIT_COMMITTER_DATE) { + Ok(s) => match s.trim().parse::() { + Ok(timestamp) => timestamp, + Err(err) => { + panic!( + "Value of environment variable `{}` does not parse as integer: {err}", + GIT_COMMITTER_DATE + ); + } + }, + Err(VarError::NotPresent) => std::time::SystemTime::now() + .duration_since(std::time::SystemTime::UNIX_EPOCH) + .expect("time is later than unix epoch") + .as_secs(), + Err(VarError::NotUnicode(_)) => { + panic!( + "Value for environment variable `{}` is not valid Unicode.", + GIT_COMMITTER_DATE + ); + } + }; + + let time = Time::new( + timestamp + .try_into() + .expect("seconds since unix epoch must fit i64"), + 0, + ); + let author = Author { + name: "radicle".to_string(), + email: public_key.to_human(), + time, + }; + + Self::new(author) + } + } + + #[cfg(any(test, feature = "test"))] + pub fn stable(public_key: &PublicKey) -> Self { + let author = Author { + name: "radicle".to_string(), + email: public_key.to_human(), + time: Time::new(0, 0), + }; + Self::new(author) } /// Construct a [`Committer`] with the provided [`Author`]. @@ -38,25 +95,6 @@ impl Committer { Self { author } } - /// Construct a [`Committer`] using the timestamp found at - /// [`env::GIT_COMMITTER_DATE`], and the given [`PublicKey`] for the email. - pub fn from_env(public_key: &PublicKey) -> Option { - let s = env::var(env::GIT_COMMITTER_DATE).ok()?; - let Ok(timestamp) = s.trim().parse::() else { - panic!( - "Invalid timestamp value {s:?} for `{}`", - env::GIT_COMMITTER_DATE - ); - }; - let time = author::Time::new(timestamp, 0); - let author = Author { - name: "radicle".to_string(), - email: public_key.to_human(), - time, - }; - Some(Self::new(author)) - } - pub fn into_inner(self) -> Author { self.author } @@ -70,8 +108,6 @@ mod git2_impls { use std::path::Path; - use radicle_core::NodeId; - use radicle_git_metadata::author::{Author, Time}; use radicle_oid::Oid; use crate::git; @@ -79,38 +115,6 @@ mod git2_impls { use super::object; use super::object::{RefsEntry, SignatureEntry}; use super::reference; - use super::Committer; - - pub fn committer(node: &NodeId, signature: &git2::Signature) -> Result { - let default = { - let name = signature - .name() - .map(|name| name.to_string()) - .ok_or(git2::Error::new( - git2::ErrorCode::Invalid, - git2::ErrorClass::Invalid, - "Invalid UTF-8 of Git signature name", - ))?; - let email = - signature - .email() - .map(|email| email.to_string()) - .ok_or(git2::Error::new( - git2::ErrorCode::Invalid, - git2::ErrorClass::Invalid, - "Invalid UTF-8 of Git signature email", - ))?; - Author { - name, - email, - time: Time::new( - signature.when().seconds(), - signature.when().offset_minutes(), - ), - } - }; - Ok(Committer::from_env_or_else(node, || default)) - } impl object::Reader for git2::Repository { fn read_commit(&self, oid: &Oid) -> Result>, object::error::ReadCommit> { diff --git a/crates/radicle/src/storage/refs/sigrefs/git/object.rs b/crates/radicle/src/storage/refs/sigrefs/git/object.rs index c567bdf3..f997314a 100644 --- a/crates/radicle/src/storage/refs/sigrefs/git/object.rs +++ b/crates/radicle/src/storage/refs/sigrefs/git/object.rs @@ -1,7 +1,7 @@ //! Traits for interacting with Git objects, necessary for implementing Radicle -//! signed references. +//! Signed References. // TODO(finto): I think these are more generally useful than just being used for -// signed references. They might be worth moving into a crate, +// Signed References. They might be worth moving into a crate, // `radicle-git-traits`, but for now they can live here. pub mod error; diff --git a/crates/radicle/src/storage/refs/sigrefs/git/reference.rs b/crates/radicle/src/storage/refs/sigrefs/git/reference.rs index 87f61730..4f46c829 100644 --- a/crates/radicle/src/storage/refs/sigrefs/git/reference.rs +++ b/crates/radicle/src/storage/refs/sigrefs/git/reference.rs @@ -1,7 +1,7 @@ //! Traits for interacting with Git references, necessary for implementing -//! Radicle signed references. +//! Radicle Signed References. // TODO(finto): I think these are more generally useful than just being used for -// signed references. They might be worth moving into a crate, +// Signed References. They might be worth moving into a crate, // `radicle-git-traits`, but for now they can live here. pub mod error; diff --git a/crates/radicle/src/storage/refs/sigrefs/read.rs b/crates/radicle/src/storage/refs/sigrefs/read.rs index ea223e6c..adc72808 100644 --- a/crates/radicle/src/storage/refs/sigrefs/read.rs +++ b/crates/radicle/src/storage/refs/sigrefs/read.rs @@ -57,6 +57,7 @@ impl VerifiedCommit { signature: self.commit.signature, id, level: self.level, + parent: self.commit.parent, }, at: self.commit.oid, } diff --git a/crates/radicle/src/storage/refs/sigrefs/write.rs b/crates/radicle/src/storage/refs/sigrefs/write.rs index 3903b951..ec71a0eb 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write.rs +++ b/crates/radicle/src/storage/refs/sigrefs/write.rs @@ -119,11 +119,33 @@ where /// ```text,no_run /// refs/namespaces//refs/rad/sigrefs /// ``` - pub fn write( + pub(in super::super::super::refs) fn write( self, committer: Committer, message: String, reflog: String, + ) -> Result { + self.write_with(committer, message, reflog, false) + } + + /// Write a commit even if the current sigrefs head contains identical refs. + /// + /// Most users will prefer [`Self::write`]. + pub(in super::super::super::refs) fn force_write( + self, + committer: Committer, + message: String, + reflog: String, + ) -> Result { + self.write_with(committer, message, reflog, true) + } + + fn write_with( + self, + committer: Committer, + message: String, + reflog: String, + force: bool, ) -> Result { let author = committer.into_inner(); let Self { @@ -138,7 +160,7 @@ where let head = HeadReader::new(&reference, repository, rid, self.signer).read(); let commit_writer = match head { - Ok(Some(head)) if head.is_unchanged(&refs) => { + Ok(Some(head)) if !force && head.is_unchanged(&refs) => { return Ok(Update::unchanged(head.verified)) } Ok(Some(head)) => CommitWriter::with_parent( @@ -190,6 +212,7 @@ impl Commit { signature: self.signature, refs: self.refs, level, + parent: self.parent, }, } } diff --git a/crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs b/crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs index 55125ba2..fb6b721d 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs +++ b/crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs @@ -46,6 +46,14 @@ fn write(refs: Refs, repo: &MockRepository) -> Result { ) } +fn force_write(refs: Refs, repo: &MockRepository) -> Result { + SignedRefsWriter::new(refs, mock::rid(), mock::node_id(), repo, &mock::AlwaysSign).force_write( + Committer::new(mock::author()), + "msg".into(), + "reflog".into(), + ) +} + #[test] fn head_error() { let repo = MockRepository::new().with_rad_sigrefs_error(&mock::node_id()); @@ -74,6 +82,28 @@ fn unchanged() { } } +#[test] +fn unchanged_force_writes_new_commit() { + let head = mock::oid(1); + let commit_oid = mock::oid(42); + let refs = some_refs(mock::oid(99)); + let repo = MockRepository::new() + .with_rad_sigrefs(&mock::node_id(), head) + .with_commit(head, mock::commit_data([])) + .with_refs(head, refs.clone()) + .with_signature(head, 1) + .with_write_tree_ok(mock::oid(99)) + .with_write_commit_ok(commit_oid) + .with_write_reference_ok(); + let update = force_write(refs.clone(), &repo).unwrap(); + let Update::Changed { entry, .. } = update else { + panic!("expected Update::Changed, got {update:?}"); + }; + assert_eq!(entry.parent, Some(head)); + assert_eq!(entry.oid, commit_oid); + assert_eq!(entry.into_refs(), refs); +} + #[test] fn commit_error() { let repo = MockRepository::new() diff --git a/crates/radicle/src/test/storage.rs b/crates/radicle/src/test/storage.rs index b4892b92..1b822ada 100644 --- a/crates/radicle/src/test/storage.rs +++ b/crates/radicle/src/test/storage.rs @@ -382,6 +382,13 @@ impl SignRepository for MockRepository { ) -> Result { todo!() } + + fn force_sign_refs>( + &self, + _signer: &Device, + ) -> Result { + todo!() + } } impl radicle_cob::Store for MockRepository {}