git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv15 2/5] run_processes_parallel: add LF when caller is sloppy

From
Stefan Beller <sbeller@google.com>
Date
Feb 24, 2016, 03:20 UTC
Message-ID
<1456284017-26141-3-git-send-email-sbeller@google.com>
In-Reply-To
<1456284017-26141-1-git-send-email-sbeller@google.com>

When the callers of parallel processing machine are sloppy with their messages, make sure the output is terminated with LF after one child process is handled. This will not mess with the internal state of the output, i.e. after all messages for one child process are process including the callbacks as well as the actual output of the child we may add an LF if the output is not ended with an LF.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
>From a discussion on a later patch, Jonathan said:
> It looks like pp_start_one takes the content of err without adding
> a \n.  That's a bug in pp_start_one and pp_collect_finished IMHO.
 run-command.c          | 17 +++++++++++++++--
 t/t0061-run-command.sh | 26 ++++++++++++++++++++++++++
 test-run-command.c     | 18 ++++++++++++++++++
 3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/run-command.c b/run-command.c
index d03ecaa..8abaae4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -897,6 +897,7 @@ struct parallel_processes {
 	struct pollfd *pfd;
 
 	unsigned shutdown : 1;
+	unsigned ended_with_newline: 1;
 
 	int output_owner;
 	struct strbuf buffered_output; /* of finished children */
@@ -979,6 +980,7 @@ static void pp_init(struct parallel_processes *pp,
 	pp->nr_processes = 0;
 	pp->output_owner = 0;
 	pp->shutdown = 0;
+	pp->ended_with_newline = 1;
 	pp->children = xcalloc(n, sizeof(*pp->children));
 	pp->pfd = xcalloc(n, sizeof(*pp->pfd));
 	strbuf_init(&pp->buffered_output, 0);
@@ -1053,6 +1055,7 @@ static int pp_start_one(struct parallel_processes *pp)
 					 pp->data,
 					 &pp->children[i].data);
 		strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
+		strbuf_addch(&pp->buffered_output, '\n');
 		strbuf_reset(&pp->children[i].err);
 		if (code)
 			pp->shutdown = 1;
@@ -1095,9 +1098,11 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout)
 static void pp_output(struct parallel_processes *pp)
 {
 	int i = pp->output_owner;
-	if (pp->children[i].state == GIT_CP_WORKING &&
-	    pp->children[i].err.len) {
+	size_t len = pp->children[i].err.len;
+	if (pp->children[i].state == GIT_CP_WORKING && len) {
 		fputs(pp->children[i].err.buf, stderr);
+		pp->ended_with_newline =
+			(pp->children[i].err.buf[len - 1] == '\n');
 		strbuf_reset(&pp->children[i].err);
 	}
 }
@@ -1107,6 +1112,7 @@ static int pp_collect_finished(struct parallel_processes *pp)
 	int i, code;
 	int n = pp->max_processes;
 	int result = 0;
+	ssize_t len;
 
 	while (pp->nr_processes > 0) {
 		for (i = 0; i < pp->max_processes; i++)
@@ -1131,12 +1137,19 @@ static int pp_collect_finished(struct parallel_processes *pp)
 		pp->pfd[i].fd = -1;
 		child_process_init(&pp->children[i].process);
 
+		len = pp->children[i].err.len - 1;
+		if (len >= 0 && pp->children[i].err.buf[len] != '\n')
+			strbuf_addch(&pp->children[i].err, '\n');
+
 		if (i != pp->output_owner) {
 			strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
 			strbuf_reset(&pp->children[i].err);
 		} else {
+			if (len == -1 && !pp->ended_with_newline)
+				strbuf_addch(&pp->children[i].err, '\n');
 			fputs(pp->children[i].err.buf, stderr);
 			strbuf_reset(&pp->children[i].err);
+			pp->ended_with_newline = 1;
 
 			/* Output all other finished child processes */
 			fputs(pp->buffered_output.buf, stderr);
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 12228b4..5c6822c 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -77,6 +77,32 @@ test_expect_success 'run_command runs in parallel with more tasks than jobs avai
 	test_cmp expect actual
 '
 
+test_expect_success 'run_command ensures each command ends in LF' '
+	test-run-command run-command-parallel 3 sh -c "printf \"%s\n%s\" Hello World" 2>actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<-EOF
+preloaded output of a child
+preloaded output of a child
+preloaded output of a child
+preloaded output of a child
+EOF
+
+test_expect_success 'run_command ensures each command ends in LF when output is only in starting cb' '
+	test-run-command run-command-parallel 3 sh -c true  2>actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<-EOF
+EOF
+
+test_expect_success 'run_command ensures each command ends in LF except when there is no output' '
+	test-run-command run-command-parallel-silent 3 sh -c true  2>actual &&
+	test_cmp expect actual
+'
+
+
 cat >expect <<-EOF
 preloaded output of a child
 asking for a quick stop
diff --git a/test-run-command.c b/test-run-command.c
index fbe0a27..ca1ee97 100644
--- a/test-run-command.c
+++ b/test-run-command.c
@@ -31,6 +31,20 @@ static int parallel_next(struct child_process *cp,
 	return 1;
 }
 
+static int parallel_next_silent(struct child_process *cp,
+			 struct strbuf *err,
+			 void *cb,
+			 void **task_cb)
+{
+	struct child_process *d = cb;
+	if (number_callbacks >= 4)
+		return 0;
+
+	argv_array_pushv(&cp->args, d->argv);
+	number_callbacks++;
+	return 1;
+}
+
 static int no_job(struct child_process *cp,
 		  struct strbuf *err,
 		  void *cb,
@@ -71,6 +85,10 @@ int main(int argc, char **argv)
 	jobs = atoi(argv[2]);
 	proc.argv = (const char **)argv + 3;
 
+	if (!strcmp(argv[1], "run-command-parallel-silent"))
+		exit(run_processes_parallel(jobs, parallel_next_silent,
+					    NULL, NULL, &proc));
+
 	if (!strcmp(argv[1], "run-command-parallel"))
 		exit(run_processes_parallel(jobs, parallel_next,
 					    NULL, NULL, &proc));
-- 
2.7.0.rc0.34.ga06e0b3.dirty
Previous: Stefan BellerNext: Junio C Hamano
Message 3 of 13 in “[PATCHv15 0/5] Expose submodule parallelism to the user”
  1. Stefan BellerFeb 24, 2016
  2. 1/5 run-command: expose default_{start_failure, task_finished}Stefan Beller, Feb 24, 2016
  3. 2/5 run_processes_parallel: add LF when caller is sloppyStefan Beller, Feb 24, 2016
  4. Junio C HamanoFeb 24, 2016
  5. Stefan BellerFeb 24, 2016
  6. Junio C HamanoFeb 24, 2016
  7. Jonathan NiederFeb 24, 2016
  8. Stefan BellerFeb 24, 2016
  9. Jonathan NiederFeb 25, 2016
  10. Junio C HamanoFeb 25, 2016
  11. 3/5 git submodule update: have a dedicated helper for cloningStefan Beller, Feb 24, 2016
  12. 4/5 submodule update: expose parallelism to the userStefan Beller, Feb 24, 2016
  13. 5/5 clone: allow an explicit argument for parallel submodule clonesStefan Beller, Feb 24, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.