repack: drop remove_temporary_files()
commit193430717a4056579201f98873bfdd152b5fdd25
authorJeff King <peff@peff.net>
Sat, 22 Oct 2022 00:21:58 +0000 (21 20:21 -0400)
committerJunio C Hamano <gitster@pobox.com>
Sat, 22 Oct 2022 01:03:52 +0000 (21 18:03 -0700)
treeb25dafeae4d1ce043d80a6d5f3c689d1273e8922
parent9cf10d8786c47c18ced6cb00e140cc18db8a7509
repack: drop remove_temporary_files()

After we've successfully finished the repack, we call
remove_temporary_files(), which looks for and removes any files matching
".tmp-$$-pack-*", where $$ is the pid of the current process. But this
is pointless. If we make it this far in the process, we've already
renamed these tempfiles into place, and there is nothing left to delete.

Nor is there a point in trying to call it to clean up when we _aren't_
successful. It's not safe for using in a signal handler, and the
previous commit already handed that job over to the tempfile API.

It might seem like it would be useful to clean up stray .tmp files left
by other invocations of git-repack. But it won't clean those files; it
only matches ones with its pid, and leaves the rest. Fortunately, those
are cleaned up naturally by successive calls to git-repack; we'll
consider .tmp-*.pack the same as normal packfiles, so "repack -ad", etc,
will roll up their contents and eventually delete them.

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.

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.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/repack.c
t/t7700-repack.sh