Re: [PATCH 0/2] commit-graph/server-info: use tempfile.h in more places

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 7, 2024, 15:40 UTC
Message-ID: <xmqq4ja4hico.fsf@gitster.g>
In-Reply-To: <cover.1717712358.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 8 quoted lines

> This pair of patches addresses two issues (in the commit-graph and
> update-server-info areas, respectively), where temporary files are
> created outside of the tempfile.h API and thus survive abnormal process
> death.
>
> The commit-graph one is more prevalent, and has been the cause of some
> minor headaches (for e.g. automated tooling to detect repository cruft
> at GitHub complaining about unknown tmp_graph_XXXXXX files left around).

;-)

I'd be very surprised if a stale "update-server-info" product causes any harm, but we unfortunately cannot remove it until we fully remove the suport for HTTP walkers.

> The fixes in both instances are relatively straightforward conversions
> to use the tempfile.h API.

Show 6 quoted lines

> Looking at the remaining uses of mkstemp(), the remaining class of
> callers that don't use the tempfile.h API are for creating temporary
> .idx, .rev files, and similar. My personal feeling is that we should
> apply similar treatment there, since these files are generated based on
> .pack data, and thus keeping around temporary copies is unnecessary when
> they can be regenerated.

Absolutely.

I wouldn't be surprised by .idx as it and .pack are so old, dating back to 2005, but anything newer like .rev I am mildly surprised that we haven't done this conversion.