Re: [PATCH 3/4] rebase: teach `reset_head()` to optionally skip the worktree

[Junio C Hamano <gitster@pobox.com>]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 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" ;-)

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  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.

>  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?

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.

Thanks.