"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: got-read-pack commit coloring fix
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 15 Aug 2025 13:15:16 +0200

Download raw body.

Thread
On Thu, Aug 14, 2025 at 06:43:45PM +0200, Omar Polo wrote:
> I really like the improvements.  ok op@!

These have been committed (commit notifications were missing due to an unrelated
bug in gotd.)

There was a problem in the final commit of this series which I have now reverted.
Here is a replacement fix which seems to work well and does not break cleanup tests:


better fix for long loop bug during commit coloring with offloading

Repaint already queued commits in the main process instead of removing them
from the queue. Otherwise we may never converge to a state where all commits
are marked as skipped. Instead such commits can inherit color keep and we'll
keep on asking got-read-pack to paint more commits for a very long time.

M  lib/pack_create_privsep.c  |  11+  10-

1 file changed, 11 insertions(+), 10 deletions(-)

commit - 5abe061a03aa6b587c595d904418740002d94956
commit + 35a48c78421f5dc5a05d329adc5e64454eac5b3c
blob - 9afa74d1d08cfdfe6e45e1bd1911a37c1585184b
blob + c29bd335ba24926a633d0fdfb8bbfb2624c42d73
--- lib/pack_create_privsep.c
+++ lib/pack_create_privsep.c
@@ -219,7 +219,7 @@ recv_painted_commit(void *arg, struct got_object_id *i
 {
 	const struct got_error *err = NULL;
 	struct recv_painted_commit_arg *a = arg;
-	struct got_object_qid *qid, *tmp;
+	struct got_object_qid *qid;
 
 	if (a->cancel_cb) {
 		err = a->cancel_cb(a->cancel_arg);
@@ -256,17 +256,18 @@ recv_painted_commit(void *arg, struct got_object_id *i
 		    "%s invalid commit color %"PRIdPTR, __func__, color);
 	}
 
-	STAILQ_FOREACH_SAFE(qid, a->ids, entry, tmp) {
+	STAILQ_FOREACH(qid, a->ids, entry) {
+		int ocolor;
 		if (got_object_id_cmp(&qid->id, id) != 0)
 			continue;
-		STAILQ_REMOVE(a->ids, qid, got_object_qid, entry);
-		color = (intptr_t)qid->data;
-		if (*(a->qid0) == qid)
-			*(a->qid0) = NULL;
-		got_object_qid_free(qid);
-		(*a->nqueued)--;
-		if (color == COLOR_SKIP)
-			(*a->nskip)--;
+		ocolor = (intptr_t)qid->data;
+		if (ocolor != color) {
+			got_pack_paint_commit(qid, color);
+			if (ocolor == COLOR_SKIP)
+				(*a->nskip)--;
+			else if (color == COLOR_SKIP)
+				(*a->nskip)++;
+		}
 		break;
 	}