Skip to content

Commit 4eabdc1

Browse files
committed
Use target branch tip as base for new independent branches
Previously, new independent branches were based on the workspace's lower bound (the lowest common ancestor of all stacks). When stacks have different merge bases with the target, this placed the new branch on an unnecessarily old commit. Now we prefer the stored target commit (target.sha) via a new `Workspace::target_tip_commit_id()` helper, which checks `target_commit` first, then `target_ref`. Falls back to the workspace lower bound for local-only workflows without a target. Also extracts `target_segment_index()` to deduplicate the target resolution logic shared with `merge_base_with_target_branch()`. Tested with four cases in `target_tip_commit_id.rs`: - Two stacks with different bases — returns the target tip - One stack above the target — still returns the target tip - No target configured — returns None - Only extra_target set — returns None (not sufficient)
1 parent 7f7756d commit 4eabdc1

4 files changed

Lines changed: 197 additions & 14 deletions

File tree

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

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,39 @@ impl Workspace {
151151
}
152152
}
153153

154+
/// Return the segment index of the target, checking [`Self::target_ref`],
155+
/// then [`Self::target_commit`], then [`Self::extra_target`] in that order.
156+
///
157+
/// Returns `None` if no target is configured.
158+
fn target_segment_index(&self) -> Option<SegmentIndex> {
159+
self.target_ref
160+
.as_ref()
161+
.map(|t| t.segment_index)
162+
.or(self.target_commit.as_ref().map(|t| t.segment_index))
163+
.or(self.extra_target)
164+
}
165+
166+
/// Return the resolved target commit ID for use as a base for new branches.
167+
///
168+
/// Prefers the stored [`Self::target_commit`] (the last-synced target SHA),
169+
/// falling back to the tip of [`Self::target_ref`] (the remote tracking branch).
170+
/// Does not consider [`Self::extra_target`].
171+
///
172+
/// Returns `None` if neither `target_commit` nor `target_ref` is configured.
173+
pub fn resolved_target_commit_id(&self) -> Option<gix::ObjectId> {
174+
self.target_commit
175+
.as_ref()
176+
.map(|t| t.commit_id)
177+
.or_else(|| {
178+
self.target_ref
179+
.as_ref()
180+
.and_then(|t| self.graph.tip_skip_empty(t.segment_index).map(|c| c.id))
181+
})
182+
}
183+
154184
/// Return the `(merge-base, target-commit-id)` of the merge-base between the `commit_to_merge`
155-
/// and either the [target-branch](Self::target_ref), the [extra-target](Self::extra_target)
156-
/// or the [target-commit](Self::target_commit), depending on which is set and encountered
185+
/// and either the [target-branch](Self::target_ref), the [target-commit](Self::target_commit),
186+
/// or the [extra-target](Self::extra_target), depending on which is set and encountered
157187
/// in this order.
158188
/// Return `None` when none of these is set, or if there was no merge-base.
159189
///
@@ -171,12 +201,7 @@ impl Workspace {
171201
.then_some(s.id)
172202
})?;
173203

174-
let target_segment_index = self
175-
.target_ref
176-
.as_ref()
177-
.map(|t| t.segment_index)
178-
.or(self.target_commit.as_ref().map(|t| t.segment_index))
179-
.or(self.extra_target)?;
204+
let target_segment_index = self.target_segment_index()?;
180205

