jj icon indicating copy to clipboard operation
jj copied to clipboard

FR: Allow `jj squash` to span multiple commits.

Open PhilipMetzger opened this issue 1 year ago • 12 comments

We've also discussed this already in Discord. Particularly useful if you need only a single patch for codereview. Ideally this is can be implemented, when/if we make squash an alias for move.

Edit: The insightful Discussion around making squash an alias of move starts here. It also proposes the feature for jj move some messages below.

PhilipMetzger avatar Dec 07 '23 19:12 PhilipMetzger

If we don't make squash and alias for move (I haven't thought much about that), I think jj move -r A::B --to C would make sense as an interface. I don't think we'd have to implement any version of this in squash, it feels confusing at first glance.

ilyagr avatar Dec 29 '23 00:12 ilyagr

I suspect we shouldn't make squash an alias.

I think jj move -r A::B --to C would make sense as an interface

There's currently jj move --from A --to C. I would suggest extending --from to accept more than a single revision. I'm not sure if we should require the all: prefix.

martinvonz avatar Dec 29 '23 01:12 martinvonz

Absolutely, I meant to say --from.

I would probably require all:, that's also a good point. In principle, we could make an exception for renders of the form A::B, but that seems a bit too complicated for the benefit.

So, the above turns into something like

jj move --from all:A::B --to C

ilyagr avatar Dec 29 '23 02:12 ilyagr

I don't think there should be a command called move that deals with commits. Other VCSs use the move concept for files. See https://en.wikipedia.org/wiki/Comparison_of_version-control_software#Basic_commands

joyously avatar Jan 30 '24 23:01 joyously

I don't think there should be a command called move that deals with commits. Other VCSs use the move concept for files. See https://en.wikipedia.org/wiki/Comparison_of_version-control_software#Basic_commands

