rewrite: don't resolve intermediate parent tree when rebasing

Let's say we're updating one parent of a merge:


```
  E            E'
 /|\          /|\
B C D   ->   B C D'
 \|/          \|/
  A            A
```

When rebasing `E` to create `E'` there, we do that by merging the
changes compared to the auto-merged parents. The auto-merged parents
before is `B+(C-A)+(D-A)`, and after it's `B+(C-A)+(D'-A)`. Then we
rebase the diff, which gives us `E' = B+(C-A)+(D'-A) + (E -
(B+(C-A)+(D-A))) = D' + (E - D')`.

However, we currently don't do quite that simplification because we
first resolve conflicts when possible in the two auto-merged parent
trees (before and after). That rarely makes a difference to the
result, but it's wasteful to do it. It does make a difference in some
cases where our merge algorithm is lossy, which currently is only the
"A+(A-B)=A" case. I added a test case showing where it does make a
difference. It's a non-obvious cases but I think the new behavior is
more correct (the old behavior was a conflict).

lib/src/rewrite.rs

@@ -249,8 +249,16 @@ impl<'repo> CommitRewriter<'repo> {
  self.old_commit.tree_id().clone(),
  )
  } else {
- let old_base_tree = merge_commit_trees(self.mut_repo, &old_parents)?;
- let new_base_tree = merge_commit_trees(self.mut_repo, &new_parents)?;
+ let old_base_tree = merge_commit_trees_no_resolve_without_repo(
+ self.mut_repo.store(),
+ self.mut_repo.index(),
+ &old_parents,
+ )?;
+ let new_base_tree = merge_commit_trees_no_resolve_without_repo(
+ self.mut_repo.store(),
+ self.mut_repo.index(),
+ &new_parents,
+ )?;
  let old_tree = self.old_commit.tree()?;
  (
  old_base_tree.id() == *self.old_commit.tree_id(),

lib/tests/test_merge_trees.rs

@@ -645,3 +645,65 @@ fn test_simplify_conflict_after_resolving_parent() {
 
 // TODO: Add tests for simplification of multi-way conflicts. Both the content
 // and the executable bit need testing.
+
+#[test]
+fn test_rebase_on_lossy_merge() {
+ let settings = testutils::user_settings();
+ let test_repo = TestRepo::init();
+ let repo = &test_repo.repo;
+
+ // Test this rebase:
+ // D foo=2 D' foo=3
+ // |\ |\
+ // | C foo=2 | C' foo=3
+ // | | => | |
+ // B | foo=2 B | foo=2
+ // |/ |/
+ // A foo=1 A foo=1
+ // Commit D effectively discarded a change from "1" to "2", so the result should
+ // be "3".
+ let path = RepoPath::from_internal_string("foo");
+ let mut tx = repo.start_transaction(&settings);
+ let repo_mut = tx.repo_mut();
+ let tree_1 = create_tree(repo, &[(path, "1")]);
+ let tree_2 = create_tree(repo, &[(path, "2")]);
+ let tree_3 = create_tree(repo, &[(path, "3")]);
+ let commit_a = repo_mut
+ .new_commit(
+ &settings,
+ vec![repo.store().root_commit_id().clone()],
+ tree_1.id(),
+ )
+ .write()
+ .unwrap();
+ let commit_b = repo_mut
+ .new_commit(&settings, vec![commit_a.id().clone()], tree_2.id())
+ .write()
+ .unwrap();
+ let commit_c = repo_mut
+ .new_commit(&settings, vec![commit_a.id().clone()], tree_2.id())
+ .write()
+ .unwrap();
+ let commit_d = repo_mut
+ .new_commit(
+ &settings,
+ vec![commit_b.id().clone(), commit_c.id().clone()],
+ tree_2.id(),
+ )
+ .write()
+ .unwrap();
+
+ let commit_c2 = repo_mut
+ .new_commit(&settings, vec![commit_a.id().clone()], tree_3.id())
+ .write()
+ .unwrap();
+ let commit_d2 = rebase_commit(
+ &settings,
+ repo_mut,
+ commit_d,
+ vec![commit_b.id().clone(), commit_c2.id().clone()],
+ )
+ .unwrap();
+
+ assert_eq!(*commit_d2.tree_id(), tree_3.id());
+}