ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

merged_trees: consider conflict-format-change-only commits empty

Browse source 
When we start writing tree-level conflicts in an existing repo, we
don't want commits that change the format to be non-empty if they
don't change any content. This patch updates `MergeTreeId::eq()` to
consider two resolved trees equal even if only their `MergedTreeId`
variant is different (one is path-level and one is tree-level).

I think I've gone through all places we compare tree ids and checked
that it's safe to compare them this way. One consequence is that
rebasing a commit without changing the parents (typically
auto-rebasing after `jj describe`) will not lead to the tree id
getting upgraded, due to an optimization we have for that case. I
don't think that's serious enough to handle specially; we'll have to
support the old format for existing repos for a while regardless of a
few commits not getting upgraded right away.

The number of failing tests with the config option enabled drop from
108 to 11 with this patch.
This commit is contained in:
Martin von Zweigbergk 2023-08-28 18:09:05 -07:00 committed by Martin von Zweigbergk
parent 8e47d2d66f
commit 61501db8ec
2 changed files with 15 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
cli/tests/test_tree_level_conflicts.rs
View file

@ -71,13 +71,13 @@ fn test_enable_tree_level_conflicts() {
let stdout = test_env.jj_cmd_success(&repo_path, &["diff"]); let stdout = test_env.jj_cmd_success(&repo_path, &["diff"]);
insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stdout, @"");
// If we create new commit off of an unconflicted commit, it also appears // If we create new commit off of an unconflicted commit, it correctly appears
// non-empty // empty
test_env.jj_cmd_success(&repo_path, &["new", "k"]); test_env.jj_cmd_success(&repo_path, &["new", "k"]);
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r=@"]); let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r=@"]);
insta::assert_snapshot!(stdout, @r###" insta::assert_snapshot!(stdout, @r###"
@ yostqsxw test.user@example.com 2001-02-03 04:05:16.000 +07:00 f5e911f1 @ yostqsxw test.user@example.com 2001-02-03 04:05:15.000 +07:00 112f0ac2
│ (no description set) │ (empty) (no description set)
~ ~
"###); "###);
let stdout = test_env.jj_cmd_success(&repo_path, &["diff"]); let stdout = test_env.jj_cmd_success(&repo_path, &["diff"]);

12
lib/src/backend.rs
View file

@ -148,7 +148,7 @@ content_hash! {
// TODO(#1624): Delete this type at some point in the future, when we decide to drop // TODO(#1624): Delete this type at some point in the future, when we decide to drop
// support for conflicts in older repos, or maybe after we have provided an upgrade // support for conflicts in older repos, or maybe after we have provided an upgrade
// mechanism. // mechanism.
#[derive(Debug, PartialEq, Eq, Clone)] #[derive(Debug, Clone)]
pub enum MergedTreeId { pub enum MergedTreeId {
/// The tree id of a legacy tree /// The tree id of a legacy tree
Legacy(TreeId), Legacy(TreeId),
@ -156,6 +156,16 @@ pub enum MergedTreeId {
Merge(Merge<TreeId>), Merge(Merge<TreeId>),
} }
impl PartialEq for MergedTreeId {
/// Overridden to make conflict-free trees be considered equal even if their
/// `MergedTreeId` variant is different.
fn eq(&self, other: &Self) -> bool {
self.to_merge() == other.to_merge()
}
}
impl Eq for MergedTreeId {}
impl ContentHash for MergedTreeId { impl ContentHash for MergedTreeId {
fn hash(&self, state: &mut impl digest::Update) { fn hash(&self, state: &mut impl digest::Update) {
match self { match self {