squash -i squashes things I didn't expect

[mr-flannery]

I'm currently trying out jj and am pretty impressed so far.

I managed to create a scenario in which I don't understand why squash -i behaves the way it does.

Here's my log:

@      oozyuxyy [email protected] 2024-10-27 16:13:27 1faf5008
├─┬─╮  merge
│ │ ○  tusrvmrw [email protected] 2024-10-27 15:12:51 feat/part3 337eaedd
│ │ │  merge branches
│ │ ○  kqkqpxvz [email protected] 2024-10-27 15:12:51 feat/part2 889011b2
│ │ │  add another change on top
│ ○ │  ywtvrslw [email protected] 2024-10-27 15:13:14 push-ywtvrslwmkpw* 63de33e1
│ ├─╯  blerg3
○ │  sywxmoxz [email protected] 2024-10-27 15:16:15 push-sywxmoxzwnxm* HEAD@git 716901aa
├─╯  (empty) blerg2
◆  lwoppnxo [email protected] 2024-10-27 15:12:01 main main@origin 6985a01e
│  (empty) Merge pull request #12 from D069022/feat/part1
~

Here's the relevant snippet of my code:

// this is a change
// and this is a change (no wait this sucks)
// and this is also a change
function two() {
  return 222;
}

I add "(no wait this sucks)" to line 2 and want to squash it into kqkqpxvz. In k the code currenty looks like this:

// this is a change
// and this is a change
function two() {
  return 222;
}

Now I run jj squash --into k -i:

So far, so good, this is what I expected, since this is the only change that I made on o (the other changes come from other parents). However, applying the squash results in a conflic in k:

@      pyprzpmx [email protected] 2024-10-27 16:22:09 8276dcf1
├─┬─╮  (empty) (no description set)
│ │ ○  tusrvmrw [email protected] 2024-10-27 16:22:09 feat/part3 9a61a1f7
│ │ │  merge branches
│ │ ×  kqkqpxvz [email protected] 2024-10-27 16:22:09 feat/part2* de94a7f4 conflict
│ │ │  add another change on top
│ ○ │  ywtvrslw [email protected] 2024-10-27 15:13:14 push-ywtvrslwmkpw* 63de33e1
│ ├─╯  blerg3
○ │  sywxmoxz [email protected] 2024-10-27 15:16:15 push-sywxmoxzwnxm* HEAD@git 716901aa
├─╯  (empty) blerg2
◆  lwoppnxo [email protected] 2024-10-27 15:12:01 main main@origin 6985a01e
│  (empty) Merge pull request #12 from D069022/feat/part1
~

And the code now looks like this:

// this is a change
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
 // and this is a change
-// and this is also a change
+++++++ Contents of side #2
// and this is a change (no wait this sucks)
// and this is also a change
>>>>>>> Conflict 1 of 1 ends
function two() {
  return 222;
}

There are two things I don't understand about this:

1. Why is // and this is a change (first of the two lines) part of the conflict? I only added to the line, so I would not have expected this to be a conflict.
2. Why is // and this is also a change part of the content of side #2? The line wasn't part of the interactive squash, so I wouldn't have expected it to be applied here.

Looking forward to your input! Thanks!

[ilyagr]

It's not clear to me what's going on.

Perhaps you meant to squash from tus to k and ended up squashing from the merge commit to k?

You could use jj op log to find the operation id from before the squash, and then post (or look at) jj log --at-op <that_op> to see what the current commit was, and jj diff --at-op <that_op>.

It's also possible that you're seeing a bug having to do with the built-in diff editor or jj improperly handling squashing from a commit resolving a conflict. You could try to reproduce it with --tool meld or some other external tool.

If you .tar.gz the entire repo, we might be able to help looking at the operation log (though I can't promise to do it immediately).

[mr-flannery]

Thanks for the prompt response!

Perhaps you meant to squash from tus to k and ended up squashing from the merge commit to k?

I did mean to specifically squash from the merge commit, though I'm wondering whether that should make a difference. Both tus and o are descendants of k, i.e. both include the line that has originally been added in k and that I'm trying to modify, and squash that modification back into k. So I'd expect it to work the same from both changes.

You could use jj op log to find the operation id from before the squash, and then post (or look at) jj log --at-op <that_op> to see what the current commit was, and jj diff --at-op <that_op>.

Here's the diff from before the squash:

$ jj diff --from k --to @ --at-op ac947e61e0a4
Modified regular file main.js:
    ...
  11   11: }
  12   12:  //fghjkl
  13   13: function three() {
       14:   console.log("blergenstergen") ### from `t`
  14   15:   return 33332;
  15   16: }
  16   17: 
  17   18: // this is a change
  18   19: // and this is a change (no wait this sucks) ### from `o`
  18   20: // and this is also a change ### from `t`
  19   21: function two() {
  20   22:   return 222;
  21   23: }
  22   24: 
       25: // dfghjklö ### from `y`
  23   26: function one() {
  24   27:   console.log("oneeeeeeeeeee");
  25   28: }
    ...

I annotated the diff with where the changes are coming from. I also reran the squash command and checked again what is and is not applied. The squash applies:

  18   19: // and this is a change (no wait this sucks) ### from `o`
  18   20: // and this is also a change ### from `t`

so one change from t that is unintended and one from o that is intended. At the same, the following changes are not being applied:

       14:   console.log("blergenstergen") ### from `t`
       25: // dfghjklö ### from `y`

So y is being ignored (fair enough), but also a change fromt is being ignored, which confuses me. I would have expected t to either be applied completely or not at all.

