absorb: add basic support for file deletion

This works if the file was added and wasn't modified within the destination
range.

Closes #6140
This commit is contained in:
Yuya Nishihara 2025-04-16 18:30:08 +09:00 committed by Martin von Zweigbergk
parent e61971c1f3
commit 39f481f2da
3 changed files with 167 additions and 21 deletions

View File

@ -49,6 +49,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* Added `duplicate_description` template, which allows [customizing the descriptions
of the commits `jj duplicate` creates](docs/config.md#duplicate-commit-description).
* `jj absorb` can now squash a deleted file if it was added by one of the
destination revisions.
### Fixed bugs
* Fixed crash on change-delete conflict resolution.

View File

@ -479,15 +479,161 @@ fn test_absorb_deleted_file() {
work_dir.run_jj(["describe", "-m1"]).success();
work_dir.write_file("file1", "1a\n");
work_dir.write_file("file2", "1a\n");
work_dir.write_file("file3", "");
work_dir.run_jj(["new"]).success();
work_dir.remove_file("file1");
work_dir.write_file("file2", ""); // emptied
work_dir.remove_file("file3"); // no content change
// Since the destinations are chosen based on content diffs, file3 cannot be
// absorbed.
let output = work_dir.run_jj(["absorb"]);
insta::assert_snapshot!(output, @r"
------- stderr -------
Warning: Skipping file1: Deleted file
Nothing changed.
Absorbed changes into 1 revisions:
qpvuntsm f3c5cd48 1
Rebased 1 descendant commits.
Working copy (@) now at: kkmpptxz 691fa544 (no description set)
Parent commit (@-) : qpvuntsm f3c5cd48 1
Remaining changes:
D file3
[EOF]
");
insta::assert_snapshot!(get_diffs(&work_dir, "mutable()"), @r"
@ kkmpptxz 691fa544 (no description set)
diff --git a/file3 b/file3
deleted file mode 100644
index e69de29bb2..0000000000
qpvuntsm f3c5cd48 1
diff --git a/file2 b/file2
~ new file mode 100644
index 0000000000..e69de29bb2
diff --git a/file3 b/file3
new file mode 100644
index 0000000000..e69de29bb2
[EOF]
");
}
#[test]
fn test_absorb_deleted_file_with_multiple_hunks() {
let test_env = TestEnvironment::default();
test_env.run_jj_in(".", ["git", "init", "repo"]).success();
let work_dir = test_env.work_dir("repo");
work_dir.run_jj(["describe", "-m1"]).success();
work_dir.write_file("file1", "1a\n1b\n");
work_dir.write_file("file2", "1a\n");
work_dir.run_jj(["new", "-m2"]).success();
work_dir.write_file("file1", "1a\n");
work_dir.write_file("file2", "1a\n1b\n");
// These changes produce conflicts because
// - for file1, "1a\n" is deleted from the commit 1,
// - for file2, two consecutive hunks are deleted.
//
// Since file2 change is split to two separate hunks, the file deletion
// cannot be propagated. If we implement merging based on interleaved delta,
// the file2 change will apply cleanly. The file1 change might be split into
// "1a\n" deletion at the commit 1 and file deletion at the commit 2, but
// I'm not sure if that's intuitive.
work_dir.run_jj(["new"]).success();
work_dir.remove_file("file1");
work_dir.remove_file("file2");
let output = work_dir.run_jj(["absorb"]);
insta::assert_snapshot!(output, @r"
------- stderr -------
Absorbed changes into 2 revisions:
kkmpptxz ca0a3d3c (conflict) 2
qpvuntsm f2703d39 (conflict) 1
Rebased 1 descendant commits.
Working copy (@) now at: zsuskuln 0156c3af (no description set)
Parent commit (@-) : kkmpptxz ca0a3d3c (conflict) 2
New conflicts appeared in 2 commits:
kkmpptxz ca0a3d3c (conflict) 2
qpvuntsm f2703d39 (conflict) 1
Hint: To resolve the conflicts, start by updating to the first one:
jj new qpvuntsm
Then use `jj resolve`, or edit the conflict markers in the file directly.
Once the conflicts are resolved, you may want to inspect the result with `jj diff`.
Then run `jj squash` to move the resolution into the conflicted commit.
Remaining changes:
D file2
[EOF]
");
insta::assert_snapshot!(get_diffs(&work_dir, "mutable()"), @r"
@ zsuskuln 0156c3af (no description set)
diff --git a/file2 b/file2
deleted file mode 100644
index 0000000000..0000000000
--- a/file2
+++ /dev/null
@@ -1,7 +0,0 @@
-<<<<<<< Conflict 1 of 1
-%%%%%%% Changes from base to side #1
--1a
- 1b
-+++++++ Contents of side #2
-1a
->>>>>>> Conflict 1 of 1 ends
× kkmpptxz ca0a3d3c (conflict) 2
diff --git a/file1 b/file1
deleted file mode 100644
index 0000000000..0000000000
--- a/file1
+++ /dev/null
@@ -1,6 +0,0 @@
-<<<<<<< Conflict 1 of 1
-%%%%%%% Changes from base to side #1
- 1a
-+1b
-+++++++ Contents of side #2
->>>>>>> Conflict 1 of 1 ends
diff --git a/file2 b/file2
--- a/file2
+++ b/file2
@@ -1,7 +1,7 @@
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
- 1a
--1b
+-1a
+ 1b
+++++++ Contents of side #2
-1b
+1a
>>>>>>> Conflict 1 of 1 ends
× qpvuntsm f2703d39 (conflict) 1
diff --git a/file1 b/file1
~ new file mode 100644
index 0000000000..0000000000
--- /dev/null
+++ b/file1
@@ -0,0 +1,6 @@
+<<<<<<< Conflict 1 of 1
+%%%%%%% Changes from base to side #1
+ 1a
++1b
++++++++ Contents of side #2
+>>>>>>> Conflict 1 of 1 ends
diff --git a/file2 b/file2
new file mode 100644
index 0000000000..0000000000
--- /dev/null
+++ b/file2
@@ -0,0 +1,7 @@
+<<<<<<< Conflict 1 of 1
+%%%%%%% Changes from base to side #1
+ 1a
+-1b
++++++++ Contents of side #2
+1b
+>>>>>>> Conflict 1 of 1 ends
[EOF]
");
}

View File

@ -117,17 +117,9 @@ pub async fn split_hunks_to_trees(
continue;
}
};
let right_text = match to_file_value(right_value) {
Ok(Some(mut value)) => value.read_all(right_path)?,
// Deleted file could be absorbed, but that would require special
// handling to propagate deletion of the tree entry
Ok(None) => {
let reason = "Deleted file".to_owned();
selected_trees
.skipped_paths
.push((right_path.to_owned(), reason));
continue;
}
let (right_text, deleted) = match to_file_value(right_value) {
Ok(Some(mut value)) => (value.read_all(right_path)?, false),
Ok(None) => (vec![], true),
Err(reason) => {
selected_trees
.skipped_paths
@ -157,14 +149,19 @@ pub async fn split_hunks_to_trees(
.entry(commit_id.clone())
.or_insert_with(|| MergedTreeBuilder::new(left_tree.id().clone()));
let new_text = combine_texts(&left_text, &right_text, ranges);
let id = repo
.store()
.write_file(left_path, &mut new_text.as_slice())
.await?;
tree_builder.set_or_remove(
left_path.to_owned(),
Merge::normal(TreeValue::File { id, executable }),
);
// Since changes to be absorbed are represented as diffs relative to
// the source parent, we can propagate file deletion only if the
// whole file content is deleted at a single destination commit.
let new_tree_value = if new_text.is_empty() && deleted {
Merge::absent()
} else {
let id = repo
.store()
.write_file(left_path, &mut new_text.as_slice())
.await?;
Merge::normal(TreeValue::File { id, executable })
};
tree_builder.set_or_remove(left_path.to_owned(), new_tree_value);
}
}