Merge pull request #13626 from gitbutlerapp/newest-base-for-new-branches

Use target branch tip as base for new independent branches

Author: mtsgrd

crates/but-graph/src/projection/workspace/api.rs

@@ -151,9 +151,39 @@ impl Workspace {
         }
     }
 
+    /// Return the segment index of the target, checking [`Self::target_ref`],
+    /// then [`Self::target_commit`], then [`Self::extra_target`] in that order.
+    ///
+    /// Returns `None` if no target is configured.
+    fn target_segment_index(&self) -> Option<SegmentIndex> {
+        self.target_ref
+            .as_ref()
+            .map(|t| t.segment_index)
+            .or(self.target_commit.as_ref().map(|t| t.segment_index))
+            .or(self.extra_target)
+    }
+
+    /// Return the resolved target commit ID for use as a base for new branches.
+    ///
+    /// Prefers the stored [`Self::target_commit`] (the last-synced target SHA),
+    /// falling back to the tip of [`Self::target_ref`] (the remote tracking branch).
+    /// Does not consider [`Self::extra_target`].
+    ///
+    /// Returns `None` if neither `target_commit` nor `target_ref` is configured.
+    pub fn resolved_target_commit_id(&self) -> Option<gix::ObjectId> {
+        self.target_commit
+            .as_ref()
+            .map(|t| t.commit_id)
+            .or_else(|| {
+                self.target_ref
+                    .as_ref()
+                    .and_then(|t| self.graph.tip_skip_empty(t.segment_index).map(|c| c.id))
+            })
+    }
+
     /// Return the `(merge-base, target-commit-id)` of the merge-base between the `commit_to_merge`
-    /// and either the [target-branch](Self::target_ref), the [extra-target](Self::extra_target)
-    /// or the [target-commit](Self::target_commit), depending on which is set and encountered
+    /// and either the [target-branch](Self::target_ref), the [target-commit](Self::target_commit),
+    /// or the [extra-target](Self::extra_target), depending on which is set and encountered
     /// in this order.
     /// Return `None` when none of these is set, or if there was no merge-base.
     ///
@@ -171,12 +201,7 @@ impl Workspace {
                 .then_some(s.id)
         })?;
 
-        let target_segment_index = self
-            .target_ref
-            .as_ref()
-            .map(|t| t.segment_index)
-            .or(self.target_commit.as_ref().map(|t| t.segment_index))
-            .or(self.extra_target)?;
+        let target_segment_index = self.target_segment_index()?;
 
         let merge_base_segment_index = self
             .graph

crates/but-graph/tests/graph/workspace/mod.rs

@@ -2,3 +2,4 @@
 mod legacy;
 mod merge_base_with_target_branch;
 mod remote_name;
+mod resolved_target_commit_id;

crates/but-graph/tests/graph/workspace/resolved_target_commit_id.rs

