* Feature request: rebase -i inside of rebase -i @ 2020-03-20 22:30 George Spelvin 2020-03-20 22:51 ` Junio C Hamano 2020-03-21 8:47 ` Johannes Sixt 0 siblings, 2 replies; 25+ messages in thread From: George Spelvin @ 2020-03-20 22:30 UTC (permalink / raw) To: git; +Cc: lkml I'm cleaning up a patch series for submission, and came across a fixup in patch #4/20 that belongs in #2/20. Unfortunately, I can't go back two patches to apply the fix until I get to the end of the current rebase, then go back down to clean it up. :-( Thinking about it, I realized that a rebase in a rebase is a perfectly well defined operation. *If* you don't bother setting a new abort point (it's not a fully nested transaction), *and* require that the tree be clean (no stashing allowed; create a WIP commit instead), it's just a matter of putting some commits back on the front of the todo-list and checking out the old version. This would make rebase work more like quilt. I'm not sure how difficult this would be, but it might be worth looking into. (Possibly gated by an extra option like --nested.) (A second thing that would be nice would be a documented way to break out of a reword and change the commit. A few times I've been improving the documentation of a patch and realized that I should change the function name.) ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-20 22:30 Feature request: rebase -i inside of rebase -i George Spelvin @ 2020-03-20 22:51 ` Junio C Hamano 2020-03-20 23:35 ` George Spelvin 2020-03-21 8:47 ` Johannes Sixt 1 sibling, 1 reply; 25+ messages in thread From: Junio C Hamano @ 2020-03-20 22:51 UTC (permalink / raw) To: George Spelvin; +Cc: git, Johannes Schindelin George Spelvin <lkml@SDF.ORG> writes: > I'm cleaning up a patch series for submission, and came across a fixup in > patch #4/20 that belongs in #2/20. > > Unfortunately, I can't go back two patches to apply the fix until I get to > the end of the current rebase, then go back down to clean it up. :-( > > Thinking about it, I realized that a rebase in a rebase is a perfectly > well defined operation. *If* you don't bother setting a new abort point > (it's not a fully nested transaction), *and* require that the tree be > clean (no stashing allowed; create a WIP commit instead), it's just a > matter of putting some commits back on the front of the todo-list and > checking out the old version. I thought that "git rebase -i" allows the todo file (i.e. list of steps still to be performed) to be edited before continuing; would your use case be supported by using that? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-20 22:51 ` Junio C Hamano @ 2020-03-20 23:35 ` George Spelvin 2020-03-21 10:51 ` Johannes Schindelin 0 siblings, 1 reply; 25+ messages in thread From: George Spelvin @ 2020-03-20 23:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Johannes Schindelin, lkml On Fri, Mar 20, 2020 at 03:51:20PM -0700, Junio C Hamano wrote: > I thought that "git rebase -i" allows the todo file (i.e. list of > steps still to be performed) to be edited before continuing; would > your use case be supported by using that? Mostly, if I do it very carefully, which is why I thought it would be easy to add. I think I could manually add the commits to the start of the todo file, reset --hard to the old state, and rebase --continue. But cutting and pasting commit IDs from git log into the todo file, and putting fixup commits in the right place is annoyingly fiddly. That's exactly the sort of thing computers are good at. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-20 23:35 ` George Spelvin @ 2020-03-21 10:51 ` Johannes Schindelin 2020-03-21 17:56 ` George Spelvin 0 siblings, 1 reply; 25+ messages in thread From: Johannes Schindelin @ 2020-03-21 10:51 UTC (permalink / raw) To: George Spelvin; +Cc: Junio C Hamano, git Hi, On Fri, 20 Mar 2020, George Spelvin wrote: > On Fri, Mar 20, 2020 at 03:51:20PM -0700, Junio C Hamano wrote: > > I thought that "git rebase -i" allows the todo file (i.e. list of > > steps still to be performed) to be edited before continuing; would > > your use case be supported by using that? > > Mostly, if I do it very carefully, which is why I thought it would > be easy to add. > > I think I could manually add the commits to the start of the todo file, > reset --hard to the old state, and rebase --continue. > > But cutting and pasting commit IDs from git log into the todo file, > and putting fixup commits in the right place is annoyingly fiddly. > That's exactly the sort of thing computers are good at. FWIW I have a super-hacky work-around for this use case that I am using in my automation of continuously rebasing Git for Windows' `master` onto the four integration branches of git.git: 1. create a throw-away worktree without checking out the commit 2. fake-run a new `rebase -i` in that worktree, with a custom "editor" (which is actually the same script) that simply consumes the todo list, aborts the `rebase -i` in the worktree, then deletes the worktree, and then inserts that todo list in the original todo list. 3. continue the rebase I never got around to implement that as a proper "nested" mode of `git rebase -i`, but it should not be too hard. The user interface would probably look somewhat like `git rebase -i --nested <arguments>...` and it would _expect_ an active interactive rebase, and it would insert the todo list into the existing one, at the beginning, with proper commenting, then reset `HEAD` after the user edited the todo list. My biggest caveat is that I had to force-exit the rebase at some stage due to reasons I only vaguely remember. It had something to do with the replacement cache not being updated when an `exec` is executed that adds a replacement object via `git replace` [*1*]. This issue might have _nothing_ to do with nested rebases, but as I said, my recollection is vague. There are a couple more concerns, of course, such as: what to do if the user deletes the entire todo list (which is traditionally the only way to abort a rebase)? My gut feeling is that it should go back to the _previous_ version of the todo list. Another big concern is what to do about `rebase.missingCommitsCheck`: with nested rebases, this will get increasingly tricky. Like, imagine you are rebasing 5 commits, the third of them results in merge conflicts, you realize that it is obsolete and so is now the first, already rebased commit. You do a nested rebase of the latest two commits to drop them, but they don't have their original commit hashes any longer. So it gets a bit finicky to keep track of what commit has been dropped on purpose and what was forgotten to pick instead. Ciao, Dscho Footnote *1*: to refresh my recollection, I would have to scour the history of the automation script, see https://github.com/git-for-windows/build-extra/commits/master/ever-green.sh ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-21 10:51 ` Johannes Schindelin @ 2020-03-21 17:56 ` George Spelvin 2020-03-25 19:26 ` Johannes Schindelin 0 siblings, 1 reply; 25+ messages in thread From: George Spelvin @ 2020-03-21 17:56 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, git, lkml On Sat, Mar 21, 2020 at 11:51:10AM +0100, Johannes Schindelin wrote: > My biggest caveat is that I had to force-exit the rebase at some stage > due to reasons I only vaguely remember. It had something to do with the > replacement cache not being updated when an `exec` is executed that adds a > replacement object via `git replace` [*1*]. This issue might have > _nothing_ to do with nested rebases, but as I said, my recollection is > vague. This is the sort of internal implementation gotcha I worry about. > There are a couple more concerns, of course, such as: what to do if the > user deletes the entire todo list (which is traditionally the only way to > abort a rebase)? My gut feeling is that it should go back to the > _previous_ version of the todo list. My assumption has been that, for simplicity, there would only be one commit in progress, and aborting it aborts everything. > Another big concern is what to do about `rebase.missingCommitsCheck`: with > nested rebases, this will get increasingly tricky. Like, imagine you are > rebasing 5 commits, the third of them results in merge conflicts, you > realize that it is obsolete and so is now the first, already rebased > commit. You do a nested rebase of the latest two commits to drop them, but > they don't have their original commit hashes any longer. So it gets a bit > finicky to keep track of what commit has been dropped on purpose and what > was forgotten to pick instead. This doesn't *seem* difficult, but I don't know how the current mechanism works. It just checks that all commits that were on the to-do list when the editor started are still listed (possibly marked "drop") when it exits. When you do a nested commit, additional commits are prepended to the to-do list, you're dropped into the editor, and the same check is made when the editor returns. If rebase.missingCommitsCheck = error is triggered, you end up with the <upstream> tree state with nothing applied and may either --continue to ignore the error or --edit-todo to put back the missing commits. Let me give an example. Suppose I have commits a-b-c-d-e, and start with rebase -i b. My to-do list starts out as c-d-e, but suppose I decide to cherry-pick z and add it to the list, so it's now z-c-d-e. So I start rebasing, and it turns out that d creates a merge conflict because I forgot a prerequisite patch y. And I really want y and z before b, anyway. So the tree state is currently a-b-z'-c', with d in progress and e yet to do. In my simple model, I have to resolve and commit d, so the tree state is a-b-z'-c'-d'. Then I can rebase -i a, and am presented with a to-do list of b-z'-c'-d'-e. If I delete any of those five commits, then rebase.missingCommitsCheck will trigger. If I put y in the list, save it, then change my mind and --edit-todo and delete y, it will also trigger. Now, it sould be nice if there were a way to say "screw this mess; just check out HEAD and put d back on the to-do list", but that could create a bit of a mess if I've split d and already committed half of it. If I used that after doung that, I'd have a to-do list of b-z'-c'-d'-d-e which might be awkward, but maybe it wouldn't be too bad. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-21 17:56 ` George Spelvin @ 2020-03-25 19:26 ` Johannes Schindelin 2020-03-26 0:18 ` George Spelvin 0 siblings, 1 reply; 25+ messages in thread From: Johannes Schindelin @ 2020-03-25 19:26 UTC (permalink / raw) To: George Spelvin; +Cc: Junio C Hamano, git Hi George, On Sat, 21 Mar 2020, George Spelvin wrote: > On Sat, Mar 21, 2020 at 11:51:10AM +0100, Johannes Schindelin wrote: > > My biggest caveat is that I had to force-exit the rebase at some stage > > due to reasons I only vaguely remember. It had something to do with the > > replacement cache not being updated when an `exec` is executed that adds a > > replacement object via `git replace` [*1*]. This issue might have > > _nothing_ to do with nested rebases, but as I said, my recollection is > > vague. > > This is the sort of internal implementation gotcha I worry about. There's plenty more gotchas like that ;-) > > There are a couple more concerns, of course, such as: what to do if the > > user deletes the entire todo list (which is traditionally the only way to > > abort a rebase)? My gut feeling is that it should go back to the > > _previous_ version of the todo list. > > My assumption has been that, for simplicity, there would only be one > commit in progress, and aborting it aborts everything. But that does not necessarily make sense. Imagine that you rebase the latest three commits, interactively. Then a merge conflict in the third makes you realize that the first commit is no longer needed. Enter the nested rebase. You manually re-schedule the failed `pick` via `git rebase --edit-todo` and then run the nested rebase: `git reset --hard && git rebase -i --nested HEAD~2`. Except that you made a typo and said `HEAD~3` instead of `HEAD~2`. You delete the entire todo list to get a chance to restart the nested rebase. But now the entire rebase gets aborted? If that would happen to me, I would unleash a whole slew of rarely used words in the vague direction of whoever implemented the nested rebase feature... > > Another big concern is what to do about `rebase.missingCommitsCheck`: with > > nested rebases, this will get increasingly tricky. Like, imagine you are > > rebasing 5 commits, the third of them results in merge conflicts, you > > realize that it is obsolete and so is now the first, already rebased > > commit. You do a nested rebase of the latest two commits to drop them, but > > they don't have their original commit hashes any longer. So it gets a bit > > finicky to keep track of what commit has been dropped on purpose and what > > was forgotten to pick instead. > > This doesn't *seem* difficult, but I don't know how the current mechanism > works. The implementation details do not matter at this stage. You have to get the design of the feature right. I am unfamiliar with the design of the feature as it is implemented right now, but I imagine that it needs to be adjusted for nested rebases, as we no longer have a single original todo list to roll back to. > It just checks that all commits that were on the to-do list when the > editor started are still listed (possibly marked "drop") when it exits. > > When you do a nested commit, additional commits are prepended to the to-do > list, you're dropped into the editor, and the same check is made when the > editor returns. > > If rebase.missingCommitsCheck = error is triggered, you end up with the > <upstream> tree state with nothing applied and may either --continue to > ignore the error or --edit-todo to put back the missing commits. > > > Let me give an example. Suppose I have commits a-b-c-d-e, and start > with rebase -i b. > > My to-do list starts out as c-d-e, but suppose I decide to cherry-pick > z and add it to the list, so it's now z-c-d-e. > > So I start rebasing, and it turns out that d creates a merge conflict > because I forgot a prerequisite patch y. And I really want y and z before > b, anyway. > > So the tree state is currently a-b-z'-c', with d in progress and e yet to > do. In my simple model, I have to resolve and commit d, so the tree > state is a-b-z'-c'-d'. Then I can rebase -i a, and am presented with > a to-do list of b-z'-c'-d'-e. > > If I delete any of those five commits, then rebase.missingCommitsCheck > will trigger. If I put y in the list, save it, then change my mind and > --edit-todo and delete y, it will also trigger. As I said, I am not using that feature myself, so I do not even know what "trigger" means in this context. It might totally be okay to use the existing code as-is in the context of a nested rebase. That remains to be verified, though, I think. > Now, it sould be nice if there were a way to say "screw this mess; just > check out HEAD and put d back on the to-do list", but that could > create a bit of a mess if I've split d and already committed half of > it. If I used that after doung that, I'd have a to-do list of > b-z'-c'-d'-d-e which might be awkward, but maybe it wouldn't be too bad. There is all kind of opportunity for messes, all right. Ciao, Johannes ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-25 19:26 ` Johannes Schindelin @ 2020-03-26 0:18 ` George Spelvin 2020-03-28 14:25 ` Johannes Schindelin 0 siblings, 1 reply; 25+ messages in thread From: George Spelvin @ 2020-03-26 0:18 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, git, lkml On Wed, Mar 25, 2020 at 08:26:48PM +0100, Johannes Schindelin wrote: > On Sat, 21 Mar 2020, George Spelvin wrote: >> My assumption has been that, for simplicity, there would only be one >> commit in progress, and aborting it aborts everything. > > But that does not necessarily make sense. Imagine that you rebase the > latest three commits, interactively. Then a merge conflict in the third > makes you realize that the first commit is no longer needed. > > Enter the nested rebase. You manually re-schedule the failed `pick` via > `git rebase --edit-todo` and then run the nested rebase: `git reset --hard > && git rebase -i --nested HEAD~2`. > > Except that you made a typo and said `HEAD~3` instead of `HEAD~2`. You > delete the entire todo list to get a chance to restart the nested rebase. > > But now the entire rebase gets aborted? Um, this example is not persuasive. If I just leave the excess commit at the front of the to-do list, then it will be recreated without change. (Note that if I choose too *small* a nubmer by accident, I can insert a "break" at the front of the list and then rebase --nested starting from there.) Okay, but what if I screw up worse and type HEAD^55 instead of HEAD^5? nd that includes multiple merges and other messy stuff? Well, perhaps a general-purpose optimization could be applied: for the first, mandatory, edit-todo, don't actually check out the tree until the edit is complete. When it is, chop off any prefix of unaltered commits and start the rebase at the first change. That would make inadvertently specifying a start point too far back reasonably harmless. It would also provide one level of nested abort in the case of a nested rebase. Until you save the initial todo, the rebase doesn't do anything except some bookkeeping. So you could have that be a special case, without providing a more general nested --abort. The main problem with a full nested rebase is that you need to define when the inner rebase completes and the outer rebase resumes. I very much want the ability to move commits around between the outer rebase and the inner one, which makes that distinction ill-defined. > If that would happen to me, I would unleash a whole slew of rarely used > words in the vague direction of whoever implemented the nested rebase > feature... The thing is, it's already quite possible to make a mess of a rebase halfway through and need to abort after you've put a lot of work in. I think a more general-purpose recovery mechanism might be more useful. For example, if the --edit-todo included a (commented-out) list of what had already been done, then after realizing that you screwed up conflict resolve b' and have now committed bad resolutions c' and d' on top of it, you could easily rebase --nested and replace b', c' and d' with the original b, c and d. Without aborting and throwing away a' as well, which was perhaps a lot of work. >> If I delete any of those five commits, then rebase.missingCommitsCheck >> will trigger. If I put y in the list, save it, then change my mind and >> --edit-todo and delete y, it will also trigger. > > As I said, I am not using that feature myself, so I do not even know what > "trigger" means in this context. It might totally be okay to use the > existing code as-is in the context of a nested rebase. That remains to be > verified, though, I think. What I mean by "trigger" is thatthe check would notice a missing commit and produce a warning or error, as configured. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-26 0:18 ` George Spelvin @ 2020-03-28 14:25 ` Johannes Schindelin 2020-03-28 16:30 ` George Spelvin 2020-03-30 14:01 ` Philip Oakley 0 siblings, 2 replies; 25+ messages in thread From: Johannes Schindelin @ 2020-03-28 14:25 UTC (permalink / raw) To: George Spelvin; +Cc: Junio C Hamano, git Hi George, On Thu, 26 Mar 2020, George Spelvin wrote: > On Wed, Mar 25, 2020 at 08:26:48PM +0100, Johannes Schindelin wrote: > > On Sat, 21 Mar 2020, George Spelvin wrote: > >> My assumption has been that, for simplicity, there would only be one > >> commit in progress, and aborting it aborts everything. > > > > But that does not necessarily make sense. Imagine that you rebase the > > latest three commits, interactively. Then a merge conflict in the third > > makes you realize that the first commit is no longer needed. > > > > Enter the nested rebase. You manually re-schedule the failed `pick` via > > `git rebase --edit-todo` and then run the nested rebase: `git reset --hard > > && git rebase -i --nested HEAD~2`. > > > > Except that you made a typo and said `HEAD~3` instead of `HEAD~2`. You > > delete the entire todo list to get a chance to restart the nested rebase. > > > > But now the entire rebase gets aborted? > > Um, this example is not persuasive. If I just leave the excess commit at > the front of the to-do list, then it will be recreated without change. There are _many_ ways to mess up a nested rebase, including (but not limited to) `--onto`, forgetting `-r`, editing the todo list too much in an editor without undo. If you are suggesting that a nested `git rebase -i` would not need a way to abort _just_ the nested rebase, then I fear we must stop the conversation right here. That's not going to fly. > (Note that if I choose too *small* a nubmer by accident, I can insert a > "break" at the front of the list and then rebase --nested starting from > there.) There are many ways how a savvy user would be able to work around the absence of a proper way to abort a nested rebase. The common theme for all of those is: - they are all quite involved and require knowledge of internals - they won't change the fact that it would be seriously negligent for us to _not_ offer a way to abort nested rebases. > Okay, but what if I screw up worse and type HEAD^55 instead of HEAD^5? > nd that includes multiple merges and other messy stuff? And Ctrl+C while the nested rebase tries to generate the todo list. > Well, perhaps a general-purpose optimization could be applied: for the > first, mandatory, edit-todo, don't actually check out the tree until the > edit is complete. When it is, chop off any prefix of unaltered commits > and start the rebase at the first change. > > That would make inadvertently specifying a start point too far back > reasonably harmless. > > It would also provide one level of nested abort in the case of a nested > rebase. Until you save the initial todo, the rebase doesn't do anything > except some bookkeeping. So you could have that be a special case, > without providing a more general nested --abort. > > The main problem with a full nested rebase is that you need to define when > the inner rebase completes and the outer rebase resumes. I very much > want the ability to move commits around between the outer rebase and the > inner one, which makes that distinction ill-defined. That probably means that we have not thought through the problem, at least not enough. If we have nested levels, then we might need to record those nested levels, including the equivalent of `ORIG_HEAD`, `onto` and the todo list. And of course make them accessible to the user. By default, we can still work on the "inner-most" rebase. The output of `git status` can hint at the nested level, to give an indication, as can the prompt. We should strive to make this as easy to use as possible, while still supporting more involved use cases (for power users such as myself, at the least). Mind you: I do not necessarily request a perfect design. I just don't want to slam the door shut when it comes to more sophisticated use cases. I _really_ would like to have a way to "just redo the latest 5 commands", for example, but I have no illusion about getting that any time soon. > > If that would happen to me, I would unleash a whole slew of rarely used > > words in the vague direction of whoever implemented the nested rebase > > feature... > > The thing is, it's already quite possible to make a mess of a rebase > halfway through and need to abort after you've put a lot of work in. Tell me about it! > I think a more general-purpose recovery mechanism might be more > useful. > > For example, if the --edit-todo included a (commented-out) list of what > had already been done, then after realizing that you screwed up > conflict resolve b' and have now committed bad resolutions c' and d' > on top of it, you could easily rebase --nested and replace b', c' and d' > with the original b, c and d. > > Without aborting and throwing away a' as well, which was perhaps a lot of > work. We do have the `done` file, but that does not discern between commands in the todo list that have been there in the first place and commands that have been added by the user _during_ the rebase. And of course it does not reflect any commands that have been removed/changed by the user during the rebase. So yeah, something like a journal, maybe. > >> If I delete any of those five commits, then rebase.missingCommitsCheck > >> will trigger. If I put y in the list, save it, then change my mind and > >> --edit-todo and delete y, it will also trigger. > > > > As I said, I am not using that feature myself, so I do not even know what > > "trigger" means in this context. It might totally be okay to use the > > existing code as-is in the context of a nested rebase. That remains to be > > verified, though, I think. > > What I mean by "trigger" is thatthe check would notice a missing commit > and produce a warning or error, as configured. I guess `missingCOmmitsCheck` really only kicks in right before/after the user edits the todo list. So maybe I was worried for nothing in this instance. Thank you for thinking about this feature, and for discussing it with me. I think it will be a really nice feature to have, and I want to avoid problems with a design that simply won't allow for certain use cases (remember how `git rebase --preserve-merges` does not allow for reordering commits, and how the design of that feature simply made this bug unfixable? I _dearly_ regret not thinking that through). Ciao, Dscho ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-28 14:25 ` Johannes Schindelin @ 2020-03-28 16:30 ` George Spelvin 2020-03-31 0:00 ` George Spelvin 2020-04-04 12:39 ` Johannes Schindelin 2020-03-30 14:01 ` Philip Oakley 1 sibling, 2 replies; 25+ messages in thread From: George Spelvin @ 2020-03-28 16:30 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, git, lkml On Sat, Mar 28, 2020 at 03:25:06PM +0100, Johannes Schindelin wrote: > There are _many_ ways to mess up a nested rebase, including (but not > limited to) `--onto`, forgetting `-r`, editing the todo list too much in > an editor without undo. > > If you are suggesting that a nested `git rebase -i` would not need a way > to abort _just_ the nested rebase, then I fear we must stop the > conversation right here. That's not going to fly. Yes, I'm seriously suggesting that. My arguments are: * The benefit is limited, because you can do pretty much all the same things with just plain --edit-todo, and I don't see the point of handing the user a loaded gun and ensuring they can't shoot themselves in their *left* foot. * The cost in implementation complexity is considerable. Even the *semantics* are awkward. In particular, if you reorder patches between the outer and inner rebases, how do you define the end of the inner rebase? Now, I *did* agree that being able to abort just the nested rebase *from the initial todo edit* is useful and achievable. That can be handled as a special case. It's allowing "git rebase --abort" from halfway throuigh the nested rebasethat introduces all kinds of hair. >> The main problem with a full nested rebase is that you need to define when >> the inner rebase completes and the outer rebase resumes. I very much >> want the ability to move commits around between the outer rebase and the >> inner one, which makes that distinction ill-defined. > > That probably means that we have not thought through the problem, at least > not enough. If we have nested levels, then we might need to record those > nested levels, including the equivalent of `ORIG_HEAD`, `onto` and the > todo list. And of course make them accessible to the user. I worry that's too much conceptual complexity being foisted on the user. > Mind you: I do not necessarily request a perfect design. I just don't want > to slam the door shut when it comes to more sophisticated use cases. I > _really_ would like to have a way to "just redo the latest 5 commands", > for example, but I have no illusion about getting that any time soon. The big use cases I see are: - Wanting to back up in the current rebase, "quilt pop" style. Many times I've found a hunk in patch #4 that should have been part of patch #2. - Wanting to move the base back a bit further than originally planned. (This is often an alternate form of the preceeding; I find a hunk that should have been in patch #-2.) - Wanting to rearrange commits across the current commit. I've gone and edited patch #4 and introduced a dependency on patch #7. I want to move patch #7 back so that my revised #4 can be tested. I think of the feature as an "edit-todo" that includes some already- applied patches. >> The thing is, it's already quite possible to make a mess of a rebase >> halfway through and need to abort after you've put a lot of work in. > > Tell me about it! Thus, my comment above about protecting the user's left foot. If they still need to be careful to not shoot off their right foot, are we saving them any mental effort? My goal for the nested-rebase feature is that it can be useful for *recovering* from such messes. Back up a few steps to before the trouble started, but *not* a full abort. >> I think a more general-purpose recovery mechanism might be more >> useful. >> >> For example, if the --edit-todo included a (commented-out) list of what >> had already been done, then after realizing that you screwed up >> conflict resolve b' and have now committed bad resolutions c' and d' >> on top of it, you could easily rebase --nested and replace b', c' and d' >> with the original b, c and d. >> >> Without aborting and throwing away a' as well, which was perhaps a lot of >> work. > > We do have the `done` file, but that does not discern between commands > in the todo list that have been there in the first place and commands that > have been added by the user _during_ the rebase. And of course it does not > reflect any commands that have been removed/changed by the user during the > rebase. > > So yeah, something like a journal, maybe. I was thinking of the "done" file. At a low level, we already have the reflog, and giving the user yet another transaction log to remember is undesirable. If we just added the original commit ID to the automatically generated reflog entry, si it also records the higher-level intention, that might do it. I.e. change bc56134cfe278 HEAD@{3}: rebase (pick): wilc1000: Use crc7 in lib/ rather than a private copy to bc56134cfe278 HEAD@{3}: rebase (pick 01cb7b5b7851): wilc1000: Use crc7 in lib/ rather than a private copy Oh! What if we recorded, for the duration of the rebase, the todo list state corresponding to reflog entries? (Or, equivalently, the reflog state at each step through the todo list.) Then we could back up the rebase with just a reflog entry pointer. I like this precisely because it doesn't give the user a new entity to keep track of; it just makes an existing entity more useful. > Thank you for thinking about this feature, and for discussing it with me. > I think it will be a really nice feature to have, and I want to avoid > problems with a design that simply won't allow for certain use cases > (remember how `git rebase --preserve-merges` does not allow for reordering > commits, and how the design of that feature simply made this bug > unfixable? I _dearly_ regret not thinking that through). Definitely, let's think it through. But remember that "perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away." KISS is a design *goal*, not just a necessary compromise. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-28 16:30 ` George Spelvin @ 2020-03-31 0:00 ` George Spelvin 2020-03-31 10:57 ` Philip Oakley 2020-04-04 12:17 ` Johannes Schindelin 2020-04-04 12:39 ` Johannes Schindelin 1 sibling, 2 replies; 25+ messages in thread From: George Spelvin @ 2020-03-31 0:00 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, git, Philip Oakley, lkml Thinking about Philip Oakley's suggestion, it dawned on me that we can *already* do a nested rebase manually, and having a less manual alias for the procedure would be reasonable. Suppose the last four commits are O-A-B-C, and whether they were created by this rebase or existed before is irrelevant. If I want to rebase --nested -i O, then I --edit-todo and prepend the following four lines: reset O pick A pick B pick C If a nested rebase command does just that, I think it would cover my use case. If it adds a comment saying "nested rebase ends here", it's easy to cancel the nested rebase if there was a mistake. A slightly fancier thing a nestrd rebase could do is see if any of the newly created picks are also used in merges that were already in the todo list. In that case, follow the pick by a label command and update the later merge to refer to the label. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-31 0:00 ` George Spelvin @ 2020-03-31 10:57 ` Philip Oakley 2020-03-31 13:36 ` Phillip Wood 2020-04-04 12:17 ` Johannes Schindelin 1 sibling, 1 reply; 25+ messages in thread From: Philip Oakley @ 2020-03-31 10:57 UTC (permalink / raw) To: George Spelvin, Johannes Schindelin; +Cc: Junio C Hamano, git Hi george, On 31/03/2020 01:00, George Spelvin wrote: > Thinking about Philip Oakley's suggestion, it dawned on me that > we can *already* do a nested rebase manually, and having a less > manual alias for the procedure would be reasonable. > > Suppose the last four commits are O-A-B-C, and whether they were created > by this rebase or existed before is irrelevant. > > If I want to rebase --nested -i O, then I --edit-todo and Maybe `--rework` as a possible alternate option name, if the concept of it being truly nested process does not work out? > prepend the following four lines: > reset O > pick A > pick B > pick C > > If a nested rebase command does just that, I think it would cover my > use case. If it adds a comment saying "nested rebase ends here", > it's easy to cancel the nested rebase if there was a mistake. > > A slightly fancier thing a nestrd rebase could do is see if any of the > newly created picks are also used in merges that were already in the todo > list. In that case, follow the pick by a label command and update the > later merge to refer to the label. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-31 10:57 ` Philip Oakley @ 2020-03-31 13:36 ` Phillip Wood 2020-04-01 16:43 ` Philip Oakley 0 siblings, 1 reply; 25+ messages in thread From: Phillip Wood @ 2020-03-31 13:36 UTC (permalink / raw) To: Philip Oakley, George Spelvin, Johannes Schindelin; +Cc: Junio C Hamano, git Hi Philip, George and Johannes I really like the idea of being able to extend or rewind an existing rebase to reedit commits. On 31/03/2020 11:57, Philip Oakley wrote: > Hi george, > > On 31/03/2020 01:00, George Spelvin wrote: >> Thinking about Philip Oakley's suggestion, it dawned on me that >> we can *already* do a nested rebase manually, and having a less >> manual alias for the procedure would be reasonable. >> >> Suppose the last four commits are O-A-B-C, and whether they were created >> by this rebase or existed before is irrelevant. >> >> If I want to rebase --nested -i O, then I --edit-todo and > > Maybe `--rework` as a possible alternate option name, if the concept of > it being truly nested process does not work out? or `--rewind` ? >> prepend the following four lines: >> reset O >> pick A >> pick B >> pick C >> >> If a nested rebase command does just that, I think it would cover my >> use case. If it adds a comment saying "nested rebase ends here", >> it's easy to cancel the nested rebase if there was a mistake. >> >> A slightly fancier thing a nestrd rebase could do is see if any of the >> newly created picks are also used in merges that were already in the todo >> list. In that case, follow the pick by a label command and update the >> later merge to refer to the label. If we're going to support rewinding a rebase that creates merges then this is a prerequisite otherwise it wont work properly. It will also need to update any existing labels that refer to a commits that get rewritten when rewinding. When cancelling the nested rebase we need to take care to restore any labels to their previous value if they have been updated by the nested rebase. We also need to restore the list or rewritten commits so that we don't report that we've rewritten the commits from the nested rebase that we're aborting. These two requirements unfortunately make it difficult to implement the nested rebase just by updating the todo list. It needs to save the current labels (and reference the commits somewhere so they don't get gc'd) and the rewritten-list. `git rebase --abort` (or whatever we end up using to abort the nested part of the rebase) needs to restore the labels and rewritten-list. I think it should probably restore the todo list as well - if the original part of the todo list gets edited during the nested rebase should we drop those changes to the list or keep them when the nested rebase is aborted? Best Wishes Phillip ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-31 13:36 ` Phillip Wood @ 2020-04-01 16:43 ` Philip Oakley 2020-04-07 15:54 ` Phillip Wood 0 siblings, 1 reply; 25+ messages in thread From: Philip Oakley @ 2020-04-01 16:43 UTC (permalink / raw) To: phillip.wood, George Spelvin, Johannes Schindelin; +Cc: Junio C Hamano, git Hi Phillip, On 31/03/2020 14:36, Phillip Wood wrote: > Hi Philip, George and Johannes > > I really like the idea of being able to extend or rewind an existing > rebase to reedit commits. > > On 31/03/2020 11:57, Philip Oakley wrote: >> Hi george, >> >> On 31/03/2020 01:00, George Spelvin wrote: >>> Thinking about Philip Oakley's suggestion, it dawned on me that >>> we can *already* do a nested rebase manually, and having a less >>> manual alias for the procedure would be reasonable. >>> >>> Suppose the last four commits are O-A-B-C, and whether they were >>> created >>> by this rebase or existed before is irrelevant. >>> >>> If I want to rebase --nested -i O, then I --edit-todo and >> >> Maybe `--rework` as a possible alternate option name, if the concept of >> it being truly nested process does not work out? > > or `--rewind` ? A possibility, though it feels a bit narrow in suggesting the capabilities > >>> prepend the following four lines: >>> reset O >>> pick A >>> pick B >>> pick C >>> >>> If a nested rebase command does just that, I think it would cover my >>> use case. If it adds a comment saying "nested rebase ends here", >>> it's easy to cancel the nested rebase if there was a mistake. >>> >>> A slightly fancier thing a nestrd rebase could do is see if any of the >>> newly created picks are also used in merges that were already in the >>> todo >>> list. In that case, follow the pick by a label command and update the >>> later merge to refer to the label. > > If we're going to support rewinding a rebase that creates merges then > this is a prerequisite otherwise it wont work properly. It will also > need to update any existing labels that refer to a commits that get > rewritten when rewinding. I would agree that the `label` instruction would need expanding to allow arbitrary refs (e.g. specific oids and other branches) to be labelled rather than just a presumed 'HEAD' ref. I did notice the man page doesn't actually define the format of the extra instructions (there was fun with awkward chars in label names on Windows). I'm of the opinion that we don't re-label/rename the previous labels - they are what they are, but we do offer the ability to provide new labels. Hence the extension to the label format to allow the labelling of arbitrary refs, not just HEAD, along with showing existing progress, so folks can _see_ the new oids etc. > > When cancelling the nested rebase we need to take care to restore any > labels to their previous value if they have been updated by the nested > rebase. We also need to restore the list or rewritten commits so that > we don't report that we've rewritten the commits from the nested > rebase that we're aborting. These two requirements unfortunately make > it difficult to implement the nested rebase just by updating the todo > list. I'm against the original conceptual idea of 'nesting' (or rewinding). What's done is done, it's in the object store, as is all the old original work, so we have a wider foundation to build from. Clearly we are not in a clean work-state, with the half complete jobs, so it's more of a recovery activity than clean coding (from the user perspective). > It needs to save the current labels (and reference the commits > somewhere so they don't get gc'd) and the rewritten-list. `git rebase > --abort` (or whatever we end up using to abort the nested part of the > rebase) needs to restore the labels and rewritten-list. I think it > should probably restore the todo list as well - if the original part > of the todo list gets edited during the nested rebase should we drop > those changes to the list or keep them when the nested rebase is aborted? If we haven't aborted, then we just have the actual sequence of work, with some of the commits, ultimately, being left as unconnected stubs (once their temporary tips have gone upon completion of the rebase - could they be explicitly dropped?). On completion, those abandoned commits could be explicitly marked as having been removed in the various book-keeping lists and logs (if it was useful and helpful). I think this is a different conceptual view of the work of the rebase - resolve process. > > Best Wishes > > Phillip -- Philip ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-04-01 16:43 ` Philip Oakley @ 2020-04-07 15:54 ` Phillip Wood 0 siblings, 0 replies; 25+ messages in thread From: Phillip Wood @ 2020-04-07 15:54 UTC (permalink / raw) To: Philip Oakley, phillip.wood, George Spelvin, Johannes Schindelin Cc: Junio C Hamano, git Hi Philip On 01/04/2020 17:43, Philip Oakley wrote: > Hi Phillip, > > On 31/03/2020 14:36, Phillip Wood wrote: >> Hi Philip, George and Johannes >> >> I really like the idea of being able to extend or rewind an existing >> rebase to reedit commits. >> >> On 31/03/2020 11:57, Philip Oakley wrote: >>> Hi george, >>> >>> On 31/03/2020 01:00, George Spelvin wrote: >>>> Thinking about Philip Oakley's suggestion, it dawned on me that >>>> we can *already* do a nested rebase manually, and having a less >>>> manual alias for the procedure would be reasonable. >>>> >>>> Suppose the last four commits are O-A-B-C, and whether they were >>>> created >>>> by this rebase or existed before is irrelevant. >>>> >>>> If I want to rebase --nested -i O, then I --edit-todo and >>> >>> Maybe `--rework` as a possible alternate option name, if the concept of >>> it being truly nested process does not work out? >> >> or `--rewind` ? > > A possibility, though it feels a bit narrow in suggesting the capabilities >> >>>> prepend the following four lines: >>>> reset O >>>> pick A >>>> pick B >>>> pick C >>>> >>>> If a nested rebase command does just that, I think it would cover my >>>> use case. If it adds a comment saying "nested rebase ends here", >>>> it's easy to cancel the nested rebase if there was a mistake. >>>> >>>> A slightly fancier thing a nestrd rebase could do is see if any of the >>>> newly created picks are also used in merges that were already in the >>>> todo >>>> list. In that case, follow the pick by a label command and update the >>>> later merge to refer to the label. >> >> If we're going to support rewinding a rebase that creates merges then >> this is a prerequisite otherwise it wont work properly. It will also >> need to update any existing labels that refer to a commits that get >> rewritten when rewinding. > > I would agree that the `label` instruction would need expanding to allow > arbitrary refs (e.g. specific oids and other branches) to be labelled > rather than just a presumed 'HEAD' ref. I did notice the man page > doesn't actually define the format of the extra instructions (there was > fun with awkward chars in label names on Windows). > > I'm of the opinion that we don't re-label/rename the previous labels - > they are what they are, but we do offer the ability to provide new > labels. Hence the extension to the label format to allow the labelling > of arbitrary refs, not just HEAD, along with showing existing progress, > so folks can _see_ the new oids etc. I'm not sure I understand how this would work. Say in the example below I decide I want to rework abc while editing fgh and run 'git rebase --rework abc^'. If we don't update the labels automatically how do I ensure that the merge will pick up the reworked abc as it's parent. #pick abc #label topic #reset cde edit fgh merge topic >> >> When cancelling the nested rebase we need to take care to restore any >> labels to their previous value if they have been updated by the nested >> rebase. We also need to restore the list or rewritten commits so that >> we don't report that we've rewritten the commits from the nested >> rebase that we're aborting. These two requirements unfortunately make >> it difficult to implement the nested rebase just by updating the todo >> list. > > I'm against the original conceptual idea of 'nesting' (or rewinding). > What's done is done, it's in the object store, as is all the old > original work, so we have a wider foundation to build from. Clearly we > are not in a clean work-state, with the half complete jobs, so it's more > of a recovery activity than clean coding (from the user perspective). I don't like the idea of nesting either but I think it would be useful to be able to get back to a known state when the changes I make with 'rebase --rework' turn out to be a bad idea so I don't throw away all the work that I did before running 'rebase --rework'. I've been using a script that rewinds and I've got myself into situations where a speculative change made while rewinding turned out to be a bad idea and I want to get back to the state before the last rewind. When working on a patch series I can rewind multiple times before completing the rebase as I jump back and forth modifying and rewording each patch. Best Wishes Phillip >> It needs to save the current labels (and reference the commits >> somewhere so they don't get gc'd) and the rewritten-list. `git rebase >> --abort` (or whatever we end up using to abort the nested part of the >> rebase) needs to restore the labels and rewritten-list. I think it >> should probably restore the todo list as well - if the original part >> of the todo list gets edited during the nested rebase should we drop >> those changes to the list or keep them when the nested rebase is aborted? > > If we haven't aborted, then we just have the actual sequence of work, > with some of the commits, ultimately, being left as unconnected stubs > (once their temporary tips have gone upon completion of the rebase - > could they be explicitly dropped?). On completion, those abandoned > commits could be explicitly marked as having been removed in the various > book-keeping lists and logs (if it was useful and helpful). I think this > is a different conceptual view of the work of the rebase - resolve process. >> >> Best Wishes >> >> Phillip > -- > Philip > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-31 0:00 ` George Spelvin 2020-03-31 10:57 ` Philip Oakley @ 2020-04-04 12:17 ` Johannes Schindelin 1 sibling, 0 replies; 25+ messages in thread From: Johannes Schindelin @ 2020-04-04 12:17 UTC (permalink / raw) To: George Spelvin; +Cc: Junio C Hamano, git, Philip Oakley Hi George, On Tue, 31 Mar 2020, George Spelvin wrote: > Thinking about Philip Oakley's suggestion, it dawned on me that > we can *already* do a nested rebase manually, and having a less > manual alias for the procedure would be reasonable. > > Suppose the last four commits are O-A-B-C, and whether they were created > by this rebase or existed before is irrelevant. > > If I want to rebase --nested -i O, then I --edit-todo and > prepend the following four lines: > reset O > pick A > pick B > pick C > > If a nested rebase command does just that, I think it would cover my > use case. If it adds a comment saying "nested rebase ends here", > it's easy to cancel the nested rebase if there was a mistake. FWIW this is precisely what I do in https://github.com/git-for-windows/build-extra/blob/70d940d1b/ever-green.sh#L184-L246 I create a new worktree (without checking out anything), start a rebase with a custom (fake) editor that simply grabs the todo list and aborts that rebase. Then I remove that worktree and insert the grabbed todo list into the current one. However, I find that it is not at all easy to cancel the nested rebase because the current `HEAD` is recorded _nowhere_. Ciao, Dscho > A slightly fancier thing a nestrd rebase could do is see if any of the > newly created picks are also used in merges that were already in the todo > list. In that case, follow the pick by a label command and update the > later merge to refer to the label. > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-28 16:30 ` George Spelvin 2020-03-31 0:00 ` George Spelvin @ 2020-04-04 12:39 ` Johannes Schindelin 2020-04-04 17:41 ` George Spelvin 1 sibling, 1 reply; 25+ messages in thread From: Johannes Schindelin @ 2020-04-04 12:39 UTC (permalink / raw) To: George Spelvin; +Cc: Junio C Hamano, git Hi George, On Sat, 28 Mar 2020, George Spelvin wrote: > On Sat, Mar 28, 2020 at 03:25:06PM +0100, Johannes Schindelin wrote: > > There are _many_ ways to mess up a nested rebase, including (but not > > limited to) `--onto`, forgetting `-r`, editing the todo list too much in > > an editor without undo. > > > > If you are suggesting that a nested `git rebase -i` would not need a way > > to abort _just_ the nested rebase, then I fear we must stop the > > conversation right here. That's not going to fly. > > Yes, I'm seriously suggesting that. My arguments are: > > * The benefit is limited, because you can do pretty much all the > same things with just plain --edit-todo, and I don't see the > point of handing the user a loaded gun and ensuring they can't > shoot themselves in their *left* foot. No, the benefit is in getting out of a fix without having to know implementation details. We do allow aborting rebases, so users will rightfully expect to be able to abort nested rebases, too. > * The cost in implementation complexity is considerable. Even the > *semantics* are awkward. In particular, if you reorder patches > between the outer and inner rebases, how do you define the end of > the inner rebase? The cost of Git's implementation complexity is also considerable. That on its own is not a reason not to do it. Besides, there are quite a few ways to achieve this on the cheap, so I would even context that it is too complex. One very trivial idea, for example, would be to rename `rebase-merge` to `rebase-nested-1` (and if that exists, find the next `rebase-nested-<n>` name and use that instead). Then run the nested rebase as per usual. Upon conclusion of the rebase (or if the user aborts it), rename back and continue. That approach has the advantage that nested rebases can use different options (such as a different merge strategy) than the enclosing rebase. > Now, I *did* agree that being able to abort just the nested rebase > *from the initial todo edit* is useful and achievable. That can > be handled as a special case. It's allowing "git rebase --abort" > from halfway throuigh the nested rebasethat introduces all kinds > of hair. This is too narrow a view. If I run a rebase, in the middle decide to try a nested rebase, and half-way through realize that the nested rebase was a bad idea, you want to _not_ give me a way to return to the enclosing rebase. That is a bad idea. > >> The main problem with a full nested rebase is that you need to define when > >> the inner rebase completes and the outer rebase resumes. I very much > >> want the ability to move commits around between the outer rebase and the > >> inner one, which makes that distinction ill-defined. > > > > That probably means that we have not thought through the problem, at least > > not enough. If we have nested levels, then we might need to record those > > nested levels, including the equivalent of `ORIG_HEAD`, `onto` and the > > todo list. And of course make them accessible to the user. > > I worry that's too much conceptual complexity being foisted on the user. > > > Mind you: I do not necessarily request a perfect design. I just don't want > > to slam the door shut when it comes to more sophisticated use cases. I > > _really_ would like to have a way to "just redo the latest 5 commands", > > for example, but I have no illusion about getting that any time soon. > > The big use cases I see are: > - Wanting to back up in the current rebase, "quilt pop" style. > Many times I've found a hunk in patch #4 that should have been > part of patch #2. > - Wanting to move the base back a bit further than originally > planned. (This is often an alternate form of the preceeding; > I find a hunk that should have been in patch #-2.) > - Wanting to rearrange commits across the current commit. I've > gone and edited patch #4 and introduced a dependency on patch #7. > I want to move patch #7 back so that my revised #4 can be tested. > > I think of the feature as an "edit-todo" that includes some already- > applied patches. > > >> The thing is, it's already quite possible to make a mess of a rebase > >> halfway through and need to abort after you've put a lot of work in. > > > > Tell me about it! > > Thus, my comment above about protecting the user's left foot. If > they still need to be careful to not shoot off their right foot, > are we saving them any mental effort? > > My goal for the nested-rebase feature is that it can be useful for > *recovering* from such messes. Back up a few steps to before the > trouble started, but *not* a full abort. That might actually be better accomplished as a `--rewind` as suggested by Phillip. It does not necessarily need a full-blown nested rebase. But as Philip hinted at, this might be quite complicated in the `--rebase-merges` case, as you cannot necessarily infer the correct labels. Example: label onto # Branch: one reset onto pick first label one # Branch: two reset one pick second label two If you try to redo both `first` and `second`, how on Earth should a nested (or rewinding) rebase figure out that it has to re-label `one`? Done right, this will be a super useful feature. I will use it all the time. It has to be done right, though, and not limit me in annoying ways. > >> I think a more general-purpose recovery mechanism might be more > >> useful. > >> > >> For example, if the --edit-todo included a (commented-out) list of what > >> had already been done, then after realizing that you screwed up > >> conflict resolve b' and have now committed bad resolutions c' and d' > >> on top of it, you could easily rebase --nested and replace b', c' and d' > >> with the original b, c and d. > >> > >> Without aborting and throwing away a' as well, which was perhaps a lot of > >> work. > > > > We do have the `done` file, but that does not discern between commands > > in the todo list that have been there in the first place and commands that > > have been added by the user _during_ the rebase. And of course it does not > > reflect any commands that have been removed/changed by the user during the > > rebase. > > > > So yeah, something like a journal, maybe. > > I was thinking of the "done" file. The `done` file will not record what commands were rescheduled, so you can end up with "double entries". > At a low level, we already have the reflog, and giving the user yet > another transaction log to remember is undesirable. The reflog does not record `label` commands. In other words, neither will give you what you need. If you want to have a record what the todo list looked like, and what `HEAD` was, at the beginning of a nested rebase, you will have to actually record both. > If we just added the original commit ID to the automatically > generated reflog entry, si it also records the higher-level intention, > that might do it. I.e. change > > bc56134cfe278 HEAD@{3}: rebase (pick): wilc1000: Use crc7 in lib/ rather than a private copy > to > bc56134cfe278 HEAD@{3}: rebase (pick 01cb7b5b7851): wilc1000: Use crc7 in lib/ rather than a private copy > > Oh! What if we recorded, for the duration of the rebase, the todo > list state corresponding to reflog entries? (Or, equivalently, > the reflog state at each step through the todo list.) Then we > could back up the rebase with just a reflog entry pointer. > > I like this precisely because it doesn't give the user a new entity > to keep track of; it just makes an existing entity more useful. > > > Thank you for thinking about this feature, and for discussing it with me. > > I think it will be a really nice feature to have, and I want to avoid > > problems with a design that simply won't allow for certain use cases > > (remember how `git rebase --preserve-merges` does not allow for reordering > > commits, and how the design of that feature simply made this bug > > unfixable? I _dearly_ regret not thinking that through). > > Definitely, let's think it through. But remember that "perfection > is achieved, not when there is nothing more to add, but when there > is nothing left to take away." KISS is a design *goal*, not just > a necessary compromise. Right. I am not even talking about adding features. I am talking about not chiseling away the features that I _need_. Ciao, Dscho ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-04-04 12:39 ` Johannes Schindelin @ 2020-04-04 17:41 ` George Spelvin 2020-04-06 10:40 ` Sebastien Bruckert 0 siblings, 1 reply; 25+ messages in thread From: George Spelvin @ 2020-04-04 17:41 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, git, lkml I'm just trying to make the point that guardrails on "git rebase --nested" which don't exist on the more powerful and dangerous "git rebase --edit-todo" are a case of installing a high-security lock on a screen door. If you can come up with something that works for both, then great. But going to significant trouble (especially in terms of design complexity and legacy burden; I'm not worrying about SMOP) for a special-case solution that only works for one is a waste of effort. Both, or neither. Just one is bad design. Regarding the semantics, consider the following case: * Initial branch history is O-A-B-C-D * I initially "git rebase A" * Then realize that I made a mistake and "git rebase --nested A^" * I reverse the order of the commits to D-C-B-A * The rebase continues, and I successfully pick D and C. (remaining commands are "pick B" and "pick A" * Then I "git rebase --abort". What state should I expect to be returned to? (Without separately abortable nested rebases, the state after the nested rebase is exactly the same as if I'd used "git rebase A^" in the first place, which doesn't seem like a terribly bad thing.) ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-04-04 17:41 ` George Spelvin @ 2020-04-06 10:40 ` Sebastien Bruckert 2020-04-06 15:24 ` George Spelvin 0 siblings, 1 reply; 25+ messages in thread From: Sebastien Bruckert @ 2020-04-06 10:40 UTC (permalink / raw) To: George Spelvin; +Cc: Johannes Schindelin, Junio C Hamano, git Hi all, What is your problem actually ? You want to edit a commit before where you are in a rebase ? O --- A --- B --- C --- D * You are in a middle of a rebase at commit C. * You want to edit A without finishing all your actual rebase. Is that right ? Then, why making a whole new rebase for that operation ? In this example, you are finally editing A with some sort of new nested operation. This operation should not do anything else than this. Like something atomical, you edit the commit / add a commit / remove one, and that's all. End of the story. Back to the original rebase, and back to commit C. If that "nested" operation made conflict with B, we can move the actual rebase to B to clean the mess you made with the "nested" operation. But you are still in only one rebase. If you abort, everything gets cleaned up. I don't know if any of this is pertinent / understandable, but I hope it gave a fresh view on that. You guys are maybe a bit too focused on what to do in case of an abort of a nested rebase. However, we don't actually know if a nested rebase is the best solution for this job. My two cents ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-04-06 10:40 ` Sebastien Bruckert @ 2020-04-06 15:24 ` George Spelvin 2020-04-07 9:16 ` Sebastien Bruckert 0 siblings, 1 reply; 25+ messages in thread From: George Spelvin @ 2020-04-06 15:24 UTC (permalink / raw) To: Sebastien Bruckert; +Cc: Johannes Schindelin, Junio C Hamano, git, lkml On Mon, Apr 06, 2020 at 06:40:07AM -0400, Sebastien Bruckert wrote: > What is your problem actually ? You want to edit a commit before where > you are in a rebase ? > > O --- A --- B --- C --- D > * You are in a middle of a rebase at commit C. > * You want to edit A without finishing all your actual rebase. > > Is that right ? > > Then, why making a whole new rebase for that operation ? Actually, it's a *bit* more complicated. I came across the need while preparing a large patch series for submission. I was going through the series, making sure the patches were in a logical order and didn't contain junk like an edit that should have been a fixup to an ealier patch. (Quite often, as I'm writing comments describing a new function, I tweak the comments later.) If I only want to fix up an old patch, I can make a fixup patch and merge it in in a later pass. If I want to edit the commit message again, I can't make the change and have git remember it for me, but I can at least make a a (possibly empty) squash commit with a note about the change. The hairy part comes when I'm doing a lot of reordering, and I realize that oh, damn it, commit C really should come before A in the patch series. (Or maybe C should be split and *part* of it should come before A.) I don't want to abort the rebase and restart, because I've already put a lot of work into rebasing A and B. (Which are each multiple patches, simplified to one for this explanation.) I've rearranged patches, changed function names and prototypes, and resolved the resultant conflicts. Just finishing up the rebase and restarting is a PITA, because I'll have more conflicts in D to resolve (again, D is not just a single commit), which will take some thinking, and by the time I"ve done all that I've forgotten what I was doing with C. What I'd like to do is just back up a few steps in the current rebase, put C there, and then resume rebasing D. Instead, I end up writing myself a note and, at the conclusion of the current rebase, starting a second one to apply the additional changes. > In this example, you are finally editing A with some sort of new > nested operation. This operation should not do anything else than > this. Like something atomical, you edit the commit / add a commit / > remove one, and that's all. End of the story. Back to the original > rebase, and back to commit C. If that "nested" operation made > conflict with B, we can move the actual rebase to B to clean the mess > you made with the "nested" operation. But you are still in only one > rebase. If you abort, everything gets cleaned up. > > I don't know if any of this is pertinent / understandable, but I hope > it gave a fresh view on that. You guys are maybe a bit too focused on > what to do in case of an abort of a nested rebase. However, we don't > actually know if a nested rebase is the best solution for this job. This sounds a lot like my original (and still preferred) design: it's not really a nested transaction in the database sense (which can be aborted independent of the outer commit), it's just one rebase that I rewind. I think a full nested transaction is too much conceptual complexity. And my primary use case is rearranging commits, so I want to move commits between the "outer" and "inner" rebase, which makes defining the boundary of the inner rebase problematic. But Johannes Schindelin seems quite forcefully opposed to the lack of a nested abort, so we're hashing it out. I'm very interested in your opinion, but please note that we already have fixup commits for amending single commits in place. The problem that currently has no good solution arises when I realize halfway through a cleanup pass that things would be a lot simpler if I moved A to after C. "Hey, rather than adding A and then updating it to take C into account, how about I just do commit C first, and then add the final code of A in one step?" That is, I want to change from O-A-B-C-D to O-B-C-A-D, but I didn't think of it until the rebase had reached O-A-B-C-. I think of it as "quilt pop" operation, taking patches off the applied list and putting them back on the todo. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-04-06 15:24 ` George Spelvin @ 2020-04-07 9:16 ` Sebastien Bruckert 2020-04-07 19:03 ` George Spelvin 0 siblings, 1 reply; 25+ messages in thread From: Sebastien Bruckert @ 2020-04-07 9:16 UTC (permalink / raw) To: George Spelvin; +Cc: Johannes Schindelin, Junio C Hamano, git Thank you for the more detailed explanation. > I'm very interested in your opinion, but please note that we already have > fixup commits for amending single commits in place. Yes sorry, I still don't know everything about git and the way it works. But I'm working on it! > The problem that currently has no good solution arises when I realize > halfway through a cleanup pass that things would be a lot simpler if I > moved A to after C. "Hey, rather than adding A and then updating it to > take C into account, how about I just do commit C first, and then add the > final code of A in one step?" That is, I want to change from O-A-B-C-D to > O-B-C-A-D, but I didn't think of it until the rebase had reached O-A-B-C-. > > I think of it as "quilt pop" operation, taking patches off the > applied list and putting them back on the todo. Hmmm so you need some way to move C before your actual commit. To make it like a pseudo command, some kind of "git rebase --reattach C --after A"? This seems closer to your original idea. Or why not modify "--edit-todo" to get commits from before your actual point? It could works like this: Before: ``` #pick b2a96fe O #pick acb7459 A #pick 0dac4a4 B edit 1f54e51 C edit cda2a7e D ``` After: ``` #pick b2a96fe O edit 1f54e51 C pick acb7459 A pick 0dac4a4 B edit cda2a7e D ``` So that you are still at C, but keeping the changes you made before on A and B, and going through them only if you have conflicts. If I understand your problem correctly (I hope), the more I think about it, the more this modified "--edit-todo" makes sense too me. Moreover, no problem for aborting, no big conceptual change, no change in the way rebase works. Only --edit-todo is enhanced. It coincides with what you were saying at the beginning: > On Fri, Mar 20, 2020 at 03:51:20PM -0700, Junio C Hamano wrote: > > I thought that "git rebase -i" allows the todo file (i.e. list of > > steps still to be performed) to be edited before continuing; would> > your use case be supported by using that? > > Mostly, if I do it very carefully, which is why I thought it would > be easy to add. > > I think I could manually add the commits to the start of the todo file, > reset --hard to the old state, and rebase --continue. > > But cutting and pasting commit IDs from git log into the todo file, > and putting fixup commits in the right place is annoyingly fiddly. > That's exactly the sort of thing computers are good at. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-04-07 9:16 ` Sebastien Bruckert @ 2020-04-07 19:03 ` George Spelvin 0 siblings, 0 replies; 25+ messages in thread From: George Spelvin @ 2020-04-07 19:03 UTC (permalink / raw) To: Sebastien Bruckert; +Cc: Johannes Schindelin, Junio C Hamano, git, lkml On Tue, Apr 07, 2020 at 05:16:50AM -0400, Sebastien Bruckert wrote: > Hmmm so you need some way to move C before your actual commit. To make > it like a pseudo command, some kind of "git rebase --reattach C > --after A"? This seems closer to your original idea. That seems too special-purpose and hard to remember. The standard rebase -i operation (generate a todo list and drop me into an editor) is perfectly adequate and more familiar. This is a very manual operation, after all. ("-i" for "interactive" means "manual".) > Or why not modify "--edit-todo" to get commits from before your actual > point? It could works like this: > > Before: > ``` > #pick b2a96fe O > #pick acb7459 A > #pick 0dac4a4 B > edit 1f54e51 C > edit cda2a7e D > ``` > > After: > ``` > #pick b2a96fe O > edit 1f54e51 C > pick acb7459 A > pick 0dac4a4 B > edit cda2a7e D > ``` > > So that you are still at C, but keeping the changes you made before on > A and B, and going through them only if you have conflicts. Because the only reason this is interesting assumes that A and B have changed! If I didn't intend to modify A somehow, I wouldn't have included it in the rebase range. They're now edited patches A' and B'. So the state I want to get to is: #pick b2a96fe O #pick acb7459 A #pick 0dac4a4 B reset b2a96fe O edit 1f54e51 C pick 7f0bcab A' pick fcd3c62 B' edit cda2a7e D where it will pick the versions of A and B that include the edits I've already made. Now, having the original commits mentioned in comments is useful, in case I made a mess of the edit and want to revert it. E.g. I can certainly see realizing, three commits later in the rebase, that I recolved a conflict wrong and should re-do it. Although this can probably be handled with a fixup. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-28 14:25 ` Johannes Schindelin 2020-03-28 16:30 ` George Spelvin @ 2020-03-30 14:01 ` Philip Oakley 2020-03-30 18:18 ` George Spelvin 1 sibling, 1 reply; 25+ messages in thread From: Philip Oakley @ 2020-03-30 14:01 UTC (permalink / raw) To: Johannes Schindelin, George Spelvin; +Cc: Junio C Hamano, git Hi all, On 28/03/2020 14:25, Johannes Schindelin wrote: > Hi George, > > On Thu, 26 Mar 2020, George Spelvin wrote: > >> On Wed, Mar 25, 2020 at 08:26:48PM +0100, Johannes Schindelin wrote: >>> On Sat, 21 Mar 2020, George Spelvin wrote: >>>> My assumption has been that, for simplicity, there would only be one >>>> commit in progress, and aborting it aborts everything. >>> But that does not necessarily make sense. Imagine that you rebase the >>> latest three commits, interactively. Then a merge conflict in the third >>> makes you realize that the first commit is no longer needed. >>> >>> Enter the nested rebase. You manually re-schedule the failed `pick` via >>> `git rebase --edit-todo` and then run the nested rebase: `git reset --hard >>> && git rebase -i --nested HEAD~2`. >>> >>> Except that you made a typo and said `HEAD~3` instead of `HEAD~2`. You >>> delete the entire todo list to get a chance to restart the nested rebase. >>> >>> But now the entire rebase gets aborted? >> Um, this example is not persuasive. If I just leave the excess commit at >> the front of the to-do list, then it will be recreated without change. > There are _many_ ways to mess up a nested rebase, including (but not > limited to) `--onto`, forgetting `-r`, editing the todo list too much in > an editor without undo. > > If you are suggesting that a nested `git rebase -i` would not need a way > to abort _just_ the nested rebase, then I fear we must stop the > conversation right here. That's not going to fly. > >> (Note that if I choose too *small* a nubmer by accident, I can insert a >> "break" at the front of the list and then rebase --nested starting from >> there.) > There are many ways how a savvy user would be able to work around the > absence of a proper way to abort a nested rebase. The common theme for all > of those is: > > - they are all quite involved and require knowledge of internals > > - they won't change the fact that it would be seriously negligent for us > to _not_ offer a way to abort nested rebases. > Perhaps we can go the other way on this one. I'd agree that attempting to nest (misunderstood mistaken) rebases is digging a too deep hole that we'd not get out of. However we do have other rebases available, specifically the "rebasing merges" https://git-scm.com/docs/git-rebase#_rebasing_merges. I know rebasing merges is way down the man page, but it has all the power and flexibility needed _if_ we can step across from the mistaken rebase step (we are at the command prompt aren't we?) into the rebasing merge mode. This will require a little bit of expansion of the insn (instruction) sheet so as to _include commented lines of the rebase steps completed_ so far, along with the labels, resets, merges, etc, so that the user can _see_ where they they are within their failed progress (along with a title line telling them their initial command and that they are now on a rebasing merge insn;-). From there they can update the insn to reset back to the correct point, redo the correct picks, and then get back to their remaining rebase steps. It's a thought anyway. HTH Philip ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-30 14:01 ` Philip Oakley @ 2020-03-30 18:18 ` George Spelvin 2020-03-30 21:53 ` Philip Oakley 0 siblings, 1 reply; 25+ messages in thread From: George Spelvin @ 2020-03-30 18:18 UTC (permalink / raw) To: Philip Oakley; +Cc: Johannes Schindelin, Junio C Hamano, git, lkml On Mon, Mar 30, 2020 at 03:01:28PM +0100, Philip Oakley wrote: > Perhaps we can go the other way on this one. > > I'd agree that attempting to nest (misunderstood mistaken) rebases is > digging a too deep hole that we'd not get out of. However we do have > other rebases available, specifically the "rebasing merges" > https://git-scm.com/docs/git-rebase#_rebasing_merges. > > I know rebasing merges is way down the man page, but it has all the > power and flexibility needed _if_ we can step across from the mistaken > rebase step (we are at the command prompt aren't we?) into the rebasing > merge mode. > > This will require a little bit of expansion of the insn (instruction) > sheet so as to _include commented lines of the rebase steps completed_ > so far, along with the labels, resets, merges, etc, so that the user can > _see_ where they they are within their failed progress (along with a > title line telling them their initial command and that they are now on a > rebasing merge insn;-). > > From there they can update the insn to reset back to the correct point, > redo the correct picks, and then get back to their remaining rebase steps. > > It's a thought anyway. I'm confused. *How* does --rebase-merge mode help? You're saying "hey, if we use this, it solves the issue" but I don't see how to pound this nail with that screwdriver. I don't see how creating a branching history helps, and I don't see how to use the reset/label/merge commands to do anything but create a branching history. I suppose it is possible to use the "reset" command in isolation to describe the jump to a new base. So you could have a history of: # Command already executed: # reset base # pick A # pick B # pick C # label rebase-1 User asked for a nested rebase # reset A' # Commands pending: pick B' pick C' # rebase-2 complete, resume rebase-1 pick D pick E Is that what you were getting at? I was thinking of it being implicit, but it might be nice for the initial "reset" in each rebase to be explicit, *and not yet executed during the initial todo edit*. That makes it really clear that deleting the todo list entirely results in no change to the tree. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-30 18:18 ` George Spelvin @ 2020-03-30 21:53 ` Philip Oakley 0 siblings, 0 replies; 25+ messages in thread From: Philip Oakley @ 2020-03-30 21:53 UTC (permalink / raw) To: George Spelvin; +Cc: Johannes Schindelin, Junio C Hamano, git Hi George, I wasn't suggesting that there already exists a complete solution... On 30/03/2020 19:18, George Spelvin wrote: > On Mon, Mar 30, 2020 at 03:01:28PM +0100, Philip Oakley wrote: >> Perhaps we can go the other way on this one. >> >> I'd agree that attempting to nest (misunderstood mistaken) rebases is >> digging a too deep hole that we'd not get out of. However we do have >> other rebases available, specifically the "rebasing merges" >> https://git-scm.com/docs/git-rebase#_rebasing_merges. I was mainly saying that folks like their "all on one page, flow chart sequence planning", and that we already have something close to that that could be extended. >> >> I know rebasing merges is way down the man page, but it has all the >> power and flexibility needed _if_ we can step across from the mistaken >> rebase step (we are at the command prompt aren't we?) into the rebasing >> merge mode. >> >> This will require a little bit of expansion of the insn (instruction) >> sheet so as to _include commented lines of the rebase steps completed_ >> so far, along with the labels, resets, merges, etc, so that the user can >> _see_ where they they are within their failed progress (along with a >> title line telling them their initial command and that they are now on a >> rebasing merge insn;-). >> >> From there they can update the insn to reset back to the correct point, >> redo the correct picks, and then get back to their remaining rebase steps. >> >> It's a thought anyway. > I'm confused. *How* does --rebase-merge mode help? You're saying > "hey, if we use this, it solves the issue" but I don't see how to > pound this nail with that screwdriver. Remember that rebasing is just a variant of creating and building a new branch in our WORM (Write once Read many) repo.. So it is "just" a case of taking the partially complete rebase, and re-working it, just like you can with an arbitrary merging rebase. > > I don't see how creating a branching history helps, and I don't see how to > use the reset/label/merge commands to do anything but create a branching > history. It is more that we could create a full instruction sheet that spans time from the start of the rebase (including labelling and resetting to get started), upto and including the current state (so you can pick from your new commits[*]), with those history steps being hash "#" commented, as well as the remaining rebase instructions. > I suppose it is possible to use the "reset" command in isolation > to describe the jump to a new base. So you could have a history of: > > # Command already executed: #git rebase <command options> (for reference & immediate use) > # reset base > # pick A > # pick B > # pick C > # label rebase-1 User asked for a nested rebase > # reset A' [*] We are missing a list of the current state of new commits, # reset base (done, see above) # placed A' (you'll need this listed if you need to pick/edit/reword # You are now at deadbeef, rebasing 23 of 75 commits .... (..suitable status message) > # Commands pending: > pick B' > pick C' > # rebase-2 complete, resume rebase-1 > pick D > pick E > > Is that what you were getting at? Correct. Given the global state problem, and the mental picture issues, let's give the user a full picture, flow chart style (it's a solid old technique that works well, even for beginners) > > I was thinking of it being implicit, but it might be nice for the initial > "reset" in each rebase to be explicit, *and not yet executed during > the initial todo edit*. > > That makes it really clear that deleting the todo list entirely > results in no change to the tree. (folks rarely notice omissions, we'll still need to tell them that it's a valid solution! sometimes empty means 'no change, carry on what you were doing';-) Philip ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Feature request: rebase -i inside of rebase -i 2020-03-20 22:30 Feature request: rebase -i inside of rebase -i George Spelvin 2020-03-20 22:51 ` Junio C Hamano @ 2020-03-21 8:47 ` Johannes Sixt 1 sibling, 0 replies; 25+ messages in thread From: Johannes Sixt @ 2020-03-21 8:47 UTC (permalink / raw) To: George Spelvin; +Cc: git Am 20.03.20 um 23:30 schrieb George Spelvin: > I'm cleaning up a patch series for submission, and came across a fixup in > patch #4/20 that belongs in #2/20. > > Unfortunately, I can't go back two patches to apply the fix until I get to > the end of the current rebase, then go back down to clean it up. :-( In such cases, I usually - unstage the changes of the current commit - stage the fixup - `git commit --fixup @~2` - stage what I had unstaged first - then continue At the end of the current rebase, a `git rebase -i --autosquash` does the final cleanup. -- Hannes ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2020-04-07 19:03 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-20 22:30 Feature request: rebase -i inside of rebase -i George Spelvin 2020-03-20 22:51 ` Junio C Hamano 2020-03-20 23:35 ` George Spelvin 2020-03-21 10:51 ` Johannes Schindelin 2020-03-21 17:56 ` George Spelvin 2020-03-25 19:26 ` Johannes Schindelin 2020-03-26 0:18 ` George Spelvin 2020-03-28 14:25 ` Johannes Schindelin 2020-03-28 16:30 ` George Spelvin 2020-03-31 0:00 ` George Spelvin 2020-03-31 10:57 ` Philip Oakley 2020-03-31 13:36 ` Phillip Wood 2020-04-01 16:43 ` Philip Oakley 2020-04-07 15:54 ` Phillip Wood 2020-04-04 12:17 ` Johannes Schindelin 2020-04-04 12:39 ` Johannes Schindelin 2020-04-04 17:41 ` George Spelvin 2020-04-06 10:40 ` Sebastien Bruckert 2020-04-06 15:24 ` George Spelvin 2020-04-07 9:16 ` Sebastien Bruckert 2020-04-07 19:03 ` George Spelvin 2020-03-30 14:01 ` Philip Oakley 2020-03-30 18:18 ` George Spelvin 2020-03-30 21:53 ` Philip Oakley 2020-03-21 8:47 ` Johannes Sixt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).