From b20f81eb0cd190e23477fc7acd5ba2f2b148027b Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 30 Mar 2023 11:52:15 +0200 Subject: [PATCH] Use cached canonical `rad/id` refs We were recomputing the `rad/id` ref every time it was accessed. --- radicle/src/cob/identity.rs | 5 +- radicle/src/rad.rs | 1 - radicle/src/storage/git.rs | 112 ++++++++++++++++-------------------- 3 files changed, 54 insertions(+), 64 deletions(-) diff --git a/radicle/src/cob/identity.rs b/radicle/src/cob/identity.rs index 34037635..0cfbea4e 100644 --- a/radicle/src/cob/identity.rs +++ b/radicle/src/cob/identity.rs @@ -230,9 +230,12 @@ impl Proposal { self.description().unwrap_or_default() ); let current = doc.update(remote, &msg, &signatures.collect::>(), repo.raw())?; + let head = repo.set_identity_head()?; + + assert_eq!(head, current); Ok(Identity { - head: current, + head, root: previous.root, current, revision: previous.revision + 1, diff --git a/radicle/src/rad.rs b/radicle/src/rad.rs index 35ca264b..73e796ba 100644 --- a/radicle/src/rad.rs +++ b/radicle/src/rad.rs @@ -165,7 +165,6 @@ pub fn fork( ) -> Result<(), ForkError> { let me = signer.public_key(); let repository = storage.repository_mut(rid)?; - // TODO: We should get the id branch pointer from a stored canonical reference. let (canonical_id, _) = repository.identity_doc()?; let (canonical_branch, canonical_head) = repository.head()?; let raw = repository.raw(); diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index 0e0a3131..a379442d 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -310,64 +310,6 @@ impl Repository { Ok(verified) } - /// Return the canonical identity [`git::Oid`] and document. - pub fn identity_doc(&self) -> Result<(Oid, identity::Doc), IdentityError> { - let head = self.identity_head()?; - - Doc::::load_at(head, self) - .map(|(doc, _)| (head, doc)) - .map_err(IdentityError::from) - } - - /// Return the canonical identity branch head. - pub fn identity_head(&self) -> Result { - let mut heads = Vec::new(); - - for remote in self.remote_ids()? { - let remote = remote?; - let oid = Doc::::head(&remote, self)?; - - heads.push(oid.into()); - } - // Keep track of the longest identity branch. - let mut longest = heads.pop().ok_or(IdentityError::MissingBranch)?; - - for head in &heads { - let base = self.raw().merge_base(*head, longest)?; - - if base == longest { - // `head` is a successor of `longest`. Update `longest`. - // - // o head - // | - // o longest (base) - // | - // - longest = *head; - } else if base == *head || *head == longest { - // `head` is an ancestor of `longest`, or equal to it. Do nothing. - // - // o longest o longest, head (base) - // | | - // o head (base) OR o - // | | - // - } else { - // The merge base between `head` and `longest` (`base`) - // is neither `head` nor `longest`. Therefore, the branches have - // diverged. - // - // longest head - // \ / - // o (base) - // | - // - return Err(IdentityError::BranchesDiverge); - } - } - Ok(longest.into()) - } - pub fn remote_ids( &self, ) -> Result> + '_, git2::Error> { @@ -554,7 +496,11 @@ impl ReadRepository for Repository { } fn identity_doc(&self) -> Result<(Oid, identity::Doc), IdentityError> { - Repository::identity_doc(self) + let head = self.identity_head()?; + + Doc::::load_at(head, self) + .map(|(doc, _)| (head, doc)) + .map_err(IdentityError::from) } fn head(&self) -> Result<(Qualified, Oid), IdentityError> { @@ -568,8 +514,6 @@ impl ReadRepository for Repository { } fn canonical_head(&self) -> Result<(Qualified, Oid), IdentityError> { - // TODO: In the `fork` function for example, we call Repository::project_identity again, - // This should only be necessary once. let (_, doc) = self.identity_doc()?; let doc = doc.verified()?; let project = doc.project()?; @@ -601,7 +545,51 @@ impl ReadRepository for Repository { } fn canonical_identity_head(&self) -> Result { - Repository::identity_head(self) + let mut heads = Vec::new(); + + for remote in self.remote_ids()? { + let remote = remote?; + let oid = Doc::::head(&remote, self)?; + + heads.push(oid.into()); + } + // Keep track of the longest identity branch. + let mut longest = heads.pop().ok_or(IdentityError::MissingBranch)?; + + for head in &heads { + let base = self.raw().merge_base(*head, longest)?; + + if base == longest { + // `head` is a successor of `longest`. Update `longest`. + // + // o head + // | + // o longest (base) + // | + // + longest = *head; + } else if base == *head || *head == longest { + // `head` is an ancestor of `longest`, or equal to it. Do nothing. + // + // o longest o longest, head (base) + // | | + // o head (base) OR o + // | | + // + } else { + // The merge base between `head` and `longest` (`base`) + // is neither `head` nor `longest`. Therefore, the branches have + // diverged. + // + // longest head + // \ / + // o (base) + // | + // + return Err(IdentityError::BranchesDiverge); + } + } + Ok(longest.into()) } }