Re: [BUG?] 'git rebase --abort' couldn't abort aborted rebase

[Elijah Newren <newren@gmail.com>]

On Fri, Feb 7, 2020 at 5:21 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Fri, Feb 07, 2020 at 12:49:54PM +0100, SZEDER Gábor wrote:
> > On Fri, Feb 07, 2020 at 12:10:08PM +0100, SZEDER Gábor wrote:
> > > That's a good subject, isn't it? :)

Indeed.  :-)

> > > So, to clarify: apparently it is possible to abort an ongoing 'git
> > > rebase' process with ctrl-C in just the right moment that a subsequent
> > > 'git rebase --abort' will refuse to clear it up.
> > >
> > > I somehow messed up the upstream and branch parameters of 'git
> > > rebase', and ended up trying to rebase a fairly recent (post v2.24.0)
> > > branch on top of v2.22.0.  Upon seeing the unexpectedly large number
> > > of patches I realized that something is wrong, hit ctrl-C right away,
> > > and this is what happened:
> > >
> > >   $ git rebase v2.22.0 <a-branch-on-top-of-2.24.0>
> > >   First, rewinding head to replay your work on top of it...
> > >   Generating patches: 100% (1108/1108), done.
> > >   Applying: send-email: move the read_config() function above getopts
> > >   Applying: send-email: rename the @bcclist variable for consistency
> > >   Applying: send-email: do defaults -> config -> getopt in that order
> > >   Using index info to reconstruct a base tree...
> > >   M       git-send-email.perl
> > >   M       t/t9001-send-email.sh
> > >   Falling back to patching base and 3-way merge...
> > >   Auto-merging t/t9001-send-email.sh
> > >   Auto-merging git-send-email.perl
> > >   ^C
> > >   ((5f07da12ac...) *|REBASE 3/1108)$ git rebase --abort
> > >   error: could not read '/home/szeder/src/git/.git/worktrees/WT/rebase-apply/head-name': No such file or directory
> > >
> > > "Fortunately" it was in a separate worktree, so I could easily get out
> > > of the situation by forcibly deleting that worktree.  Unfortunately,
> > > that was exactly what I did, instead of securing the failed state for
> > > later analysis...  sorry.
> >
> > > All this is with a git built from current 'next', with a bunch of
> > > unrelated (none of them touches rebase or the sequencer) patches on
> > > top.
> >
> > Trying to reproduce it is a hit or miss... well, mostly miss :)
> >
> > There is a rather short window when 'git rebase' applies patches
> > before hitting a first merge conflict.  If the ctrl-C arrives before
> > 'git rebase' starts applying patches, then it cleans everything up,
> > and we are not rebasing, so there is no need for 'git rebase --abort'.
> > Once 'git rebase' stops because of the merge conflict we get our shell
> > back, and 'git rebase --abort' works as it should.  But after a good
> > couple of tries I managed to hit ctrl-C while 'git rebase' was
> > applying patches:
>
> This patch below increases the size of the window where a ctrl-C gets
> us into that problematic situation:
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 8181c2aef3..5d62766000 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1737,6 +1737,14 @@ static void am_run(struct am_state *state, int resume)
>                         exit(1);
>
>                 say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg);
> +               {
> +                       int i;
> +                       for (i = 0; i < 60; i++) {
> +                               fprintf(stderr, "sleeping... %d\r", i);
> +                               sleep(1);
> +                       }
> +                       fprintf(stderr, "\n");
> +               }
>
>                 apply_status = run_apply(state, NULL);
>
> And then we can reliably reproduce the issue even when rebasing only a
> single commit:
>
>   $ ./bin-wrappers/git rebase v2.25.0 9c8a294
>   First, rewinding head to replay your work on top of it...
>   Applying: sha1-file: remove OBJECT_INFO_SKIP_CACHED
>   ^Ceeping... 3
>   ((v2.25.0)|REBASE 1/1)$ ./bin-wrappers/git rebase --abort
>   error: could not read '.git/rebase-apply/head-name': No such file or directory
>
> Note that the sleep() calls were added to 'builtin/am.c', i.e. this
> issue is present in the 'am' rebase backend.  I tried to break 'git
> rebase -m ...' by adding sleep()s to various places in pick_commits()
> in 'sequencer.c', but, FWIW, the subsequent 'git rebase --abort'
> always worked as expected.
>
> So we may have yet another reason to switch the default rebase backend
> from 'am' to 'merge'.
>
>
> > Finally, note the 'v2.24.0^{commit}' parameter, in particular the
> > '^{commit}' part.  That's important, because without it we stumble
> > upon _another_ bug:
> >
> >   $ git rebase v2.22.0 v2.24.0
> >   error: Object 1cc4bc0fcd93f816d514d77c29f2cc9ffdd8ae09 not a commit
> >   First, rewinding head to replay your work on top of it...
> >   Generating patches: 100% (1049/1049), done.
> >   Applying: send-email: move the read_config() function above getopts
> >   < ... applying further patches and hitting a merge conflict ... >
> >   Resolve all conflicts manually, mark them as resolved with
> >   "git add/rm <conflicted_files>", then run "git rebase --continue".
> >   You can instead skip this commit: run "git rebase --skip".
> >   To abort and get back to the state before "git rebase", run "git rebase --abort".
> >   (detached HEAD *+|REBASE 7/1049)$ git rebase --abort
> >   error: update_ref failed for ref 'HEAD': cannot update ref 'HEAD': trying to write non-commit object 1cc4bc0fcd93f816d514d77c29f2cc9ffdd8ae09 to branch 'HEAD'
> >
> > So 'git rebase' shows an error right at the beginning when rebasing a
> > tag, but then continues anyway.  However, 'git rebase --abort' can't
> > restore the original state.
>
> Now let's rebase a tag with the 'merge' backend, and then try to abort
> it:
>
>   $ ./BUILDS/v2.25.0/bin/git rebase -m v2.22.0 v2.24.0
>   error: Object 1cc4bc0fcd93f816d514d77c29f2cc9ffdd8ae09 not a commit
>   Auto-merging t/t9001-send-email.sh
>   Auto-merging git-send-email.perl
>   CONFLICT (content): Merge conflict in git-send-email.perl
>   Auto-merging Documentation/git-send-email.txt
>   error: could not apply 3ff15040e2... send-email: fix regression in sendemail.identity parsing
>   Resolve all conflicts manually, mark them as resolved with
>   "git add/rm <conflicted_files>", then run "git rebase --continue".
>   You can instead skip this commit: run "git rebase --skip".
>   To abort and get back to the state before "git rebase", run "git rebase --abort".
>   Recorded preimage for 'git-send-email.perl'
>   Could not apply 3ff15040e2... send-email: fix regression in sendemail.identity parsing
>   (detached HEAD *+|REBASE 7/1049)$ ./BUILDS/v2.25.0/bin/git rebase --abort
>   ((v2.24.0))$
>
> So it prints the same error as the 'am' backend (or perhaps that error
> comes from the common, backend-independent parts of rebase?  I didn't
> look), and it continues all the same, but in the end '--abort' is
> capable to abort the operation.  So we do have yet another reason to
> switch the default backend.

Ooh, interesting; thanks for investigating and providing a heads up.
I should probably add a note about this to the "differences between
backends"; I'll roll it into whatever changes I might need to make
with Phillip's review of the series.