Merge pull request #7343 from gitbutlerapp/write-refs

Write real git refs and keep them up to date

crates/but-workspace/src/commit_engine/refs.rs

@@ -32,6 +32,7 @@ pub fn rewrite(
         == 1;
     for (old, new) in changed_commits {
         let old_git2 = old.to_git2();
+        let mut already_updated_refs = vec![];
         'stacks: for stack in &mut branches_ordered {
             if stack.head == old_git2 {
                 stack.head = new.to_git2();
@@ -64,7 +65,11 @@ pub fn rewrite(
                     }
                 };
                 if id == old {
-                    branch.set_head(CommitOrChangeId::CommitId(new.to_string()));
+                    if let Some(refname) =
+                        branch.set_head(CommitOrChangeId::CommitId(new.to_string()), repo)?
+                    {
+                        already_updated_refs.push(refname)
+                    }
                     updated_refs.push(UpdatedReference {
                         old_commit_id: old,
                         new_commit_id: new,
@@ -82,6 +87,9 @@ pub fn rewrite(
         };
 
         for name in refs_to_rewrite {
+            if already_updated_refs.iter().any(|r| name.as_bstr() == r) {
+                continue;
+            }
             use gix::refs::{
                 transaction::{Change, LogChange, RefEdit, RefLog},
                 Target,

crates/but-workspace/tests/workspace/commit_engine/refs_update.rs

@@ -144,6 +144,7 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> {
             ("s1-b/first", repo.rev_parse_single("@~1")?.detach()),
             ("s1-b/init", repo.rev_parse_single("@~2")?.detach()),
         ],
+        &repo,
     );
     vb.branches.insert(stack.id, stack);
 
@@ -155,6 +156,7 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> {
             ("s2-b/first", repo.rev_parse_single("@~1")?.detach()),
             ("s2-b/init", repo.rev_parse_single("@~2")?.detach()),
         ],
+        &repo,
     );
     vb.branches.insert(stack.id, stack);
 
@@ -274,7 +276,12 @@ fn insert_commit_into_single_stack_with_signatures() -> anyhow::Result<()> {
     * ecd6722 (tag: first-commit, first-commit) init
     ");
 
-    let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", initial_commit_id)]);
+    let stack = stack_with_branches(
+        "s1",
+        head_commit_id,
+        [("s1-b/init", initial_commit_id)],
+        &repo,
+    );
     vb.branches.insert(stack.id, stack);
     // Add 10 lines to the end.
     write_sequence(&repo, "file", [(30, None)])?;
@@ -430,7 +437,12 @@ fn branch_tip_below_non_merge_workspace_commit() -> anyhow::Result<()> {
     * 4342edf (tag: first-commit) init
     ");
 
-    let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", initial_commit_id)]);
+    let stack = stack_with_branches(
+        "s1",
+        head_commit_id,
+        [("s1-b/init", initial_commit_id)],
+        &repo,
+    );
     vb.branches.insert(stack.id, stack);
 
     write_sequence(&repo, "file", [(110, None)])?;
@@ -529,7 +541,7 @@ fn insert_commits_into_workspace() -> anyhow::Result<()> {
 
     let mut vb = VirtualBranchesState::default();
     let stack1_head = repo.rev_parse_single("merge^1")?.detach();
-    let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", stack1_head)]);
+    let stack = stack_with_branches("s1", head_commit_id, [("s1-b/init", stack1_head)], &repo);
     vb.branches.insert(stack.id, stack);
 
     // another 10 to the end (HEAD range is 1-30).
@@ -657,7 +669,7 @@ fn merge_commit_remains_unsigned_in_remerge() -> anyhow::Result<()> {
 
     let branch_a = repo.rev_parse_single("A")?.detach();
     let mut vb = VirtualBranchesState::default();
-    let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)]);
+    let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)], &repo);
     vb.branches.insert(stack.id, stack);
 
     // initial is 1-30, remove first 5
@@ -745,6 +757,7 @@ fn commit_on_top_of_branch_in_workspace() -> anyhow::Result<()> {
         // The order indicates which one actually is on top, even though they both point to the
         // same commit.
         [("s1-b/below-top", branch_a), ("s1-b/top", branch_a)],
+        &repo,
     );
     vb.branches.insert(stack.id, stack);
 
@@ -826,7 +839,7 @@ fn amend_on_top_of_branch_in_workspace() -> anyhow::Result<()> {
 
     let branch_a = repo.rev_parse_single("A")?.detach();
     let mut vb = VirtualBranchesState::default();
-    let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)]);
+    let stack = stack_with_branches("s1", branch_a, [("s1-b/top", branch_a)], &repo);
     vb.branches.insert(stack.id, stack);
 
     // initial is 1-30, make a change that transfers correctly to A where it is 5-20.
@@ -910,6 +923,7 @@ mod utils {
         name: &str,
         tip: gix::ObjectId,
         branches: impl IntoIterator<Item = (&'static str, gix::ObjectId)>,
+        repo: &gix::Repository,
     ) -> gitbutler_stack::Stack {
         gitbutler_stack::Stack {
             id: Default::default(),
@@ -918,7 +932,8 @@ mod utils {
             head: tip.to_git2(),
             heads: branches
                 .into_iter()
-                .map(|(name, target_id)| new_stack_branch(name, target_id))
+                .map(|(name, target_id)| new_stack_branch(name, target_id, repo))
+                .filter_map(Result::ok)
                 .collect(),
             notes: String::new(),
             source_refname: None,
@@ -936,11 +951,16 @@ mod utils {
         }
     }
 
-    fn new_stack_branch(name: &str, head: gix::ObjectId) -> gitbutler_stack::StackBranch {
+    fn new_stack_branch(
+        name: &str,
+        head: gix::ObjectId,
+        repo: &gix::Repository,
+    ) -> anyhow::Result<gitbutler_stack::StackBranch> {
         gitbutler_stack::StackBranch::new(
             CommitOrChangeId::CommitId(head.to_string()),
             name.into(),
             None,
+            repo,
         )
     }
 

crates/gitbutler-branch-actions/src/stack.rs

@@ -45,11 +45,12 @@ pub fn create_series(
     assure_open_workspace_mode(ctx).context("Requires an open workspace mode")?;
     let mut stack = ctx.project().virtual_branches().get_stack(stack_id)?;
     let normalized_head_name = normalize_branch_name(&req.name)?;
+    let repo = ctx.gix_repository()?;
     // If target_patch is None, create a new head that points to the top of the stack (most recent patch)
     if let Some(target_patch) = req.target_patch {
         stack.add_series(
             ctx,
-            StackBranch::new(target_patch, normalized_head_name, req.description),
+            StackBranch::new(target_patch, normalized_head_name, req.description, &repo)?,
             req.preceding_head,
         )
     } else {

crates/gitbutler-stack/Cargo.toml

@@ -27,6 +27,7 @@ gitbutler-repo.workspace = true
 gitbutler-commit.workspace = true
 gitbutler-oxidize.workspace = true
 uuid.workspace = true
+bstr.workspace = true
 
 [[test]]
 name = "stack"

crates/gitbutler-stack/src/heads.rs

@@ -20,6 +20,7 @@ pub(crate) fn get_head(heads: &[StackBranch], name: &str) -> Result<(usize, Stac
 pub(crate) fn remove_head(
     mut heads: Vec<StackBranch>,
     name: String,
+    repo: &gix::Repository,
 ) -> Result<(Vec<StackBranch>, bool)> {
     // find the head that corresponds to the supplied name, together with its index
     let (idx, head) = get_head(&heads, &name)?;
@@ -34,18 +35,19 @@ pub(crate) fn remove_head(
         let prior_head = heads
             .get_mut(idx - 1)
             .ok_or_else(|| anyhow!("Cannot get the head before the head being removed"))?;
-        prior_head.set_head(head.head().to_owned());
+        prior_head.set_head(head.head().to_owned(), repo)?;
         moved_another_reference = true;
     }
     heads.remove(idx);
+    head.delete_reference(repo)?;
     Ok((heads, moved_another_reference))
 }
 
 /// Takes the list of current existing heads and a new head.
 /// Returns new, updated list of heads with the new head added in the correct position.
 /// If there are multiple heads pointing to the same patch, it uses `preceding_head` to disambiguate the order.
 // TODO: when there is a patch reference for a commit ID and a patch reference for a change ID, recognize if they are equivalent (i.e. point to the same commit)
-pub(crate) fn add_head(
+pub fn add_head(
     existing_heads: Vec<StackBranch>,
     new_head: StackBranch,
     preceding_head: Option<StackBranch>,
@@ -137,41 +139,3 @@ pub(crate) fn add_head(
     }
     Ok(updated_heads)
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-    #[test]
-    fn add_head_with_archived_bottom_head() -> Result<()> {
-        let mut head_1_archived = StackBranch::new(
-            CommitOrChangeId::CommitId("328447a2-08aa-4c4d-a1bc-08d5cd82bcd4".to_string()),
-            "kv-branch-3".to_string(),
-            None,
-        );
-        head_1_archived.archived = true;
-        let head_2 = StackBranch::new(
-            CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()),
-            "more-on-top".to_string(),
-            None,
-        );
-        let existing_heads = vec![head_1_archived.clone(), head_2.clone()];
-        let new_head = StackBranch::new(
-            CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()),
-            "abcd".to_string(),
-            None,
-        );
-        let patches = vec![
-            CommitOrChangeId::CommitId("92a89ae608d77ff75c1ce52ea9dccc0bccd577e9".to_string()),
-            CommitOrChangeId::CommitId("11609175-039d-44ee-9d4a-6baa9ad2a750".to_string()),
-        ];
-
-        let updated_heads = add_head(
-            existing_heads,
-            new_head.clone(),
-            Some(head_2.clone()),
-            patches,
-        )?;
-        assert_eq!(updated_heads, vec![head_1_archived, head_2, new_head]);
-        Ok(())
-    }
-}

crates/gitbutler-stack/src/lib.rs

@@ -13,6 +13,7 @@ pub use state::{VirtualBranches as VirtualBranchesState, VirtualBranchesHandle};
 pub use target::Target;
 
 mod heads;
+pub use heads::add_head;
 pub use stack::{commit_by_oid_or_change_id, CommitsForId, PatchReferenceUpdate, TargetUpdate};
 
 // This is here because CommitOrChangeId::ChangeId is deprecated, for some reason allow cant be done on the CommitOrChangeId struct