Yeah, that's totally fair. As both https://github.com/martinvonz/jj/issues/2678#issuecomment-1871651445 and https://github.com/martinvonz/jj/issues/2678#issuecomment-1871667071 hinted at, this shouldn't be the way forward. But recently there has been a discussion (#2882) to move jj move into jj squash which everyone probably prefers.

PhilipMetzger avatar Jan 31 '24 18:01 PhilipMetzger

Let’s leave jj squash to deal with a single revision. For dealing with a revset, I suggest a new command jj collapse -r REVISIONS.

The way it would work is up for debate. My idea is to identify the roots and the heads within the revset, associate each root with the set of heads that are descended from it, virtually merge each set of heads, and finally “collapse” the merged sets of heads down to their respective roots. In the final result only the roots of the revset remain, and their contents are of the virtual merges of their descendant heads; the descendants of the former heads of the revset are rebased onto the roots of it.

In the simple case of a single linear descent, the result is intuitive. In other cases, maybe an all: prefix or some other form of acknowledgement should be required. The other cases may get as ugly as overlapping sets of descendant heads, which themselves may have descendants that are merges… :face_with_head_bandage:

amiryal avatar Feb 20 '24 07:02 amiryal

Let’s leave jj squash to deal with a single revision. For dealing with a revset, I suggest a new command jj collapse -r REVISIONS.

This sounds a lot like what the proposed absorb would be.

joyously avatar Feb 20 '24 14:02 joyously

Let’s leave jj squash to deal with a single revision.

Why though? I think its in our complexity/novelty budget to give it new powers.

For dealing with a revset, I suggest a new command jj collapse -r REVISIONS.

That sounds like sl fold which I find a better synonym, so I'd rather have jj fold if we even go through with it.

The way it would work is up for debate. My idea is to identify the roots and the heads within the revset, associate each root with the set of heads that are descended from it, virtually merge each set of heads, and finally “collapse” the merged sets of heads down to their respective roots. In the final result only the roots of the revset remain, and their contents are of the virtual merges of their descendant heads; the descendants of the former heads of the revset are rebased onto the roots of it.

In the simple case of a single linear descent, the result is intuitive. In other cases, maybe an all: prefix or some other form of acknowledgement should be required. The other cases may get as ugly as overlapping sets of descendant heads, which themselves may have descendants that are merges… 🤕

This really sounds like #170.

PhilipMetzger avatar Feb 20 '24 15:02 PhilipMetzger

That sounds like sl fold

Yes, but if I understand sl fold correctly, it refuses to work on a non-linear chain, and keeps only the head, not the root (like jj unsquash, but unlike jj squash or my suggested jj collapse). Thus, using the same name might be confusing.

This sounds a lot like what the proposed absorb would be.

No; let me try to explain.

What jj squash currently does to a single commit C: move its changes to its parent commit, and if C is left empty, delete it.

What this FR suggests to do to a linear chain of commits of length N: move the cumulated changes from the top N - 1 commits to the first commit and delete the top N - 1 commits.

What my understanding of absorb would do to a single commit C: divide the changes from C, between some or all of its unpublished ancestor commits, by using an algorithm that figures out which chunk of changes should go into which ancestor commit.

The way I suggested that jj collapse would work is a generalisation, from “a linear chain of commits of length N” as described above, to a revset with potentially multiple roots, heads and merges in between, not necessarily interconnected.

amiryal avatar Feb 20 '24 18:02 amiryal

Probably not constructive to this conversation, but I was wanting to do this and I concocted this monstrocity:

jj log -r master+..tzyrpmnw --template 'change_id ++ "\n"' --no-graph | xargs -I {} jj squash -r {} -m "Final commit message"

Which will one-by-one squash all commits in a given range. Just dumping this here in case it is useful to anyone who stumbles across this issue.

julienvincent avatar Feb 21 '24 17:02 julienvincent

What this FR suggests to do to a linear chain of commits of length N: move the cumulated changes from the top N - 1 commits to the first commit and delete the top N - 1 commits.

What I had in mind was actually not to limit it to a linear chain. I think it makes sense to move/squash any number of changes into a given destination. All the source revisions would become empty and abandoned. You could combine that with paths too, which means some of the source revisions might remain. Combining it with -i would be possible but would be annoying to use if it opens up the diff editor once per source revision.

martinvonz avatar Feb 21 '24 17:02 martinvonz

That sounds like sl fold

Yes, but if I understand sl fold correctly, it refuses to work on a non-linear chain, and keeps only the head, not the root (like jj unsquash, but unlike jj squash or my suggested jj collapse). Thus, using the same name might be confusing.

I don't think this will matter as there are way less Sapling users than Git, so we could adopt the name. It will only be confusing initially.

What jj squash currently does to a single commit C: move its changes to its parent commit, and if C is left empty, delete it.

What this FR suggests to do to a linear chain of commits of length N: move the cumulated changes from the top N - 1 commits to the first commit and delete the top N - 1 commits. [...] The way I suggested that jj collapse would work is a generalisation, from “a linear chain of commits of length N” as described above, to a revset with potentially multiple roots, heads and merges in between, not necessarily interconnected.

Yes, that seems to be a agreeable solution, although having the option to squash everything would be better.

What my understanding of absorb would do to a single commit C: divide the changes from C, between some or all of its unpublished ancestor commits, by using an algorithm that figures out which chunk of changes should go into which ancestor commit.

Yes, thats also my understanding.

What this FR suggests to do to a linear chain of commits of length N: move the cumulated changes from the top N - 1 commits to the first commit and delete the top N - 1 commits.

What I had in mind was actually not to limit it to a linear chain. I think it makes sense to move/squash any number of changes into a given destination. All the source revisions would become empty and abandoned. You could combine that with paths too, which means some of the source revisions might remain. Combining it with -i would be possible but would be annoying to use if it opens up the diff editor once per source revision.

That's my favorite solution so far. It'd also make sense if we follow-through with #2882 .

PhilipMetzger avatar Feb 21 '24 18:02 PhilipMetzger