Re: [PATCH 3/5] midx-write.c: simplify fill_packs_from_midx()

On Mon, May 26, 2025 at 09:23:59AM +0200, Patrick Steinhardt wrote:

On Sun, May 25, 2025 at 02:41:57PM -0400, Taylor Blau wrote:

quoted

diff --git a/midx-write.c b/midx-write.c
index e4a3830d45..7802a4b694 100644
--- a/midx-write.c
+++ b/midx-write.c

@@ -943,6 +943,19 @@ static int fill_packs_from_midx(struct write_midx_context *ctx,
 {
 	struct multi_pack_index *m;

+	if (preferred_pack_name) {
+		/*
+		 * If a preferred pack is specified, need to have
+		 * packed_git's loaded to ensure the chosen preferred
+		 * pack has a non-zero object count.
+		 *
+		 * Trick ourselves into thinking that we're writing a
+		 * reverse index in this case in order to open up the
+		 * pack index file.
+		 */
+		flags |= MIDX_WRITE_REV_INDEX;
+	}

This change feels a bit weird to me. Sure, it does allow us to pull out
the loop in the subsequent patch. But honestly, that makes this
workaround even weirder in that we now set unrelated flags in some
function and expect a different function to only honor it in order to
open the packfile.

Shouldn't we instead have a separate flag for the new function that
tells it whether or not it is supposed to prepare the pack?

Yeah, I like that much better, thanks!

Thanks,
Taylor