Re: [PATCH v3 2/9] midx.c: make changing the preferred pack safe

> On Fri, Jan 14, 2022 at 01:43:55PM -0800, Junio C Hamano wrote:
>> 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.
>
> Exactly. In this case, we really did want to overwrite an existing .rev
> file with the same name. That's because prior to this patch, we didn't
> store the object order in the MIDX itself. That made it possible for us
> to change the object order, but leave the MIDX checksum alone.