interdiff: allow gaps in multi-revision revsets

This commit is contained in:
Gaëtan Lehmann
2026-06-08 14:44:17 +02:00
parent 1d02a288ad
commit 84c28a23f1
3 changed files with 249 additions and 121 deletions

View File

@@ -21,7 +21,6 @@ use tracing::instrument;
use crate::cli_util::CommandHelper;
use crate::cli_util::RevisionArg;
use crate::cli_util::print_unmatched_explicit_paths;
use crate::cli_util::short_commit_hash;
use crate::command_error::CommandError;
use crate::command_error::user_error;
use crate::complete;
@@ -93,24 +92,8 @@ pub(crate) async fn cmd_interdiff(
let from_evaluator = workspace_command
.parse_union_revsets(ui, &[args.from.clone().unwrap_or(RevisionArg::AT)])?;
let from_expression = from_evaluator.expression();
let mut from_gaps = workspace_command
.attach_revset_evaluator(
from_expression
.roots()
.range(&from_expression.heads())
.minus(from_expression),
)
.evaluate_to_commit_ids()?;
if let Some(commit_id) = from_gaps.try_next().await? {
return Err(
user_error("Cannot diff revsets with gaps in --from.").hinted(format!(
"Revision {} would need to be in the set.",
short_commit_hash(&commit_id)
)),
);
}
let from_commits: Vec<_> = workspace_command
.attach_revset_evaluator(from_expression.heads())
.attach_revset_evaluator(from_expression.clone())
.evaluate_to_commits()?
.try_collect()
.await?;
@@ -118,24 +101,8 @@ pub(crate) async fn cmd_interdiff(
let to_evaluator =
workspace_command.parse_union_revsets(ui, &[args.to.clone().unwrap_or(RevisionArg::AT)])?;
let to_expression = to_evaluator.expression();
let mut to_gaps = workspace_command
.attach_revset_evaluator(
to_expression
.roots()
.range(&to_expression.heads())
.minus(to_expression),
)
.evaluate_to_commit_ids()?;
if let Some(commit_id) = to_gaps.try_next().await? {
return Err(
user_error("Cannot diff revsets with gaps in --to.").hinted(format!(
"Revision {} would need to be in the set.",
short_commit_hash(&commit_id)
)),
);
}
let to_commits: Vec<_> = workspace_command
.attach_revset_evaluator(to_expression.heads())
.attach_revset_evaluator(to_expression.clone())
.evaluate_to_commits()?
.try_collect()
.await?;

View File

@@ -75,7 +75,6 @@ use jj_lib::repo_path::InvalidRepoPathError;
use jj_lib::repo_path::RepoPath;
use jj_lib::repo_path::RepoPathUiConverter;
use jj_lib::rewrite::merge_commit_trees;
use jj_lib::rewrite::rebase_to_dest_parent;
use jj_lib::settings::UserSettings;
use jj_lib::store::Store;
use thiserror::Error;
@@ -613,6 +612,64 @@ impl<'a> DiffRenderer<'a> {
Ok(())
}
/// Compute the merged tree of the parents of the roots of
/// `commits`. Roots are commits with no ancestor also in the set,
/// so their parents are guaranteed external.
async fn roots_parent_base_tree(&self, commits: &[Commit]) -> BackendResult<MergedTree> {
let roots: Vec<&Commit> = commits
.iter()
.filter(|c| {
!commits.iter().any(|other| {
other.id() != c.id()
&& self
.repo
.index()
.is_ancestor(other.id(), c.id())
.unwrap_or(false)
})
})
.collect();
let mut parents: Vec<Commit> = Vec::new();
let mut seen = std::collections::HashSet::new();
for root in &roots {
for parent in root.parents().await? {
if seen.insert(parent.id().clone()) {
parents.push(parent);
}
}
}
merge_commit_trees(self.repo, &parents).await
}
/// Replay each commit in `commits` onto `base_tree`, labeling conflict
/// markers with `side` (e.g. "from" / "to") to distinguish which side
/// of the interdiff produced them.
async fn replay_changes_onto_base_tree(
&self,
commits: &[Commit],
base_tree: MergedTree,
side: &str,
) -> BackendResult<MergedTree> {
let mut tree = base_tree;
for source in commits.iter().rev() {
let old_parents = source.parents().await?;
let old_base = merge_commit_trees(self.repo, &old_parents).await?;
tree = MergedTree::merge(Merge::from_vec(vec![
(tree, format!(" ({side} context)")),
(
old_base,
format!("{} ({side} parent)", source.parents_conflict_label().await?),
),
(
source.tree(),
format!("{} ({side} revision)", source.conflict_label()),
),
]))
.await?;
}
Ok(tree)
}
/// Generates diff between `from_commits` and `to_commits` based off their
/// parents. The `from_commits` will temporarily be rebased onto the
/// `to_commits` parents to exclude unrelated changes.
@@ -647,48 +704,16 @@ impl<'a> DiffRenderer<'a> {
.build()
.simplify()
};
let from_tree = if let [to_commit] = to_commits {
rebase_to_dest_parent(self.repo, from_commits, to_commit).await?
} else {
let mut all_parents: Vec<Commit> = Vec::new();
let mut seen_ids = std::collections::HashSet::new();
for to_commit in to_commits {
for parent in to_commit.parents().await? {
if seen_ids.insert(parent.id().clone()) {
all_parents.push(parent);
}
}
}
let merged_parent_tree = merge_commit_trees(self.repo, &all_parents).await?;
let diffs: Vec<_> = futures::future::try_join_all(from_commits.iter().map(
async |source| -> BackendResult<_> {
Ok(Diff::new(
(
source.parent_tree(self.repo).await?,
format!(
"{} (original parents)",
source.parents_conflict_label().await?
),
),
(
source.tree(),
format!("{} (original revision)", source.conflict_label()),
),
))
},
))
// Compute base tree from to_commits' roots' external parents.
// This is the context onto which from_commits are rebased.
let base = self.roots_parent_base_tree(to_commits).await?;
let from_tree = self
.replay_changes_onto_base_tree(from_commits, base.clone(), "from")
.await?;
let to_tree = self
.replay_changes_onto_base_tree(to_commits, base, "to")
.await?;
MergedTree::merge(Merge::from_diffs(
(merged_parent_tree, " (rebased parents)".to_string()),
diffs,
))
.await?
};
let to_tree = if let [to_commit] = to_commits {
to_commit.tree()
} else {
merge_commit_trees(self.repo, to_commits).await?
};
let copy_records = CopyRecords::default(); // TODO
self.show_diff_commit_descriptions(
*formatter,

View File

@@ -197,11 +197,11 @@ fn test_interdiff_conflicting() {
+++ b/file
@@ -1,8 +1,1 @@
-<<<<<<< conflict 1 of 1
-%%%%%%% diff from: qpvuntsm d0c049cd (original parents)
-\\\\\\\ to: zsuskuln 0b2c304e (new parents)
-%%%%%%% diff from: qpvuntsm d0c049cd (from parent)
-\\\\\\\ to: (from context)
--foo
-+abc
-+++++++ rlvkpnrz b23f92c3 (original revision)
-+++++++ rlvkpnrz b23f92c3 (from revision)
-bar
->>>>>>> conflict 1 of 1 ends
+def
@@ -230,12 +230,13 @@ fn test_interdiff_conflicting() {
}
#[test]
fn test_interdiff_gap_detection() {
fn test_interdiff_allows_gaps() {
let test_env = TestEnvironment::default();
test_env.run_jj_in(".", ["git", "init", "repo"]).success();
let work_dir = test_env.work_dir("repo");
// Create a linear chain A -> B -> C
// Create a linear chain A -> B -> C, each writing file
// then overwriting it in the subsequent commit (old pattern).
work_dir.write_file("file", "a\n");
work_dir.run_jj(["new", "-mcommit-a"]).success();
work_dir
@@ -254,25 +255,11 @@ fn test_interdiff_gap_detection() {
.run_jj(["bookmark", "create", "-r@", "c"])
.success();
// Gap in --from (A|C where B is between them)
// A|C has a gap (B missing), but interdiff allows it — no error
let output = work_dir.run_jj(["interdiff", "--from", "a|c", "--to", "c"]);
insta::assert_snapshot!(output, @"
------- stderr -------
Error: Cannot diff revsets with gaps in --from.
Hint: Revision 7772739fe4c7 would need to be in the set.
[EOF]
[exit status: 1]
");
// Gap in --to (A|C where B is between them)
output.success();
let output = work_dir.run_jj(["interdiff", "--from", "c", "--to", "a|c"]);
insta::assert_snapshot!(output, @"
------- stderr -------
Error: Cannot diff revsets with gaps in --to.
Hint: Revision 7772739fe4c7 would need to be in the set.
[EOF]
[exit status: 1]
");
output.success();
}
#[test]
@@ -343,42 +330,191 @@ fn test_interdiff_range_duplicate() {
test_env.run_jj_in(".", ["git", "init", "repo"]).success();
let work_dir = test_env.work_dir("repo");
// Set up: a -> b -> c (chain modifying f1)
// a -> d -> b2 -> c2 (d adds f2, then b2/c2 duplicate b/c on d)
// b::c and b2::c2 should have same changes => empty interdiff
// main: create f0
work_dir.run_jj(["desc", "-mfoo"]).success();
work_dir.write_file("f0", "foo\n");
work_dir.run_jj(["bookmark", "create", "main"]).success();
// a: create f1 with "base"
// a: create f1
work_dir.run_jj(["new", "-ma"]).success();
work_dir.write_file("f1", "base\n");
work_dir.write_file("f1", "a\n");
work_dir.run_jj(["bookmark", "create", "a"]).success();
// b: modify f1 to add "b"
// b: add f2
work_dir.run_jj(["new", "-mb"]).success();
work_dir.write_file("f1", "base\nb\n");
work_dir.write_file("f2", "b\n");
work_dir.run_jj(["bookmark", "create", "b"]).success();
// c: modify f1 to add "c"
// c: modify f1 (inherits f2 from b)
work_dir.run_jj(["new", "-mc"]).success();
work_dir.write_file("f1", "base\nb\nc\n");
work_dir.write_file("f1", "c\n");
work_dir.run_jj(["bookmark", "create", "c"]).success();
// d: add f2 from a (create separate branch from a)
work_dir.run_jj(["new", "a", "-md"]).success();
work_dir.write_file("f2", "d\n");
work_dir.run_jj(["bookmark", "create", "d"]).success();
// Duplicate b::c on top of d, creating b2 and c2
// duplicate b on top of main, move main to b'
work_dir
.run_jj(["duplicate", "-r", "b::c", "--onto", "d"])
.run_jj(["duplicate", "-r", "b", "--onto", "main"])
.success();
work_dir
.run_jj(["bookmark", "create", "b2", "-r", "d+"])
.success();
work_dir
.run_jj(["bookmark", "create", "c2", "-r", "d++"])
.run_jj(["bookmark", "set", "main", "-r", "main+ ~ a"])
.success();
// interdiff between b::c and b2::c2: identical changes => empty
let output = work_dir.run_jj(["interdiff", "--from", "b::c", "--to", "b2::c2"]);
// create a new revision on top of main which adds content to f2, and move main
// there
work_dir.run_jj(["new", "main", "-md"]).success();
work_dir.write_file("f2", "b\nd\n");
work_dir
.run_jj(["bookmark", "set", "main", "-r", "main+"])
.success();
// Duplicate a and c on top of main (skipping b)
work_dir
.run_jj(["duplicate", "-r", "a|c", "--onto", "main"])
.success();
work_dir
.run_jj(["bookmark", "create", "a2", "-r", "main+"])
.success();
work_dir
.run_jj(["bookmark", "create", "c2", "-r", "main++"])
.success();
let output = work_dir.run_jj(["log", "-T builtin_log_oneline"]);
insta::assert_snapshot!(output, @"
○ rsllmpnm test.user 2001-02-03 08:05:20 c2 2bae6fbe c
○ lylxulpl test.user 2001-02-03 08:05:20 a2 c70c1a5c a
@ kmkuslsw test.user 2001-02-03 08:05:19 main a141fe7b d
○ znkkpsqq test.user 2001-02-03 08:05:16 91d546a0 b
│ ○ vruxwmqv test.user 2001-02-03 08:05:15 c f3c4ef5b c
│ ○ royxmykx test.user 2001-02-03 08:05:13 b 0f303e9f b
│ ○ zsuskuln test.user 2001-02-03 08:05:11 a d3a2d994 a
├─╯
○ qpvuntsm test.user 2001-02-03 08:05:09 85736a58 foo
◆ zzzzzzzz root() 00000000
[EOF]
");
// comparing a|c to a2::c2 should show no diff
let output = work_dir.run_jj(["interdiff", "--from", "a|c", "--to", "a2::c2"]);
insta::assert_snapshot!(output, @"");
// a::c includes b (adds f2=b). the content of b should show when comparing a::c
// and a2::c2
let output = work_dir.run_jj(["interdiff", "--from", "a::c", "--to", "a2::c2"]);
insta::assert_snapshot!(output, @r#"
Modified commit description:
1 1: <<<<<<< conflict 1 of 1
2 : %%%%%%% diff from: base #1
2: %%%%%%% diff from: base
3 3: \\\\\\\ to: side #1
4 4: +c
5 : %%%%%%% diff from: base #2
6 : \\\\\\\ to: side #2
7 : +b
8 : +++++++ side #3
5: +++++++ side #2
9 6: a
10 7: >>>>>>> conflict 1 of 1 ends
Resolved conflict in f2:
1 : <<<<<<< conflict 1 of 1
2 : +++++++ (from context)
3 1: b
4 2: d
5 : %%%%%%% diff from: zsuskuln d3a2d994 "a" (from parent)
6 : \\\\\\\ to: vruxwmqv f3c4ef5b "c" (from revision)
7 : +b
8 : >>>>>>> conflict 1 of 1 ends
[EOF]
"#);
}
#[test]
fn test_interdiff_to_with_gap() {
let test_env = TestEnvironment::default();
test_env.run_jj_in(".", ["git", "init", "repo"]).success();
let work_dir = test_env.work_dir("repo");
// main: create f0
work_dir.run_jj(["desc", "-mfoo"]).success();
work_dir.write_file("f0", "foo\n");
work_dir.run_jj(["bookmark", "create", "main"]).success();
// a: create f1
work_dir.run_jj(["new", "-m", "revision a"]).success();
work_dir.write_file("f1", "a\n");
work_dir.run_jj(["bookmark", "create", "a"]).success();
// b: add f2
work_dir.run_jj(["new", "-m", "revision b"]).success();
work_dir.write_file("f2", "b\n");
work_dir.run_jj(["bookmark", "create", "b"]).success();
// c: modify f1
work_dir.run_jj(["new", "-m", "revision c"]).success();
work_dir.write_file("f1", "c\n");
work_dir.run_jj(["bookmark", "create", "c"]).success();
// Duplicate a::c onto main, creating a2, b2, c2 as children
work_dir
.run_jj(["duplicate", "-r", "a::c", "--onto", "main"])
.success();
// Bookmark the duplicates
// After duplicate, the structure is root() -> a2 -> b2 -> c2
work_dir
.run_jj(["bookmark", "create", "a2", "-r", "latest(main+)"])
.success();
work_dir
.run_jj(["bookmark", "create", "b2", "-r", "children(a2)"])
.success();
work_dir
.run_jj(["bookmark", "create", "c2", "-r", "children(b2)"])
.success();
// Insert d2 between b2 and c2 (d2 sets "d" to f3)
work_dir
.run_jj(["new", "-A", "b2", "-m", "revision d"])
.success();
work_dir.write_file("f3", "d\n");
work_dir.run_jj(["bookmark", "create", "d2"]).success();
// Rebase c2 on top of d2 so that d2 is now between b2 and c2
work_dir
.run_jj(["rebase", "-r", "c2", "-d", "d2"])
.success();
let output = work_dir.run_jj(["log", "-T builtin_log_oneline"]);
insta::assert_snapshot!(output, @"
○ lpnsqqnl test.user 2001-02-03 08:05:21 c2 4331391a revision c
@ lylxulpl test.user 2001-02-03 08:05:21 d2 fdf3a6a6 revision d
○ uuzqqzqu test.user 2001-02-03 08:05:16 b2 476b8d40 revision b
○ znkkpsqq test.user 2001-02-03 08:05:16 a2 86f06054 revision a
│ ○ vruxwmqv test.user 2001-02-03 08:05:15 c cf0a2baf revision c
│ ○ royxmykx test.user 2001-02-03 08:05:13 b 37b3bfe7 revision b
│ ○ zsuskuln test.user 2001-02-03 08:05:11 a d1657f43 revision a
├─╯
○ qpvuntsm test.user 2001-02-03 08:05:09 main 85736a58 foo
◆ zzzzzzzz root() 00000000
[EOF]
");
let output = work_dir.run_jj(["interdiff", "--from", "a::c", "--to", "a2::c2 ~ d2"]);
insta::assert_snapshot!(output, @"");
let output = work_dir.run_jj(["interdiff", "--from", "a::c", "--to", "a2::c2"]);
insta::assert_snapshot!(output, @r"
Modified commit description:
...
4 4: +revision c
5 5: %%%%%%% diff from: base #2
6 6: \\\\\\\ to: side #2
7: +revision d
8: %%%%%%% diff from: base #3
9: \\\\\\\ to: side #3
7 10: +revision b
8 : +++++++ side #3
11: +++++++ side #4
9 12: revision a
10 13: >>>>>>> conflict 1 of 1 ends
Added regular file f3:
1: d
[EOF]
");
}