181206
let merge_base_segment_index = self
182207
.graph

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@
22
mod legacy;
33
mod merge_base_with_target_branch;
44
mod remote_name;
5+
mod resolved_target_commit_id;
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
use but_graph::Graph;
2+
use but_testsupport::visualize_commit_graph_all;
3+
4+
use crate::init::utils::{
5+
add_workspace, add_workspace_with_target, add_workspace_without_target,
6+
read_only_in_memory_scenario, standard_options, standard_options_with_extra_target,
7+
};
8+
9+
#[test]
10+
fn returns_target_tip_when_stacks_have_different_bases() -> anyhow::Result<()> {
11+
let (repo, mut meta) = read_only_in_memory_scenario("ws/two-branches-one-below-base")?;
12+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
13+
* e82dfab (HEAD -> gitbutler/workspace) GitButler Workspace Commit
14+
|\
15+
| * 6fdab32 (A) A1
16+
* | 78b1b59 (B) B1
17+
| | * 938e6f2 (origin/main, main) M4
18+
| |/
19+
|/|
20+
* | f52fcec M3
21+
|/
22+
* bce0c5e M2
23+
* 3183e43 M1
24+
");
25+
26+
// A branches from M2, B branches from M3.
27+
// resolved_target_commit_id should return M4 (the tip of origin/main).
28+
add_workspace(&mut meta);
29+
30+
let ws = Graph::from_head(&repo, &*meta, standard_options())?
31+
.validated()?
32+
.into_workspace()?;
33+
34+
let tip = ws.resolved_target_commit_id();
35+
let expected_m4 = repo.rev_parse_single(":/M4")?.detach();
36+
assert_eq!(
37+
tip,
38+
Some(expected_m4),
39+
"should return M4, the tip of origin/main"
40+
);
41+
42+
Ok(())
43+
}
44+
45+
#[test]
46+
fn returns_target_tip_when_one_stack_is_above_target() -> anyhow::Result<()> {
47+
let (repo, mut meta) = read_only_in_memory_scenario("ws/two-branches-one-above-base")?;
48+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
49+
* c5587c9 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
50+
|\
51+
| * de6d39c (A) A1
52+
| * a821094 (origin/main, main) M3
53+
* | ce25240 (B) B1
54+
|/
55+
* bce0c5e M2
56+
* 3183e43 M1
57+
");
58+
59+
// A branches from M3 (which is also origin/main), B branches from M2.
60+
// resolved_target_commit_id should return M3 (the tip of origin/main).
61+
add_workspace(&mut meta);
62+
63+
let ws = Graph::from_head(&repo, &*meta, standard_options())?
64+
.validated()?
65+
.into_workspace()?;
66+
67+
let tip = ws.resolved_target_commit_id();
68+
let expected_m3 = repo.rev_parse_single(":/M3")?.detach();
69+
assert_eq!(
70+
tip,
71+
Some(expected_m3),
72+
"should return M3, the tip of origin/main"
73+
);
74+
75+
Ok(())
76+
}
77+
78+
#[test]
79+
fn prefers_target_commit_over_target_ref() -> anyhow::Result<()> {
80+
let (repo, mut meta) = read_only_in_memory_scenario("ws/local-target-and-stack")?;
81+
insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r"
82+
* 59a427f (HEAD -> gitbutler/workspace) GitButler Workspace Commit
83+
|\
84+
| * a62b0de (A) A2
85+
| * 120a217 A1
86+
* | 0a415d8 (main) M3
87+
| | * 1f5c47b (origin/main) RM1
88+
| |/
89+
|/|
90+
* | 73ba99d M2
91+
|/
92+
* fafd9d0 init
93+
");
94+
95+
// Set target_commit (default_target.sha) to M2, while target_ref points to origin/main (RM1).
96+
let m2 = repo.rev_parse_single(":/M2")?.detach();
97+
add_workspace_with_target(&mut meta, m2);
98+
99+
let ws = Graph::from_head(&repo, &*meta, standard_options())?
100+
.validated()?
101+
.into_workspace()?;
102+
103+
assert!(ws.target_ref.is_some(), "target_ref should be set");
104+
assert!(ws.target_commit.is_some(), "target_commit should be set");
105+
106+
let result = ws.resolved_target_commit_id();
107+
assert_eq!(
108+
result,
109+
Some(m2),
110+
"should prefer stored target_commit (M2) over target_ref tip (RM1)"
111+
);
112+
113+
Ok(())
114+
}
115+
116+
#[test]
117+
fn returns_none_when_no_target() -> anyhow::Result<()> {
118+
let (repo, mut meta) = read_only_in_memory_scenario("ws/no-target-without-ws-commit")?;
119+
120+
add_workspace_without_target(&mut meta);
121+
let ws = Graph::from_head(&repo, &*meta, standard_options())?
122+
.validated()?
123+
.into_workspace()?;
124+
125+
assert!(
126+
ws.resolved_target_commit_id().is_none(),
127+
"should return None when no target is set"
128+
);
129+
130+
Ok(())
131+
}
132+
133+
#[test]
134+
fn returns_none_with_only_extra_target() -> anyhow::Result<()> {
135+
let (repo, mut meta) = read_only_in_memory_scenario("ws/two-branches-one-below-base")?;
136+
137+
add_workspace(&mut meta);
138+
meta.data_mut().default_target = None;
139+
140+
let ws = Graph::from_head(
141+
&repo,
142+
&*meta,
143+
standard_options_with_extra_target(&repo, "main"),
144+
)?
145+
.validated()?
146+
.into_workspace()?;
147+
148+
assert!(
149+
ws.resolved_target_commit_id().is_none(),
150+
"extra_target alone is not sufficient — need target_commit or target_ref"
151+
);
152+
153+
Ok(())
154+
}

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,15 @@ pub(super) mod function {
192192
instruction,
193193
)
194194
} else {
195-
let base = ws_base.with_context(|| {
196-
format!(
197-
"workspace at {} is missing a base",
198-
workspace.ref_name_display()
199-
)
200-
})?;
195+
let base = workspace
196+
.resolved_target_commit_id()
197+
.or(ws_base)
198+
.with_context(|| {
199+
format!(
200+
"workspace at {} is missing a base",
201+
workspace.ref_name_display()
202+
)
203+
})?;
201204
(
202205
// do not validate, as the base is expectedly outside of workspace
203206
false,

0 commit comments

Comments
 (0)