Re: ab/config-based-hooks-N status (was Re: [PATCH v5 24/36] run-command: add stdin callback for parallelization)

On Wed, Oct 06 2021, Ævar Arnfjörð Bjarmason wrote:

On Thu, Sep 02 2021, Ævar Arnfjörð Bjarmason wrote:

Emily, there's a...:

quoted

diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 7ae03dc7123..9348184d303 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c

@@ -32,8 +32,13 @@ static int parallel_next(struct child_process *cp,
 		return 0;
 
 	strvec_pushv(&cp->args, d->argv);
+	cp->in = d->in;
+	cp->no_stdin = d->no_stdin;
 	strbuf_addstr(err, "preloaded output of a child\n");
 	number_callbacks++;
+
+	*task_cb = xmalloc(sizeof(int));
+	*(int*)(*task_cb) = 2;
 	return 1;
 }

Probably trivial to solve failure here in t0061-run-command.sh if you
compile with SANITIZE=leak. This failed in combination with my[1] (but
for anyone reading along, this patch has been ejected from "seen" a
while ago).

More generally: The equivalent of 01-07/36 of this series is being
merged into "next". As described in a plan to submit this topic
incrementally I was hoping to do 08-20/36 next, i.e. up to "run-command:
remove old run_hook_{le,ve}() hook API". See [2] for that plan.

You've been inactive on-list recently, it would be nice to time this so
that by the time it gets to 21-36/36 (which I was planning to split in
two per [2]) that you'd have time to review/help with outstanding issues
etc, for eventually re-submitting your "config based hooks" on top once
this all lands.

1. https://lore.kernel.org/git/patch-02.10-9a8804e1d9a-20211006T094705Z-avarab@gmail.com/ (local)
2. https://lore.kernel.org/git/875yut8nns.fsf@evledraar.gmail.com/ (local)

Since I had a reason to look at this again, this fixes it. I've squashed
it into my "base" branch, but it won't be in the next batch I submit (I
have the cut-off point before this commit):

diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index fa25bcbbc0d..e9b4214d163 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c

@@ -61,11 +61,21 @@ static void test_consume_sideband(struct strbuf *output, void *cb)
 	fclose(sideband);
 }
 
+static int task_free(int result,
+		     struct strbuf *err,
+		     void *pp_cb,
+		     void *pp_task_cb)
+{
+	free(pp_task_cb);
+	return 0;
+}
+
 static int task_finished(int result,
 			 struct strbuf *err,
 			 void *pp_cb,
 			 void *pp_task_cb)
 {
+	task_free(0, NULL, NULL, pp_task_cb);
 	strbuf_addstr(err, "asking for a quick stop\n");
 	return 1;
 }

@@ -438,7 +448,7 @@ int cmd__run_command(int argc, const char **argv)
 
 	if (!strcmp(argv[1], "run-command-parallel"))
 		exit(run_processes_parallel(jobs, parallel_next,
-					    NULL, NULL, NULL, NULL, &proc));
+					    NULL, NULL, NULL, task_free, &proc));
 
 	if (!strcmp(argv[1], "run-command-abort"))
 		exit(run_processes_parallel(jobs, parallel_next,

@@ -452,12 +462,12 @@ int cmd__run_command(int argc, const char **argv)
 		proc.in = -1;
 		proc.no_stdin = 0;
 		exit (run_processes_parallel(jobs, parallel_next, NULL,
-					     test_stdin, NULL, NULL, &proc));
+					     test_stdin, NULL, task_free, &proc));
 	}
 
 	if (!strcmp(argv[1], "run-command-sideband"))
 		exit(run_processes_parallel(jobs, parallel_next, NULL, NULL,
-					    test_consume_sideband, NULL,
+					    test_consume_sideband, task_free,
 					    &proc));
 
 	fprintf(stderr, "check usage\n");