replace and remove run_command_v_opt()

Replace the remaining calls of run_command_v_opt() with run_command()
calls and explict struct child_process variables.  This is more verbose,
but not by much overall.  The code becomes more flexible, e.g. it's easy
to extend to conditionally add a new argument.

Then remove the now unused function and its own flag names, simplifying
the run-command API.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>

diff --git a/bisect.c b/bisect.c
index 090aa5c4b4c51255c9bce97166f693f3e92b4732..ec7487e6836c7ec5819f0b22cc7b08bce0e59fa0 100644 (file)
--- a/bisect.c
+++ b/bisect.c
@@ -737,11 +737,12 @@ 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

diff --git a/builtin/am.c b/builtin/am.c
index 5781e7a95ec3013a5439ae2f8db2bf16b3920d13..20aea0d2487b2dfe851d44f753e7de7439698223 100644 (file)
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2187,11 +2187,12 @@ 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) {

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 5c63ba6994f70b4a1b65cb1529583448e1d36a2c..1d2ce8a0e12c8b6ebfc5c452c2a0b5b688e13d39 100644 (file)
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -764,10 +764,12 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a
                strbuf_read_file(&start_head, git_path_bisect_start(), 0);
                strbuf_trim(&start_head);
                if (!no_checkout) {
-                       const char *argv[] = { "checkout", start_head.buf,
-                                              "--", NULL };
+                       struct child_process cmd = CHILD_PROCESS_INIT;
 
-                       if (run_command_v_opt(argv, RUN_GIT_CMD)) {
+                       cmd.git_cmd = 1;
+                       strvec_pushl(&cmd.args, "checkout", start_head.buf,
+                                    "--", NULL);
+                       if (run_command(&cmd)) {
                                res = error(_("checking out '%s' failed."
                                                 " Try 'git bisect start "
                                                 "<valid-branch>'."),
@@ -1141,9 +1143,12 @@ static int get_first_good(const char *refname UNUSED,
 
 static int do_bisect_run(const char *command)
 {
-       const char *argv[] = { command, NULL };
+       struct child_process cmd = CHILD_PROCESS_INIT;
+
        printf(_("running %s\n"), command);
-       return run_command_v_opt(argv, RUN_USING_SHELL);
+       cmd.use_shell = 1;
+       strvec_push(&cmd.args, command);
+       return run_command(&cmd);
 }
 
 static int verify_good(const struct bisect_terms *terms, const char *command)

diff --git a/builtin/clone.c b/builtin/clone.c
index 56e7775daefe7f6ae26138c1fbf1cb132c15c9f9..0e4348686b6efb13bc1aa8847f8012a416e2080c 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -865,11 +865,15 @@ static void write_refspec_config(const char *src_ref_prefix,
 
 static void dissociate_from_references(void)
 {
-       static const char* argv[] = { "repack", "-a", "-d", NULL };
        char *alternates = git_pathdup("objects/info/alternates");
 
        if (!access(alternates, F_OK)) {
-               if (run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDIN))
+               struct child_process cmd = CHILD_PROCESS_INIT;
+
+               cmd.git_cmd = 1;
+               cmd.no_stdin = 1;
+               strvec_pushl(&cmd.args, "repack", "-a", "-d", NULL);
+               if (run_command(&cmd))
                        die(_("cannot repack to clean up"));
                if (unlink(alternates) && errno != ENOENT)
                        die_errno(_("cannot unlink temporary alternates file"));

diff --git a/builtin/difftool.c b/builtin/difftool.c
index 22bcc3444b1c239eaf4deffb665850db6f69a933..d7f08c8a7fa201750465d37a3b6b8fbb7b91ac6a 100644 (file)
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -44,8 +44,11 @@ static int difftool_config(const char *var, const char *value, void *cb)
 
 static int print_tool_help(void)
 {
-       const char *argv[] = { "mergetool", "--tool-help=diff", NULL };
-       return run_command_v_opt(argv, RUN_GIT_CMD);
+       struct child_process cmd = CHILD_PROCESS_INIT;
+
+       cmd.git_cmd = 1;
+       strvec_pushl(&cmd.args, "mergetool", "--tool-help=diff", NULL);
+       return run_command(&cmd);
 }
 
 static int parse_index_info(char *p, int *mode1, int *mode2,

diff --git a/builtin/fetch.c b/builtin/fetch.c
index a0fca93bb6a63e441e3921832f7c7d9a7bd3dd78..dd060dd65af2ea083e4a0a02d34cbafe8499fecc 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1965,14 +1965,17 @@ static int fetch_multiple(struct string_list *list, int max_children)
        } else
                for (i = 0; i < list->nr; i++) {
                        const char *name = list->items[i].string;
-                       strvec_push(&argv, name);
+                       struct child_process cmd = CHILD_PROCESS_INIT;
+
+                       strvec_pushv(&cmd.args, argv.v);
+                       strvec_push(&cmd.args, name);
                        if (verbosity >= 0)
                                printf(_("Fetching %s\n"), name);
-                       if (run_command_v_opt(argv.v, RUN_GIT_CMD)) {
+                       cmd.git_cmd = 1;
+                       if (run_command(&cmd)) {
                                error(_("could not fetch %s"), name);
                                result = 1;
                        }
-                       strvec_pop(&argv);
                }
 
        strvec_clear(&argv);

diff --git a/builtin/gc.c b/builtin/gc.c
index 87ad0077d8cc81b9d308d500f282947cf034b1ce..962bab61f93758528872c9b2edbdda44f2d63956 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -167,9 +167,11 @@ static void gc_config(void)
 struct maintenance_run_opts;
 static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)
 {
-       const char *argv[] = { "pack-refs", "--all", "--prune", NULL };
+       struct child_process cmd = CHILD_PROCESS_INIT;
 
-       return run_command_v_opt(argv, RUN_GIT_CMD);
+       cmd.git_cmd = 1;
+       strvec_pushl(&cmd.args, "pack-refs", "--all", "--prune", NULL);
+       return run_command(&cmd);
 }
 
 static int too_many_loose_objects(void)
@@ -535,8 +537,14 @@ static void gc_before_repack(void)
        if (pack_refs && maintenance_task_pack_refs(NULL))
                die(FAILED_RUN, "pack-refs");
 
-       if (prune_reflogs && run_command_v_opt(reflog.v, RUN_GIT_CMD))
-               die(FAILED_RUN, reflog.v[0]);
+       if (prune_reflogs) {
+               struct child_process cmd = CHILD_PROCESS_INIT;
+
+               cmd.git_cmd = 1;
+               strvec_pushv(&cmd.args, reflog.v);
+               if (run_command(&cmd))
+                       die(FAILED_RUN, reflog.v[0]);
+       }
 }
 
 int cmd_gc(int argc, const char **argv, const char *prefix)
@@ -550,6 +558,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
        int daemonized = 0;
        int keep_largest_pack = -1;
        timestamp_t dummy;
+       struct child_process rerere_cmd = CHILD_PROCESS_INIT;
 
        struct option builtin_gc_options[] = {
                OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -671,11 +680,17 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
        gc_before_repack();
 
        if (!repository_format_precious_objects) {
-               if (run_command_v_opt(repack.v,
-                                     RUN_GIT_CMD | RUN_CLOSE_OBJECT_STORE))
+               struct child_process repack_cmd = CHILD_PROCESS_INIT;
+
+               repack_cmd.git_cmd = 1;
+               repack_cmd.close_object_store = 1;
+               strvec_pushv(&repack_cmd.args, repack.v);
+               if (run_command(&repack_cmd))
                        die(FAILED_RUN, repack.v[0]);
 
                if (prune_expire) {
+                       struct child_process prune_cmd = CHILD_PROCESS_INIT;
+
                        /* run `git prune` even if using cruft packs */
                        strvec_push(&prune, prune_expire);
                        if (quiet)
@@ -683,18 +698,26 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
                        if (has_promisor_remote())
                                strvec_push(&prune,
                                            "--exclude-promisor-objects");
-                       if (run_command_v_opt(prune.v, RUN_GIT_CMD))
+                       prune_cmd.git_cmd = 1;
+                       strvec_pushv(&prune_cmd.args, prune.v);
+                       if (run_command(&prune_cmd))
                                die(FAILED_RUN, prune.v[0]);
                }
        }
 
        if (prune_worktrees_expire) {
+               struct child_process prune_worktrees_cmd = CHILD_PROCESS_INIT;
+
                strvec_push(&prune_worktrees, prune_worktrees_expire);
-               if (run_command_v_opt(prune_worktrees.v, RUN_GIT_CMD))
+               prune_worktrees_cmd.git_cmd = 1;
+               strvec_pushv(&prune_worktrees_cmd.args, prune_worktrees.v);
+               if (run_command(&prune_worktrees_cmd))
                        die(FAILED_RUN, prune_worktrees.v[0]);
        }
 
-       if (run_command_v_opt(rerere.v, RUN_GIT_CMD))
+       rerere_cmd.git_cmd = 1;
+       strvec_pushv(&rerere_cmd.args, rerere.v);
+       if (run_command(&rerere_cmd))
                die(FAILED_RUN, rerere.v[0]);
 
        report_garbage = report_pack_garbage;

diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index c0383fe9df9a3eb2e34f8c5869f996ebc81fce2e..012f52bd007ce769b2b067cff72b8d8259060dbe 100644 (file)
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -12,6 +12,7 @@ static int merge_entry(int pos, const char *path)
        const char *arguments[] = { pgm, "", "", "", path, "", "", "", NULL };
        char hexbuf[4][GIT_MAX_HEXSZ + 1];
        char ownbuf[4][60];
+       struct child_process cmd = CHILD_PROCESS_INIT;
 
        if (pos >= active_nr)
                die("git merge-index: %s not in the cache", path);
@@ -31,7 +32,8 @@ static int merge_entry(int pos, const char *path)
        if (!found)
                die("git merge-index: %s not in the cache", path);
 
-       if (run_command_v_opt(arguments, 0)) {
+       strvec_pushv(&cmd.args, arguments);
+       if (run_command(&cmd)) {
                if (one_shot)
                        err++;
                else {

diff --git a/run-command.c b/run-command.c
index 923bad37fee992355d5f8e4c02f393500b7e7aa7..23e100dffc4d8257dffcf4dac33b3445d0d921f5 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -1004,21 +1004,6 @@ int run_command(struct child_process *cmd)
        return finish_command(cmd);
 }
 
-int run_command_v_opt(const char **argv, int opt)
-{
-       struct child_process cmd = CHILD_PROCESS_INIT;
-       strvec_pushv(&cmd.args, argv);
-       cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
-       cmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
-       cmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
-       cmd.silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;
-       cmd.use_shell = opt & RUN_USING_SHELL ? 1 : 0;
-       cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
-       cmd.wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0;
-       cmd.close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0;
-       return run_command(&cmd);
-}
-
 #ifndef NO_PTHREADS
 static pthread_t main_thread;
 static int main_thread_set;

diff --git a/run-command.h b/run-command.h
index 63ed6e18ff8f716aa680ada474f38bfcd327ea26..41cc8470dc3c0439afcc4488eda36645b36ae22e 100644 (file)
--- a/run-command.h
+++ b/run-command.h
@@ -150,8 +150,7 @@ struct child_process {
 }
 
 /**
- * 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.
@@ -223,26 +222,6 @@ int run_command(struct child_process *);
  */
 int run_auto_maintenance(int quiet);
 
-#define RUN_COMMAND_NO_STDIN           (1<<0)
-#define RUN_GIT_CMD                    (1<<1)
-#define RUN_COMMAND_STDOUT_TO_STDERR   (1<<2)
-#define RUN_SILENT_EXEC_FAILURE                (1<<3)
-#define RUN_USING_SHELL                        (1<<4)
-#define RUN_CLEAN_ON_EXIT              (1<<5)
-#define RUN_WAIT_AFTER_CLEAN           (1<<6)
-#define RUN_CLOSE_OBJECT_STORE         (1<<7)
-
-/**
- * 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_STDOUT_TO_STDERR`, or `RUN_SILENT_EXEC_FAILURE`
- * that correspond to the members .no_stdin, .git_cmd,
- * .stdout_to_stderr, .silent_exec_failure of `struct child_process`.
- */
-int run_command_v_opt(const char **argv, int opt);
-
 /**
  * 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

diff --git a/sequencer.c b/sequencer.c
index 31e24f38f8f2221dcd2942add9ed59512bb26487..643744fb9b9f0f876744d9a2a03e7a80e8ab005d 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -3555,11 +3555,13 @@ 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)

diff --git a/shell.c b/shell.c
index 7ff4109db7058b5677be2fa8775f699b54e4d4f1..af0d7c734f8347082ab094cf1fc8898cbf26b64d 100644 (file)
--- a/shell.c
+++ b/shell.c
@@ -52,21 +52,24 @@ static void cd_to_homedir(void)
 static void run_shell(void)
 {
        int done = 0;
-       static const char *help_argv[] = { HELP_COMMAND, NULL };
+       struct child_process help_cmd = CHILD_PROCESS_INIT;
 
        if (!access(NOLOGIN_COMMAND, F_OK)) {
                /* Interactive login disabled. */
-               const char *argv[] = { NOLOGIN_COMMAND, NULL };
+               struct child_process nologin_cmd = CHILD_PROCESS_INIT;
                int status;
 
-               status = run_command_v_opt(argv, 0);
+               strvec_push(&nologin_cmd.args, NOLOGIN_COMMAND);
+               status = run_command(&nologin_cmd);
                if (status < 0)
                        exit(127);
                exit(status);
        }
 
        /* Print help if enabled */
-       run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
+       help_cmd.silent_exec_failure = 1;
+       strvec_push(&help_cmd.args, HELP_COMMAND);
+       run_command(&help_cmd);
 
        do {
                const char *prog;
@@ -125,9 +128,13 @@ static void run_shell(void)
                           !strcmp(prog, "exit") || !strcmp(prog, "bye")) {
                        done = 1;
                } else if (is_valid_cmd_name(prog)) {
+                       struct child_process cmd = CHILD_PROCESS_INIT;
+
                        full_cmd = make_cmd(prog);
                        argv[0] = full_cmd;
-                       code = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE);
+                       cmd.silent_exec_failure = 1;
+                       strvec_pushv(&cmd.args, argv);
+                       code = run_command(&cmd);
                        if (code == -1 && errno == ENOENT) {
                                fprintf(stderr, "unrecognized command '%s'\n", prog);
                        }

diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
index a714130ece77c334c982b8a9520d7fb1b9fa6a3d..94fd8ccf513f6f5dd157eddad78e99b81414024b 100644 (file)
--- a/t/helper/test-trace2.c
+++ b/t/helper/test-trace2.c
@@ -132,6 +132,7 @@ static int ut_003error(int argc, const char **argv)
  */
 static int ut_004child(int argc, const char **argv)
 {
+       struct child_process cmd = CHILD_PROCESS_INIT;
        int result;
 
        /*
@@ -141,7 +142,8 @@ static int ut_004child(int argc, const char **argv)
        if (!argc)
                return 0;
 
-       result = run_command_v_opt(argv, 0);
+       strvec_pushv(&cmd.args, argv);
+       result = run_command(&cmd);
        exit(result);
 }