Thread (146 messages) 146 messages, 6 authors, 2017-01-02

Re: [PATCH v2 28/34] run_command_opt(): optionally hide stderr when the command succeeds

From: Johannes Sixt <hidden>
Date: 2016-12-14 08:34:29
Subsystem: the rest · Maintainer: Linus Torvalds

Am 13.12.2016 um 16:32 schrieb Johannes Schindelin:
quoted hunk ↗ jump to hunk
This will be needed to hide the output of `git commit` when the
sequencer handles an interactive rebase's script.

Signed-off-by: Johannes Schindelin <redacted>
---
 run-command.c | 23 +++++++++++++++++++++++
 run-command.h |  1 +
 2 files changed, 24 insertions(+)
diff --git a/run-command.c b/run-command.c
index ca905a9e80..5bb957afdd 100644
--- a/run-command.c
+++ b/run-command.c
@@ -589,6 +589,29 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
 	cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
 	cmd.dir = dir;
 	cmd.env = env;
+
+	if (opt & RUN_HIDE_STDERR_ON_SUCCESS) {
+		struct strbuf buf = STRBUF_INIT;
+		int res;
+
+		cmd.err = -1;
+		if (start_command(&cmd) < 0)
+			return -1;
+
+		if (strbuf_read(&buf, cmd.err, 0) < 0) {
+			close(cmd.err);
+			finish_command(&cmd); /* throw away exit code */
+			return -1;
+		}
+
+		close(cmd.err);
+		res = finish_command(&cmd);
+		if (res)
+			fputs(buf.buf, stderr);
+		strbuf_release(&buf);
+		return res;
+	}
+
 	return run_command(&cmd);
 }
Clearly, this is a bolted-on feature. It's not the worst move that you
did not advertise the flag in Documentation/technical/api-run-command.txt...

I wanted to see what it would look like if we make it the caller's
responsibility to throw away stderr. The patch is below, as fixup
of patch 29/34. The change is gross, but the end result is not that
bad, though not really a delightful read, either, mostly due to the
strange cleanup semantics of the start_command/finish_command combo,
so... I dunno.
diff --git a/sequencer.c b/sequencer.c
index 41be4cde16..f375880bd7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -660,15 +660,16 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 			  int cleanup_commit_message)
 {
 	char **env = NULL;
-	struct argv_array array;
-	int opt = RUN_GIT_CMD, rc;
+	int rc;
 	const char *value;
+	struct strbuf errout = STRBUF_INIT;
+	struct child_process cmd = CHILD_PROCESS_INIT;
+
+	cmd.git_cmd = 1;
 
 	if (is_rebase_i(opts)) {
-		if (!edit) {
-			opt |= RUN_COMMAND_STDOUT_TO_STDERR;
-			opt |= RUN_HIDE_STDERR_ON_SUCCESS;
-		}
+		if (!edit)
+			cmd.stdout_to_stderr = 1;
 
 		env = read_author_script();
 		if (!env) {
@@ -679,36 +680,58 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 		}
 	}
 
-	argv_array_init(&array);
-	argv_array_push(&array, "commit");
-	argv_array_push(&array, "-n");
+	argv_array_push(&cmd.args, "commit");
+	argv_array_push(&cmd.args, "-n");
 
 	if (amend)
-		argv_array_push(&array, "--amend");
+		argv_array_push(&cmd.args, "--amend");
 	if (opts->gpg_sign)
-		argv_array_pushf(&array, "-S%s", opts->gpg_sign);
+		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
 	if (opts->signoff)
-		argv_array_push(&array, "-s");
+		argv_array_push(&cmd.args, "-s");
 	if (defmsg)
-		argv_array_pushl(&array, "-F", defmsg, NULL);
+		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
 	if (cleanup_commit_message)
-		argv_array_push(&array, "--cleanup=strip");
+		argv_array_push(&cmd.args, "--cleanup=strip");
 	if (edit)
-		argv_array_push(&array, "-e");
+		argv_array_push(&cmd.args, "-e");
 	else if (!cleanup_commit_message &&
 		 !opts->signoff && !opts->record_origin &&
 		 git_config_get_value("commit.cleanup", &value))
-		argv_array_push(&array, "--cleanup=verbatim");
+		argv_array_push(&cmd.args, "--cleanup=verbatim");
 
 	if (allow_empty)
-		argv_array_push(&array, "--allow-empty");
+		argv_array_push(&cmd.args, "--allow-empty");
 
 	if (opts->allow_empty_message)
-		argv_array_push(&array, "--allow-empty-message");
+		argv_array_push(&cmd.args, "--allow-empty-message");
+
+	cmd.env = (const char *const *)env;
+
+	if (cmd.stdout_to_stderr) {
+		/* hide stderr on success */
+		cmd.err = -1;
+		rc = -1;
+		if (start_command(&cmd) < 0)
+			goto cleanup;
+
+		if (strbuf_read(&errout, cmd.err, 0) < 0) {
+			close(cmd.err);
+			finish_command(&cmd); /* throw away exit code */
+			goto cleanup;
+		}
+
+		close(cmd.err);
+		rc = finish_command(&cmd);
+		if (rc)
+			fputs(errout.buf, stderr);
+	} else {
+		rc = run_command(&cmd);
+	}
 
-	rc = run_command_v_opt_cd_env(array.argv, opt, NULL,
-			(const char *const *)env);
-	argv_array_clear(&array);
+cleanup:
+	child_process_clear(&cmd);
+	strbuf_release(&errout);
 	free(env);
 
 	return rc;
-- 
2.11.0.79.g263f27a
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help