Radish alpha
h
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
radicle: Split `WriteRepository::set_head` up
Draft lorenz opened 7 months ago

The method set_head did 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
8 files changed +66 -36 646d4360 f00a5ed4
modified crates/radicle-node/src/worker/fetch.rs
@@ -119,11 +119,9 @@ impl Handle {
                // points to a repository that is temporary and gets moved by [`mv`].
                let repo = storage.repository(rid)?;
                repo.set_identity_head()?;
-
                match repo.set_head() {
-
                    Ok(head) => {
-
                        if head.is_updated() {
-
                            log::trace!(target: "worker", "Set HEAD to {}", head.new);
-
                        }
+
                match repo.set_head_to_default_branch() {
+
                    Ok(()) => {
+
                        log::trace!(target: "worker", "Set HEAD successfully");
                    }
                    Err(RepositoryError::Quorum(e)) => {
                        log::warn!(target: "worker", "Fetch could not set HEAD: {e}")
@@ -369,9 +367,6 @@ fn set_canonical_refs(
    applied: &Applied,
) -> Result<Option<UpdatedCanonicalRefs>, error::Canonical> {
    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
        .canonical_refs_or_default(|| {
            let rule = identity.doc().default_branch_rule()?;
modified crates/radicle-remote-helper/src/push.rs
@@ -282,8 +282,7 @@ pub fn run(
    }
    let delegates = stored.delegates()?;
    let identity = stored.identity()?;
-
    let project = identity.project()?;
-
    let canonical_ref = git::refs::branch(project.default_branch());
+
    let canonical_ref = identity.default_branch()?;
    let mut set_canonical_refs: Vec<(git::Qualified, git::canonical::Object)> =
        Vec::with_capacity(specs.len());
    let working = git::raw::Repository::open(working)?;
@@ -401,14 +400,8 @@ pub fn run(

            // N.b. special case for handling the canonical ref, since it
            // creates a symlink to HEAD
-
            if *refname == canonical_ref
-
                && stored
-
                    .set_head()
-
                    .map(|head| head.is_updated())
-
                    .unwrap_or(false)
-
            {
-
                print_update();
-
                continue;
+
            if *refname == canonical_ref {
+
                stored.set_head_to_default_branch()?;
            }

            match stored.backend.refname_to_id(refname.as_str()) {
modified crates/radicle/src/identity/doc.rs
@@ -742,12 +742,16 @@ impl Doc {
        Ok(proj)
    }

+
    /// Gets the qualified reference name of the default branch,
+
    /// according to the project payload in this document.
+
    pub fn default_branch(&self) -> Result<git::Qualified, PayloadError> {
+
        Ok(git::refs::branch(self.project()?.default_branch()))
+
    }
+

    pub fn default_branch_rule(
        &self,
    ) -> Result<(rules::Pattern, rules::ValidRule), DefaultBranchRuleError> {
-
        let proj = self.project()?;
-
        let refname = proj.default_branch();
-
        let pattern = rules::Pattern::try_from(git::refs::branch(refname).to_owned())?;
+
        let pattern = rules::Pattern::try_from(self.default_branch()?.to_owned())?;
        let rule = rules::Rule::new(
            rules::ResolvedDelegates::Delegates(self.delegates.clone()),
            self.threshold,
modified crates/radicle/src/rad.rs
@@ -147,7 +147,8 @@ where
    )?;
    stored.set_remote_identity_root_to(pk, 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)?;

modified crates/radicle/src/storage.rs
@@ -522,6 +522,12 @@ pub trait ReadRepository: Sized + ValidateRepository {
    /// Returns the [`Oid`] as well as the qualified reference name.
    fn head(&self) -> Result<(Qualified, Oid), RepositoryError>;

+
    /// Gets the qualified reference name of the default branch of self,
+
    /// according to the project payload in the identity document.
+
    fn default_branch(&self) -> Result<Qualified, RepositoryError> {
+
        Ok(self.identity_doc()?.default_branch()?.to_owned())
+
    }
+

    /// Compute the canonical head of this repository.
    ///
    /// Ignores any existing `HEAD` reference.
@@ -664,9 +670,16 @@ where

/// Allows read-write access to a repository.
pub trait WriteRepository: ReadRepository + SignRepository {
-
    /// Set the repository head to the canonical branch.
-
    /// This computes the head based on the delegate set.
-
    fn set_head(&self) -> Result<SetHead, RepositoryError>;
+
    /// Sets the symbolic reference `HEAD` to target the default branch.
+
    /// This only depends on the value for the default branch in the identity
+
    /// 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.
    fn set_identity_head(&self) -> Result<Oid, RepositoryError> {
        let head = self.canonical_identity_head()?;
modified crates/radicle/src/storage/git.rs
@@ -786,7 +786,7 @@ impl ReadRepository for Repository {

    fn canonical_head(&self) -> Result<(Qualified, Oid), RepositoryError> {
        let doc = self.identity_doc()?;
-
        let refname = git::refs::branch(doc.project()?.default_branch());
+
        let refname = doc.default_branch()?.to_owned();
        let crefs = match doc.canonical_refs()? {
            Some(crefs) => crefs,
            // Fallback to constructing the default branch via the project
@@ -878,16 +878,39 @@ impl ReadRepository 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 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 git::ext::is_not_found_err(&err) => {
+
                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
            .raw()
-
            .refname_to_id(&head_ref)
+
            .refname_to_id(&branch_ref)
            .ok()
            .map(|oid| oid.into());

-
        let (branch_ref, new) = self.canonical_head()?;
-

        if old == Some(new) {
            return Ok(SetHead { old, new });
        }
@@ -895,10 +918,6 @@ impl WriteRepository for Repository {
        self.raw()
            .reference(&branch_ref, *new, 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 })
    }

modified crates/radicle/src/test.rs
@@ -58,7 +58,8 @@ pub fn fetch<W: WriteRepository>(
    drop(opts);

    repo.set_identity_head()?;
-
    repo.set_head()?;
+
    repo.set_head_to_default_branch()?;
+
    repo.set_default_branch_to_canonical_head()?;

    let validations = repo.validate()?;
    if !validations.is_empty() {
modified crates/radicle/src/test/storage.rs
@@ -326,7 +326,11 @@ impl WriteRepository for MockRepository {
        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!()
    }