Merge pull request #5340 from gitbutlerapp/kv-branch-1

Prune integrated heads upon inegrating upstream

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

@@ -322,6 +322,7 @@ pub(crate) fn integrate_upstream(
             };
 
             branch.set_stack_head(command_context, *head, Some(*tree))?;
+            branch.prune_integrated_heads(command_context)?;
         }
 
         // checkout_branch_trees won't checkout anything if there are no

crates/gitbutler-stack/src/stack.rs

@@ -437,6 +437,24 @@ impl Stack {
         state.set_branch(self.clone())
     }
 
+    /// Removes any heads that are refering to commits that are no longer between the stack head and the merge base
+    pub fn prune_integrated_heads(&mut self, ctx: &CommandContext) -> Result<()> {
+        if !self.initialized() {
+            return Err(anyhow!("Stack has not been initialized"));
+        }
+        self.updated_timestamp_ms = gitbutler_time::time::now_ms();
+        let state = branch_state(ctx);
+        let commit_ids = stack_patches(ctx, &state, self.head(), true)?;
+        let new_heads = self
+            .heads
+            .iter()
+            .filter(|h| commit_ids.contains(&h.target))
+            .cloned()
+            .collect_vec();
+        self.heads = new_heads;
+        state.set_branch(self.clone())
+    }
+
     /// Prepares push details according to the series to be pushed (picking out the correct sha and remote refname)
     /// This operation will error out if the target has no push remote configured.
     pub fn push_details(&self, ctx: &CommandContext, branch_name: String) -> Result<PushDetails> {
@@ -480,9 +498,19 @@ impl Stack {
         let mut previous_head = repo.merge_base(self.head(), default_target.sha)?;
         for head in self.heads.clone() {
             let head_commit =
-                commit_by_oid_or_change_id(&head.target, repo, self.head(), merge_base)?
-                    .head
-                    .id();
+                match commit_by_oid_or_change_id(&head.target, repo, self.head(), merge_base) {
+                    Ok(commits_for_id) => commits_for_id.head.id(),
+                    Err(e) => {
+                        // The series may have been integrated
+                        tracing::warn!(
+                            "Failed to find commit with commit_or_change_id: {} for head: {}, {}",
+                            head.target,
+                            head.name,
+                            e
+                        );
+                        continue;
+                    }
+                };
 
             let mut local_patches = vec![];
             for commit in repo

crates/gitbutler-stack/tests/mod.rs

@@ -168,6 +168,11 @@ fn add_multiple_series() -> Result<()> {
         head_names(&test_ctx),
         vec!["head_1", "head_2", "a-branch-2", "head_4"]
     );
+
+    // prune is noop
+    let before_prune = test_ctx.branch.heads.clone();
+    test_ctx.branch.prune_integrated_heads(&ctx)?;
+    assert_eq!(before_prune, test_ctx.branch.heads);
     Ok(())
 }
 
@@ -1086,6 +1091,78 @@ fn set_legacy_refname_pushed() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn prune_heads_noop() -> Result<()> {
+    let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
+    let mut test_ctx = test_ctx(&ctx)?;
+    test_ctx.branch.initialize(&ctx)?;
+    let initial_state = test_ctx.branch.heads.clone();
+    test_ctx.branch.prune_integrated_heads(&ctx)?;
+    assert_eq!(initial_state, test_ctx.branch.heads);
+    // Assert persisted
+    assert_eq!(
+        test_ctx.branch,
+        test_ctx.handle.get_branch(test_ctx.branch.id)?
+    );
+    Ok(())
+}
+
+#[test]
+fn prune_heads_success() -> Result<()> {
+    let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
+    let mut test_ctx = test_ctx(&ctx)?;
+    test_ctx.branch.initialize(&ctx)?;
+    let initial_state = test_ctx.branch.heads.clone();
+    // adding a commit that is not in the stack
+    test_ctx.branch.heads.insert(
+        0,
+        PatchReference {
+            target: test_ctx.other_commits.first().cloned().unwrap().into(),
+            name: "foo".to_string(),
+            description: None,
+        },
+    );
+    assert_eq!(test_ctx.branch.heads.len(), 2);
+    test_ctx.branch.prune_integrated_heads(&ctx)?;
+    assert_eq!(test_ctx.branch.heads.len(), 1);
+    assert_eq!(initial_state, test_ctx.branch.heads);
+    // Assert persisted
+    assert_eq!(
+        test_ctx.branch,
+        test_ctx.handle.get_branch(test_ctx.branch.id)?
+    );
+    Ok(())
+}
+
+#[test]
+fn does_not_prune_head_on_merge_base() -> Result<()> {
+    let (ctx, _temp_dir) = command_ctx("multiple-commits")?;
+    let mut test_ctx = test_ctx(&ctx)?;
+    test_ctx.branch.initialize(&ctx)?;
+    let merge_base = ctx.repository().find_commit(
+        ctx.repository()
+            .merge_base(test_ctx.branch.head(), test_ctx.default_target.sha)?,
+    )?;
+    test_ctx.branch.add_series(
+        &ctx,
+        PatchReference {
+            target: merge_base.into(),
+            name: "bottom".to_string(),
+            description: None,
+        },
+        None,
+    )?;
+    let initial_state = test_ctx.branch.heads.clone();
+    test_ctx.branch.prune_integrated_heads(&ctx)?;
+    assert_eq!(initial_state, test_ctx.branch.heads);
+    // Assert persisted
+    assert_eq!(
+        test_ctx.branch,
+        test_ctx.handle.get_branch(test_ctx.branch.id)?
+    );
+    Ok(())
+}
+
 fn command_ctx(name: &str) -> Result<(CommandContext, TempDir)> {
     gitbutler_testsupport::writable::fixture("stacking.sh", name)
 }