Thread (27 messages) 27 messages, 4 authors, 2025-06-03

Re: [PATCH 5/5] midx: return a `packed_git` pointer from `prepare_midx_pack()`

From: Taylor Blau <hidden>
Date: 2025-05-28 02:18:09

On Mon, May 26, 2025 at 09:24:03AM +0200, Patrick Steinhardt wrote:
On Sun, May 25, 2025 at 02:42:03PM -0400, Taylor Blau wrote:
quoted
diff --git a/midx.c b/midx.c
index fbce88bd46..f7f509cf46 100644
--- a/midx.c
+++ b/midx.c
@@ -449,50 +449,48 @@ static uint32_t midx_for_pack(struct multi_pack_index **_m,
 	return pack_int_id - m->num_packs_in_base;
 }

-int prepare_midx_pack(struct repository *r, struct multi_pack_index *m,
-		      uint32_t pack_int_id)
+struct packed_git *prepare_midx_pack(struct repository *r,
+				     struct multi_pack_index *m,
+				     uint32_t pack_int_id)
 {
-	struct strbuf pack_name = STRBUF_INIT;
-	struct strbuf key = STRBUF_INIT;
-	struct packed_git *p;
+	uint32_t pack_pos = midx_for_pack(&m, pack_int_id);

-	pack_int_id = midx_for_pack(&m, pack_int_id);
+	if (!m->packs[pack_pos]) {
+		struct strbuf pack_name = STRBUF_INIT;
+		struct strbuf key = STRBUF_INIT;
+		struct packed_git *p;

-	if (m->packs[pack_int_id] == (void *)(intptr_t)-1)
-		return 1;
Ah, so this series builds on top of my patch that introduces the
negative lookup cache? That wasn't quite clear to me and makes it a bit
hard to iterate on my patch now.
Eek, sorry for the confusion. I mentioned it in the beginning of my
cover letter as well as the (less visible) "base-commit" identifier. Is
there another spot where I could have highlighted the dependency more
clearly?
Could I suggest that you maybe include that patch as part of this
series so that those can be iterated on in tandem?
I could, however your branch is already tentatively marked for 'next',
and Junio applied this topic to a branch based on yours as I had
suggested. So we could change it, but I think it would be more of a
hassle for Junio, so I'd rather avoid doing so.

Are there changes that you want to make on top of your patch?
Overall I think that this change is quite sensible and hides away at
least some of the complexity. Thanks!
Much appreciated :-).

Thanks,
Taylor
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help