| Message ID | 8d1dec51b704c45bf36af24f657cc40f006989e2.1542065154.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Assorted fixes revolving around rebase and merges | expand |
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > 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@gmx.de> > --- > 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? 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. > ret = finish_rebase(&options); > goto cleanup; > }
Hi Junio, On Tue, 13 Nov 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > 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@gmx.de> > > --- > > 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; > > } >
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(); ret = finish_rebase(&options); goto cleanup; }