It's also possible that you're seeing a bug having to do with the built-in diff editor or jj improperly handling squashing from a commit resolving a conflict. You could try to reproduce it with --tool meld or some other external tool.

I used git-delta, meld behaved weirdly on my machine 🤷
Here's what happens:

$ jj squash --into k -i --tool=delta
added: var/folders/f6/55cxd8z148x_cy4v_sqd46wm0000gn/T/jj-diff-KCWSfS/right/JJ-INSTRUCTIONS
─────────────────────────────────────────────────────────────────────────────────────────────

───┐
1: │
───┘
You are moving changes from: oozyuxyy 1faf5008 merge
into commit: kqkqpxvz 889011b2 feat/part2 | add another change on top

The left side of the diff shows the contents of the parent commit. The
right side initially shows the contents of the commit you're moving
changes from.

Adjust the right side until the diff shows the changes you want to move
to the destination. If you don't make any changes, then all the changes
from the source will be moved into the destination.

var/folders/f6/55cxd8z148x_cy4v_sqd46wm0000gn/T/jj-diff-KCWSfS/left/main.js ⟶   var/folders/f6/55cxd8z148x_cy4v_sqd46wm0000gn/T/jj-diff-KCWSfS/right/main.js
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

───────────────────────┐
16: function three() { │
───────────────────────┘
}

// this is a change
// and this is a change
// and this is a change (no wait this sucks)
:

It shows the same lines. However, when running git diff manually on the commits, it reports the same as running jj diff manually:

$ git diff 889011b2 1faf5008
diff --git a/main.js b/main.js
index c2796de..c92110e 100644
--- a/main.js
+++ b/main.js
@@ -11,15 +11,18 @@ function print(str) {
 }
  //fghjkl
 function three() {
+  console.log("blergenstergen")
   return 33332;
 }
 
 // this is a change
-// and this is a change
+// and this is a change (no wait this sucks)
+// and this is also a change
 function two() {
   return 222;
 }
 
+// dfghjklö
 function one() {
   console.log("oneeeeeeeeeee");
 }

Is it possible that the input for the interactive diff editor is not the same as for the squash itself?

If you .tar.gz the entire repo, we might be able to help looking at the operation log (though I can't promise to do it immediately).

Absolutely :) Here's the repo:
jj-playground.tar.gz

[ilyagr]

I didn't follow all you wrote here (yet, sorry about that), but I'm wondering if the problem is that the squash command doesn't do quite what you think it does. In particular, I'm responding (perhaps too quickly) to your statement that "So I'd expect it to work the same from both changes", which is wrong (in general, at least).

Let's say we have this tree:

X     file = 3
| Y   file = 2
|/
A     file = 2
B     file = 1

Then, jj restore --from Y --to B would set file to 2 in B, as one would probably expect.

However, jj squash --from Y --to B is quite different. In this case, it would not do anything. This is because Y is an "empty commit"; its diff relative to its parent is empty. jj squash is intended to squash this diff into B, so it has nothing to do for an empty diff.

Going back to the statement "So I'd expect it to work the same from both changes.", note that in this case, jj squash --from Y --to B is quite different from jj squash --from A --to B (which, confusingly, does the same thing as jj restore --from A --to B; the operations coincide if B is the direct and only parent of A).

Update: I confusingly used Z below, I meant X (now fixed)

Meanwhile, jj restore --from X --to B would set file to 3 in B, unsurprisingly. This would also cause a conflict in A, since it tries to apply a diff of "file=1 -> file=2" to "file=3".

However, jj squash --from X --to B would cause a conflict. This is because X's diff relative to parent is "file = 2 -> file = 3". The squash operation would try to apply this diff to "B", which has value "file = 1"; this cannot be done without a conflict.

Does this make sense? Also, apologies if I still misunderstand the problem and you know all this.

[mr-flannery]

but I'm wondering if the problem is that the squash command doesn't do quite what you think it does.
I'm pretty sure that's where the problem lies 😅 Let me try to rephrase what I think I understood from your message:

jj squash works on the diff to its parent change. At some level I think that does explain it, but at some level I'm still confused. I deleted line 20 (// this is also a change) from t, then went back to o, applied the same edit (i.e. added (no wait this sucks) to line 19), then ran jj squash --into k -i again, and now it worked. Which, given the fact that squash works on diffs, makes sense because now that nothing around line 19 changed, it can still apply the diff "across t" to k. But doesn't make intuitive sense to me (not having any knowledge of diff algorithms) is why a new line after line 19 causes the diffs to "break" between k -> t and t -> o. (I hope that phrasing made sense).

Regardless of whether my understand is correct or not, is there a recommend way to squash (or by other means back propagate) a such a (code) change across multiple (jj) changes? I could of course just resolve the conflict in k, but then t is also conflicted and has to be resolved, so that's kinda tedious.

Alternatively, I just tried squashing from o to t and then from t to k, which worked without any conflicts, but is generally also tedious across larger distances. Is there a way to do this without conflicts in a single step, or does not simply not work given how diffs work in jj?

[arxanas]

Since it's a case of moving a line into an ancestor, an interleaved deltas approach might work without conflict, like how https://sapling-scm.com/docs/dev/internals/linelog is used for the sl absorb subcommand. jj doesn't implement this at present.

[yuja]

fwiw, I'm thinking of adding jj absorb, but the initial version wouldn't use weave for hunk propagation, so it wouldn't help resolve hunks with no context lines.

We can also do word-level conflict resolution, but I'm not sure if it should be enabled by default.