On Tue Nov 19, 2024 at 11:54, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> "Bence Ferdinandy" <bence@xxxxxxxxxxxxxx> writes:
>
>> No, it is not, but it's also a mistake. It should be `updateres == 1`.
>> refs_update_symref_extended outputs -1 for "not a symref" and 1 for any other
>> error currently. Before I touched the code it was 1 for any error, so I left
>> that as is. So we want to error out on set_head if we get a 1 and continue if
>> we get 0 or -1 (and handle the difference in the report_set_head_auto).
>>
>> Thanks for noticing, I'll get that fixed in v14.
>
> It is good that somebody noticed it (and it may have happened to be
> me), but if it is a "mistake" as you said, I wonder why none of your
> tests caught it. Do we have a gap in test coverage?
I think there is no test that is testing this branch:
updateres = refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf,
"remote set-head", &b_local_head, 0);
if (updateres == 1) {
result |= error(_("Could not setup %s"), b_head.buf);
goto cleanup;
Running this in t/
grep -r "Could not setup"
also yield nothing, so that's probably true. I'm wondering what would be the
best way to trigger this error, refs_update_symref needs to fail for this.
