Re: [PATCH] rebase --abort: cleanup refs/rewritten

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Phillip,

On Wed, 1 May 2019, Phillip Wood wrote:

> On 30/04/2019 23:49, Johannes Schindelin wrote:
> >
> > On Tue, 30 Apr 2019, Phillip Wood wrote:
> >
> > > On 29/04/2019 17:07, Johannes Schindelin wrote:
> > > >
> > > > On Fri, 26 Apr 2019, Phillip Wood wrote:
> > > >
> > > > > From: Phillip Wood <phillip.wood@dunelm.org.uk>
> > > > >
> > > > > When `rebase -r` finishes it removes any refs under refs/rewritten
> > > > > that it has created. However if the rebase is aborted these refs are
> > > > > not removed. This can cause problems for future rebases. For example I
> > > > > recently wanted to merge a updated version of a topic branch into an
> > > > > integration branch so ran `rebase -ir` and removed the picks and label
> > > > > for the topic branch from the todo list so that
> > > > >       merge -C <old-merge> topic
> > > > > would pick up the new version of topic. Unfortunately
> > > > > refs/rewritten/topic already existed from a previous rebase that had
> > > > > been aborted so the rebase just used the old topic, not the new one.
> > > > >
> > > > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> > > > > ---
> > > >
> > > > Makes a ton of sense, and I feel a bit embarrassed that I forgot about
> > > > that item on my TODO list. The patch looks obviously correct!
> > >
> > > Thanks, after I sent it I realized that --quit should probably clear
> > > refs/rewritten as well, so I'll re-roll with that added. (One could argue
> > > that
> > > a user might want them after quitting the rebase but there is no way to
> > > clean
> > > them up safely once we've deleted the state files and I suspect most users
> > > would be suprised if they were left laying around)
> >
> > I am not so sure. `--quit` is essentially all about "leave the state
> > as-is, but still abort the rebase".
>
> I think it depends on what you mean by "state" `--quit` is about removing
> state specific to rebases while preserving HEAD, the index and worktree.

I guess the fault is mine for bleeding out internal rebase state into the
refs namespace.

While I cannot really imagine any harm from this patch in practice, it is
slightly worrisome that deleting refs also deletes their reflogs, which
makes it an unrecoverable problem *iff* any user runs into trouble with
this.

Ciao,
Dscho