On Wed, Aug 28, 2024 at 02:33:56PM -0400, Jeff King wrote: > Is that right, though? It looks like the caller might pass in a > tempfile name like .../pack/multi-pack-index.d/tmp_midx_XXXXXX, > if we're in incremental mode. But we'll write directly to > "multi-pack-index-$hash.bitmap" in the same directory. I'm not sure to > what degree it matters, since that's the name we want in the long run. > But would we possibly overwrite an active-in-use file rather than doing > the atomic rename-into-place if we happened to generate the same midx? > > It feels like we should still respect the name the caller is using for > tempfiles, and then rename it into the correct spot at the end. In either case, we're going to write to a temporary file initialized by the pack-bitmap machinery and then rename() it into place at the end of bitmap_writer_finish(). On the caller side, in the non-incremental mode, we'll pass $GIT_DIR/objects/pack/multi-pack-index-$hash.bitmap as the location, write its contents into a temporary file, and then rename() it there. But in the incremental mode this series introduces, I think it would be a bug to pass a tmp_midx_XXXXXX file path there, since nobody would move it from tmp_midx_XXXXX-$HASH.bitmap into its final location. So I think what's written here with the fixup! patch is right (and should be squashed into 13/13 in the next round), but let me know if I'm missing something. Thanks, Taylor