Re: [PATCH 03/11] reset_head(): don't run checkout hook if there is an error

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Oct 4, 2021, 10:00 UTC
Message-ID: <b298f8cb-f55f-86f0-52fc-8d9db0047525@gmail.com>
In-Reply-To: <xmqqtui05uxr.fsf@gitster.g>

Hi Junio

On 01/10/2021 21:52, Junio C Hamano wrote:

Show 11 quoted lines

> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> The hook should only be run if the worktree and refs were successfully
>> updated.
> 
> OK.  This is a behaviour change visible to end-users, and deserves a
> mention in the release notes.
> 
>   - When "git rebase" attempted to check out a branch (or detached

It only affects "git rebase --apply" as the "merge" backend forks "git checkout" which does not run the hook if it cannot update the worktree or refs.

Show 8 quoted lines

>     the HEAD) to work on, we used to always call the "post-checkout"
>     hook, even if the checkout failed to update the ref.  The hook is
>     no longer called if the checkout fails.
> 
> or something.
> 
> Again, can the bug this step fixes be protected with a new test in
> t/ please?

I'll try and come up with something - it should be possible to arrange an untracked file to make unpack_trees() to fail, I'm not sure how we'd make update_ref() fail

Best Wishes

Phillip

Show 19 quoted lines

>>
>> 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 fc4dae3fd2d..5abb1a5683e 100644
>> --- a/reset.c
>> +++ b/reset.c
>> @@ -125,7 +125,7 @@ reset_head_refs:
>>   			ret = create_symref("HEAD", switch_to_branch,
>>   					    reflog_head);
>>   	}
>> -	if (run_hook)
>> +	if (!ret && run_hook)
>>   		run_hook_le(NULL, "post-checkout",
>>   			    oid_to_hex(orig ? orig : null_oid()),
>>   			    oid_to_hex(oid), "1", NULL);