Skip to content

Commit 1ca2fb6

Browse files
committed
fix(graph): Gracefully handle squash-merges
Before, when doing a `git-stack --pull`, we'd rebase the merged branch onto master. `git` would the automatically detect the commit is merged and drop it. We'd then associate the branch name with the same commit master points to and have to delete it manually. This tries to detect that case so we can automatically delete the stale branch on behalf of users. This does not help if your branch is behind master because those extra commits will impact how we are detecting this case (tree id). This is a spiritual revert of 80f5158, re-implementing 892117b to handle all of our changes and with a little more experience and context. This is a part of #28.
1 parent 1df3bcc commit 1ca2fb6

File tree

3 files changed

+96
-0
lines changed

3 files changed

+96
-0
lines changed

src/bin/git-stack/stack.rs

+2
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ fn plan_rebase(state: &State, stack: &StackState) -> eyre::Result<git_stack::git
349349
git_stack::graph::protect_branches(&mut root, &state.repo, &state.protected_branches);
350350

351351
git_stack::graph::rebase_branches(&mut root, stack.onto.id);
352+
git_stack::graph::drop_by_tree_id(&mut root);
352353

353354
let script = git_stack::graph::to_script(&root);
354355

@@ -389,6 +390,7 @@ fn show(state: &State, colored_stdout: bool) -> eyre::Result<()> {
389390
if state.dry_run {
390391
// Show as-if we performed all mutations
391392
git_stack::graph::rebase_branches(&mut root, stack.onto.id);
393+
git_stack::graph::drop_by_tree_id(&mut root);
392394
}
393395

394396
eyre::Result::Ok(root)

src/git/repo.rs

+7
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ impl Commit {
7777
.next()
7878
}
7979
}
80+
81+
pub fn revert_summary(&self) -> Option<&bstr::BStr> {
82+
self.summary
83+
.strip_prefix(b"Revert ")
84+
.and_then(|s| s.strip_suffix(b"\""))
85+
.map(ByteSlice::as_bstr)
86+
}
8087
}
8188

8289
pub struct GitRepo {

src/graph/ops.rs

+87
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,93 @@ fn pushable_node(node: &mut Node, mut cause: Option<&str>) {
127127
}
128128
}
129129

130+
/// Quick pass for what is droppable
131+
///
132+
/// We get into this state when a branch is squashed. The id would be different due to metadata
133+
/// but the tree_id, associated with the repo, is the same if your branch is up-to-date.
134+
///
135+
/// The big risk is if a commit was reverted. To protect against this, we only look at the final
136+
/// state of the branch and then check if it looks like a revert.
137+
///
138+
/// To avoid walking too much of the tree, we are going to assume only the first branch in a stack
139+
/// could have been squash-merged.
140+
///
141+
/// This assumes that the Node was rebased onto all of the new potentially squash-merged Nodes and
142+
/// we extract the potential tree_id's from those protected commits.
143+
pub fn drop_by_tree_id(node: &mut Node) {
144+
if node.action.is_protected() {
145+
track_protected_tree_id(node, std::collections::HashSet::new());
146+
}
147+
}
148+
149+
fn track_protected_tree_id(
150+
node: &mut Node,
151+
mut protected_tree_ids: std::collections::HashSet<git2::Oid>,
152+
) {
153+
assert!(node.action.is_protected());
154+
protected_tree_ids.insert(node.local_commit.tree_id);
155+
156+
match node.children.len() {
157+
0 => (),
158+
1 => {
159+
let child = node.children.values_mut().next().unwrap();
160+
if child.action.is_protected() {
161+
track_protected_tree_id(child, protected_tree_ids);
162+
} else {
163+
drop_first_branch_by_tree_id(child, protected_tree_ids);
164+
}
165+
}
166+
_ => {
167+
for child in node.children.values_mut() {
168+
if child.action.is_protected() {
169+
track_protected_tree_id(child, protected_tree_ids.clone());
170+
} else {
171+
drop_first_branch_by_tree_id(child, protected_tree_ids.clone());
172+
}
173+
}
174+
}
175+
}
176+
}
177+
178+
fn drop_first_branch_by_tree_id(
179+
node: &mut Node,
180+
protected_tree_ids: std::collections::HashSet<git2::Oid>,
181+
) -> bool {
182+
assert!(!node.action.is_protected());
183+
if node.branches.is_empty() {
184+
match node.children.len() {
185+
0 => false,
186+
1 => {
187+
let child = node.children.values_mut().next().unwrap();
188+
let all_dropped = drop_first_branch_by_tree_id(child, protected_tree_ids);
189+
if all_dropped {
190+
node.action = crate::graph::Action::Delete;
191+
}
192+
all_dropped
193+
}
194+
_ => {
195+
let mut all_dropped = true;
196+
for child in node.children.values_mut() {
197+
all_dropped &= drop_first_branch_by_tree_id(child, protected_tree_ids.clone());
198+
}
199+
if all_dropped {
200+
node.action = crate::graph::Action::Delete;
201+
}
202+
all_dropped
203+
}
204+
}
205+
} else if !protected_tree_ids.contains(&node.local_commit.tree_id) {
206+
false
207+
} else if node.local_commit.revert_summary().is_some() {
208+
// Might not *actually* be a revert or something more complicated might be going on. Let's
209+
// just be cautious.
210+
false
211+
} else {
212+
node.action = crate::graph::Action::Delete;
213+
true
214+
}
215+
}
216+
130217
pub fn to_script(node: &Node) -> crate::git::Script {
131218
let mut script = crate::git::Script::new();
132219

0 commit comments

Comments
 (0)