From: Mark Jamsek Subject: Re: fix pack exclusion via an ancestor commit To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Thu, 30 Jan 2025 22:15:41 +1100 Stefan Sperling 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 GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68