Re: [PATCH RFC 00/11] Introduce git-history(1) command for easy history editing
- From
Elijah Newren <newren@gmail.com>
- Date
- Dec 19, 2025, 16:30 UTC
- Message-ID
- <CABPp-BGE1PC0RhpkfABUL74Yade6HkMQd35bv0my9A2+1VY6AA@mail.gmail.com>
- In-Reply-To
- <aUVDax0PbkaXGB61@pks.im>
On Fri, Dec 19, 2025 at 4:22 AM Patrick Steinhardt <ps@pks.im> wrote:
>
[...]
Show 14 quoted lines
> Okay, so the majority of folks here seem to favor rewriting all > dependent branches, which is also the default that JJ uses here, and > git-replay(1) does it, too. > > There is one major difference between git-replay(1) and git-history(1) > though: the former works with revision ranges, whereas the latter does > not. By using revision ranges we avoid the problem I have mentioned in a > different branch of this discussion, which is that we have no easy way > to figure out which branches we'd have to touch in the first place. This > is because we simply walk the revision range there and then look at > which of our references point into that range. That's simple enough. > > But in our case we're not working with ranges, we are working with a > singular commit.
I don't understand the distinction at all. `git replay edit` also took a single commit, and then implemented the obvious (and jj-like) behavior of rewriting all branches that descended from that commit.
> In my head this meant that we'd have to basically do a > revision walk that starts from all of our branches so that we can figure > out which of them would eventually reach the commit that we are about to > rewrite.
Yes, and it's only a few lines of code, as I showed earlier.
> And that of course doesn't scale.
That's quite an assumption about scaling; I don't believe it. Under what conditions would this be slow enough for users to notice and be bothered? commit-graphs not enabled + weird local clone with thousands of local branches? Also, isn't jj specifically designed for large repositories and with scaling in mind, and yet this is their default behavior?
More importantly, this is being used to justify a large principle of least astonishment violation (disconnecting branches with shared history), so we'd not only need to show that walking all branches was slower enough for users to notice, but slower enough that the negative user performance experience offsets the negative user experience from the astonishing behavior. Typically, spending extra cycles to provide users with good warnings/errors is a good use of time, especially when it'll take them far longer to discover and recover from negative surprises.
> Now we could of course also introduce ranges into git-history(1). That > would indeed solve the issue,
I actually don't follow; how would this help? I'm not even sure how it would make sense; am I missing something?
> as we can reuse the same architecture as > we already have in git-replay(1). But I don't really want to go there as > it is leaking complexity to the user: they want to rewrite a single > commit, why should they have to think about ranges?
I totally agree that they shouldn't have to think about ranges. They rewrite a commit and every branch that descends from it is rewritten for them. If the commit the user tried to edit was part of an immutable branch (to be implemented later), by default you throw an error. For the special cases where users do want to disconnect connected histories, you can provide an option for users to specify that they only want the current branch (or only the branches which the current branch contains).
> But now that I've thought about the problem a bit I think we can avoid > that issue by implicitly identifying the range
Yes! As I've been saying, anything that descends from the commit being rewritten.
> it's all the commits > between the commit we're about to rewrite and HEAD.
Huh?
Show 9 quoted lines
> So, same as with > git-replay(1), the set of branches that we'd need to rewrite is any one > branch that points into that range. It keeps the UI simple as the user > still only has to think about a singular commit, should be sufficiently > fast to compute in most cases, and it allows mega-merge workflows like > JJ supports. > > Does that make sense to everyone? If so, I'll revise my stance and will > adapt the current implementation to do exactly that.
No, it doesn't make any sense to me at all. It'll avoid the principle of least astonishment violation in some cases, but leave it present for others (e.g. other branches which contain this one, or other branches which share the specified commit even if they diverge afterwards). I think we shouldn't have a principle of least astonishment violation. There are three ways to avoid such a violation:
(1) rewrite all branches (refs/heads/*) that descend from the commit (code for this already previously provided and is shorter than the existing code in this series) (2) walk all branches that descend from the commit so we can give the user a warning/error when multiple branches are affected (3) force the user to be explicit about what they want. Provide a set of mutually exclusive flags, and error if none are provided. One flag would be for rewriting all branches, one would be for rewriting only the current branch, and we could add others (e.g. rewriting all branches contained in the current branch).
I don't think retaining this POLA violation makes sense as a starting point.