Re: [PATCH 05/11] reset_head(): factor out ref updates

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 01/10/2021 22:00, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> In the next commit we will stop trying to update HEAD when we are just
>> clearing changes from the working tree. Move the code that updates the
> 
> "clearing changes" meaning...?  Like "rebase --abort"?

"rebase --abort" will update HEAD, "rebase --skip" would be a better 
example, I'll expand the commit message.

Best Wishes

Phillip

>> refs to its own function in preparation for that.
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   reset.c | 112 +++++++++++++++++++++++++++++++-------------------------
>>   1 file changed, 62 insertions(+), 50 deletions(-)
>>
>> diff --git a/reset.c b/reset.c
>> index 9ab007c0c34..668c7639127 100644
>> --- a/reset.c
>> +++ b/reset.c
>> @@ -8,26 +8,75 @@
>>   #include "tree.h"
>>   #include "unpack-trees.h"
>>   
>> +static int update_refs(const struct object_id *oid, const char *switch_to_branch,
>> +		       const char *reflog_head, const char *reflog_orig_head,
>> +		       const char *default_reflog_action, unsigned flags)
>> +{
>> +	unsigned detach_head = flags & RESET_HEAD_DETACH;
>> +	unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
>> +	unsigned update_orig_head = flags & RESET_ORIG_HEAD;
>> +	struct object_id *orig = NULL, oid_orig, *old_orig = NULL, oid_old_orig;
>> +	struct strbuf msg = STRBUF_INIT;
>> +	const char *reflog_action;
>> +	size_t prefix_len;
>> +	int ret;
>> +
>> +	reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
>> +	strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : default_reflog_action);
>> +	prefix_len = msg.len;
>> +
>> +	if (update_orig_head) {
>> +		if (!get_oid("ORIG_HEAD", &oid_old_orig))
>> +			old_orig = &oid_old_orig;
>> +		if (!get_oid("HEAD", &oid_orig)) {
>> +			orig = &oid_orig;
>> +			if (!reflog_orig_head) {
>> +				strbuf_addstr(&msg, "updating ORIG_HEAD");
>> +				reflog_orig_head = msg.buf;
>> +			}
>> +			update_ref(reflog_orig_head, "ORIG_HEAD", orig,
>> +				   old_orig, 0, UPDATE_REFS_MSG_ON_ERR);
>> +		} else if (old_orig)
>> +			delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
>> +	}
>> +
>> +	if (!reflog_head) {
>> +		strbuf_setlen(&msg, prefix_len);
>> +		strbuf_addstr(&msg, "updating HEAD");
>> +		reflog_head = msg.buf;
>> +	}
>> +	if (!switch_to_branch)
>> +		ret = update_ref(reflog_head, "HEAD", oid, orig,
>> +				 detach_head ? REF_NO_DEREF : 0,
>> +				 UPDATE_REFS_MSG_ON_ERR);
>> +	else {
>> +		ret = update_ref(reflog_head, switch_to_branch, oid,
>> +				 NULL, 0, UPDATE_REFS_MSG_ON_ERR);
>> +		if (!ret)
>> +			ret = create_symref("HEAD", switch_to_branch,
>> +					    reflog_head);
>> +	}
>> +	if (!ret && run_hook)
>> +		run_hook_le(NULL, "post-checkout",
>> +			    oid_to_hex(orig ? orig : null_oid()),
>> +			    oid_to_hex(oid), "1", NULL);
>> +	strbuf_release(&msg);
>> +	return ret;
>> +}
>> +
>>   int reset_head(struct repository *r, struct object_id *oid,
>>   	       const char *switch_to_branch, unsigned flags,
>>   	       const char *reflog_orig_head, const char *reflog_head,
>>   	       const char *default_reflog_action)
>>   {
>> -	unsigned detach_head = flags & RESET_HEAD_DETACH;
>>   	unsigned reset_hard = flags & RESET_HEAD_HARD;
>> -	unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
>>   	unsigned refs_only = flags & RESET_HEAD_REFS_ONLY;
>> -	unsigned update_orig_head = flags & RESET_ORIG_HEAD;
>>   	struct object_id head_oid;
>>   	struct tree_desc desc[2] = { { NULL }, { NULL } };
>>   	struct lock_file lock = LOCK_INIT;
>>   	struct unpack_trees_options unpack_tree_opts = { 0 };
>>   	struct tree *tree;
>> -	const char *action, *reflog_action;
>> -	struct strbuf msg = STRBUF_INIT;
>> -	size_t prefix_len;
>> -	struct object_id *orig = NULL, oid_orig,
>> -		*old_orig = NULL, oid_old_orig;
>> +	const char *action;
>>   	int ret = 0, nr = 0;
>>   
>>   	if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
>> @@ -47,7 +96,9 @@ int reset_head(struct repository *r, struct object_id *oid,
>>   		oid = &head_oid;
>>   
>>   	if (refs_only)
>> -		goto reset_head_refs;
>> +		return update_refs(oid, switch_to_branch, reflog_head,
>> +				   reflog_orig_head, default_reflog_action,
>> +				   flags);
>>   
>>   	action = reset_hard ? "reset" : "checkout";
>>   	setup_unpack_trees_porcelain(&unpack_tree_opts, action);
>> @@ -90,49 +141,10 @@ int reset_head(struct repository *r, struct object_id *oid,
>>   		goto leave_reset_head;
>>   	}
>>   
>> -reset_head_refs:
>> -	reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
>> -	strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : default_reflog_action);
>> -	prefix_len = msg.len;
>> -
>> -	if (update_orig_head) {
>> -		if (!get_oid("ORIG_HEAD", &oid_old_orig))
>> -			old_orig = &oid_old_orig;
>> -		if (!get_oid("HEAD", &oid_orig)) {
>> -			orig = &oid_orig;
>> -			if (!reflog_orig_head) {
>> -				strbuf_addstr(&msg, "updating ORIG_HEAD");
>> -				reflog_orig_head = msg.buf;
>> -			}
>> -			update_ref(reflog_orig_head, "ORIG_HEAD", orig,
>> -				   old_orig, 0, UPDATE_REFS_MSG_ON_ERR);
>> -		} else if (old_orig)
>> -			delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
>> -	}
>> -
>> -	if (!reflog_head) {
>> -		strbuf_setlen(&msg, prefix_len);
>> -		strbuf_addstr(&msg, "updating HEAD");
>> -		reflog_head = msg.buf;
>> -	}
>> -	if (!switch_to_branch)
>> -		ret = update_ref(reflog_head, "HEAD", oid, orig,
>> -				 detach_head ? REF_NO_DEREF : 0,
>> -				 UPDATE_REFS_MSG_ON_ERR);
>> -	else {
>> -		ret = update_ref(reflog_head, switch_to_branch, oid,
>> -				 NULL, 0, UPDATE_REFS_MSG_ON_ERR);
>> -		if (!ret)
>> -			ret = create_symref("HEAD", switch_to_branch,
>> -					    reflog_head);
>> -	}
>> -	if (!ret && run_hook)
>> -		run_hook_le(NULL, "post-checkout",
>> -			    oid_to_hex(orig ? orig : null_oid()),
>> -			    oid_to_hex(oid), "1", NULL);
>> +	ret = update_refs(oid, switch_to_branch, reflog_head, reflog_orig_head,
>> +			  default_reflog_action, flags);
>>   
>>   leave_reset_head:
>> -	strbuf_release(&msg);
>>   	rollback_lock_file(&lock);
>>   	clear_unpack_trees_porcelain(&unpack_tree_opts);
>>   	while (nr)