Jeff King <peff@xxxxxxxx> writes:
> On Thu, Jan 18, 2018 at 02:38:41PM +0100, Mathias Rav wrote:
>
>> Running git clone --single-branch --mirror -b TAGNAME previously
>> triggered the following error message:
>>
>> fatal: multiple updates for ref 'refs/tags/TAGNAME' not allowed.
>>
>> This error condition is handled in files_initial_transaction_commit().
>>
>> 42c7f7ff9 ("commit_packed_refs(): remove call to `packed_refs_unlock()`", 2017-06-23)
>> introduced incorrect unlocking in the error path of this function,
>> which changes the error message to
>>
>> fatal: BUG: packed_refs_unlock() called when not locked
>>
>> Move the call to packed_refs_unlock() above the "cleanup:" label
>> since the unlocking should only be done in the last error path.
>
> Thanks, this solution looks correct to me. It's pretty low-impact since
> the locking is the second-to-last thing in the function, so we don't
> have to re-add the unlock to a bunch of error code paths. But one
> alternative would be to just do:
>
> if (packed_refs_is_locked(refs))
> packed_refs_unlock(refs->packed_ref_store);
>
> in the cleanup section.
Yeah, that may be a more future-proof alternative, and just as you
said the patch as posted would be sufficient, too.
Thanks.