@@ -0,0 +1,154 @@
+use but_graph::Graph;
+use but_testsupport::visualize_commit_graph_all;
+
+use crate::init::utils::{
+    add_workspace, add_workspace_with_target, add_workspace_without_target,
+    read_only_in_memory_scenario, standard_options, standard_options_with_extra_target,
+};
+
+#[test]
+fn returns_target_tip_when_stacks_have_different_bases() -> anyhow::Result<()> {
+    let (repo, mut meta) = read_only_in_memory_scenario("ws/two-branches-one-below-base")?;
+    insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
+    *   e82dfab (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    |\  
+    | * 6fdab32 (A) A1
+    * | 78b1b59 (B) B1
+    | | * 938e6f2 (origin/main, main) M4
+    | |/  
+    |/|   
+    * | f52fcec M3
+    |/  
+    * bce0c5e M2
+    * 3183e43 M1
+    ");
+
+    // A branches from M2, B branches from M3.
+    // resolved_target_commit_id should return M4 (the tip of origin/main).
+    add_workspace(&mut meta);
+
+    let ws = Graph::from_head(&repo, &*meta, standard_options())?
+        .validated()?
+        .into_workspace()?;
+
+    let tip = ws.resolved_target_commit_id();
+    let expected_m4 = repo.rev_parse_single(":/M4")?.detach();
+    assert_eq!(
+        tip,
+        Some(expected_m4),
+        "should return M4, the tip of origin/main"
+    );
+
+    Ok(())
+}
+
+#[test]
+fn returns_target_tip_when_one_stack_is_above_target() -> anyhow::Result<()> {
+    let (repo, mut meta) = read_only_in_memory_scenario("ws/two-branches-one-above-base")?;
+    insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
+    *   c5587c9 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    |\  
+    | * de6d39c (A) A1
+    | * a821094 (origin/main, main) M3
+    * | ce25240 (B) B1
+    |/  
+    * bce0c5e M2
+    * 3183e43 M1
+    ");
+
+    // A branches from M3 (which is also origin/main), B branches from M2.
+    // resolved_target_commit_id should return M3 (the tip of origin/main).
+    add_workspace(&mut meta);
+
+    let ws = Graph::from_head(&repo, &*meta, standard_options())?
+        .validated()?
+        .into_workspace()?;
+
+    let tip = ws.resolved_target_commit_id();
+    let expected_m3 = repo.rev_parse_single(":/M3")?.detach();
+    assert_eq!(
+        tip,
+        Some(expected_m3),
+        "should return M3, the tip of origin/main"
+    );
+
+    Ok(())
+}
+
+#[test]
+fn prefers_target_commit_over_target_ref() -> anyhow::Result<()> {
+    let (repo, mut meta) = read_only_in_memory_scenario("ws/local-target-and-stack")?;
+    insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
+    *   59a427f (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    |\  
+    | * a62b0de (A) A2
+    | * 120a217 A1
+    * | 0a415d8 (main) M3
+    | | * 1f5c47b (origin/main) RM1
+    | |/  
+    |/|   
+    * | 73ba99d M2
+    |/  
+    * fafd9d0 init
+    ");
+
+    // Set target_commit (default_target.sha) to M2, while target_ref points to origin/main (RM1).
+    let m2 = repo.rev_parse_single(":/M2")?.detach();
+    add_workspace_with_target(&mut meta, m2);
+
+    let ws = Graph::from_head(&repo, &*meta, standard_options())?
+        .validated()?
+        .into_workspace()?;
+
+    assert!(ws.target_ref.is_some(), "target_ref should be set");
+    assert!(ws.target_commit.is_some(), "target_commit should be set");
+
+    let result = ws.resolved_target_commit_id();
+    assert_eq!(
+        result,
+        Some(m2),
+        "should prefer stored target_commit (M2) over target_ref tip (RM1)"
+    );
+
+    Ok(())
+}
+
+#[test]
+fn returns_none_when_no_target() -> anyhow::Result<()> {
+    let (repo, mut meta) = read_only_in_memory_scenario("ws/no-target-without-ws-commit")?;
+
+    add_workspace_without_target(&mut meta);
+    let ws = Graph::from_head(&repo, &*meta, standard_options())?
+        .validated()?
+        .into_workspace()?;
+
+    assert!(
+        ws.resolved_target_commit_id().is_none(),
+        "should return None when no target is set"
+    );
+
+    Ok(())
+}
+
+#[test]
+fn returns_none_with_only_extra_target() -> anyhow::Result<()> {
+    let (repo, mut meta) = read_only_in_memory_scenario("ws/two-branches-one-below-base")?;
+
+    add_workspace(&mut meta);
+    meta.data_mut().default_target = None;
+
+    let ws = Graph::from_head(
+        &repo,
+        &*meta,
+        standard_options_with_extra_target(&repo, "main"),
+    )?
+    .validated()?
+    .into_workspace()?;
+
+    assert!(
+        ws.resolved_target_commit_id().is_none(),
+        "extra_target alone is not sufficient — need target_commit or target_ref"
+    );
+
+    Ok(())
+}

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

@@ -192,12 +192,15 @@ pub(super) mod function {
                             instruction,
                         )
                     } else {
-                        let base = ws_base.with_context(|| {
-                            format!(
-                                "workspace at {} is missing a base",
-                                workspace.ref_name_display()
-                            )
-                        })?;
+                        let base = workspace
+                            .resolved_target_commit_id()
+                            .or(ws_base)
+                            .with_context(|| {
+                                format!(
+                                    "workspace at {} is missing a base",
+                                    workspace.ref_name_display()
+                                )
+                            })?;
                         (
                             // do not validate, as the base is expectedly outside of workspace
                             false,