Re: [PATCH v7 1/7] run-command: add duplicate_output_fn to run_processes_parallel_opts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Calvin

On 07/02/2023 18:17, Calvin Wan wrote:
Add duplicate_output_fn as an optionally set function in
run_process_parallel_opts. If set, output from each child process is
copied and passed to the callback function whenever output from the
child process is buffered to allow for separate parsing.

Signed-off-by: Calvin Wan <calvinwan@xxxxxxxxxx>
---
  run-command.c               | 16 ++++++++++++---
  run-command.h               | 27 +++++++++++++++++++++++++
  t/helper/test-run-command.c | 21 ++++++++++++++++++++
  t/t0061-run-command.sh      | 39 +++++++++++++++++++++++++++++++++++++
  4 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/run-command.c b/run-command.c
index 756f1839aa..cad88befe0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1526,6 +1526,9 @@ static void pp_init(struct parallel_processes *pp,
  	if (!opts->get_next_task)
  		BUG("you need to specify a get_next_task function");
+ if (opts->duplicate_output && opts->ungroup)
+		BUG("duplicate_output and ungroup are incompatible with each other");
+
  	CALLOC_ARRAY(pp->children, n);
  	if (!opts->ungroup)
  		CALLOC_ARRAY(pp->pfd, n);
@@ -1645,14 +1648,21 @@ static void pp_buffer_stderr(struct parallel_processes *pp,
  	for (size_t i = 0; i < opts->processes; i++) {
  		if (pp->children[i].state == GIT_CP_WORKING &&
  		    pp->pfd[i].revents & (POLLIN | POLLHUP)) {
-			int n = strbuf_read_once(&pp->children[i].err,
-						 pp->children[i].process.err, 0);
+			ssize_t n = strbuf_read_once(&pp->children[i].err,
+						     pp->children[i].process.err, 0);
  			if (n == 0) {
  				close(pp->children[i].process.err);
  				pp->children[i].state = GIT_CP_WAIT_CLEANUP;
-			} else if (n < 0)
+			} else if (n < 0) {
  				if (errno != EAGAIN)
  					die_errno("read");
+			} else {
+				if (opts->duplicate_output)
+					opts->duplicate_output(&pp->children[i].err,
+					       strlen(pp->children[i].err.buf) - n,

Looking at how this is used in patch 7 I think it would be better to pass a const char*, length pair rather than a struct strbuf*, offset pair.
i.e.
opts->duplicate_output(pp->children[i].err.buf + pp->children[i].err.len - n, n, ...)

That would make it clear that we do not expect duplicate_output() to alter the buffer and would avoid the duplicate_output() having to add the offset to the start of the buffer to find the new data.

Best Wishes

Phillip


+					       opts->data,
+					       pp->children[i].data);
+			}
  		}
  	}
  }
diff --git a/run-command.h b/run-command.h
index 072db56a4d..6dcf999f6c 100644
--- a/run-command.h
+++ b/run-command.h
@@ -408,6 +408,27 @@ typedef int (*start_failure_fn)(struct strbuf *out,
  				void *pp_cb,
  				void *pp_task_cb);
+/**
+ * This callback is called whenever output from a child process is buffered
+ *
+ * See run_processes_parallel() below for a discussion of the "struct
+ * strbuf *out" parameter.
+ *
+ * The offset refers to the number of bytes originally in "out" before
+ * the output from the child process was buffered. Therefore, the buffer
+ * range, "out + buf" to the end of "out", would contain the buffer of
+ * the child process output.
+ *
+ * pp_cb is the callback cookie as passed into run_processes_parallel,
+ * pp_task_cb is the callback cookie as passed into get_next_task_fn.
+ *
+ * This function is incompatible with "ungroup"
+ */
+typedef void (*duplicate_output_fn)(struct strbuf *out,
+				    size_t offset,
+				    void *pp_cb,
+				    void *pp_task_cb);
+
  /**
   * This callback is called on every child process that finished processing.
   *
@@ -461,6 +482,12 @@ struct run_process_parallel_opts
  	 */
  	start_failure_fn start_failure;
