diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 9d56a38ccf2..970c40a276a 100644 --- a/cli/src/cli_util.rs +++ b/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!( diff --git a/cli/src/commands/bookmark/list.rs b/cli/src/commands/bookmark/list.rs index bb9f26bc071..102616c86b4 100644 --- a/cli/src/commands/bookmark/list.rs +++ b/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, diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index f89ca4982c3..98c8e383114 100644 --- a/cli/src/commands/git/push.rs +++ b/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())?; diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index 07769970cc3..e88ecc5ed83 100644 --- a/cli/src/commands/resolve.rs +++ b/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(); diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs index 3221ca12e86..041a2cac691 100644 --- a/cli/src/commands/status.rs +++ b/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" diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index 8f69d0f340b..0d72702265b 100644 --- a/cli/src/commit_templater.rs +++ b/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()) }, ); diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index 541fc1ec94e..b5ed656d0f6 100644 --- a/cli/tests/test_operations.rs +++ b/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" diff --git a/cli/tests/test_working_copy.rs b/cli/tests/test_working_copy.rs index c474b7e6443..e33766b4c8b 100644 --- a/cli/tests/test_working_copy.rs +++ b/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 { } 249 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 { } 289 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 { } 130 None "file" [EOF] "#); diff --git a/lib/src/backend.rs b/lib/src/backend.rs index 0ce098ada25..8287dcdcebc 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -152,41 +152,34 @@ pub struct SecureSig { pub type SigningFn<'a> = dyn FnMut(&[u8]) -> SignResult> + 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` 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), -} - -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, } -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` - pub fn to_merge(&self) -> Merge { - match self { - Self::Legacy(tree_id) => Merge::resolved(tree_id.clone()), - Self::Merge(tree_ids) => tree_ids.clone(), - } + /// Create a `MergedTreeId` from a `Merge`. + pub fn new(tree_ids: Merge) -> Self { + Self { tree_ids } + } + + /// Returns the underlying `Merge`. + pub fn as_merge(&self) -> &Merge { + &self.tree_ids + } + + /// Extracts the underlying `Merge`. + pub fn into_merge(self) -> Merge { + self.tree_ids } } diff --git a/lib/src/commit.rs b/lib/src/commit.rs index 9bbdfb0e2cc..b0bfbcf4259 100644 --- a/lib/src/commit.rs +++ b/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 { - 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 { diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 52bb4735996..857e388d4a5 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/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); diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 4bcad75c7bd..5f9624a7c09 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -494,7 +494,7 @@ impl GitBackend { repo: &'repo gix::Repository, id: &CommitId, ) -> BackendResult> { - let tree = self.read_commit(id).block_on()?.root_tree.to_merge(); + let tree = self.read_commit(id).block_on()?.root_tree.into_merge(); // TODO(kfm): probably want to do something here if it is a merge let tree_id = tree.first().clone(); let gix_id = validate_git_object_id(&tree_id)?; @@ -554,13 +554,12 @@ fn root_tree_from_git_extra_header(value: &BStr) -> Result { if tree_ids.len() == 1 || tree_ids.len() % 2 == 0 { return Err(()); } - Ok(MergedTreeId::Merge(Merge::from_vec(tree_ids))) + Ok(MergedTreeId::new(Merge::from_vec(tree_ids))) } fn commit_from_git_without_root_parent( id: &CommitId, git_object: &gix::Object, - uses_tree_conflict_format: bool, is_shallow: bool, ) -> BackendResult { let commit = git_object @@ -593,11 +592,7 @@ fn commit_from_git_without_root_parent( .map_err(|()| to_read_object_err("Invalid jj:trees header", id))? .unwrap_or_else(|| { let tree_id = TreeId::from_bytes(commit.tree().as_bytes()); - if uses_tree_conflict_format { - MergedTreeId::resolved(tree_id) - } else { - MergedTreeId::Legacy(tree_id) - } + MergedTreeId::resolved(tree_id) }); // Use lossy conversion as commit message with "mojibake" is still better than // nothing. @@ -723,14 +718,13 @@ fn serialize_extras(commit: &Commit) -> Vec { change_id: commit.change_id.to_bytes(), ..Default::default() }; - if let MergedTreeId::Merge(tree_ids) = &commit.root_tree { - proto.uses_tree_conflict_format = true; - if !tree_ids.is_resolved() { - // This is done for the sake of jj versions <0.28 (before commit - // f7b14be) being able to read the repo. At some point in the - // future, we can stop doing it. - proto.root_tree = tree_ids.iter().map(|r| r.to_bytes()).collect(); - } + proto.uses_tree_conflict_format = true; + let tree_ids = commit.root_tree.as_merge(); + if !tree_ids.is_resolved() { + // This is done for the sake of jj versions <0.28 (before commit + // f7b14be) being able to read the repo. At some point in the + // future, we can stop doing it. + proto.root_tree = tree_ids.iter().map(|r| r.to_bytes()).collect(); } for predecessor in &commit.predecessors { proto.predecessors.push(predecessor.to_bytes()); @@ -743,22 +737,16 @@ fn deserialize_extras(commit: &mut Commit, bytes: &[u8]) { if !proto.change_id.is_empty() { commit.change_id = ChangeId::new(proto.change_id); } - if let MergedTreeId::Legacy(legacy_tree_id) = &commit.root_tree + if commit.root_tree.as_merge().is_resolved() && proto.uses_tree_conflict_format + && !proto.root_tree.is_empty() { - if !proto.root_tree.is_empty() { - let merge_builder: MergeBuilder<_> = proto - .root_tree - .iter() - .map(|id_bytes| TreeId::from_bytes(id_bytes)) - .collect(); - commit.root_tree = MergedTreeId::Merge(merge_builder.build()); - } else { - // uses_tree_conflict_format was set but there was no root_tree override in the - // proto, which means we should just promote the tree id from the - // git commit to be a known-conflict-free tree - commit.root_tree = MergedTreeId::resolved(legacy_tree_id.clone()); - } + let merge_builder: MergeBuilder<_> = proto + .root_tree + .iter() + .map(|id_bytes| TreeId::from_bytes(id_bytes)) + .collect(); + commit.root_tree = MergedTreeId::new(merge_builder.build()); } for predecessor in &proto.predecessors { commit.predecessors.push(CommitId::from_bytes(predecessor)); @@ -951,7 +939,7 @@ fn import_extra_metadata_entries_from_heads( // TODO(#1624): Should we read the root tree here and check if it has a // `.jjconflict-...` entries? That could happen if the user used `git` to e.g. // change the description of a commit with tree-level conflicts. - let commit = commit_from_git_without_root_parent(&id, &git_object, true, is_shallow)?; + let commit = commit_from_git_without_root_parent(&id, &git_object, is_shallow)?; mut_table.add_entry(id.to_bytes(), serialize_extras(&commit)); work_ids.extend( commit @@ -1206,7 +1194,7 @@ impl Backend for GitBackend { .find_object(git_commit_id) .map_err(|err| map_not_found_err(err, id))?; let is_shallow = self.shallow_root_ids(&locked_repo)?.contains(id); - commit_from_git_without_root_parent(id, &git_object, false, is_shallow)? + commit_from_git_without_root_parent(id, &git_object, is_shallow)? }; if commit.parents.is_empty() { commit.parents.push(self.root_commit_id.clone()); @@ -1237,12 +1225,10 @@ impl Backend for GitBackend { assert!(contents.secure_sig.is_none(), "commit.secure_sig was set"); let locked_repo = self.lock_git_repo(); - let git_tree_id = match &contents.root_tree { - MergedTreeId::Legacy(tree_id) => validate_git_object_id(tree_id)?, - MergedTreeId::Merge(tree_ids) => match tree_ids.as_resolved() { - Some(tree_id) => validate_git_object_id(tree_id)?, - None => write_tree_conflict(&locked_repo, tree_ids)?, - }, + let tree_ids = contents.root_tree.as_merge(); + let git_tree_id = match tree_ids.as_resolved() { + Some(tree_id) => validate_git_object_id(tree_id)?, + None => write_tree_conflict(&locked_repo, tree_ids)?, }; let author = signature_to_git(&contents.author); let mut committer = signature_to_git(&contents.committer); @@ -1271,9 +1257,7 @@ impl Backend for GitBackend { } } let mut extra_headers: Vec<(BString, BString)> = vec![]; - if let MergedTreeId::Merge(tree_ids) = &contents.root_tree - && !tree_ids.is_resolved() - { + if !tree_ids.is_resolved() { let value = tree_ids.iter().map(|id| id.hex()).join(" "); extra_headers.push((JJ_TREES_COMMIT_HEADER.into(), value.into())); } @@ -1665,10 +1649,9 @@ mod tests { assert_eq!(commit.parents, vec![CommitId::from_bytes(&[0; 20])]); assert_eq!(commit.predecessors, vec![]); assert_eq!( - commit.root_tree.to_merge(), - Merge::resolved(TreeId::from_bytes(root_tree_id.as_bytes())) + commit.root_tree, + MergedTreeId::resolved(TreeId::from_bytes(root_tree_id.as_bytes())) ); - assert_matches!(commit.root_tree, MergedTreeId::Merge(_)); assert_eq!(commit.description, "git commit message"); assert_eq!(commit.author.name, "git author"); assert_eq!(commit.author.email, "git.author@example.com"); @@ -1731,10 +1714,9 @@ mod tests { assert_eq!(commit2.parents, vec![commit_id.clone()]); assert_eq!(commit.predecessors, vec![]); assert_eq!( - commit.root_tree.to_merge(), - Merge::resolved(TreeId::from_bytes(root_tree_id.as_bytes())) + commit.root_tree, + MergedTreeId::resolved(TreeId::from_bytes(root_tree_id.as_bytes())) ); - assert_matches!(commit.root_tree, MergedTreeId::Merge(_)); } #[test] @@ -1923,7 +1905,7 @@ mod tests { let commit = Commit { parents: vec![backend.root_commit_id().clone()], predecessors: vec![], - root_tree: MergedTreeId::Legacy(backend.empty_tree_id().clone()), + root_tree: MergedTreeId::resolved(backend.empty_tree_id().clone()), change_id: original_change_id.clone(), description: "initial".to_string(), author: create_signature(), @@ -2014,7 +1996,7 @@ mod tests { let mut commit = Commit { parents: vec![], predecessors: vec![], - root_tree: MergedTreeId::Legacy(backend.empty_tree_id().clone()), + root_tree: MergedTreeId::resolved(backend.empty_tree_id().clone()), change_id: ChangeId::from_hex("abc123"), description: "".to_string(), author: create_signature(), @@ -2100,7 +2082,7 @@ mod tests { let mut commit = Commit { parents: vec![backend.root_commit_id().clone()], predecessors: vec![], - root_tree: MergedTreeId::Merge(root_tree.clone()), + root_tree: MergedTreeId::new(root_tree.clone()), change_id: ChangeId::from_hex("abc123"), description: "".to_string(), author: create_signature(), @@ -2200,7 +2182,7 @@ mod tests { let commit = Commit { parents: vec![backend.root_commit_id().clone()], predecessors: vec![], - root_tree: MergedTreeId::Legacy(backend.empty_tree_id().clone()), + root_tree: MergedTreeId::resolved(backend.empty_tree_id().clone()), change_id: ChangeId::new(vec![42; 16]), description: "initial".to_string(), author: signature.clone(), @@ -2280,7 +2262,7 @@ mod tests { let commit1 = Commit { parents: vec![backend.root_commit_id().clone()], predecessors: vec![], - root_tree: MergedTreeId::Legacy(backend.empty_tree_id().clone()), + root_tree: MergedTreeId::resolved(backend.empty_tree_id().clone()), change_id: ChangeId::from_hex("7f0a7ce70354b22efcccf7bf144017c4"), description: "initial".to_string(), author: create_signature(), @@ -2322,7 +2304,7 @@ mod tests { let commit = Commit { parents: vec![backend.root_commit_id().clone()], predecessors: vec![], - root_tree: MergedTreeId::Legacy(backend.empty_tree_id().clone()), + root_tree: MergedTreeId::resolved(backend.empty_tree_id().clone()), change_id: ChangeId::new(vec![42; 16]), description: "initial".to_string(), author: create_signature(), diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 8dc2784d9e5..f00540ad464 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -910,7 +910,7 @@ impl TreeState { .iter() .map(|id| TreeId::new(id.clone())) .collect(); - self.tree_id = MergedTreeId::Merge(tree_ids_builder.build()); + self.tree_id = MergedTreeId::new(tree_ids_builder.build()); } self.file_states = FileStatesMap::from_proto(proto.file_states, proto.is_file_states_sorted); @@ -919,18 +919,15 @@ impl TreeState { Ok(()) } - #[expect(clippy::assigning_clones)] + #[expect(clippy::assigning_clones, clippy::field_reassign_with_default)] pub fn save(&mut self) -> Result<(), TreeStateError> { let mut proto: crate::protos::local_working_copy::TreeState = Default::default(); - match &self.tree_id { - MergedTreeId::Legacy(_) => { - unreachable!(); - } - MergedTreeId::Merge(tree_ids) => { - proto.tree_ids = tree_ids.iter().map(|id| id.to_bytes()).collect(); - } - } - + proto.tree_ids = self + .tree_id + .as_merge() + .iter() + .map(|id| id.to_bytes()) + .collect(); proto.file_states = self.file_states.data.clone(); // `FileStatesMap` is guaranteed to be sorted. proto.is_file_states_sorted = true; diff --git a/lib/src/merged_tree.rs b/lib/src/merged_tree.rs index 4b002446377..87a411e66dd 100644 --- a/lib/src/merged_tree.rs +++ b/lib/src/merged_tree.rs @@ -87,7 +87,7 @@ impl MergedTree { } /// Extracts the underlying `Merge`. - pub fn take(self) -> Merge { + pub fn into_merge(self) -> Merge { self.trees } @@ -206,7 +206,7 @@ impl MergedTree { /// The tree's id pub fn id(&self) -> MergedTreeId { - MergedTreeId::Merge(self.trees.map(|tree| tree.id().clone())) + MergedTreeId::new(self.trees.map(|tree| tree.id().clone())) } /// Look up the tree at the given path. @@ -954,9 +954,7 @@ impl Stream for DiffStreamForFileSystem<'_> { /// /// You start by creating an instance of this type with one or more /// base trees. You then add overrides on top. The overrides may be -/// conflicts. Then you can write the result as a legacy tree -/// (allowing path-level conflicts) or as multiple conflict-free -/// trees. +/// conflicts. Then you can write the result as a merge of trees. pub struct MergedTreeBuilder { base_tree_id: MergedTreeId, overrides: BTreeMap, @@ -981,15 +979,12 @@ impl MergedTreeBuilder { /// Create new tree(s) from the base tree(s) and overrides. pub fn write_tree(self, store: &Arc) -> BackendResult { - let base_tree_ids = match self.base_tree_id.clone() { - MergedTreeId::Legacy(base_tree_id) => Merge::resolved(base_tree_id), - MergedTreeId::Merge(base_tree_ids) => base_tree_ids, - }; + let base_tree_ids = self.base_tree_id.as_merge().clone(); let new_tree_ids = self.write_merged_trees(base_tree_ids, store)?; match new_tree_ids.simplify().into_resolved() { Ok(single_tree_id) => Ok(MergedTreeId::resolved(single_tree_id)), Err(tree_id) => { - let tree = store.get_root_tree(&MergedTreeId::Merge(tree_id))?; + let tree = store.get_root_tree(&MergedTreeId::new(tree_id))?; let resolved = tree.resolve().block_on()?; Ok(resolved.id()) } diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 3de2923d965..25f9929fe4c 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -79,7 +79,7 @@ pub async fn merge_commit_trees_no_resolve_without_repo( .try_map_async(async |commit_id| { let commit = store.get_commit_async(commit_id).await?; let tree = commit.tree_async().await?; - Ok::<_, BackendError>(tree.take()) + Ok::<_, BackendError>(tree.into_merge()) }) .await?; Ok(MergedTree::new(tree_merge.flatten().simplify())) diff --git a/lib/src/simple_backend.rs b/lib/src/simple_backend.rs index 1bb51e5523e..ca452eb39d1 100644 --- a/lib/src/simple_backend.rs +++ b/lib/src/simple_backend.rs @@ -359,14 +359,12 @@ pub fn commit_to_proto(commit: &Commit) -> crate::protos::simple_store::Commit { for predecessor in &commit.predecessors { proto.predecessors.push(predecessor.to_bytes()); } - match &commit.root_tree { - MergedTreeId::Legacy(_) => { - panic!("The simple backend doesn't support legacy trees"); - } - MergedTreeId::Merge(tree_ids) => { - proto.root_tree = tree_ids.iter().map(|id| id.to_bytes()).collect(); - } - } + proto.root_tree = commit + .root_tree + .as_merge() + .iter() + .map(|id| id.to_bytes()) + .collect(); proto.change_id = commit.change_id.to_bytes(); proto.description = commit.description.clone(); proto.author = Some(signature_to_proto(&commit.author)); @@ -385,7 +383,7 @@ fn commit_from_proto(mut proto: crate::protos::simple_store::Commit) -> Commit { let parents = proto.parents.into_iter().map(CommitId::new).collect(); let predecessors = proto.predecessors.into_iter().map(CommitId::new).collect(); let merge_builder: MergeBuilder<_> = proto.root_tree.into_iter().map(TreeId::new).collect(); - let root_tree = MergedTreeId::Merge(merge_builder.build()); + let root_tree = MergedTreeId::new(merge_builder.build()); let change_id = ChangeId::new(proto.change_id); Commit { parents, diff --git a/lib/src/store.rs b/lib/src/store.rs index 6a879cd1065..8b75f5f8346 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -223,18 +223,11 @@ impl Store { self: &Arc, id: &MergedTreeId, ) -> BackendResult { - match &id { - MergedTreeId::Legacy(id) => { - let tree = self.get_tree_async(RepoPathBuf::root(), id).await?; - Ok(MergedTree::resolved(tree)) - } - MergedTreeId::Merge(ids) => { - let trees = ids - .try_map_async(|id| self.get_tree_async(RepoPathBuf::root(), id)) - .await?; - Ok(MergedTree::new(trees)) - } - } + let trees = id + .as_merge() + .try_map_async(|id| self.get_tree_async(RepoPathBuf::root(), id)) + .await?; + Ok(MergedTree::new(trees)) } pub async fn write_tree( diff --git a/lib/tests/test_id_prefix.rs b/lib/tests/test_id_prefix.rs index 26a9f213b0d..6720ce3a191 100644 --- a/lib/tests/test_id_prefix.rs +++ b/lib/tests/test_id_prefix.rs @@ -566,7 +566,7 @@ fn test_id_prefix_shadowed_by_ref() { let commit_id_sym = commit.id().to_string(); let change_id_sym = commit.change_id().to_string(); - insta::assert_snapshot!(commit_id_sym, @"ccde67f2a6ece4661cf4"); + insta::assert_snapshot!(commit_id_sym, @"38b5c5aebe81a8441470"); insta::assert_snapshot!(change_id_sym, @"sryyqqkqmuumyrlruupspprvnulvovzm"); let context = IdPrefixContext::default(); diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs index 3699240b126..376104b9a61 100644 --- a/lib/tests/test_local_working_copy.rs +++ b/lib/tests/test_local_working_copy.rs @@ -541,7 +541,7 @@ fn test_tree_builder_file_directory_transition() { let workspace_root = ws.workspace_root().to_owned(); let mut check_out_tree = |tree_id: &TreeId| { let tree = repo.store().get_tree(RepoPathBuf::root(), tree_id).unwrap(); - let commit = commit_with_tree(repo.store(), MergedTreeId::Legacy(tree.id().clone())); + let commit = commit_with_tree(repo.store(), MergedTreeId::resolved(tree.id().clone())); ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); }; diff --git a/lib/tests/test_merge_trees.rs b/lib/tests/test_merge_trees.rs index 6b0b64ebbca..985a238c0bc 100644 --- a/lib/tests/test_merge_trees.rs +++ b/lib/tests/test_merge_trees.rs @@ -251,12 +251,12 @@ fn test_rebase_on_lossy_merge(same_change: SameChange) { SameChange::Keep => assert_eq!(*commit_d2.tree_id(), tree_3.id()), SameChange::Accept => { let expected_tree_id = Merge::from_vec(vec![ - tree_2.id().to_merge(), - tree_1.id().to_merge(), - tree_3.id().to_merge(), + tree_2.id().into_merge(), + tree_1.id().into_merge(), + tree_3.id().into_merge(), ]) .flatten(); - assert_eq!(*commit_d2.tree_id(), MergedTreeId::Merge(expected_tree_id)); + assert_eq!(*commit_d2.tree_id(), MergedTreeId::new(expected_tree_id)); } } } diff --git a/lib/tests/test_merged_tree.rs b/lib/tests/test_merged_tree.rs index 60ddb3ea2ea..14e6f6245e5 100644 --- a/lib/tests/test_merged_tree.rs +++ b/lib/tests/test_merged_tree.rs @@ -81,7 +81,7 @@ fn test_merged_tree_builder_resolves_conflict() { let tree2 = create_single_tree(repo, &[(path1, "bar")]); let tree3 = create_single_tree(repo, &[(path1, "bar")]); - let base_tree_id = MergedTreeId::Merge(Merge::from_removes_adds( + let base_tree_id = MergedTreeId::new(Merge::from_removes_adds( [tree1.id().clone()], [tree2.id().clone(), tree3.id().clone()], )); diff --git a/lib/tests/test_revset_optimized.rs b/lib/tests/test_revset_optimized.rs index b8b761cd7d8..29a2d6c794e 100644 --- a/lib/tests/test_revset_optimized.rs +++ b/lib/tests/test_revset_optimized.rs @@ -196,15 +196,15 @@ fn test_mostly_linear() { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @r" 00000000000000000000 - 1 b454727d1ac1243807d5 - 2 efbe8bc183cdad501010 - 3 668852e79ac986cbb24a - 4 e2cfc9485a41e3039864 - 5 9f4cab37e672b9a20029 - 6 7433850ea79a09758b78 - 7 11f067071cc8223b818b - 8 480c23000c48225eec16 - 9 773cad10cdad4b30c9bf + 1 0481b93947ec320582da + 2 cf00f20ba8d03cfe27d7 + 3 db1fb816b776ac1257a1 + 4 3fa336059698229e1869 + 5 f1d81483ce728dac9c5c + 6 c0620144c8381147c3eb + 7 f1355db0abd492eb4df4 + 8 2aa8feb562b8426f485f + 9 fee08f51862cdd69739d "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); @@ -250,14 +250,14 @@ fn test_weird_merges() { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @r" 00000000000000000000 - 1 b454727d1ac1243807d5 - 2 efbe8bc183cdad501010 - 3 8e6b4a4aa763e550916a - 4 ff56a4d7893e7c13b323 - 5 f1fbd424801c4550decb - 6 8d3dad20495f63c76f5e - 7 116c29c5f0f7d1eef9cb - 8 50774213daae44ce0e66 + 1 0481b93947ec320582da + 2 cf00f20ba8d03cfe27d7 + 3 eae7b745114ccd1e7c2b + 4 8ff5f4ecfd6e51f7fb46 + 5 0701d1a6ff427cc5d1c1 + 6 8b2aa399c528813d0bfc + 7 abd7903b1690685bb9c8 + 8 0f916c2bef3fe0aa2f54 "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); @@ -326,15 +326,15 @@ fn test_feature_branches() { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @r" 00000000000000000000 - 1 b454727d1ac1243807d5 - 2 7550acbc09948087c024 - 3 8e6b4a4aa763e550916a - 4 c4b7645a58d514040403 - 5 5dd33d2a7d5364d62825 - 6 b2b4c490c091aea33575 - 7 d35b7eb1ae4195a195b8 - 8 59707ff883b28584ec23 - 9 c482d0862bb841b200de + 1 0481b93947ec320582da + 2 dfc04cc2cdd8ddb7b55b + 3 eae7b745114ccd1e7c2b + 4 8e272dcf1dfef181cb22 + 5 62bb52a8eb37e1dffcb1 + 6 900d6e697d7e53fe31c5 + 7 74bcf8cf11c54a565c0c + 8 01c9367ecaaa7c0bfc47 + 9 60b2edccd2b633d4b164 "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); @@ -396,15 +396,15 @@ fn test_rewritten() { insta::assert_snapshot!( commits.iter().map(|c| format!("{:<2} {}\n", c.description(), c.id())).join(""), @r" 00000000000000000000 - 1 b454727d1ac1243807d5 - 2 efbe8bc183cdad501010 - 3 bf42a9771bc9322180c3 - 4 fb270b6eeef978c0a12b - 5 479dec6a96f336043233 - 2b d0d01d78d2d69c0afa23 - 3 1894713af307b49e5644 - 5 9c5d36510c963d5a5ca3 - 5 e8b2f867cecdf8cce16d + 1 0481b93947ec320582da + 2 cf00f20ba8d03cfe27d7 + 3 0ae48179bdee2dc5cbee + 4 01e8f78ec985350a98f5 + 5 2586f14733ac1c4f2b89 + 2b 4dc5572169ee6230a7fc + 3 9efc19868da6032ea4f1 + 5 ea4b6ce59436a2ccfa67 + 5 31f5e38f8d68c839c6f6 "); let commit_ids = commits.iter().map(|c| c.id().clone()).collect_vec(); diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index bdfe247ceeb..d626f13158f 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -562,7 +562,7 @@ pub fn dump_tree(store: &Arc, tree_id: &MergedTreeId) -> String { &mut buf, "tree {}", tree_id - .to_merge() + .as_merge() .iter() .map(|tree_id| tree_id.hex()) .join("&")