Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, Jan 27, 2025 at 10:42:50AM +0100, Stefan Sperling wrote:
> > On Mon, Jan 27, 2025 at 10:33:32AM +0100, Stefan Sperling wrote:
> > > Extend test coverage to cover both code paths in gotd
> > 
> > I just realized this part (the addition of the 
> > modify_protected_branch_with_deep_history test case) is bogus.
> > 
> > Because with this packing bug fixed, there will always be an empty
> > pack file sent to gotd, regardless of the number of parent commits
> > used by this test.
> > 
> > So I will drop the regress/gotd changes before commit, and write a
> > better test case later (which would try to replace the tip commit,
> > rather than simply trying to remove the tip).
> 
> Here is a patch on top to ensure that got-read-pack's internal
> commit coloring loop gets the same fix. This is a bit involved
> because it turns out that we had a state-mismatch between the
> main process and got-read-pack, so they would run slightly
> different traversals. This is fixed here as well.
> 
> OK?

ok (and sorry for the delay in reviewing this!)

Here is a diff on top of yours that zaps an extra blank line and
trailing tab, and unwraps a pre-existing, unnecessary line break,
which I'll commit after you.


M  lib/pack_create_privsep.c  |  1+  2-
M  lib/privsep.c              |  1+  2-

2 files changed, 2 insertions(+), 4 deletions(-)

path + /home/mark/src/got
commit - 5e63c5e6f8f06908e5fc0192306788c424ded24e
blob - b8de462a868ea019db806d3f84da929a32cc590b
file + lib/pack_create_privsep.c
--- lib/pack_create_privsep.c
+++ lib/pack_create_privsep.c
@@ -116,9 +116,8 @@ send_filtered_id_queue(struct imsgbuf *ibuf, struct go
 				return err;
 			nids = 0;
 		}
-		
 	}
-	
+
 	if (nids > 0) {
 		err = got_privsep_send_object_idlist(ibuf, filtered_ids, nids);
 		if (err)
commit - 5e63c5e6f8f06908e5fc0192306788c424ded24e
blob - 9c7646c870a7ddbf9a22618d09e3549ef5d9cdb4
file + lib/privsep.c
--- lib/privsep.c
+++ lib/privsep.c
@@ -3378,8 +3378,7 @@ const struct got_error *
 got_privsep_init_commit_painting(struct imsgbuf *ibuf)
 {
 	if (imsg_compose(ibuf, GOT_IMSG_COMMIT_PAINTING_INIT,
-	    0, 0, -1, NULL, 0)
-	    == -1)
+	    0, 0, -1, NULL, 0) == -1)
 		return got_error_from_errno("imsg_compose "
 		    "COMMIT_PAINTING_INIT");
 


> 
> rework got-read-pack's commit coloring loop
> 
> Port the parent commit coloring fix to got-read-pack, and ensure that
> it starts off with the same state as the main process.
> got-read-pack did not have access to the main process's ids array, and
> was thus working with a different initial state. With these changes the
> same commit traversal happens regardless of whether coloring is "offloaded"
> to got-read-pack or not (verified manually by placing debug printfs).
> 
> M  lib/got_lib_privsep.h                  |    1+   9-
> M  lib/pack_create_privsep.c              |  141+  63-
> M  lib/privsep.c                          |    2+  10-
> M  libexec/got-read-pack/got-read-pack.c  |  144+  15-
> M  regress/cmdline/pack.sh                |   70+   0-
> 
> 5 files changed, 358 insertions(+), 97 deletions(-)


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68