Re: [PATCH v2 2/2] rebase: set REF_HEAD_DETACH in checkout_up_to_date()

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mar 14, 2022, 14:05 UTC
Message-ID: <d5151784-3218-ab85-8226-f3d941f649b2@gmail.com>
In-Reply-To: <b35edfb8-9b48-d5b3-8d77-cb61a22f85ef@gmail.com>

On 14/03/2022 10:54, Phillip Wood wrote:

Show 24 quoted lines

> On 13/03/2022 07:58, Junio C Hamano wrote:
>> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> diff --git a/reset.c b/reset.c
>>> index e3383a93343..f8e32fcc240 100644
>>> --- a/reset.c
>>> +++ b/reset.c
>>> @@ -101,6 +101,9 @@ int reset_head(struct repository *r, const struct 
>>> reset_head_opts *opts)
>>>       if (opts->branch_msg && !opts->branch)
>>>           BUG("branch reflog message given without a branch");
>>> +    if (switch_to_branch && opts->flags & RESET_HEAD_DETACH)
>>
>> It's just style thing but it probably is easier to read to have
>> an extra () around the bitwise-&.
>>
>>> +        BUG("attempting to detach HEAD when branch is given");
>>
>> I wonder if there is a valid use case NOT to use RESET_HEAD_DETACH
>> when switch_to_branch == NULL.  If there isn't, it could be that
>> we can get rid of RESET_HEAD_DETACH bit and base this decision
>> solely on switch_to_branch'es NULLness.
> 
> "rebase --skip" and "rebase --autostash" are two such uses I think

Those don't update any refs though so are not helpful examples. Possibly when we fast-forward because HEAD is an ancestor of 'onto' is a potential case but at the moment I think we detach HEAD when checking out 'onto' and then update and checkout the branch if there is one (I've been thinking about fixing that so we only detach HEAD if we're going to rebase).

Best Wishes

Phillip

> Best Wishes
> 
> Phillip