radicle/storage: Split `WriteRepository::set_head`

The method `set_head` does two things:
 1. Compute the canonical head and set the default branch to target.
 2. Set the symbolic reference `HEAD` to target the default branch.

Split these two concerns into:
 1. `set_default_branch_to_canonical_head`
 2. `set_head_to_default_branch`
This commit is contained in:
Lorenz Leutgeb 2025-09-07 14:01:27 +02:00 committed by Fintan Halpenny
parent 0eba6caf9f
commit bfb2858054
7 changed files with 55 additions and 31 deletions

View File

@ -131,11 +131,9 @@ impl Handle {
// points to a repository that is temporary and gets moved by [`mv`]. // points to a repository that is temporary and gets moved by [`mv`].
let repo = storage.repository(rid)?; let repo = storage.repository(rid)?;
repo.set_identity_head()?; repo.set_identity_head()?;
match repo.set_head() { match repo.set_head_to_default_branch() {
Ok(head) => { Ok(()) => {
if head.is_updated() { log::trace!(target: "worker", "Set HEAD successfully");
log::trace!(target: "worker", "Set HEAD to {}", head.new);
}
} }
Err(RepositoryError::Quorum(e)) => { Err(RepositoryError::Quorum(e)) => {
log::warn!(target: "worker", "Fetch could not set HEAD for {rid}: {e}") log::warn!(target: "worker", "Fetch could not set HEAD for {rid}: {e}")
@ -352,9 +350,6 @@ fn set_canonical_refs(
applied: &Applied, applied: &Applied,
) -> Result<Option<UpdatedCanonicalRefs>, error::Canonical> { ) -> Result<Option<UpdatedCanonicalRefs>, error::Canonical> {
let identity = repo.identity()?; let identity = repo.identity()?;
// 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 let rules = identity
.canonical_refs_or_default(|| { .canonical_refs_or_default(|| {
let rule = identity.doc().default_branch_rule()?; let rule = identity.doc().default_branch_rule()?;

View File

@ -426,14 +426,8 @@ pub(super) fn run(
// N.b. special case for handling the canonical ref, since it // N.b. special case for handling the canonical ref, since it
// creates a symlink to HEAD // creates a symlink to HEAD
if *refname == canonical_ref if *refname == canonical_ref {
&& stored stored.set_head_to_default_branch()?;
.set_head()
.map(|head| head.is_updated())
.unwrap_or(false)
{
print_update();
continue;
} }
match stored.backend.refname_to_id(refname.as_str()) { match stored.backend.refname_to_id(refname.as_str()) {

View File

@ -143,7 +143,8 @@ where
)?; )?;
stored.set_remote_identity_root_to(pk, identity)?; stored.set_remote_identity_root_to(pk, identity)?;
stored.set_identity_head_to(identity)?; stored.set_identity_head_to(identity)?;
stored.set_head()?; stored.set_head_to_default_branch()?;
stored.set_default_branch_to_canonical_head()?;
let signed = stored.sign_refs(signer)?; let signed = stored.sign_refs(signer)?;

View File

@ -115,7 +115,7 @@ impl FromIterator<PublicKey> for Namespaces {
} }
} }
/// Output of [`WriteRepository::set_head`]. /// Output of [`WriteRepository::set_default_branch_to_canonical_head`].
pub struct SetHead { pub struct SetHead {
/// Old branch head. /// Old branch head.
pub old: Option<Oid>, pub old: Option<Oid>,
@ -668,9 +668,16 @@ where
/// Allows read-write access to a repository. /// Allows read-write access to a repository.
pub trait WriteRepository: ReadRepository + SignRepository { pub trait WriteRepository: ReadRepository + SignRepository {
/// Set the repository head to the canonical branch. /// Sets the symbolic reference `HEAD` to target the default branch.
/// This computes the head based on the delegate set. /// This only depends on the value for the default branch in the identity
fn set_head(&self) -> Result<SetHead, RepositoryError>; /// document, and does not require the canonical reference behind the
/// default branch to be computed, or even exist.
fn set_head_to_default_branch(&self) -> Result<(), RepositoryError>;
/// Computes the head of the default branch based on the delegate set,
/// and sets it.
fn set_default_branch_to_canonical_head(&self) -> Result<SetHead, RepositoryError>;
/// Set the repository 'rad/id' to the canonical commit, agreed by quorum. /// Set the repository 'rad/id' to the canonical commit, agreed by quorum.
fn set_identity_head(&self) -> Result<Oid, RepositoryError> { fn set_identity_head(&self) -> Result<Oid, RepositoryError> {
let head = self.canonical_identity_head()?; let head = self.canonical_identity_head()?;

View File

@ -943,16 +943,42 @@ impl ReadRepository for Repository {
} }
impl WriteRepository for Repository { impl WriteRepository for Repository {
fn set_head(&self) -> Result<SetHead, RepositoryError> { fn set_head_to_default_branch(&self) -> Result<(), RepositoryError> {
let head_ref = refname!("HEAD"); let head_ref = refname!("HEAD");
let branch_ref = self.default_branch()?;
match self.raw().find_reference(head_ref.as_str()) {
Ok(mut head_ref) => {
if head_ref
.symbolic_target()
.is_some_and(|t| t != branch_ref.as_str())
{
head_ref.symbolic_set_target(branch_ref.as_str(), "set-head (radicle)")?;
}
Ok(())
}
Err(err) if err.is_not_found() => {
self.raw().reference_symbolic(
head_ref.as_str(),
branch_ref.as_str(),
true,
"set-head (radicle)",
)?;
Ok(())
}
Err(err) => Err(err.into()),
}
}
fn set_default_branch_to_canonical_head(&self) -> Result<SetHead, RepositoryError> {
let (branch_ref, new) = self.canonical_head()?;
let old = self let old = self
.raw() .raw()
.refname_to_id(&head_ref) .refname_to_id(&branch_ref)
.ok() .ok()
.map(|oid| oid.into()); .map(|oid| oid.into());
let (branch_ref, new) = self.canonical_head()?;
if old == Some(new) { if old == Some(new) {
return Ok(SetHead { old, new }); return Ok(SetHead { old, new });
} }
@ -960,10 +986,6 @@ impl WriteRepository for Repository {
self.raw() self.raw()
.reference(&branch_ref, new.into(), true, "set-local-branch (radicle)")?; .reference(&branch_ref, new.into(), true, "set-local-branch (radicle)")?;
log::debug!(target: "storage", "Setting ref: {head_ref} -> {branch_ref}");
self.raw()
.reference_symbolic(&head_ref, &branch_ref, true, "set-head (radicle)")?;
Ok(SetHead { old, new }) Ok(SetHead { old, new })
} }

View File

@ -59,7 +59,8 @@ pub fn fetch<W: WriteRepository>(
drop(opts); drop(opts);
repo.set_identity_head()?; repo.set_identity_head()?;
repo.set_head()?; repo.set_head_to_default_branch()?;
repo.set_default_branch_to_canonical_head()?;
let validations = repo.validate()?; let validations = repo.validate()?;
if !validations.is_empty() { if !validations.is_empty() {

View File

@ -344,7 +344,11 @@ impl WriteRepository for MockRepository {
todo!() todo!()
} }
fn set_head(&self) -> Result<SetHead, RepositoryError> { fn set_head_to_default_branch(&self) -> Result<(), RepositoryError> {
todo!()
}
fn set_default_branch_to_canonical_head(&self) -> Result<SetHead, RepositoryError> {
todo!() todo!()
} }