backend: remove MergedTreeId::Legacy variant

cli/src/cli_util.rs

@@ -2000,7 +2000,7 @@ See https://jj-vcs.github.io/jj/latest/working-copy/#stale-working-copy \
         print_checkout_stats(ui, stats, new_commit)?;
         if Some(new_commit) != maybe_old_commit
             && let Some(mut formatter) = ui.status_formatter()
-            && new_commit.has_conflict()?
+            && new_commit.has_conflict()
         {
             let conflicts = new_commit.tree()?.conflicts().collect_vec();
             writeln!(

cli/src/commands/bookmark/list.rs

@@ -459,7 +459,7 @@ mod tests {
         Arc::new(backend::Commit {
             parents: vec![],
             predecessors: vec![],
-            root_tree: MergedTreeId::Legacy(TreeId::new(vec![])),
+            root_tree: MergedTreeId::resolved(TreeId::new(vec![])),
             change_id: ChangeId::new(vec![]),
             description: String::new(),
             author,

cli/src/commands/git/push.rs

@@ -557,7 +557,7 @@ fn validate_commits_ready_to_push(
         {
             reasons.push("it has no author and/or committer set");
         }
-        if commit.has_conflict()? {
+        if commit.has_conflict() {
             reasons.push("it has conflicts");
         }
         let is_private = is_private(commit.id())?;

cli/src/commands/resolve.rs

@@ -135,7 +135,7 @@ pub(crate) fn cmd_resolve(
     // be printed by the `tx.finish()` instead.
     if workspace_command.get_wc_commit_id() != Some(new_commit.id())
         && let Some(mut formatter) = ui.status_formatter()
-        && new_commit.has_conflict()?
+        && new_commit.has_conflict()
     {
         let new_tree = new_commit.tree()?;
         let new_conflicts = new_tree.conflicts().collect_vec();

cli/src/commands/status.rs

@@ -141,7 +141,7 @@ pub(crate) fn cmd_status(
             writeln!(formatter)?;
         }
 
-        if wc_commit.has_conflict()? {
+        if wc_commit.has_conflict() {
             // TODO: Conflicts should also be filtered by the `matcher`. See the related
             // TODO on `MergedTree::conflicts()`.
             let conflicts = wc_commit.tree()?.conflicts().collect_vec();
@@ -169,7 +169,7 @@ pub(crate) fn cmd_status(
         } else {
             for parent in wc_commit.parents() {
                 let parent = parent?;
-                if parent.has_conflict()? {
+                if parent.has_conflict() {
                     writeln!(
                         formatter.labeled("hint").with_heading("Hint: "),
                         "Conflict in parent commit has been resolved in working copy"

cli/src/commit_templater.rs

@@ -1196,7 +1196,7 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
         "conflict",
         |_language, _diagnostics, _build_ctx, self_property, function| {
             function.expect_no_arguments()?;
-            let out_property = self_property.and_then(|commit| Ok(commit.has_conflict()?));
+            let out_property = self_property.map(|commit| commit.has_conflict());
             Ok(out_property.into_dyn_wrapped())
         },
     );

cli/tests/test_operations.rs

@@ -678,7 +678,7 @@ fn test_op_abandon_ancestors() {
     ");
     insta::assert_snapshot!(work_dir.run_jj(["debug", "local-working-copy", "--ignore-working-copy"]), @r#"
     Current operation: OperationId("1675333b7de89b5da012c696d797345bad2a6ce55a4b605e85c3897f818f05e11e8c53de19d34c2fee38a36528dc95bd2a378f72ac0877f8bec2513a68043253")
-    Current tree: Merge(Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")))
+    Current tree: MergedTreeId { tree_ids: Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")) }
     [EOF]
     "#);
     insta::assert_snapshot!(work_dir.run_jj(["op", "log"]), @r"
@@ -739,7 +739,7 @@ fn test_op_abandon_ancestors() {
     ");
     insta::assert_snapshot!(work_dir.run_jj(["debug", "local-working-copy", "--ignore-working-copy"]), @r#"
     Current operation: OperationId("ce6a0300b7346109e75a6dcc97e3ff9e1488ce43a4073dd9eb81afb7f463b4543d3f15cf9a42a9864a4aaf6daab900b6b037dbdcb95f87422e891f7e884641aa")
-    Current tree: Merge(Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")))
+    Current tree: MergedTreeId { tree_ids: Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")) }
     [EOF]
     "#);
     insta::assert_snapshot!(work_dir.run_jj(["op", "log"]), @r"
@@ -787,7 +787,7 @@ fn test_op_abandon_without_updating_working_copy() {
     ");
     insta::assert_snapshot!(work_dir.run_jj(["debug", "local-working-copy", "--ignore-working-copy"]), @r#"
     Current operation: OperationId("0d4bb8e4a2babc4c216be0f9bde32aeef888abebde0062aeb1c204dde5e1f476fa951fcbeceb2263cf505008ba87a834849469dede30dfc589f37d5073aedfbe")
-    Current tree: Merge(Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")))
+    Current tree: MergedTreeId { tree_ids: Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")) }
     [EOF]
     "#);
     insta::assert_snapshot!(work_dir.run_jj(["op", "log", "-n1", "--ignore-working-copy"]), @r"
@@ -809,7 +809,7 @@ fn test_op_abandon_without_updating_working_copy() {
     ");
     insta::assert_snapshot!(work_dir.run_jj(["debug", "local-working-copy", "--ignore-working-copy"]), @r#"
     Current operation: OperationId("0d4bb8e4a2babc4c216be0f9bde32aeef888abebde0062aeb1c204dde5e1f476fa951fcbeceb2263cf505008ba87a834849469dede30dfc589f37d5073aedfbe")
-    Current tree: Merge(Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")))
+    Current tree: MergedTreeId { tree_ids: Resolved(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")) }
     [EOF]
     "#);
     insta::assert_snapshot!(work_dir.run_jj(["op", "log", "-n1", "--ignore-working-copy"]), @r"

cli/tests/test_working_copy.rs

@@ -366,7 +366,7 @@ fn test_conflict_marker_length_stored_in_working_copy() {
     let output = work_dir.run_jj(["debug", "local-working-copy"]);
     insta::assert_snapshot!(output.normalize_stdout_with(redact_output), @r#"
     Current operation: OperationId("da3b34243efe5ea04830cd2211b5be79444fbc2ef23681361fd2f551ebb86772bff21695da95b72388306e028bf04c6d76db10bf4cbd3a08eb34bf744c8900c7")
-    Current tree: Merge(Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("f56b8223da0dab22b03b8323ced4946329aeb4e0")]))
+    Current tree: MergedTreeId { tree_ids: Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("f56b8223da0dab22b03b8323ced4946329aeb4e0")]) }
     Normal { <executable> }           249 <timestamp> Some(MaterializedConflictData { conflict_marker_len: 11 }) "file"
     [EOF]
     "#);
@@ -429,7 +429,7 @@ fn test_conflict_marker_length_stored_in_working_copy() {
     let output = work_dir.run_jj(["debug", "local-working-copy"]);
     insta::assert_snapshot!(output.normalize_stdout_with(redact_output), @r#"
     Current operation: OperationId("3de33bbfe3a9df8a052cc243aeedac6a3240d6115cb88f2779a1b6f1289288c6e78153875e48e41c17c098418f681bc872c54743e76b9e210f08533c50fc5a26")
-    Current tree: Merge(Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("3329c18c95f7b7a55c278c2259e9c4ce711fae59")]))
+    Current tree: MergedTreeId { tree_ids: Conflicted([TreeId("381273b50cf73f8c81b3f1502ee89e9bbd6c1518"), TreeId("771f3d31c4588ea40a8864b2a981749888e596c2"), TreeId("3329c18c95f7b7a55c278c2259e9c4ce711fae59")]) }
     Normal { <executable> }           289 <timestamp> Some(MaterializedConflictData { conflict_marker_len: 11 }) "file"
     [EOF]
     "#);
@@ -464,7 +464,7 @@ fn test_conflict_marker_length_stored_in_working_copy() {
     let output = work_dir.run_jj(["debug", "local-working-copy"]);
     insta::assert_snapshot!(output.normalize_stdout_with(redact_output), @r#"
     Current operation: OperationId("2676b66a8d17cf7913d2260285abe6f3ca4c8dc8f3fdfb3f54a4d566c9199670f80123a7174b553ff67c13c20c6827cde2429847a7949c19bc52f2397139e4c9")
-    Current tree: Merge(Resolved(TreeId("6120567b3cb2472d549753ed3e4b84183d52a650")))
+    Current tree: MergedTreeId { tree_ids: Resolved(TreeId("6120567b3cb2472d549753ed3e4b84183d52a650")) }
     Normal { <executable> }           130 <timestamp> None "file"
     [EOF]
     "#);

lib/src/backend.rs

@@ -152,41 +152,34 @@ pub struct SecureSig {
 
 pub type SigningFn<'a> = dyn FnMut(&[u8]) -> SignResult<Vec<u8>> + Send + 'a;
 
-/// Identifies a single legacy tree, which may have path-level conflicts, or a
-/// merge of multiple trees, where the individual trees do not have conflicts.
-// 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
-// mechanism.
-#[derive(ContentHash, Debug, Clone)]
-pub enum MergedTreeId {
-    /// The tree id of a legacy tree
-    Legacy(TreeId),
+/// Identifies a merge of multiple trees. Can be read as a `MergedTree`.
+// TODO: this type doesn't add anything over `Merge<TreeId>` currently, but conflict labels could be
+// added here in the future if we also add them to `MergedTree`.
+#[derive(ContentHash, Debug, PartialEq, Eq, Clone)]
+pub struct MergedTreeId {
     /// The tree id(s) of a merge tree
-    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()
-    }
+    tree_ids: Merge<TreeId>,
 }
 
-impl Eq for MergedTreeId {}
-
 impl MergedTreeId {
     /// Create a resolved `MergedTreeId` from a single regular tree.
     pub fn resolved(tree_id: TreeId) -> Self {
-        Self::Merge(Merge::resolved(tree_id))
+        Self::new(Merge::resolved(tree_id))
     }
 
-    /// Return this id as `Merge<TreeId>`
-    pub fn to_merge(&self) -> Merge<TreeId> {
-        match self {
-            Self::Legacy(tree_id) => Merge::resolved(tree_id.clone()),
-            Self::Merge(tree_ids) => tree_ids.clone(),
-        }
+    /// Create a `MergedTreeId` from a `Merge<TreeId>`.
+    pub fn new(tree_ids: Merge<TreeId>) -> Self {
+        Self { tree_ids }
+    }
+
+    /// Returns the underlying `Merge<TreeId>`.
+    pub fn as_merge(&self) -> &Merge<TreeId> {
+        &self.tree_ids
+    }
+
+    /// Extracts the underlying `Merge<TreeId>`.
+    pub fn into_merge(self) -> Merge<TreeId> {
+        self.tree_ids
     }
 }
 

lib/src/commit.rs

@@ -151,12 +151,8 @@ impl Commit {
         is_backend_commit_empty(repo, &self.store, &self.data)
     }
 
-    pub fn has_conflict(&self) -> BackendResult<bool> {
-        if let MergedTreeId::Merge(tree_ids) = self.tree_id() {
-            Ok(!tree_ids.is_resolved())
-        } else {
-            Ok(self.tree()?.has_conflict())
-        }
+    pub fn has_conflict(&self) -> bool {
+        !self.tree_id().as_merge().is_resolved()
     }
 
     pub fn change_id(&self) -> &ChangeId {

lib/src/default_index/revset_engine.rs

@@ -1312,7 +1312,7 @@ fn build_predicate_fn(
         RevsetFilterPredicate::HasConflict => box_pure_predicate_fn(move |index, pos| {
             let entry = index.commits().entry_by_pos(pos);
             let commit = store.get_commit(&entry.commit_id())?;
-            Ok(commit.has_conflict()?)
+            Ok(commit.has_conflict())
         }),
         RevsetFilterPredicate::Signed => box_pure_predicate_fn(move |index, pos| {
             let entry = index.commits().entry_by_pos(pos);