Revert "Merge branch 'ar/run-command-hook'"

This reverts commit f406b8955295d01089ba2baf35eceadff2d11cae,
reversing changes made to 1627809eeff75e6ec936fc609e7be46d5eb2fa9e.

It seems to have caused a few regressions, two of the three known
ones we have proposed solutions for.  Let's give ourselves a bit
more room to maneuver during the pre-release freeze period and
restart once the 2.53 ships.

1 files changed, 24 insertions, 118 deletions

diff --git a/run-command.c b/run-command.c
index 2d3c2ac55c..e3e02475cc 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1478,32 +1478,15 @@ enum child_state {
 	GIT_CP_WAIT_CLEANUP,
 };
 
-struct parallel_child {
-	enum child_state state;
-	struct child_process process;
-	struct strbuf err;
-	void *data;
-};
-
-static int child_is_working(const struct parallel_child *pp_child)
-{
-	return pp_child->state == GIT_CP_WORKING;
-}
-
-static int child_is_ready_for_cleanup(const struct parallel_child *pp_child)
-{
-	return child_is_working(pp_child) && !pp_child->process.in;
-}
-
-static int child_is_receiving_input(const struct parallel_child *pp_child)
-{
-	return child_is_working(pp_child) && pp_child->process.in > 0;
-}
-
 struct parallel_processes {
 	size_t nr_processes;
 
-	struct parallel_child *children;
+	struct {
+		enum child_state state;
+		struct child_process process;
+		struct strbuf err;
+		void *data;
+	} *children;
 	/*
 	 * The struct pollfd is logically part of *children,
 	 * but the system call expects it as its own array.
@@ -1526,7 +1509,7 @@ static void kill_children(const struct parallel_processes *pp,
 			  int signo)
 {
 	for (size_t i = 0; i < opts->processes; i++)
-		if (child_is_working(&pp->children[i]))
+		if (pp->children[i].state == GIT_CP_WORKING)
 			kill(pp->children[i].process.pid, signo);
 }
 
@@ -1595,10 +1578,7 @@ static void pp_cleanup(struct parallel_processes *pp,
 	 * When get_next_task added messages to the buffer in its last
 	 * iteration, the buffered output is non empty.
 	 */
-	if (opts->consume_output)
-		opts->consume_output(&pp->buffered_output, opts->data);
-	else
-		strbuf_write(&pp->buffered_output, stderr);
+	strbuf_write(&pp->buffered_output, stderr);
 	strbuf_release(&pp->buffered_output);
 
 	sigchain_pop_common();
@@ -1672,44 +1652,6 @@ static int pp_start_one(struct parallel_processes *pp,
 	return 0;
 }
 
-static void pp_buffer_stdin(struct parallel_processes *pp,
-			    const struct run_process_parallel_opts *opts)
-{
-	/* Buffer stdin for each pipe. */
-	for (size_t i = 0; i < opts->processes; i++) {
-		struct child_process *proc = &pp->children[i].process;
-		int ret;
-
-		if (!child_is_receiving_input(&pp->children[i]))
-			continue;
-
-		/*
-		 * child input is provided via path_to_stdin when the feed_pipe cb is
-		 * missing, so we just signal an EOF.
-		 */
-		if (!opts->feed_pipe) {
-			close(proc->in);
-			proc->in = 0;
-			continue;
-		}
-
-		/**
-		 * Feed the pipe:
-		 *   ret < 0 means error
-		 *   ret == 0 means there is more data to be fed
-		 *   ret > 0 means feeding finished
-		 */
-		ret = opts->feed_pipe(proc->in, opts->data, pp->children[i].data);
-		if (ret < 0)
-			die_errno("feed_pipe");
-
-		if (ret) {
-			close(proc->in);
-			proc->in = 0;
-		}
-	}
-}
-
 static void pp_buffer_stderr(struct parallel_processes *pp,
 			     const struct run_process_parallel_opts *opts,
 			     int output_timeout)
@@ -1723,7 +1665,7 @@ static void pp_buffer_stderr(struct parallel_processes *pp,
 
 	/* Buffer output from all pipes. */
 	for (size_t i = 0; i < opts->processes; i++) {
-		if (child_is_working(&pp->children[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);
@@ -1737,17 +1679,13 @@ static void pp_buffer_stderr(struct parallel_processes *pp,
 	}
 }
 
-static void pp_output(const struct parallel_processes *pp,
-		      const struct run_process_parallel_opts *opts)
+static void pp_output(const struct parallel_processes *pp)
 {
 	size_t i = pp->output_owner;
 
-	if (child_is_working(&pp->children[i]) &&
+	if (pp->children[i].state == GIT_CP_WORKING &&
 	    pp->children[i].err.len) {
-		if (opts->consume_output)
-			opts->consume_output(&pp->children[i].err, opts->data);
-		else
-			strbuf_write(&pp->children[i].err, stderr);
+		strbuf_write(&pp->children[i].err, stderr);
 		strbuf_reset(&pp->children[i].err);
 	}
 }
@@ -1784,7 +1722,6 @@ static int pp_collect_finished(struct parallel_processes *pp,
 		pp->children[i].state = GIT_CP_FREE;
 		if (pp->pfd)
 			pp->pfd[i].fd = -1;
-		pp->children[i].process.in = 0;
 		child_process_init(&pp->children[i].process);
 
 		if (opts->ungroup) {
@@ -1795,15 +1732,11 @@ static int pp_collect_finished(struct parallel_processes *pp,
 		} else {
 			const size_t n = opts->processes;
 
-			/* Output errors, then all other finished child processes */
-			if (opts->consume_output) {
-				opts->consume_output(&pp->children[i].err, opts->data);
-				opts->consume_output(&pp->buffered_output, opts->data);
-			} else {
-				strbuf_write(&pp->children[i].err, stderr);
-				strbuf_write(&pp->buffered_output, stderr);
-			}
+			strbuf_write(&pp->children[i].err, stderr);
 			strbuf_reset(&pp->children[i].err);
+
+			/* Output all other finished child processes */
+			strbuf_write(&pp->buffered_output, stderr);
 			strbuf_reset(&pp->buffered_output);
 
 			/*
@@ -1815,7 +1748,7 @@ static int pp_collect_finished(struct parallel_processes *pp,
 			 * running process time.
 			 */
 			for (i = 0; i < n; i++)
-				if (child_is_working(&pp->children[(pp->output_owner + i) % n]))
+				if (pp->children[(pp->output_owner + i) % n].state == GIT_CP_WORKING)
 					break;
 			pp->output_owner = (pp->output_owner + i) % n;
 		}
@@ -1823,27 +1756,6 @@ static int pp_collect_finished(struct parallel_processes *pp,
 	return result;
 }
 
-static void pp_handle_child_IO(struct parallel_processes *pp,
-				const struct run_process_parallel_opts *opts,
-				int output_timeout)
-{
-	/*
-	 * First push input, if any (it might no-op), to child tasks to avoid them blocking
-	 * after input. This also prevents deadlocks when ungrouping below, if a child blocks
-	 * while the parent also waits for them to finish.
-	 */
-	pp_buffer_stdin(pp, opts);
-
-	if (opts->ungroup) {
-		for (size_t i = 0; i < opts->processes; i++)
-			if (child_is_ready_for_cleanup(&pp->children[i]))
-				pp->children[i].state = GIT_CP_WAIT_CLEANUP;
-	} else {
-		pp_buffer_stderr(pp, opts, output_timeout);
-		pp_output(pp, opts);
-	}
-}
-
 void run_processes_parallel(const struct run_process_parallel_opts *opts)
 {
 	int i, code;
@@ -1863,16 +1775,6 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
 					   "max:%"PRIuMAX,
 					   (uintmax_t)opts->processes);
 
-	if (opts->ungroup && opts->consume_output)
-		BUG("ungroup and reading output are mutualy exclusive");
-
-	/*
-	 * Child tasks might receive input via stdin, terminating early (or not), so
-	 * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which
-	 * actually writes the data to children stdin fds.
-	 */
-	sigchain_push(SIGPIPE, SIG_IGN);
-
 	pp_init(&pp, opts, &pp_sig);
 	while (1) {
 		for (i = 0;
@@ -1890,7 +1792,13 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
 		}
 		if (!pp.nr_processes)
 			break;
-		pp_handle_child_IO(&pp, opts, output_timeout);
+		if (opts->ungroup) {
+			for (size_t i = 0; i < opts->processes; i++)
+				pp.children[i].state = GIT_CP_WAIT_CLEANUP;
+		} else {
+			pp_buffer_stderr(&pp, opts, output_timeout);
+			pp_output(&pp);
+		}
 		code = pp_collect_finished(&pp, opts);
 		if (code) {
 			pp.shutdown = 1;
@@ -1901,8 +1809,6 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
 
 	pp_cleanup(&pp, opts);
 
-	sigchain_pop(SIGPIPE);
-
 	if (do_trace2)
 		trace2_region_leave(tr2_category, tr2_label, NULL);
 }