Re: [PATCH 4/4] repack: drop remove_temporary_files()

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 21, 2022, 23:51 UTC
Message-ID: <Y1Mwhdn83MzNMsUx@nand.local>
In-Reply-To: <Y1MTtgOOWNovPawH@coredump.intra.peff.net>

On Fri, Oct 21, 2022 at 05:48:38PM -0400, Jeff King wrote:

Show 8 quoted lines

> The one case that could matter is if pack-objects generates an extension
> we don't know about, like ".tmp-pack-$$-$hash.some-new-ext". The current
> code will quietly delete such a file, while after this patch we'd leave
> it in place. In practice this doesn't happen, and would be indicative of
> a bug. Leaving the file as cruft is arguably a better behavior, as it
> means somebody is more likely to eventually notice and fix the bug.  If
> we really wanted to be paranoid, we could scan for and warn about such
> files, but that seems like overkill.

I agree, that would definitely be overkill.

> There's nothing to test with regard to the removal of this function. It
> was doing nothing, so the behavior should be the same.  However, we can
> verify (and protect) our assumption that "repack -ad" will eventually
> remove stray files by adding a test for that.

Thanks for adding such a test. This patch is beautiful :-).

Thanks, Taylor