add a test to assure non-workspace refs are *not* updated

Author: Byron

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

@@ -348,22 +348,57 @@ pub fn create_commit_and_update_refs(
         return Ok(out);
     };
 
-    let commit_to_find = match destination {
+    let (commit_to_find, is_amend) = match destination {
         Destination::NewCommit {
             parent_commit_id, ..
-        } => parent_commit_id,
-        Destination::AmendCommit(commit) => Some(commit),
+        } => (parent_commit_id, false),
+        Destination::AmendCommit(commit) => (Some(commit), true),
     };
 
     if let Some(commit_in_graph) = commit_to_find {
         let mut all_refs_by_id = gix::hashtable::HashMap::<_, Vec<_>>::default();
-        for (commit_id, git_reference) in repo
-            .references()?
-            .prefixed("refs/heads/")?
-            .chain(repo.references()?.prefixed("refs/gitbutler/")?)
-            .filter_map(Result::ok)
-            .filter_map(|r| r.try_id().map(|id| (id.detach(), r.inner.name)))
-        {
+        let mut checked_out_ref_name = None;
+        let checked_out_ref = repo.head_ref()?.and_then(|mut r| {
+            let id = r.peel_to_id_in_place().ok()?.detach();
+            checked_out_ref_name = Some(r.inner.name.clone());
+            Some((id, r.inner.name))
+        });
+        let (platform_storage, platform_storage_2);
+        let checked_out_and_gitbutler_refs =
+            checked_out_ref
+                .into_iter()
+                // TODO: remove this as `refs/gitbutler/` won't contain relevant refs anymore.
+                .chain({
+                    platform_storage_2 = repo.references()?;
+                    platform_storage_2
+                        .prefixed("refs/gitbutler/")?
+                        .filter_map(Result::ok)
+                        .filter_map(|r| r.try_id().map(|id| (id.detach(), r.inner.name)))
+                })
+                .chain(
+                    // When amending, we want to update all branches that pointed to the old commit to now point to the new commit.
+                    if is_amend {
+                        Box::new({
+                            platform_storage = repo.references()?;
+                            platform_storage
+                                .prefixed("refs/heads/")?
+                                .filter_map(Result::ok)
+                                .filter_map(|r| {
+                                    let is_checked_out = checked_out_ref_name.as_ref().is_some_and(
+                                        |checked_out_ref| checked_out_ref == &r.inner.name,
+                                    );
+                                    if is_checked_out {
+                                        None
+                                    } else {
+                                        r.try_id().map(|id| (id.detach(), r.inner.name))
+                                    }
+                                })
+                        }) as Box<dyn Iterator<Item = _>>
+                    } else {
+                        Box::new(std::iter::empty())
+                    },
+                );
+        for (commit_id, git_reference) in checked_out_and_gitbutler_refs {
             all_refs_by_id
                 .entry(commit_id)
                 .or_default()

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

@@ -36,15 +36,7 @@ pub fn rewrite(
                 }
             }
             if stack.head_oid(repo)? == old {
-                // Perhaps skip this - the head will be updated later in this call
-                // stack.set_stack_head_without_persisting(repo, new.to_git2(), None)?;
-                // Does it make sense to set stack tree in v3? I think not
-                // stack.tree = new
-                //     .attach(repo)
-                //     .object()?
-                //     .into_commit()
-                //     .tree_id()?
-                //     .to_git2();
+                // The actual head will be updated later.
                 updated_refs.push(UpdatedReference {
                     old_commit_id: old,
                     new_commit_id: new,
@@ -65,15 +57,16 @@ pub fn rewrite(
                     });
             for (idx, branch) in stack.heads.iter_mut().rev().enumerate() {
                 let id = branch.head_oid(repo)?;
-                if id == old {
-                    if update_up_to_idx.is_some() && Some(idx) > update_up_to_idx {
-                        // Make sure the actual refs also don't update (later)
-                        already_updated_refs.push(format!("refs/heads/{}", branch.name()).into());
-                        continue;
-                    }
-                    if let Some(full_refname) = branch.set_head(new, repo)? {
-                        already_updated_refs.push(full_refname)
-                    }
+                if id != old {
+                    continue;
+                }
+                if update_up_to_idx.is_some() && Some(idx) > update_up_to_idx {
+                    // Make sure the actual refs also don't update (later)
+                    already_updated_refs.push(format!("refs/heads/{}", branch.name()).into());
+                    continue;
+                }
+                if let Some(full_refname) = branch.set_head(new, repo)? {
+                    already_updated_refs.push(full_refname);
                     updated_refs.push(UpdatedReference {
                         old_commit_id: old,
                         new_commit_id: new,

crates/but-workspace/tests/fixtures/scenario/three-commits-with-line-offset-and-workspace-commit.sh

@@ -10,6 +10,6 @@ git init
 seq 10 >file
 git add . && git commit -m init && git tag first-commit
 
-{ seq 4; seq 10; } >file && git commit -am "insert 5 lines to the top"
+{ seq 4; seq 10; } >file && git commit -am "insert 5 lines to the top" && git branch feat1
 
 git commit -m $'GitButler Workspace Commit\n\njust a fake - only the subject matters right now' --allow-empty

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

@@ -110,8 +110,8 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> {
     // The HEAD reference was updated, along with all other tag-references that pointed to it.
     let new_commit = outcome.new_commit.expect("a new commit was created");
     insta::assert_snapshot!(visualize_commit_graph(&repo, new_commit)?, @r"
-    * 1c4bb33 (HEAD -> main, another-tip) third commit
-    * f1b87da (tag: tag-that-should-not-move) second commit
+    * 1c4bb33 (HEAD -> main) third commit
+    * f1b87da (tag: tag-that-should-not-move, another-tip) second commit
     * 0939187 initial commit
     ");
 
@@ -132,8 +132,8 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> {
     assure_no_worktree_changes(&repo)?;
     // The top commit has a different hash now thanks to amending.
     insta::assert_snapshot!(graph_commit_outcome(&repo, &outcome)?, @r"
-    * 2abfa5c (HEAD -> main, another-tip) third commit
-    * f1b87da (tag: tag-that-should-not-move) second commit
+    * 2abfa5c (HEAD -> main) third commit
+    * f1b87da (tag: tag-that-should-not-move, another-tip) second commit
     * 0939187 initial commit
     ");
 
@@ -194,15 +194,6 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> {
             Sha1(273aeca7ca98af0f7972af6e7859a3ae7fde497a),
         ),
         references: [
-            UpdatedReference {
-                reference: Git(
-                    FullName(
-                        "refs/heads/another-tip",
-                    ),
-                ),
-                old_commit_id: Sha1(2abfa5cc3c7c48b8b9eabbd10c21b88347801f15),
-                new_commit_id: Sha1(189ac82eb44ddb97677d7d7b1859cf6f2e33a473),
-            },
             UpdatedReference {
                 reference: Git(
                     FullName(
@@ -234,9 +225,9 @@ fn new_commits_to_tip_from_unborn_head() -> anyhow::Result<()> {
     write_vrbranches_to_refs(&vb, &repo)?;
     // It updates stack heads and stack branch heads.
     insta::assert_snapshot!(graph_commit_outcome(&repo, &outcome)?, @r"
-    * 189ac82 (HEAD -> main, s2-b/second, s1-b/second, another-tip) fourth commit
+    * 189ac82 (HEAD -> main, s2-b/second, s1-b/second) fourth commit
     * 2abfa5c third commit
-    * f1b87da (tag: tag-that-should-not-move, s2-b/first, s1-b/first) second commit
+    * f1b87da (tag: tag-that-should-not-move, s2-b/first, s1-b/first, another-tip) second commit
     * 0939187 (s2-b/init, s1-b/init) initial commit
     ");
     insta::assert_snapshot!(visualize_tree(&repo, &outcome)?, @r#"
@@ -269,7 +260,7 @@ fn new_stack_receives_commit_and_adds_it_to_workspace_commit() -> anyhow::Result
     let workspace_commit_id = repo.rev_parse_single("@")?.detach();
     insta::assert_snapshot!(visualize_commit_graph(&repo, workspace_commit_id)?, @r"
     * 47c9e16 (HEAD -> main) GitButler Workspace Commit
-    * b451685 insert 5 lines to the top
+    * b451685 (feat1) insert 5 lines to the top
     * d15b5ae (tag: first-commit) init
     ");
 
@@ -312,7 +303,7 @@ fn new_stack_receives_commit_and_adds_it_to_workspace_commit() -> anyhow::Result
     *   7051951 (HEAD -> main) GitButler Workspace Commit
     |\  
     | * 00fbfba (s2/top) new file with 15 lines
-    * | b451685 (s1/top) insert 5 lines to the top
+    * | b451685 (s1/top, feat1) insert 5 lines to the top
     |/  
     * d15b5ae (tag: first-commit) init
     ");
@@ -611,8 +602,8 @@ fn insert_commit_into_single_stack_with_signatures() -> anyhow::Result<()> {
     let rewritten_head_id = repo.head_id()?.detach();
     insta::assert_snapshot!(visualize_commit_graph(&repo, rewritten_head_id)?, @r"
     * 3d1262e (HEAD -> main) insert 10 lines to the top
-    * 3aec753 (s1-b/init, first-commit) between initial and former first
-    * ecd6722 (tag: first-commit) init
+    * 3aec753 (s1-b/init) between initial and former first
+    * ecd6722 (tag: first-commit, first-commit) init
     ");
     insta::assert_snapshot!(but_testsupport::visualize_tree(rewritten_head_id.attach(&repo)), @r#"
     5fdd313
@@ -639,15 +630,6 @@ fn insert_commit_into_single_stack_with_signatures() -> anyhow::Result<()> {
                 old_commit_id: Sha1(ecd67221705b069c4f46365a46c8f2cd8a97ec19),
                 new_commit_id: Sha1(3aec75308383b83d85a78a90308a618755a7b0f8),
             },
-            UpdatedReference {
-                reference: Git(
-                    FullName(
-                        "refs/heads/first-commit",
-                    ),
-                ),
-                old_commit_id: Sha1(ecd67221705b069c4f46365a46c8f2cd8a97ec19),
-                new_commit_id: Sha1(3aec75308383b83d85a78a90308a618755a7b0f8),
-            },
             UpdatedReference {
                 reference: Git(
                     FullName(
@@ -710,8 +692,8 @@ fn insert_commit_into_single_stack_with_signatures() -> anyhow::Result<()> {
     let rewritten_head_id = repo.head_id()?;
     insta::assert_snapshot!(visualize_commit_graph(&repo, rewritten_head_id)?, @r"
     * 4b649eb (HEAD -> main) insert 10 lines to the top
-    * d26d789 (s1-b/init, first-commit) between initial and former first
-    * ecd6722 (tag: first-commit) init
+    * d26d789 (s1-b/init) between initial and former first
+    * ecd6722 (tag: first-commit, first-commit) init
     ");
     insta::assert_snapshot!(but_testsupport::visualize_tree(rewritten_head_id), @r#"
     683b451
@@ -879,8 +861,8 @@ fn insert_commits_into_workspace() -> anyhow::Result<()> {
     *   a97960f (HEAD -> merge) Merge branch 'A' into merge
     |\  
     | * 3538622 (A) add 10 to the beginning
-    * | 46991ae (s1-b/init, B) add 10 more lines at end
-    * | e81b470 add 10 to the end
+    * | 46991ae (s1-b/init) add 10 more lines at end
+    * | e81b470 (B) add 10 to the end
     |/  
     * 9cf2979 (main) init
     ");
@@ -1006,8 +988,8 @@ fn merge_commit_remains_unsigned_in_remerge() -> anyhow::Result<()> {
     insta::assert_snapshot!(visualize_commit_graph(&repo, rewritten_head_id)?, @r"
     *   595a255 (HEAD -> merge) Merge branch 'A' into merge
     |\  
-    | * 057f154 (s1-b/top, A) remove 5 lines from beginning
-    | * eede47d add 10 to the beginning
+    | * 057f154 (s1-b/top) remove 5 lines from beginning
+    | * eede47d (A) add 10 to the beginning
     * | 16fe86e (B) add 10 to the end
     |/  
     * 6074509 (main) init
@@ -1227,8 +1209,8 @@ fn commit_on_top_of_branch_in_workspace() -> anyhow::Result<()> {
     insta::assert_snapshot!(visualize_commit_graph(&repo, rewritten_head_id)?, @r"
     *   09ac476 (HEAD -> merge) Merge branch 'A' into merge
     |\  
-    | * 99f3a1c (s1-b/top, A) remove 5 lines from beginning
-    | * 7f389ed (s1-b/below-top) add 10 to the beginning
+    | * 99f3a1c (s1-b/top) remove 5 lines from beginning
+    | * 7f389ed (s1-b/below-top, A) add 10 to the beginning
     * | 91ef6f6 (s2-b/top, s2-b/below-top, B) add 10 to the end
     |/  
     * ff045ef (main) init
@@ -1315,10 +1297,10 @@ fn commit_on_top_of_branch_in_workspace() -> anyhow::Result<()> {
     insta::assert_snapshot!(visualize_commit_graph(&repo, rewritten_head_id)?, @r"
     *   098effd (HEAD -> merge) Merge branch 'A' into merge
     |\  
-    | * 99f3a1c (s1-b/top, A) remove 5 lines from beginning
-    | * 7f389ed (s1-b/below-top) add 10 to the beginning
-    * | 03e9b6f (s2-b/top, B) remove 5 lines from the end
-    * | 91ef6f6 (s2-b/below-top) add 10 to the end
+    | * 99f3a1c (s1-b/top) remove 5 lines from beginning
+    | * 7f389ed (s1-b/below-top, A) add 10 to the beginning
+    * | 03e9b6f (s2-b/top) remove 5 lines from the end
+    * | 91ef6f6 (s2-b/below-top, B) add 10 to the end
     |/  
     * ff045ef (main) init
     ");

crates/gitbutler-stack/src/stack.rs

@@ -210,11 +210,10 @@ impl Stack {
         self.try_into()
     }
 
-    // TODO: derive this from the last head
     pub fn head_oid(&self, repo: &gix::Repository) -> Result<gix::ObjectId> {
         self.heads
             .last()
-            .map(|head| head.head_oid(repo))
+            .map(|branch| branch.head_oid(repo))
             .ok_or_else(|| anyhow!("Stack is uninitialized"))?
     }
 

crates/gitbutler-stack/src/stack_branch.rs

@@ -143,14 +143,17 @@ impl StackBranch {
         }
     }
 
-    /// This will update the commit that this points to (the virtual reference in virtual_branches.toml) as well as update of create a real git reference.
-    /// If this points to a change id, it's a noop operation. In practice, moving forward, new CommitOrChangeId entries will always be CommitId and ChangeId may only appear in deserialized data.
-    pub fn set_head<T>(&mut self, head: T, repo: &gix::Repository) -> Result<Option<BString>>
+    /// This will update the commit that real git reference points to, so it points to `target`,
+    /// as well as the cached data in this instance.
+    /// Returns the full reference name like `refs/heads/name`.
+    /// If this points to a change id, it's a noop operation. In practice, moving forward, new
+    /// `CommitOrChangeId` entries will always be CommitId and ChangeId may only appear in deserialized data.
+    pub fn set_head<T>(&mut self, target: T, repo: &gix::Repository) -> Result<Option<BString>>
     where
         T: Into<CommitOrChangeId> + Clone,
     {
-        let refname = self.set_real_reference(repo, &head)?;
-        self.head = head.into();
+        let refname = self.set_real_reference(repo, &target)?;
+        self.head = target.into();
         Ok(refname)
     }