From: "René Scharfe" <l.s.r@web.de>
To: Git List <git@vger.kernel.org>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Taylor Blau" <me@ttaylorr.com>, "Jeff King" <peff@peff.net>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 0/12] run-command: remove run_command_v_*()
Date: Sun, 30 Oct 2022 12:40:23 +0100 [thread overview]
Message-ID: <ea061164-b36b-485c-963f-8c13e813a47e@web.de> (raw)
In-Reply-To: <7407e074-4bd8-b351-7fa4-baf59b41880c@web.de>
Replace the convenience functions run_command_v_opt() et. al. and use
struct child_process and run_command() directly instead, for an overall
code reduction and a simpler and more flexible API that allows creating
argument lists without magic numbers and reduced risk of memory leaks.
Changes since v1:
- Do the return value fix earlier; it was only an afterthought before.
Keep the colon (no "while at it, ...").
- Break out the xstrdup(), oid_to_hex_r() and C99 cleanups.
- Convert tricky string arrays before strvecs because Ævar didn't like
the opposite order.
- Extend the example code in tmp-objdir.h so it still only requires
"cmd".
Ævar Arnfjörð Bjarmason (1):
merge: remove always-the-same "verbose" arguments
René Scharfe (11):
run-command: fix return value comment
am: simplify building "show" argument list
bisect: simplify building "checkout" argument list
bisect--helper: factor out do_bisect_run()
sequencer: simplify building argument list in do_exec()
use child_process member "args" instead of string array variable
use child_process members "args" and "env" directly
replace and remove run_command_v_opt_cd_env()
replace and remove run_command_v_opt_tr2()
replace and remove run_command_v_opt_cd_env_tr2()
replace and remove run_command_v_opt()
add-interactive.c | 9 ++-
bisect.c | 12 ++--
builtin/add.c | 19 +++--
builtin/am.c | 12 ++--
builtin/bisect--helper.c | 68 +++++++++---------
builtin/clone.c | 41 ++++++-----
builtin/difftool.c | 24 ++++---
builtin/fetch.c | 9 ++-
builtin/gc.c | 55 ++++++++++-----
builtin/merge-index.c | 4 +-
builtin/merge.c | 53 ++++++--------
builtin/pull.c | 147 +++++++++++++++++++--------------------
builtin/remote.c | 40 +++++------
compat/mingw.c | 11 +--
diff.c | 27 ++++---
fsmonitor-ipc.c | 10 ++-
git.c | 15 ++--
ll-merge.c | 7 +-
merge.c | 18 ++---
run-command.c | 35 ----------
run-command.h | 34 +--------
scalar.c | 13 ++--
sequencer.c | 32 ++++-----
shell.c | 17 +++--
t/helper/test-fake-ssh.c | 7 +-
t/helper/test-trace2.c | 4 +-
tmp-objdir.h | 6 +-
27 files changed, 346 insertions(+), 383 deletions(-)
Range-Diff gegen v1:
1: c0b2b88500 = 1: 113a9e0a81 merge: remove always-the-same "verbose" arguments
-: ---------- > 2: d10e70460b run-command: fix return value comment
-: ---------- > 3: c8cd913e39 am: simplify building "show" argument list
-: ---------- > 4: 4bb142e4a9 bisect: simplify building "checkout" argument list
2: 387db545d1 = 5: 5fcbe94eb4 bisect--helper: factor out do_bisect_run()
-: ---------- > 6: 962403cf22 sequencer: simplify building argument list in do_exec()
4: 348bc6d32c ! 7: f1689abe85 use child_process member "args" instead of string array variable
@@ Commit message
Use run_command() with a struct child_process variable and populate its
"args" member directly instead of building a string array and passing it
- to run_command_v_opt(). This avoids the use of magic index numbers.
-
- ## bisect.c ##
-@@ bisect.c: static struct oid_array skipped_revs;
-
- static struct object_id *current_bad_oid;
-
--static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
--
- static const char *term_bad;
- static const char *term_good;
-
-@@ bisect.c: static int is_expected_rev(const struct object_id *oid)
- enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
- int no_checkout)
- {
-- char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
- struct commit *commit;
- struct pretty_print_context pp = {0};
- struct strbuf commit_msg = STRBUF_INIT;
-
-- oid_to_hex_r(bisect_rev_hex, bisect_rev);
- update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
-
-- argv_checkout[2] = bisect_rev_hex;
- if (no_checkout) {
- update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
- UPDATE_REFS_DIE_ON_ERR);
- } else {
-- if (run_command_v_opt(argv_checkout, RUN_GIT_CMD))
-+ struct child_process cmd = CHILD_PROCESS_INIT;
-+
-+ cmd.git_cmd = 1;
-+ strvec_pushl(&cmd.args, "checkout", "-q",
-+ oid_to_hex(bisect_rev), "--", NULL);
-+ if (run_command(&cmd))
- /*
- * Errors in `run_command()` itself, signaled by res < 0,
- * and errors in the child process, signaled by res > 0
-
- ## builtin/am.c ##
-@@ builtin/am.c: static int show_patch(struct am_state *state, enum show_patch_type sub_mode)
- int len;
-
- if (!is_null_oid(&state->orig_commit)) {
-- const char *av[4] = { "show", NULL, "--", NULL };
-- char *new_oid_str;
-- int ret;
-+ struct child_process cmd = CHILD_PROCESS_INIT;
-
-- av[1] = new_oid_str = xstrdup(oid_to_hex(&state->orig_commit));
-- ret = run_command_v_opt(av, RUN_GIT_CMD);
-- free(new_oid_str);
-- return ret;
-+ strvec_pushl(&cmd.args, "show", oid_to_hex(&state->orig_commit),
-+ "--", NULL);
-+ cmd.git_cmd = 1;
-+ return run_command(&cmd);
- }
-
- switch (sub_mode) {
+ to run_command_v_opt(). This avoids the use of magic index numbers and
+ makes simplifies the possible addition of more arguments in the future.
## builtin/difftool.c ##
@@ builtin/difftool.c: static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
@@ ll-merge.c: static enum ll_merge_result ll_ext_merge(const struct ll_merge_drive
if (fd < 0)
goto bad;
- ## sequencer.c ##
-@@ sequencer.c: static int error_failed_squash(struct repository *r,
-
- static int do_exec(struct repository *r, const char *command_line)
- {
-- const char *child_argv[] = { NULL, NULL };
-+ struct child_process cmd = CHILD_PROCESS_INIT;
- int dirty, status;
-
- fprintf(stderr, _("Executing: %s\n"), command_line);
-- child_argv[0] = command_line;
-- status = run_command_v_opt(child_argv, RUN_USING_SHELL);
-+ cmd.use_shell = 1;
-+ strvec_push(&cmd.args, command_line);
-+ status = run_command(&cmd);
-
- /* force re-reading of the cache */
- if (discard_index(r->index) < 0 || repo_read_index(r) < 0)
-
## t/helper/test-fake-ssh.c ##
@@ t/helper/test-fake-ssh.c: int cmd_main(int argc, const char **argv)
struct strbuf buf = STRBUF_INIT;
3: 7745e16df4 = 8: a467a4ecb5 use child_process members "args" and "env" directly
5: eeaa6aa86d ! 9: dc27358775 replace and remove run_command_v_opt_cd_env()
@@ run-command.h
@@ run-command.h: struct child_process {
/**
- * The functions: child_process_init, start_command, finish_command,
-- * run_command, run_command_v_opt, run_command_v_opt_cd_env, child_process_clear
-- * do the following:
-+ * run_command, run_command_v_opt, child_process_clear do the following:
+ * The functions: start_command, finish_command, run_command,
+- * run_command_v_opt, run_command_v_opt_cd_env do the following:
++ * run_command_v_opt do the following:
*
* - If a system call failed, errno is set and -1 is returned. A diagnostic
* is printed.
@@ run-command.h: int run_command_v_opt_tr2(const char **argv, int opt, const char
## tmp-objdir.h ##
@@
+ *
* Example:
*
++ * struct child_process child = CHILD_PROCESS_INIT;
* struct tmp_objdir *t = tmp_objdir_create("incoming");
- * if (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) &&
- * !tmp_objdir_migrate(t))
-+ * strvec_pushv(&cmd.env, tmp_objdir_env(t));
-+ * if (!run_command(&cmd)) && !tmp_objdir_migrate(t))
++ * strvec_push(&child.args, cmd);
++ * strvec_pushv(&child.env, tmp_objdir_env(t));
++ * if (!run_command(&child)) && !tmp_objdir_migrate(t))
* printf("success!\n");
* else
* die("failed...tmp_objdir will clean up for us");
6: 95b5dcbb66 = 10: dcd65580c6 replace and remove run_command_v_opt_tr2()
7: 5e111ab053 ! 11: 389ec8ef54 replace and remove run_command_v_opt_cd_env_tr2()
@@ run-command.h: int run_auto_maintenance(int quiet);
/**
- * Convenience functions that encapsulate a sequence of
-+ * Convenience function that encapsulate a sequence of
++ * Convenience function that encapsulates a sequence of
* start_command() followed by finish_command(). The argument argv
* specifies the program and its arguments. The argument opt is zero
* or more of the flags `RUN_COMMAND_NO_STDIN`, `RUN_GIT_CMD`,
8: f1f438d724 ! 12: a6e7135e31 replace and remove run_command_v_opt()
@@ Commit message
Suggested-by: Jeff King <peff@peff.net>
+ ## bisect.c ##
+@@ bisect.c: enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
+ update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
+ UPDATE_REFS_DIE_ON_ERR);
+ } else {
+- const char *argv_checkout[] = {
+- "checkout", "-q", oid_to_hex(bisect_rev), "--", NULL
+- };
++ struct child_process cmd = CHILD_PROCESS_INIT;
+
+- if (run_command_v_opt(argv_checkout, RUN_GIT_CMD))
++ cmd.git_cmd = 1;
++ strvec_pushl(&cmd.args, "checkout", "-q",
++ oid_to_hex(bisect_rev), "--", NULL);
++ if (run_command(&cmd))
+ /*
+ * Errors in `run_command()` itself, signaled by res < 0,
+ * and errors in the child process, signaled by res > 0
+
+ ## builtin/am.c ##
+@@ builtin/am.c: static int show_patch(struct am_state *state, enum show_patch_type sub_mode)
+ int len;
+
+ if (!is_null_oid(&state->orig_commit)) {
+- const char *av[] = {
+- "show", oid_to_hex(&state->orig_commit), "--", NULL
+- };
++ struct child_process cmd = CHILD_PROCESS_INIT;
+
+- return run_command_v_opt(av, RUN_GIT_CMD);
++ strvec_pushl(&cmd.args, "show", oid_to_hex(&state->orig_commit),
++ "--", NULL);
++ cmd.git_cmd = 1;
++ return run_command(&cmd);
+ }
+
+ switch (sub_mode) {
+
## builtin/bisect--helper.c ##
@@ builtin/bisect--helper.c: static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a
strbuf_read_file(&start_head, git_path_bisect_start(), 0);
@@ run-command.c: int run_command(struct child_process *cmd)
## run-command.h ##
@@ run-command.h: struct child_process {
+ }
/**
- * The functions: child_process_init, start_command, finish_command,
-- * run_command, run_command_v_opt, child_process_clear do the following:
-+ * run_command, child_process_clear do the following:
+- * The functions: start_command, finish_command, run_command,
+- * run_command_v_opt do the following:
++ * The functions: start_command, finish_command, run_command do the following:
*
* - If a system call failed, errno is set and -1 is returned. A diagnostic
* is printed.
@@ run-command.h: int run_command(struct child_process *);
-#define RUN_CLOSE_OBJECT_STORE (1<<7)
-
-/**
-- * Convenience function that encapsulate a sequence of
+- * Convenience function that encapsulates a sequence of
- * start_command() followed by finish_command(). The argument argv
- * specifies the program and its arguments. The argument opt is zero
- * or more of the flags `RUN_COMMAND_NO_STDIN`, `RUN_GIT_CMD`,
@@ run-command.h: int run_command(struct child_process *);
* Execute the given command, sending "in" to its stdin, and capturing its
* stdout and stderr in the "out" and "err" strbufs. Any of the three may
+ ## sequencer.c ##
+@@ sequencer.c: static int error_failed_squash(struct repository *r,
+
+ static int do_exec(struct repository *r, const char *command_line)
+ {
+- const char *child_argv[] = { command_line, NULL };
++ struct child_process cmd = CHILD_PROCESS_INIT;
+ int dirty, status;
+
+ fprintf(stderr, _("Executing: %s\n"), command_line);
+- status = run_command_v_opt(child_argv, RUN_USING_SHELL);
++ cmd.use_shell = 1;
++ strvec_push(&cmd.args, command_line);
++ status = run_command(&cmd);
+
+ /* force re-reading of the cache */
+ if (discard_index(r->index) < 0 || repo_read_index(r) < 0)
+
## shell.c ##
@@ shell.c: static void cd_to_homedir(void)
static void run_shell(void)
9: 59677c9598 < -: ---------- run-command: fix return value comment
--
2.38.1
next prev parent reply other threads:[~2022-10-30 11:40 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-27 16:30 [PATCH 0/8] run-command: remove run_command_v_*() René Scharfe
2022-10-27 16:35 ` [PATCH 1/8] merge: remove always-the-same "verbose" arguments René Scharfe
2022-10-29 18:12 ` Taylor Blau
2022-10-27 16:35 ` [PATCH 2/8] bisect--helper: factor out do_bisect_run() René Scharfe
2022-10-27 22:26 ` Ævar Arnfjörð Bjarmason
2022-10-29 18:16 ` Taylor Blau
2022-10-27 16:36 ` [PATCH 3/8] use child_process members "args" and "env" directly René Scharfe
2022-10-27 18:28 ` Junio C Hamano
2022-10-27 22:37 ` Ævar Arnfjörð Bjarmason
2022-10-27 16:37 ` [PATCH 4/8] use child_process member "args" instead of string array variable René Scharfe
2022-10-27 21:09 ` Ævar Arnfjörð Bjarmason
2022-10-28 14:23 ` René Scharfe
2022-10-29 18:30 ` Taylor Blau
2022-10-29 18:26 ` Taylor Blau
2022-10-27 16:38 ` [PATCH 5/8] replace and remove run_command_v_opt_cd_env() René Scharfe
2022-10-27 20:16 ` Ævar Arnfjörð Bjarmason
2022-10-29 19:26 ` Taylor Blau
2022-10-27 16:39 ` [PATCH 6/8] replace and remove run_command_v_opt_tr2() René Scharfe
2022-10-27 16:40 ` [PATCH 7/8] replace and remove run_command_v_opt_cd_env_tr2() René Scharfe
2022-10-27 22:46 ` Ævar Arnfjörð Bjarmason
2022-10-28 14:23 ` René Scharfe
2022-10-27 16:41 ` [PATCH 8/8] replace and remove run_command_v_opt() René Scharfe
2022-10-27 22:41 ` Ævar Arnfjörð Bjarmason
2022-10-28 14:23 ` René Scharfe
2022-10-27 20:11 ` [PATCH 0/8] run-command: remove run_command_v_*() Jeff King
2022-10-28 14:23 ` René Scharfe
2022-10-27 21:46 ` Ævar Arnfjörð Bjarmason
2022-10-28 16:04 ` René Scharfe
2022-10-28 16:11 ` Ævar Arnfjörð Bjarmason
2022-10-28 17:16 ` René Scharfe
2022-10-29 2:17 ` Ævar Arnfjörð Bjarmason
2022-10-29 10:05 ` René Scharfe
2022-10-29 19:32 ` Taylor Blau
2022-10-30 11:40 ` René Scharfe [this message]
2022-10-30 11:42 ` [PATCH v2 01/12] merge: remove always-the-same "verbose" arguments René Scharfe
2022-10-30 11:45 ` [PATCH v2 02/12] run-command: fix return value comment René Scharfe
2022-10-30 11:46 ` [PATCH v2 03/12] am: simplify building "show" argument list René Scharfe
2022-10-30 11:47 ` [PATCH v2 04/12] bisect: simplify building "checkout" " René Scharfe
2022-10-30 11:48 ` [PATCH v2 05/12] bisect--helper: factor out do_bisect_run() René Scharfe
2022-10-30 11:49 ` [PATCH v2 06/12] sequencer: simplify building argument list in do_exec() René Scharfe
2022-10-30 11:50 ` [PATCH v2 07/12] use child_process member "args" instead of string array variable René Scharfe
2022-10-30 11:51 ` [PATCH v2 08/12] use child_process members "args" and "env" directly René Scharfe
2022-10-30 11:51 ` [PATCH v2 09/12] replace and remove run_command_v_opt_cd_env() René Scharfe
2022-10-30 11:52 ` [PATCH v2 10/12] replace and remove run_command_v_opt_tr2() René Scharfe
2022-10-30 11:53 ` [PATCH v2 11/12] replace and remove run_command_v_opt_cd_env_tr2() René Scharfe
2022-10-30 11:55 ` [PATCH v2 12/12] replace and remove run_command_v_opt() René Scharfe
2022-10-30 11:58 ` [PATCH v2 0/12] run-command: remove run_command_v_*() René Scharfe
2022-10-30 18:05 ` Taylor Blau
2022-10-31 13:35 ` Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ea061164-b36b-485c-963f-8c13e813a47e@web.de \
--to=l.s.r@web.de \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).