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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 12 Oct 2021 14:59:45 +0200

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

> On Thu, Sep 02 2021, Ævar Arnfjörð Bjarmason wrote:
>
> Emily, there's a...:
>
>> 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/[email-masked]/
> 2. https://lore.kernel.org/git/[email-masked]/

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");