From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 04 Oct 2021 09:58:59 GMT
Subject: Re: [PATCH 02/11] reset_head(): fix checkout
Message-ID: <648396f6-56ce-b727-4567-2b6eba2f4f46@gmail.com>
In-Reply-To: <xmqqee947aqn.fsf@gitster.g>

Hi Junio

On 01/10/2021 21:26, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> The reset bit should only be set if flags contains RESET_HEAD_HARD.
>> The test for `!deatch_head` dates back to the original implementation
>> of reset_head() in ac7f467fef ("builtin/rebase: support running "git
>> rebase <upstream>"", 2018-08-07) and was correct until e65123a71d
>> ("builtin rebase: support `git rebase <upstream> <switch-to>`",
>> 2018-09-04) started using reset_head() to checkout <switch-to> when
>> fast-forwarding.
> 
> Sorry, but it is not quite clear what exactly is "fix checkout" in
> the context of this change, even with the above paragraph that
> describes the internals but not any end-user visible effect.

"git checkout" refuses to overwrite untracked files but reset_head() 
does when checking out a branch.

> Can this step come with its own addition to t/ to demonstrate the
> breakage that is fixed?

I can add a test to check that a checkout does not remove untracked 
files. However such a test would pass on top of 
en/remaving-untracked-fixes without the fix in this patch. I cannot 
think of a way to specifically test that unpack_tree_opts.reset == 0 
unless RESET_HEAD_HARD is given after en/removing-untracked-fixes is 
merged. Elijah's fixes will stop the "reset" mode of reset_head() from 
wiping untracked files which is good. The reset flag to unpack trees 
also affects unmerged entries but rebase does not try to checkout 
anything if the index contains unmerged entries.

Best Wishes

Phillip

>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   reset.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/reset.c b/reset.c
>> index 79310ae071b..fc4dae3fd2d 100644
>> --- a/reset.c
>> +++ b/reset.c
>> @@ -57,7 +57,7 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
>>   	unpack_tree_opts.update = 1;
>>   	unpack_tree_opts.merge = 1;
>>   	init_checkout_metadata(&unpack_tree_opts.meta, switch_to_branch, oid, NULL);
>> -	if (!detach_head)
>> +	if (reset_hard)
>>   		unpack_tree_opts.reset = 1;
>>   
>>   	if (repo_read_index_unmerged(r) < 0) {