+ /**
+	 * duplicate_output: See duplicate_output_fn() above. This should be
+	 * NULL unless process specific output is needed
+	 */
+	duplicate_output_fn duplicate_output;
+
  	/**
  	 * task_finished: See task_finished_fn() above. This can be
  	 * NULL to omit any special handling.
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 3ecb830f4a..ffd3cd0045 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -52,6 +52,21 @@ static int no_job(struct child_process *cp,
  	return 0;
  }
+static void duplicate_output(struct strbuf *out,
+			size_t offset,
+			void *pp_cb UNUSED,
+			void *pp_task_cb UNUSED)
+{
+	struct string_list list = STRING_LIST_INIT_DUP;
+
+	string_list_split(&list, out->buf + offset, '\n', -1);
+	for (size_t i = 0; i < list.nr; i++) {
+		if (strlen(list.items[i].string) > 0)
+			fprintf(stderr, "duplicate_output: %s\n", list.items[i].string);
+	}
+	string_list_clear(&list, 0);
+}
+
  static int task_finished(int result,
  			 struct strbuf *err,
  			 void *pp_cb,
@@ -439,6 +454,12 @@ int cmd__run_command(int argc, const char **argv)
  		opts.ungroup = 1;
  	}
+ if (!strcmp(argv[1], "--duplicate-output")) {
+		argv += 1;
+		argc -= 1;
+		opts.duplicate_output = duplicate_output;
+	}
+
  	jobs = atoi(argv[2]);
  	strvec_clear(&proc.args);
  	strvec_pushv(&proc.args, (const char **)argv + 3);
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index e2411f6a9b..879e536638 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -135,6 +135,15 @@ test_expect_success 'run_command runs in parallel with more jobs available than
  	test_cmp expect actual
  '
+test_expect_success 'run_command runs in parallel with more jobs available than tasks --duplicate-output' '
+	test-tool run-command --duplicate-output run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
+	test_must_be_empty out &&
+	test 4 = $(grep -c "duplicate_output: Hello" err) &&
+	test 4 = $(grep -c "duplicate_output: World" err) &&
+	sed "/duplicate_output/d" err > err1 &&
+	test_cmp expect err1
+'
+
  test_expect_success 'run_command runs ungrouped in parallel with more jobs available than tasks' '
  	test-tool run-command --ungroup run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
  	test_line_count = 8 out &&
@@ -147,6 +156,15 @@ test_expect_success 'run_command runs in parallel with as many jobs as tasks' '
  	test_cmp expect actual
  '
+test_expect_success 'run_command runs in parallel with as many jobs as tasks --duplicate-output' '
+	test-tool run-command --duplicate-output run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
+	test_must_be_empty out &&
+	test 4 = $(grep -c "duplicate_output: Hello" err) &&
+	test 4 = $(grep -c "duplicate_output: World" err) &&
+	sed "/duplicate_output/d" err > err1 &&
+	test_cmp expect err1
+'
+
  test_expect_success 'run_command runs ungrouped in parallel with as many jobs as tasks' '
  	test-tool run-command --ungroup run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
  	test_line_count = 8 out &&
@@ -159,6 +177,15 @@ test_expect_success 'run_command runs in parallel with more tasks than jobs avai
  	test_cmp expect actual
  '
+test_expect_success 'run_command runs in parallel with more tasks than jobs available --duplicate-output' '
+	test-tool run-command --duplicate-output run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
+	test_must_be_empty out &&
+	test 4 = $(grep -c "duplicate_output: Hello" err) &&
+	test 4 = $(grep -c "duplicate_output: World" err) &&
+	sed "/duplicate_output/d" err > err1 &&
+	test_cmp expect err1
+'
+
  test_expect_success 'run_command runs ungrouped in parallel with more tasks than jobs available' '
  	test-tool run-command --ungroup run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
  	test_line_count = 8 out &&
@@ -180,6 +207,12 @@ test_expect_success 'run_command is asked to abort gracefully' '
  	test_cmp expect actual
  '
+test_expect_success 'run_command is asked to abort gracefully --duplicate-output' '
+	test-tool run-command --duplicate-output run-command-abort 3 false >out 2>err &&
+	test_must_be_empty out &&
+	test_cmp expect err
+'
+
  test_expect_success 'run_command is asked to abort gracefully (ungroup)' '
  	test-tool run-command --ungroup run-command-abort 3 false >out 2>err &&
  	test_must_be_empty out &&
@@ -196,6 +229,12 @@ test_expect_success 'run_command outputs ' '
  	test_cmp expect actual
  '
+test_expect_success 'run_command outputs --duplicate-output' '
+	test-tool run-command --duplicate-output run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
+	test_must_be_empty out &&
+	test_cmp expect err
+'
+
  test_expect_success 'run_command outputs (ungroup) ' '
  	test-tool run-command --ungroup run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err &&
  	test_must_be_empty out &&



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux