Hi Junio,
On Tue, 13 Nov 2018, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> > The scripted version of the rebase used to execute `git reset --hard`
> > when skipping or aborting. When we ported this to C, we did update the
> > worktree and some reflogs, but we failed to imitate `git reset --hard`'s
> > behavior regarding files in .git/ such as MERGE_HEAD.
> >
> > Let's address this oversight.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > builtin/rebase.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 0ee06aa363..017dce1b50 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -23,6 +23,7 @@
> > #include "revision.h"
> > #include "commit-reach.h"
> > #include "rerere.h"
> > +#include "branch.h"
> >
> > static char const * const builtin_rebase_usage[] = {
> > N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] "
> > @@ -1002,6 +1003,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> >
> > if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
> > die(_("could not discard worktree changes"));
> > + remove_branch_state();
> > if (read_basic_state(&options))
> > exit(1);
> > goto run_rebase;
> > @@ -1019,6 +1021,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> > options.head_name, 0, NULL, NULL) < 0)
> > die(_("could not move back to %s"),
> > oid_to_hex(&options.orig_head));
> > + remove_branch_state();
>
> Hmph. Among 5 or so callsites of reset_head(), only these two
> places need it, so reset_head() is clearly not a substitute for
> "reset --hard HEAD", and it sometimes used to switch branches or
> something?
Indeed. There is also the `git reset --hard` call in the scripted
version's autostash code path. But that definitely does not need to remove
the branch state, as it is not recovering from a merge or cherry-pick or
revert.
> Perhaps we may need to rename it to avoid confusion but it can wait
> until we actually decide to make it externally available. Until then,
> it's OK as-is, I would think.
Okay.
Ciao,
Dscho
>
> > ret = finish_rebase(&options);
> > goto cleanup;
> > }
>
