From 0d96af5d07de3fe52c6d70203f77176aa4f4cecb Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Wed, 20 Aug 2025 09:16:37 +0100 Subject: [PATCH] node: update default branch for canonical refs After a fetch is performed, `set_head` is called, which essentially updates the default branch based on the canonical reference rule for that branch. Later, `set_canonical_refs` is called, which originally could be the empty set of rules. This would mean that the event for the default branch would never be emitted. The approach taken here is to always use the default branch rule if there are no rules set in the identity document. Unfortunately, this ends up in computing the default branch twice, due to `set_head`. Another approach is to use the result of `set_head` to update the set of `UpdatedCanonicalRefs`, to prevent the need for defaulting to the default branch rule. However, this does not prevent the double counting as soon as the rules are added to the identity document, since the default branch rule will then be synthesised into the rule set. --- crates/radicle-node/src/worker/fetch.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/radicle-node/src/worker/fetch.rs b/crates/radicle-node/src/worker/fetch.rs index 2f115cca..b5302060 100644 --- a/crates/radicle-node/src/worker/fetch.rs +++ b/crates/radicle-node/src/worker/fetch.rs @@ -1,3 +1,5 @@ +use radicle::identity::doc::CanonicalRefsError; +use radicle::identity::CanonicalRefs; pub(crate) use radicle_protocol::worker::fetch::error; use std::collections::BTreeSet; @@ -367,14 +369,16 @@ fn set_canonical_refs( applied: &Applied, ) -> Result, error::Canonical> { let identity = repo.identity()?; - let rules = match identity - .canonical_refs()? - .map(|crefs| crefs.rules().clone()) - .filter(|rules| !rules.is_empty()) - { - None => return Ok(None), - Some(rules) => rules, - }; + // TODO(finto): it's unfortunate that we may end up computing the default + // branch again after `set_head` is called after the fetch. This is due to + // the storage capabilities being leaked to this part of the code base. + let rules = identity + .canonical_refs_or_default(|| { + let rule = identity.doc().default_branch_rule()?; + Ok::<_, CanonicalRefsError>(CanonicalRefs::from_iter([rule])) + })? + .rules() + .clone(); let mut updated_refs = UpdatedCanonicalRefs::default(); let refnames = applied @@ -389,6 +393,7 @@ fn set_canonical_refs( _ => None, }) .collect::>(); + for name in refnames { let canonical = match rules.canonical(name.clone(), repo) { Some(canonical) => canonical,