Hi Junio,
On Fri, 4 Jan 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> > This is what the legacy (scripted) rebase does in
> > `move_to_original_branch`, and we will need this functionality in the
> > next commit.
>
> The move-to-original-branch helper does:
>
> - point $head_name to the commit pointed at by HEAD
> - point HEAD symref to $head_name
>
> without touching the index or the working tree files. It's not
> exactly "reset --soft" but more like "switch-branch --soft" ;-)
Right, but it does reset the HEAD softly.
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > builtin/rebase.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 768bea0da8..303175fdf1 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -337,6 +337,7 @@ static void add_var(struct strbuf *buf, const char *name, const char *value)
> >
> > #define RESET_HEAD_DETACH (1<<0)
> > #define RESET_HEAD_HARD (1<<1)
> > +#define RESET_HEAD_REFS_ONLY (1<<2)
>
> In the future codebase in 'pu', we have 1<<2 already taken by
> another topic, so I'll tell my rerere database that the bit
> assignment needs to be adjusted.
Okay.
> > static int reset_head(struct object_id *oid, const char *action,
> > const char *switch_to_branch, unsigned flags,
> > @@ -344,6 +345,7 @@ static int reset_head(struct object_id *oid, const char *action,
> > {
> > unsigned detach_head = flags & RESET_HEAD_DETACH;
> > unsigned reset_hard = flags & RESET_HEAD_HARD;
> > + unsigned refs_only = flags & RESET_HEAD_REFS_ONLY;
> > struct object_id head_oid;
> > struct tree_desc desc[2] = { { NULL }, { NULL } };
> > struct lock_file lock = LOCK_INIT;
> > @@ -359,7 +361,7 @@ static int reset_head(struct object_id *oid, const char *action,
> > if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
> > BUG("Not a fully qualified branch: '%s'", switch_to_branch);
> >
> > - if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
> > + if (!refs_only && hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
> > ret = -1;
> > goto leave_reset_head;
> > }
>
> Not touching the index, so no need to lock the index. Makes sense.
>
> > @@ -372,6 +374,9 @@ static int reset_head(struct object_id *oid, const char *action,
> > if (!oid)
> > oid = &head_oid;
> >
> > + if (flags & RESET_HEAD_REFS_ONLY)
> > + goto reset_head_refs;
> > +
>
> Why not "refs_only" that we already prepared above??? Are we
> munging that secondary variable before control comes here?
No, just an oversight. Will fix.
> In any case, not touching the index nor the working tree, so no need
> to call into the unpack_trees machinery. Makes sense.
>
> > memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
> > setup_unpack_trees_porcelain(&unpack_tree_opts, action);
> > unpack_tree_opts.head_idx = 1;
> > @@ -412,6 +417,7 @@ static int reset_head(struct object_id *oid, const char *action,
> > goto leave_reset_head;
> > }
> >
> > +reset_head_refs:
> > reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
>
> And the control continues from the point we update the reflog.
> Makes sense.
>
> > strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
> > prefix_len = msg.len;
>
> This helper is touched by two other topics in flight, and that was
> one of the reason why it took a bit longer than usual for me to
> merge this topic. Please sanity-check the result of the conflict
> resolution at the tip of 'pu' branch.
Sorry, I *just now* had time to take care of this patch series. I will
gladly check once you integrate the new patch series iteration.
Thanks,
Dscho
