| Message ID | 5ea636009e7858e50357f0f6f8d8fa42e056db60.1638975482.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ee464c4e37c7f34c4e5ba2fce35df4149083e5ea |
| Headers | show |
| Series | rebase: reset_head() related fixes and improvements | expand |
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > If ORIG_HEAD is not set by passing RESET_ORIG_HEAD then there is no > need to pass anything for reflog_orig_head. In addition to the callers > fixed in this commit move_to_original_branch() also passes > reflog_orig_head without setting ORIG_HEAD. That caller is mistakenly > passing the message it wants to put in the branch reflog which is not > currently possible so we delay fixing that caller until we can pass > the message as the branch reflog. As I hinted elsewhere, these rules should be spelled out in a comment before "int reset_head(...)" either in reset.[ch]. For this particular one, I wonder if (A) we can lose RESET_ORIG_HEAD bit and use the presence of reflog_orig_head to mean what that bit currently means, or (B) we keep the current code strucure, but make it a BUG() if a non-NULL reflog_orig_head is given without RESET_ORIG_HEAD bit set. > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > builtin/rebase.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 3d78b5c8bef..fdd822c470f 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -675,7 +675,7 @@ static int run_am(struct rebase_options *opts) > > reset_head(the_repository, &opts->orig_head, > opts->head_name, 0, > - "HEAD", NULL, DEFAULT_REFLOG_ACTION); > + NULL, NULL, DEFAULT_REFLOG_ACTION); > error(_("\ngit encountered an error while preparing the " > "patches to replay\n" > "these revisions:\n" > @@ -1777,7 +1777,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > options.head_name ? options.head_name : "detached HEAD", > oid_to_hex(&options.onto->object.oid)); > reset_head(the_repository, NULL, options.head_name, > - RESET_HEAD_REFS_ONLY, "HEAD", msg.buf, NULL); > + RESET_HEAD_REFS_ONLY, NULL, msg.buf, NULL); > strbuf_release(&msg); > ret = finish_rebase(&options); > goto cleanup;
On 09/12/2021 19:26, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> If ORIG_HEAD is not set by passing RESET_ORIG_HEAD then there is no >> need to pass anything for reflog_orig_head. In addition to the callers >> fixed in this commit move_to_original_branch() also passes >> reflog_orig_head without setting ORIG_HEAD. That caller is mistakenly >> passing the message it wants to put in the branch reflog which is not >> currently possible so we delay fixing that caller until we can pass >> the message as the branch reflog. > > As I hinted elsewhere, these rules should be spelled out in a > comment before "int reset_head(...)" either in reset.[ch]. > > For this particular one, I wonder if > > (A) we can lose RESET_ORIG_HEAD bit and use the presence of > reflog_orig_head to mean what that bit currently means, or > > (B) we keep the current code strucure, but make it a BUG() if > a non-NULL reflog_orig_head is given without RESET_ORIG_HEAD > bit set. We do (B) later in the series, it cannot be done in this commit as we leave move_to_original_branch() unchanged here and that would BUG() out. I've added a comment it the commit message to explain this. Best Wishes Phillip > >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> builtin/rebase.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/builtin/rebase.c b/builtin/rebase.c >> index 3d78b5c8bef..fdd822c470f 100644 >> --- a/builtin/rebase.c >> +++ b/builtin/rebase.c >> @@ -675,7 +675,7 @@ static int run_am(struct rebase_options *opts) >> >> reset_head(the_repository, &opts->orig_head, >> opts->head_name, 0, >> - "HEAD", NULL, DEFAULT_REFLOG_ACTION); >> + NULL, NULL, DEFAULT_REFLOG_ACTION); >> error(_("\ngit encountered an error while preparing the " >> "patches to replay\n" >> "these revisions:\n" >> @@ -1777,7 +1777,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) >> options.head_name ? options.head_name : "detached HEAD", >> oid_to_hex(&options.onto->object.oid)); >> reset_head(the_repository, NULL, options.head_name, >> - RESET_HEAD_REFS_ONLY, "HEAD", msg.buf, NULL); >> + RESET_HEAD_REFS_ONLY, NULL, msg.buf, NULL); >> strbuf_release(&msg); >> ret = finish_rebase(&options); >> goto cleanup;
diff --git a/builtin/rebase.c b/builtin/rebase.c index 3d78b5c8bef..fdd822c470f 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -675,7 +675,7 @@ static int run_am(struct rebase_options *opts) reset_head(the_repository, &opts->orig_head, opts->head_name, 0, - "HEAD", NULL, DEFAULT_REFLOG_ACTION); + NULL, NULL, DEFAULT_REFLOG_ACTION); error(_("\ngit encountered an error while preparing the " "patches to replay\n" "these revisions:\n" @@ -1777,7 +1777,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.head_name ? options.head_name : "detached HEAD", oid_to_hex(&options.onto->object.oid)); reset_head(the_repository, NULL, options.head_name, - RESET_HEAD_REFS_ONLY, "HEAD", msg.buf, NULL); + RESET_HEAD_REFS_ONLY, NULL, msg.buf, NULL); strbuf_release(&msg); ret = finish_rebase(&options); goto cleanup;