Feature request: rebase -i inside of rebase -i

Feature request: rebase -i inside of rebase -i

[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. :-(

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.)

Re: Feature request: rebase -i inside of rebase -i

[Junio C Hamano]

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?

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.

Re: Feature request: rebase -i inside of rebase -i

[Johannes Sixt]

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

Re: Feature request: rebase -i inside of rebase -i

[Johannes Schindelin]

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

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.

Re: Feature request: rebase -i inside of rebase -i

[Johannes Schindelin]

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

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.

Re: Feature request: rebase -i inside of rebase -i

[Johannes Schindelin]

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

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.

Re: Feature request: rebase -i inside of rebase -i

[Philip Oakley]

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

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.

Re: Feature request: rebase -i inside of rebase -i

[Philip Oakley]

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

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.

Re: Feature request: rebase -i inside of rebase -i

[Philip Oakley]

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.

Re: Feature request: rebase -i inside of rebase -i

[Phillip Wood]

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

Re: Feature request: rebase -i inside of rebase -i

[Philip Oakley]

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

Re: Feature request: rebase -i inside of rebase -i

[Johannes Schindelin]

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.
>

Re: Feature request: rebase -i inside of rebase -i

[Johannes Schindelin]

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

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.)

Re: Feature request: rebase -i inside of rebase -i

[Sebastien Bruckert]

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

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.

Re: Feature request: rebase -i inside of rebase -i

[Sebastien Bruckert]

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.

Re: Feature request: rebase -i inside of rebase -i

[Phillip Wood]

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
> 

Re: Feature request: rebase -i inside of rebase -i

[George Spelvin]

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.