`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 https://github.com/martinvonz/jj/discussions/2882 and https://github.com/martinvonz/jj/pull/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.

[WaffleLapkin]

Just gotten into this exact issue (tryng to squash a change into descendant always causes a conflict). Is there any progress on solving this issue / can I help?

[martinvonz]

There hasn't been any progress. I'm not even sure what we can do about this. The problem is that the changes you select in the diff editor are applied in reverse to the source commit, so if you're selecting only the "bar" line to move out, then that's a change from an empty (or missing) file to one that contains only "bar\n", so the patch that's applied to the parent is one that removes "bar\n" from a file so it becomes empty. That doesn't apply to a file that contains "foo\nbar\nbaz\n".

One thing I can think of is to add a jj squash -i --select-what-to-keep to select what to keep instead of selecting what to move out of the commit as we currently do. But my current feeling is that this feature is not requested frequently enough that it's worth having that option. I suspect many users who run into this case won't think to try that option even if we have it.

[WaffleLapkin]

Why would this be an option? Since this is only needed if squashing into descendants (and always happens if you do so!), we can "just" check if the --into change is descendant of the --from change, in which case we need to change the behavior.

[bnjmnt4n]

Squashing into a descendant does work in some cases; IIRC it's only when part of a hunk is selected then these types of conflicts occur. I think it would also be confusing if the behavior of the command inverted based on that.

[WaffleLapkin]

Squashing into a descendant does work in some cases; IIRC it's only when part of a hunk is selected then these types of conflicts occur.

I would argue that selecting parts of a hunk is a pretty common operation.

I think it would also be confusing if the behavior of the command inverted based on that.

I'm not sure I follow. If the implementation needed for squashing part of a hunk inverts what is kept/moved, couldn't we also invert selection, so that what is kept/moved is still the same wrt the selection?

[arialdomartini]

When it comes to moving some changes from the current revset to its direct descendant, instead of:

jj squash --to <descendant> --interactive

that would produce a conflict in @, one could use:

jj restore --interactive --restore-descendants

or

jj diff-edit --restore-descendants

The former lets you select the lines to be removed from @ and moved to its descendant. The latter, lets you select what to keep.

I could not find anything else to make this work but with the immediate direct descendant.