SQUASH ME

Author: Byron

crates/but-core/src/ref_metadata.rs

@@ -36,8 +36,27 @@ pub struct Workspace {
     pub push_remote: Option<String>,
 }
 
+impl Workspace {}
+
 /// Mutations
 impl Workspace {
+    /// Remove the named segment `branch`, which removes the whole stack if it's empty after removing a segment
+    /// of that name.
+    /// Returns `true` if it was removed or `false` if it wasn't found.
+    pub fn remove_segment(&mut self, branch: &FullNameRef) -> bool {
+        let Some((stack_idx, segment_idx)) = self.find_owner_indexes_by_name(branch) else {
+            return false;
+        };
+
+        let stack = &mut self.stacks[stack_idx];
+        stack.branches.remove(segment_idx);
+
+        if stack.branches.is_empty() {
+            self.stacks.remove(stack_idx);
+        }
+        true
+    }
+
     /// Insert `branch` as new stack if it's not yet contained in the workspace and if `order` is not `None` or push
     /// it to the end of the stack list.
     /// Note that `order` is only relevant at insertion time.

crates/but-core/tests/core/ref_metadata.rs

@@ -2,7 +2,7 @@ mod workspace {
     use but_core::ref_metadata::Workspace;
 
     #[test]
-    fn add_new_stack_if_not_present() {
+    fn add_new_stack_if_not_present_journey() {
         let mut ws = Workspace::default();
         assert_eq!(ws.stacks.len(), 0);
 
@@ -18,10 +18,26 @@ mod workspace {
         let c_ref = r("refs/heads/C");
         assert!(ws.add_or_insert_new_stack_if_not_present(c_ref, None));
         assert_eq!(ws.stack_names().collect::<Vec<_>>(), [b_ref, a_ref, c_ref]);
+
+        assert!(ws.remove_segment(a_ref));
+        assert!(ws.remove_segment(b_ref));
+        assert!(!ws.remove_segment(b_ref));
+        assert!(ws.remove_segment(c_ref));
+        assert!(!ws.remove_segment(c_ref));
+
+        // Everything should be removed.
+        insta::assert_debug_snapshot!(ws, @r"
+        Workspace {
+            ref_info: RefInfo { created_at: None, updated_at: None },
+            stacks: [],
+            target_ref: None,
+            push_remote: None,
+        }
+        ");
     }
 
     #[test]
-    fn insert_new_segment_above_anchor_if_not_present() {
+    fn insert_new_segment_above_anchor_if_not_present_journey() {
         let mut ws = Workspace::default();
         assert_eq!(ws.stacks.len(), 0);
 
@@ -82,6 +98,20 @@ mod workspace {
             push_remote: None,
         }
         "#);
+
+        assert!(ws.remove_segment(b_ref));
+        assert!(ws.remove_segment(a_ref));
+        assert!(ws.remove_segment(c_ref));
+
+        // Everything should be removed.
+        insta::assert_debug_snapshot!(ws, @r"
+        Workspace {
+            ref_info: RefInfo { created_at: None, updated_at: None },
+            stacks: [],
+            target_ref: None,
+            push_remote: None,
+        }
+        ");
     }
 
     fn r(name: &str) -> &gix::refs::FullNameRef {

crates/but-workspace/src/branch/apply.rs

@@ -263,39 +263,7 @@ pub(crate) mod function {
         {
             let ws_mut: &mut Workspace = &mut ws_md;
             for rn in &branches_to_apply {
-                // Here we have to check if the new ref would be able to become its own stack,
-                // or if it has to be a dependent branch. Stacks only work if the ref rests on a base
-                // outside the workspace, so if we find it in the workspace (in an ambiguous spot) it must be
-                // a dependent branch
-                if let Some(segment_to_insert_above) = workspace
-                    .stacks
-                    .iter()
-                    .flat_map(|stack| stack.segments.iter())
-                    .find_map(|segment| {
-                        segment.commits.iter().flat_map(|c| c.refs.iter()).find_map(
-                            |ambiguous_rn| {
-                                (ambiguous_rn.as_ref() == *rn)
-                                    .then_some(segment.ref_name.as_ref())
-                                    .flatten()
-                            },
-                        )
-                    })
-                {
-                    if ws_mut
-                        .insert_new_segment_above_anchor_if_not_present(
-                            rn,
-                            segment_to_insert_above.as_ref(),
-                        )
-                        .is_none()
-                    {
-                        // For now bail, until we know it's worth fixing this case automatically.
-                        bail!(
-                            "Missing reference {segment_to_insert_above} which should be known to workspace metadata to serve as insertion position for {rn}"
-                        );
-                    }
-                } else {
-                    ws_mut.add_or_insert_new_stack_if_not_present(rn, order);
-                }
+                ws_mut.add_or_insert_new_stack_if_not_present(rn, order);
             }
         }
         let ws_md_override = Some((workspace_ref_name_to_update.clone(), (*ws_md).clone()));
@@ -368,8 +336,9 @@ pub(crate) mod function {
             );
         }
         // At this point, the workspace-metadata already knows the new branch(es), but the workspace itself
-        // doesn't see one or more of to-be-applied branches. These are, however, part of the graph by now,
-        // and we want to try to create a workspace merge.
+        // doesn't see one or more of to-be-applied branches (to become stacks).
+        // These are, however, part of the graph by now, and we want to try to create a workspace
+        // merge.
         let mut in_memory_repo = repo.clone().for_tree_diffing()?.with_object_memory();
         let merge_result = WorkspaceCommit::from_new_merge_with_metadata(
             &ws_md.stacks,
@@ -390,33 +359,104 @@ pub(crate) mod function {
             .entrypoint_commit()
             .context("BUG: how is it possible that there is no head commit?")?
             .id;
-        let new_head_id = merge_result.workspace_commit_id;
-        let graph = graph.redo_traversal_with_overlay(
-            &in_memory_repo,
-            meta,
-            overlay.with_entrypoint(new_head_id, Some(workspace_ref_name_to_update.clone())),
-        )?;
+        let mut new_head_id = merge_result.workspace_commit_id;
+        let overlay =
+            overlay.with_entrypoint(new_head_id, Some(workspace_ref_name_to_update.clone()));
+        let mut graph =
+            graph.redo_traversal_with_overlay(&in_memory_repo, meta, overlay.clone())?;
 
         let workspace = graph.to_workspace()?;
-        let unapplied_branches: Vec<_> = branches_to_apply
-            .iter()
-            .filter_map(|rn| {
-                if workspace.refname_is_segment(rn) {
-                    None
-                } else {
-                    Some(rn)
-                }
-            })
-            .collect();
+        let collect_unapplied_branches = |workspace: &but_graph::projection::Workspace| {
+            branches_to_apply
+                .iter()
+                .filter(|rn| workspace.refname_is_segment(rn))
+                .collect::<Vec<_>>()
+        };
+        let unapplied_branches = collect_unapplied_branches(&workspace);
         if !unapplied_branches.is_empty() {
-            bail!(
-                "Unexpectedly failed to apply {branches} which is/are still not in the workspace",
-                branches = unapplied_branches
+            // Now that the merge is done, try to redo the operation one last time with dependent branches instead.
+            // Only do that for the still unapplied branches, which should always find some sort of anchor.
+            let ws_mut: &mut Workspace = &mut ws_md;
+            for branch_to_remove in &unapplied_branches {
+                ws_mut.remove_segment(branch_to_remove);
+            }
+            for rn in &unapplied_branches {
+                // Here we have to check if the new ref would be able to become its own stack,
+                // or if it has to be a dependent branch. Stacks only work if the ref rests on a base
+                // outside the workspace, so if we find it in the workspace (in an ambiguous spot) it must be
+                // a dependent branch
+                if let Some(segment_to_insert_above) = workspace
+                    .stacks
                     .iter()
-                    .map(|rn| rn.shorten().to_string())
-                    .collect::<Vec<_>>()
-                    .join(", ")
-            )
+                    .flat_map(|stack| stack.segments.iter())
+                    .find_map(|segment| {
+                        segment.commits.iter().flat_map(|c| c.refs.iter()).find_map(
+                            |ambiguous_rn| {
+                                (ambiguous_rn.as_ref() == **rn)
+                                    .then_some(segment.ref_name.as_ref())
+                                    .flatten()
+                            },
+                        )
+                    })
+                {
+                    if ws_mut
+                        .insert_new_segment_above_anchor_if_not_present(
+                            rn,
+                            segment_to_insert_above.as_ref(),
+                        )
+                        .is_none()
+                    {
+                        // For now bail, until we know it's worth fixing this case automatically.
+                        bail!(
+                            "Missing reference {segment_to_insert_above} which should be known to workspace metadata to serve as insertion position for {rn}"
+                        );
+                    }
+                } else {
+                    bail!(
+                        "Unexpectedly failed to find anchor for {rn} to make it a dependent branch"
+                    )
+                }
+            }
+
+            // Redo the merge, with the different stack configuration.
+            // Note that this is the exception, typically using stacks will be fine.
+            let merge_result = WorkspaceCommit::from_new_merge_with_metadata(
+                &ws_md.stacks,
+                workspace.graph,
+                &in_memory_repo,
+                Some(branch),
+            )?;
+
+            if merge_result.has_conflicts() && on_workspace_conflict.should_abort() {
+                return Ok(Outcome {
+                    graph: Cow::Owned(graph),
+                    workspace_ref_created: needs_ws_ref_creation,
+                    workspace_merge: Some(merge_result),
+                });
+            }
+            new_head_id = merge_result.workspace_commit_id;
+            let ws_md_override = Some((workspace_ref_name_to_update.clone(), (*ws_md).clone()));
+
+            graph = graph.redo_traversal_with_overlay(
+                &in_memory_repo,
+                meta,
+                overlay
+                    .with_entrypoint(new_head_id, Some(workspace_ref_name_to_update.clone()))
+                    .with_workspace_metadata_override(ws_md_override),
+            )?;
+            let workspace = graph.to_workspace()?;
+            let unapplied_branches = collect_unapplied_branches(&workspace);
+
+            if !unapplied_branches.is_empty() {
+                bail!(
+                    "Unexpectedly failed to apply {branches} which is/are still not in the workspace",
+                    branches = unapplied_branches
+                        .iter()
+                        .map(|rn| rn.shorten().to_string())
+                        .collect::<Vec<_>>()
+                        .join(", ")
+                )
+            }
         }
 
         // All work is done, persist and exit.

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

@@ -70,6 +70,7 @@ fn go_back_to_integration(ctx: &CommandContext, default_target: &Target) -> Resu
             &gix_repo,
             but_workspace::branch::checkout::Options {
                 uncommitted_changes: UncommitedWorktreeChanges::KeepAndAbortOnConflict,
+                skip_head_update: false,
             },
         )?;
     } else {