Skip to content

Commit 9c994a6

Browse files
feat(split): support extracting renamed files
This is a 1st pass; still TODO: - confirm rename w/ changed contents can split - confirm rename w/ other files can split
1 parent 3c2e421 commit 9c994a6

3 files changed

Lines changed: 164 additions & 14 deletions

File tree

git-branchless-lib/src/git/diff.rs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ pub struct Diff<'repo> {
1616
}
1717

1818
impl Diff<'_> {
19+
/// Enable rename and copy detection for this diff.
20+
///
21+
/// Must be called before iterating deltas if you want renames to be
22+
/// reported as a single `Renamed` delta rather than separate add/delete
23+
/// deltas.
24+
pub fn find_similar(&mut self) -> eyre::Result<()> {
25+
self.inner.find_similar(None).map_err(|e| eyre::eyre!(e))
26+
}
27+
1928
/// Summarize this diff into a single line "short" format.
2029
pub fn short_stats(&self) -> eyre::Result<String> {
2130
let stats = self.inner.stats()?;
@@ -48,11 +57,15 @@ pub fn summarize_diff_for_temporary_commit(diff: &Diff) -> eyre::Result<String>
4857
// returns an Err, so catch and ignore it
4958
let _ = diff.inner.foreach(
5059
&mut |delta: git2::DiffDelta, _| {
51-
let relevant_path = delta
52-
.old_file()
53-
.path()
54-
.or(delta.new_file().path())
55-
.unwrap_or_else(|| unreachable!("diff should have contained at least 1 file"));
60+
// For deletions show the old (removed) path; for everything
61+
// else (including renames) prefer the new path so that e.g.
62+
// a rename "foo.txt → bar.txt" is labelled as "bar.txt".
63+
let relevant_path = if delta.status() == git2::Delta::Deleted {
64+
delta.old_file().path().or_else(|| delta.new_file().path())
65+
} else {
66+
delta.new_file().path().or_else(|| delta.old_file().path())
67+
}
68+
.unwrap_or_else(|| unreachable!("diff should have contained at least 1 file"));
5669
filename = Some(format!("{}", relevant_path.display()));
5770
false
5871
},
@@ -67,7 +80,9 @@ pub fn summarize_diff_for_temporary_commit(diff: &Diff) -> eyre::Result<String>
6780
};
6881

6982
let ins_del = match (stats.insertions(), stats.deletions()) {
70-
(0, 0) => unreachable!("empty diff"),
83+
// A diff with no insertions or deletions still has a file change,
84+
// e.g. a pure rename or a file-mode change.
85+
(0, 0) => "0".to_string(),
7186
(i, 0) => format!("+{i}"),
7287
(0, d) => format!("-{d}"),
7388
(i, d) => format!("+{i}/-{d}"),

git-branchless/src/commands/split.rs

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ use lib::{
2828
},
2929
git::{
3030
CherryPickFastOptions, FileMode, GitRunInfo, MaybeZeroOid, NonZeroOid, Repo,
31-
ResolvedReferenceInfo, hydrate_tree, make_empty_tree, process_diff_for_record,
32-
summarize_diff_for_temporary_commit,
31+
ResolvedReferenceInfo, get_changed_paths_between_trees, hydrate_tree, make_empty_tree,
32+
process_diff_for_record, summarize_diff_for_temporary_commit,
3333
},
3434
try_exit_code,
3535
util::{ExitCode, EyreExitOr},
@@ -298,6 +298,41 @@ pub fn split(
298298
}
299299
};
300300

301+
// Pre-compute a map of exact renames in the target commit: new_path -> old_path.
302+
// A rename is detected when a path is absent from parent_tree but present in
303+
// target_tree, and another path with the SAME blob OID is absent from target_tree
304+
// but present in parent_tree (same content = exact rename, no content change).
305+
//
306+
// This map is used when extracting a renamed file to correctly restore the
307+
// original path in the remainder tree, preventing cherry-pick conflicts.
308+
let rename_map: HashMap<PathBuf, PathBuf> = {
309+
let changed_paths =
310+
get_changed_paths_between_trees(&repo, Some(&parent_tree), Some(&target_tree))?;
311+
let mut deleted_by_oid: HashMap<NonZeroOid, PathBuf> = HashMap::new();
312+
let mut added_entries: Vec<(PathBuf, NonZeroOid)> = Vec::new();
313+
for path in changed_paths {
314+
let parent_oid = parent_tree.get_oid_for_path(&path)?;
315+
let target_oid = target_tree.get_oid_for_path(&path)?;
316+
match (parent_oid, target_oid) {
317+
(Some(MaybeZeroOid::NonZero(p_oid)), None | Some(MaybeZeroOid::Zero)) => {
318+
deleted_by_oid.insert(p_oid, path);
319+
}
320+
(None | Some(MaybeZeroOid::Zero), Some(MaybeZeroOid::NonZero(t_oid))) => {
321+
added_entries.push((path, t_oid));
322+
}
323+
_ => {}
324+
}
325+
}
326+
added_entries
327+
.into_iter()
328+
.filter_map(|(new_path, blob_oid)| {
329+
deleted_by_oid
330+
.get(&blob_oid)
331+
.map(|old_path| (new_path, old_path.clone()))
332+
})
333+
.collect()
334+
};
335+
301336
for (spec, path) in resolved_specs.iter() {
302337
let path = path.as_path();
303338

@@ -326,21 +361,50 @@ pub fn split(
326361

327362
let target_entry = target_tree.get_path(path)?;
328363
let temp_tree_oid = match (parent_entry, target_entry, &split_mode) {
329-
// added or modified & InsertBefore => add to extracted commit
330-
(None, Some(commit_entry), SplitMode::InsertBefore)
331-
| (Some(_), Some(commit_entry), SplitMode::InsertBefore) => {
364+
// modified & InsertBefore => add to extracted commit (no rename handling needed)
365+
(Some(_), Some(commit_entry), SplitMode::InsertBefore) => {
332366
remainder_tree.add_or_replace(&repo, path, &commit_entry)?
333367
}
334368

369+
// added & InsertBefore => add to extracted commit; for renames, also remove
370+
// the original path (which is present in the parent-based remainder tree)
371+
(None, Some(commit_entry), SplitMode::InsertBefore) => {
372+
let oid = remainder_tree.add_or_replace(&repo, path, &commit_entry)?;
373+
if let Some(old_path) = rename_map.get(path) {
374+
let interim = repo
375+
.find_tree(oid)?
376+
.expect("hydrated tree should always be findable");
377+
interim.remove(&repo, old_path)?
378+
} else {
379+
oid
380+
}
381+
}
382+
335383
// removed & InsertBefore => remove from remainder commit
336384
(Some(_), None, SplitMode::InsertBefore) => {
337385
remainder_tree.remove(&repo, path)?
338386
}
339387

340-
// added => remove from remainder commit
388+
// added => remove from remainder commit; for renames, also restore the
389+
// original path from the parent tree so the remainder accurately reflects
390+
// only the non-extracted changes
341391
(None, Some(_), SplitMode::InsertAfter)
342392
| (None, Some(_), SplitMode::DetachAfter)
343-
| (None, Some(_), SplitMode::Discard) => remainder_tree.remove(&repo, path)?,
393+
| (None, Some(_), SplitMode::Discard) => {
394+
let oid = remainder_tree.remove(&repo, path)?;
395+
if let Some(old_path) = rename_map.get(path) {
396+
if let Some(old_entry) = parent_tree.get_path(old_path)? {
397+
let interim = repo
398+
.find_tree(oid)?
399+
.expect("hydrated tree should always be findable");
400+
interim.add_or_replace(&repo, old_path, &old_entry)?
401+
} else {
402+
oid
403+
}
404+
} else {
405+
oid
406+
}
407+
}
344408

345409
// deleted or modified => replace w/ parent content in split commit
346410
(Some(parent_entry), _, _) => {
@@ -489,12 +553,16 @@ pub fn split(
489553
} else {
490554
(&remainder_tree, &target_tree)
491555
};
492-
let diff = repo.get_diff_between_trees(
556+
let mut diff = repo.get_diff_between_trees(
493557
effects,
494558
Some(old_tree),
495559
new_tree,
496560
0, // we don't care about the context here
497561
)?;
562+
// Enable rename detection so that a pure rename shows as one file
563+
// changed with 0 insertions/deletions rather than two separate
564+
// add/delete entries.
565+
diff.find_similar()?;
498566

499567
summarize_diff_for_temporary_commit(&diff)?
500568
};

git-branchless/tests/test_split.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,73 @@ fn test_split_deleted_file() -> eyre::Result<()> {
238238
Ok(())
239239
}
240240

241+
#[test]
242+
fn test_split_renamed_file() -> eyre::Result<()> {
243+
let git = make_git()?;
244+
git.init_repo()?;
245+
git.detach_head()?;
246+
247+
git.commit_file("test1", 1)?;
248+
249+
git.run(&["mv", "test1.txt", "test1-renamed.txt"])?;
250+
git.write_file_txt("test2", "new contents")?;
251+
git.run(&["add", "."])?;
252+
git.run(&["commit", "-m", "first commit"])?;
253+
254+
{
255+
let (stdout, _stderr) = git.branchless("smartlog", &[])?;
256+
insta::assert_snapshot!(stdout, @r###"
257+
O f777ecc (master) create initial.txt
258+
|
259+
o 62fc20d create test1.txt
260+
|
261+
@ d5713a5 first commit
262+
"###);
263+
}
264+
265+
{
266+
let (stdout, _stderr) = git.run(&["show", "--pretty=format:", "--stat", "HEAD"])?;
267+
insta::assert_snapshot!(&stdout, @"
268+
test1.txt => test1-renamed.txt | 0
269+
test2.txt | 1 +
270+
2 files changed, 1 insertion(+)
271+
");
272+
}
273+
274+
{
275+
let (stdout, _stderr) = git.branchless("split", &["HEAD", "test1-renamed.txt"])?;
276+
insta::assert_snapshot!(&stdout, @r###"
277+
branchless: running command: <git-executable> checkout 495b4c09b4cc1755847ba0fd42c903f9c7eecc00 --
278+
Nothing to restack.
279+
O f777ecc (master) create initial.txt
280+
|
281+
o 62fc20d create test1.txt
282+
|
283+
@ 495b4c0 first commit
284+
|
285+
o ca4a140 temp(split): test1-renamed.txt (0)
286+
"###);
287+
}
288+
289+
{
290+
git.branchless("next", &[])?;
291+
292+
let (stdout, _stderr) = git.run(&["show", "--pretty=format:", "--stat", "HEAD~"])?;
293+
insta::assert_snapshot!(&stdout, @"
294+
test2.txt | 1 +
295+
1 file changed, 1 insertion(+)
296+
");
297+
298+
let (stdout, _stderr) = git.run(&["show", "--pretty=format:", "--stat", "HEAD"])?;
299+
insta::assert_snapshot!(&stdout, @"
300+
test1.txt => test1-renamed.txt | 0
301+
1 file changed, 0 insertions(+), 0 deletions(-)
302+
");
303+
}
304+
305+
Ok(())
306+
}
307+
241308
#[test]
242309
fn test_split_multiple_files() -> eyre::Result<()> {
243310
let git = make_git()?;

0 commit comments

Comments
 (0)