jj move from parent to child creates conflict

[matts1]

Steps to Reproduce the Problem

Create an empty jj repo, then:

echo "foo
bar
baz" > file
jj new -m "child"
jj move --from @- --to @ --interactive

Then select just the line "bar"

I came across this use case by running jj split, then realizing I missed putting something in the child, so I expect this to be a somewhat common use case.

Expected Behavior

Neither commit should have a conflict in it

Actual Behavior

The parent commit has a conflict which is then resolved in the child

Specifications

• Platform: Linux
• Version: jj google-0.17.1-jj-client_20240515_00_RC00-633966000

[matts1]

Also, just double checked. The same problem also applies to squash, which is supposed to replace move.

[martinvonz]

jj google-0.17.1-jj-client_20240515_00_RC00-633966000

That's an internal version. Does the problem happen with a regular upstream build too?

[ilyagr]

I think I can reproduce this. The conflict I get in the parent is:

<<<<<<< Conflict 1 of 1
+++++++ Contents of side #1
foo
bar
baz
%%%%%%% Changes from base to side #2
-bar
>>>>>>>

I was wondering if this has to do with tracking missing files in conflicts (like the bug about deletion-emptyfile conflict), but I no longer think this is the case. This bug seems to happen even if you start with an empty file file (instead of creating the file in the parent commit).

[ilyagr]

If you start with

foo
bar
baz
oof
zab

and leave only

bar
baz

during the split, there is no conflict by the way. It feels like this might be about reaching some threshold in the diff algorithm.

Update: I used the wrong command. If you do a squash, a very similar conflict appears.

[bnjmnt4n]

Correct me if I'm wrong, but it seems like there will always be a conflict when removing changes from any commit when doing a squash operation if there are hunks with more than 1 line, with at least 1 and less than all the lines in the hunk being selected. This means that there will always be a conflict when "removing" the selected lines for squashing, since the 3-way merge will result in a conflicting hunk (added hunk, a few lines selected from added hunk, empty hunk) which cannot be resolved trivially.

It seems like this isn't an issue most of the time because squashing is typically done from descendant to ancestor, and the conflict will be resolved after rebasing onto the modified descendant.

[matts1]

Suppose you have a commit A with the file f containing:

foo
bar
baz

If we try to move the line bar to a parent commit P (the usual case), I believe what happens internally roughly corresponds to (algorithmically speaking - obviously it doesn't actually run these commands):

1. Run jj split, creating a parent commit A' containing +bar. Commit A now contains +foo,+baz
2. Rebase commits such that A' is a direct child of P (this is a no-op, since it already is)
3. Merge A' and P. Since A' is now the direct child of P, the merge is trivial (just use the tree A')

If we try to move the line bar to a child commit C, on the other hand, I believe this is what happens:

1. Run jj split, creating a parent commit A' containing +bar. Commit A now contains +foo,+baz
2. Rebase commits such that A' is a direct child of C (this is a no-op, since it already is)

• This creates a merge conflict because we had the commits A', A, and then C, and swapping the ordering of A and A' creates a merge conflict in A, but is resolved in A'

3. Merge A' and C. Since A' is now the direct child of C, we just use the tree A'

• Now, A has a conflict, but it's resolved in C

I think it'd be trivial to fix this by simply changing the ordering. I think we could do the following instead, which would be conflict-free (the differences are in bold):

1. Run jj split on the inverse of the things to be moved (+bar,+baz), creating a child commit A' containing +bar. Commit A now contains +foo,+baz
2. Rebase commits such that A' is a direct parent of C (this is a no-op, since it already is)
3. Merge A' and C. Since A' is now the direct parent of C, we just use the tree C

[matts1]

I happened to notice jj unsquash today. It seems to do what I specified, but in a more constrained form (no --from and --to), only supporting a revision and it's direct parent.

FWIW, my two cents on this are:

• jj move should be the canonical, rather than jj squash. I think move provides a far better mental model for what it actually does - moves a patch between Commits.
  • I think that the beauty of jj is in its simplicity, and it's sufficiently simple that rather a command being named after how it's intended to be used, it should be named after what it does, since a user can tell how to use it much more easily than something like git.
  • For example, cherry-pick in git has been replaced with duplicate in jj, which I think is a great name.
• jj squash should be an alias for jj move, if we even want it to exist
• jj unsquash should not exist.

I'm interested in whether my take is controversial, or whether others agree with me.

[bnjmnt4n]

I think this change actually went the opposite way (jj move was deprecated for jj squash). See #2882 and #3271. As mentioned in the discussion, move is a bit ambiguous since it could refer to moving commits (although that's done by jj rebase), or moving files.

[matts1]

Yeah, I was aware it went the opposite way, I was mostly just voicing my thoughts on it. Move being ambiguous is a good point though.

I still maintain that squash and unsquash just adds confusion though.

[matts1]

I've investigated further. Suppose we have a source commit S a parent commit P, and we select the diff D from S - P. Suppose in our example, S = "abca", P = "", and we select the first "a" as our diff.

• We currently do is S = S - (P + D) + P.
• That is, "abca" - "a" + ""
• Hence, we get a conflict, since we can't determine which "a" it's trying to remove
• Even if the A wasn't repeated, this is basically like rebasing abc from a parent b onto the empty string, which will also conflict

In reality, what we need is S = S + ~D

• ~D can be calculated from the builtin diff tool, as it returns scm_record::File objects which contain diffs.
• External diff tools, however, return files (P + D, not D), and thus cannot reconstruct ~D.

It appears that unsquash solves this problem by directing you to select the contents you want to keep in the parent commit, rather than the contents you want to move. We could potentially do the same, but I don't think it'd be a great user experience, so I wouldn't recommend it. One option we could take is to add a --invert flag, so instead of marking what we want to move we mark what we want to keep, but honestly that doesn't feel great either.