`rebase --skip-empty` on merge commits doesn't remove deleted parents

[matts1]

Description

Given the chain:

C
| \
A  B
| /
old_main

If you submit B, fetch, then run rebase --skip-empty, we currently give the user:

C' --------+
|          |
A'         |
|          | 
new_main   |
|          |
...        |
|          |
B' --------+

However, ideally, we should rewrite it to:

C'
|
A'
|
new_main

I propose that after doing the rebase with --skip-empty, when rewriting a commit C to C', instead of setting the parents of a commit to be the revset p, we instead set it to be heads(p). In this example, we would, instead of setting it to the revset A' | B', set it to heads(A' | B'), which would evaluate to A'.

Would this work, or am I missing some edge cases?

[bnjmnt4n]

I guess this is somewhat related to https://github.com/martinvonz/jj/issues/3565.

[matts1]

After reading that thread, I agree that it's related, but I think that even if we choose not to simplify ancestor merges in general without a --simplify-merges flag, semantically --skip-empty means "If this code has already been submitted, please delete the local commits". I think in a case like this, keeping a reference to that submitted commit doesn't make much sense.

[martinvonz]

FWIW, the jj piper sync command we have at Google does simplify merges. I think we should make the planned jj git sync also do that. I hope to get back to that command some day...

[matts1]

Until that command has been implemented, I've had success with git fetch && jj rebase -s 'all:roots(mutable())' -d 'trunk()' --skip-empty

[bnjmnt4n]

We now have the simplify-parents command which can be explicitly used to simplify merges. I'm not sure if this should be accepted because right now --skip-emptied refers to skipping commits, and overloading it to mean both skipping emptied commits and parent edges doesn't seem like a good idea. Perhaps we should add a --simplify-merges flag to rebase?