From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 14 Jan 2022 21:43:55 GMT
Subject: Re: [PATCH v3 2/9] midx.c: make changing the preferred pack safe
Message-ID: <xmqqsftq9en8.fsf@gitster.g>
In-Reply-To: <xmqqwnj29f1v.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Taylor Blau <me@ttaylorr.com> writes:
>
>> ... It's likely we were using
>> finalize_object_file() instead of a pure rename() because the former
>> also adjusts shared permissions.
>
> I thought the primary reason why we use finalize was because we
> ignore EEXIST (and the assumption is that the files with the same
> contents get the same name computed from their contents).
>
>>  	tmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr,
>>  					midx_hash, WRITE_REV);
>>  
>> -	if (finalize_object_file(tmp_file, buf.buf))
>> +	if (rename(tmp_file, buf.buf))
>>  		die(_("cannot store reverse index file"));
>
> Doesn't your new code die with it if buf.buf names an existing file?

Ah, scratch that.  rename() discards the old one atomically, so as
long as tmp_file and buf.buf are in the same directory (which I
think it is in this case), we wouldn't be affected by the bug that
is worked around with "Coda hack" in finalize_object_file(), either.