jj icon indicating copy to clipboard operation
jj copied to clipboard

FR: `jj split --horizontal/--siblings/--sideways`

Open martinvonz opened this issue 2 years ago • 8 comments

Is your feature request related to a problem? Please describe.

Sometimes it's useful to split a commit "sideways", producing two or more siblings instead of a chain of commits.

Describe the solution you'd like

We talked about adding a way of splitting commits sideways on Discord at some point. The same request just came up for Mercurial in a chat group at Google.

Describe alternatives you've considered

An alternative is to let the user finish splitting the commits and then let them flatten the result using something like #1079.

martinvonz avatar Sep 19 '23 23:09 martinvonz

I keep running into the lack of this. In my case, jj split --sibling -i seems like the most natural way to get something like git stash -p — I want to split off some part of my current changes (so they leave the working copy) and come back to them later. Naturally, they have to be based on the same parent as the working copy.

solson avatar Nov 11 '23 02:11 solson

This shouldn't be hard to implement, if you're interested. Let's say we have history like this:

D
|\
B C
|/
A

The user now runs jj split --siblings -r B and we call the diff editor as usual. The output from that is the tree contents of the first commit. We would create commit B1 with the same parents as B, i.e. A. We would then create commit B2 by merging the changes between the new B1 and B into A's parent tree. We would then rewrite the children of B (i.e. D) to have B1 and B2 as parents instead of B. So in this case, D would have B1, B2, and C as parents. I don't quite remember, but it looks like we might be able to just add both new commits where we currently have one commit here.

martinvonz avatar Nov 11 '23 06:11 martinvonz

Thanks for the explanation! I may have time to work on some jj PRs some weekend soon.

solson avatar Nov 11 '23 12:11 solson

I'm going to look into implementing this, but I'd like a clarification. It sounds like Solson and Martin are describing different things.

I think that Solson is describing a command that does something like this:

jj split --siblings -r B

C      C
|      |
B  ->  B1  B2       
|      |  /
A      A--

I think Martin is describing a command that does this:

jj split --siblings -r B

C        --C--
|       /     \
B  ->  B1     B2       
|       \     /
A        --A--

Is this feature meant to do 1, 2, both, or something completely different?

emesterhazy avatar Feb 06 '24 18:02 emesterhazy

jj split --siblings -r B

C        --C--
|       /     \
B  ->  B1     B2       
|       \     /
A        --A--

This is what I would expect, personally. In my earlier comment I was referring to a usecase where the revision I'm splitting has no children in the first place, i.e.

jj split --siblings

B  ->  B1     B2       
|       \     /
A        --A--

Where B is the working copy and A is the working copy parent, and B1 is the new working copy afterwards. But this is just a special case.

solson avatar Feb 06 '24 18:02 solson

Ok got it. So then to get to the first state we could do this:

jj split --siblings -r B

C        --C--
|       /     \
B  ->  B1     B2
|       \     /
A        --A--

jj rebase -s C -d B1

  --C--      C
 /     \     |
B1     B2 -> B1  B2
 \     /     |  /
  --A--      A--

emesterhazy avatar Feb 06 '24 19:02 emesterhazy

I have this halfway working in #3037. By halfway, I mean that it is able to handle the simple case:

jj split --siblings

B  ->  B1     B2       
|       \     /
A        --A--

I'm still familiarizing myself with the internals to figure out the best way to handle the case where commit B has children that need to be rebased onto B1 and B2. If anyone has suggestions please let me know. Adding both siblings to the set_rewritten_commit call doesn't work and just ends up creating a divergent commit.

In that case, the log looks like this, where the commits with descriptions "Add file3" and "Add file1 and file2" are the originals and qtulxkyw is the commit being split.

◉  prnuroxt emesterhazy@ 2024-02-13 10:30:06.000 -05:00 83845508
│  Add file2
│ @  qtulxkyw?? emesterhazy@ 2024-02-13 10:30:03.000 -05:00 09f4d061
├─╯  Add file1
│ ◉  spltwkum emesterhazy@ 2024-02-13 10:00:45.000 -05:00 a0726d5f
│ │  Add file3
│ ◉  qtulxkyw?? emesterhazy@ 2024-02-11 10:58:42.000 -05:00 c98b9d59
├─╯  Add file1 and file2

I think I'm going to need to change how rebase_descendants works or manually rebase the children of the split commit onto the new siblings.

emesterhazy avatar Feb 13 '24 15:02 emesterhazy

set_rewritten_commit call

I actually thought that would work. Anyway, the way regular (serial) split is a bit weird and it's probably a good idea to do the rebasing of children manually for that case, so maybe it's good to do it for sibling split as well. The drawback is that we would need to replicate some of the logic from rebase_descendants(), in particular the logic for updating branches (should presumably point to both commits) and the working copy (we'll have to pick one of the commits).

martinvonz avatar Feb 13 '24 16:02 martinvonz

I think I have this done and ready for review and polish in #3037.

emesterhazy avatar Mar 31 '24 01:03 emesterhazy

I'm not sure all of the considerations in https://github.com/martinvonz/jj/issues/2274#issuecomment-1941992244 are addressed though. I'm still setting the rewritten commit to the second commit, so any branches are moved to the second commit and it is checked out as the working copy.

I don't think that checking out the second commit is an issue (presumably we have to pick one), but perhaps we should leave any branches in a conflicted state. Thoughts?

emesterhazy avatar Mar 31 '24 01:03 emesterhazy

I don't think that checking out the second commit is an issue (presumably we have to pick one), but perhaps we should leave any branches in a conflicted state. Thoughts?

Good question. I think it would make more sense to leave the branch in a conflicted state, but it seems like it's practically quite useful to leave it on one of the commits instead. That way it requires action from the user in some of the cases instead of all of the cases. I'd say we should leave it that way for now and change it later if we hear that it's confusing - or worse, that it leads people to push the wrong commit to a remote.

By the way, we have had a discussion internally at Google about where regular (parent/child) jj split should put the branch. It currently leaves it on the child, but it lets the parent keep the change id. We use branches for tracking code reviews at Google (I know you're at Google, @emesterhazy; I'm just providing context for others). For that use case, it's confusing that the branch doesn't follow the change id. It makes sense for the branch to move to the child in more Git-like workflows, since branches typically point to the tip of a chain of commits in Git. However, I don't want us to design a behavior that makes less sense just because it better matches Git. What do people think?

martinvonz avatar Mar 31 '24 16:03 martinvonz

I created https://github.com/martinvonz/jj/issues/3419 to discuss what happens to branches when a commit is split. Currently both jj split and jj split --siblings move the branch to the second commit, so at least they are consistent. If we want to change the behavior for either of these then let's pick up the discussion over there.

emesterhazy avatar Apr 01 '24 23:04 emesterhazy

Closed by #3037. The new option is --siblings, although in hindsight maybe we should have named the flag --parallelize or --parallel to match jj parallelize. The short name -p would also be less ambiguous than -s, which means source for jj rebase.

If anyone has opinions on the name we can still easily change it until the next release. Feel free to open a new issue :)

emesterhazy avatar Apr 01 '24 23:04 emesterhazy