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

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

Junio C Hamano <gitster@pobox.com> writes:

> "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.

The topics are os/rebase-runs-post-checkout-hook and nd/backup-log
IIRC.