Re: [PATCH v4 4/8] pack-objects: use finalize_object_file() to rename pack/idx/etc

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> This has some test and real-world fallout, as seen in the adjustment to
> t5303 below. That test script assumes that we can "fix" corruption by
> repacking into a good state, including when the pack generated by that
> repack operation collides with a (corrupted) pack with the same hash.
> This violates our assumption from the previous adjustments to
> finalize_object_file() that if we're moving a new file over an existing
> one, that since their checksums match, so too must their contents.
>
> This makes "fixing" corruption like this a more explicit operation,
> since the test (and users, who may fix real-life corruption using a
> similar technique) must first move the broken contents out of the way.

Nicely described.

> @@ -528,9 +529,9 @@ static void rename_tmp_packfile(struct strbuf *name_prefix, const char *source,
>  	size_t name_prefix_len = name_prefix->len;
>  
>  	strbuf_addstr(name_prefix, ext);
> -	if (rename(source, name_prefix->buf))
> -		die_errno("unable to rename temporary file to '%s'",
> -			  name_prefix->buf);
> +	if (finalize_object_file(source, name_prefix->buf))
> +		die("unable to rename temporary file to '%s'",
> +		    name_prefix->buf);
>  	strbuf_setlen(name_prefix, name_prefix_len);
>  }

Looking good.

Thanks.