On Fri, Mar 10, 2023 at 07:24:36AM +0100, Patrick Steinhardt wrote: > > I think one could also make an argument that index_pack_lockfile() > > should return a tempfile struct itself, but I didn't look too closely at > > the other caller on the fetch side (but it should be conceptually the > > same). > > I had a look at it, but git-fetch-pack(1) works quite differently in > that regard as it also supports the case where the packfile lock should > stay locked after it exits via the `--keep` switch. So the logic is more > intricate here. > > Furthermore, git-fetch-pack(1) only does the locking, but never unlocks > the packfiles. That is instead handled by git-fetch(1). So converting > the interface to use tempfiles directly wouldn't work as we are crossing > process boundaries here. I think the calls into fetch-pack.c that handle the pack lockfiles can happen in-process from git-fetch itself. But I also think there are probably cases where they don't (v0 git-over-http should use a separate "fetch-pack --stateless-rpc", I believe). So yeah, it's probably too complicated to worry about lumping in here, especially since you noted that it handles cleanup correctly already. Thanks for looking into it. -Peff
