From: Junio C Hamano Date: Tue, 24 Sep 2024 21:34:37 GMT Subject: Re: [PATCH v4 4/8] pack-objects: use finalize_object_file() to rename pack/idx/etc Message-ID: In-Reply-To: <3cc7f7b1f67fe823834c36f3be20be8ee56e16a4.1727199118.git.me@ttaylorr.com> Taylor Blau 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.