@matheusbsbs / matheus-bagliano / commits / ddbb47fde9

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>

René Scharfe committed Oct 30, 2022 at 12:55 UTC ddbb47fde9b6d8cd9f3728847a378f634318cfb1
13 files changed +92 -75
bisect.c
+5 -4
@@ -737,11 +737,12 @@ enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
737 update_ref(NULL, "BISECT_HEAD", bisect_rev, NULL, 0,
738 UPDATE_REFS_DIE_ON_ERR);
739 } else {
740 - const char *argv_checkout[] = {
741 - "checkout", "-q", oid_to_hex(bisect_rev), "--", NULL
742 - };
740 + struct child_process cmd = CHILD_PROCESS_INIT;
741
744 - if (run_command_v_opt(argv_checkout, RUN_GIT_CMD))
742 + cmd.git_cmd = 1;
743 + strvec_pushl(&cmd.args, "checkout", "-q",
744 + oid_to_hex(bisect_rev), "--", NULL);
745 + if (run_command(&cmd))
746 /*
747 * Errors in `run_command()` itself, signaled by res < 0,
748 * and errors in the child process, signaled by res > 0
builtin/am.c
+5 -4
@@ -2187,11 +2187,12 @@ static int show_patch(struct am_state *state, enum show_patch_type sub_mode)
2187 int len;
2188
2189 if (!is_null_oid(&state->orig_commit)) {
2190 - const char *av[] = {
2191 - "show", oid_to_hex(&state->orig_commit), "--", NULL
2192 - };
2190 + struct child_process cmd = CHILD_PROCESS_INIT;
2191
2194 - return run_command_v_opt(av, RUN_GIT_CMD);
2192 + strvec_pushl(&cmd.args, "show", oid_to_hex(&state->orig_commit),
2193 + "--", NULL);
2194 + cmd.git_cmd = 1;
2195 + return run_command(&cmd);
2196 }
2197
2198 switch (sub_mode) {
builtin/bisect--helper.c
+10 -5
@@ -764,10 +764,12 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a
764 strbuf_read_file(&start_head, git_path_bisect_start(), 0);
765 strbuf_trim(&start_head);
766 if (!no_checkout) {
767 - const char *argv[] = { "checkout", start_head.buf,
768 - "--", NULL };
767 + struct child_process cmd = CHILD_PROCESS_INIT;
768
770 - if (run_command_v_opt(argv, RUN_GIT_CMD)) {
769 + cmd.git_cmd = 1;
770 + strvec_pushl(&cmd.args, "checkout", start_head.buf,
771 + "--", NULL);
772 + if (run_command(&cmd)) {
773 res = error(_("checking out '%s' failed."
774 " Try 'git bisect start "
775 "<valid-branch>'."),
@@ -1141,9 +1143,12 @@ static int get_first_good(const char *refname UNUSED,
1143
1144 static int do_bisect_run(const char *command)
1145 {
1144 - const char *argv[] = { command, NULL };
1146 + struct child_process cmd = CHILD_PROCESS_INIT;
1147 +
1148 printf(_("running %s\n"), command);
1146 - return run_command_v_opt(argv, RUN_USING_SHELL);
1149 + cmd.use_shell = 1;
1150 + strvec_push(&cmd.args, command);
1151 + return run_command(&cmd);
1152 }
1153
1154 static int verify_good(const struct bisect_terms *terms, const char *command)
builtin/clone.c
+6 -2
@@ -865,11 +865,15 @@ static void write_refspec_config(const char *src_ref_prefix,
865
866 static void dissociate_from_references(void)
867 {
868 - static const char* argv[] = { "repack", "-a", "-d", NULL };
868 char *alternates = git_pathdup("objects/info/alternates");
869
870 if (!access(alternates, F_OK)) {
872 - if (run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDIN))
871 + struct child_process cmd = CHILD_PROCESS_INIT;
872 +
873 + cmd.git_cmd = 1;
874 + cmd.no_stdin = 1;
875 + strvec_pushl(&cmd.args, "repack", "-a", "-d", NULL);
876 + if (run_command(&cmd))
877 die(_("cannot repack to clean up"));
878 if (unlink(alternates) && errno != ENOENT)
879 die_errno(_("cannot unlink temporary alternates file"));
builtin/difftool.c
+5 -2
@@ -44,8 +44,11 @@ static int difftool_config(const char *var, const char *value, void *cb)
44
45 static int print_tool_help(void)
46 {
47 - const char *argv[] = { "mergetool", "--tool-help=diff", NULL };
48 - return run_command_v_opt(argv, RUN_GIT_CMD);
47 + struct child_process cmd = CHILD_PROCESS_INIT;
48 +
49 + cmd.git_cmd = 1;
50 + strvec_pushl(&cmd.args, "mergetool", "--tool-help=diff", NULL);
51 + return run_command(&cmd);
52 }
53
54 static int parse_index_info(char *p, int *mode1, int *mode2,
builtin/fetch.c
+6 -3
@@ -1965,14 +1965,17 @@ static int fetch_multiple(struct string_list *list, int max_children)
1965 } else
1966 for (i = 0; i < list->nr; i++) {
1967 const char *name = list->items[i].string;
1968 - strvec_push(&argv, name);
1968 + struct child_process cmd = CHILD_PROCESS_INIT;
1969 +
1970 + strvec_pushv(&cmd.args, argv.v);
1971 + strvec_push(&cmd.args, name);
1972 if (verbosity >= 0)
1973 printf(_("Fetching %s\n"), name);
1971 - if (run_command_v_opt(argv.v, RUN_GIT_CMD)) {
1974 + cmd.git_cmd = 1;
1975 + if (run_command(&cmd)) {
1976 error(_("could not fetch %s"), name);
1977 result = 1;
1978 }
1975 - strvec_pop(&argv);
1979 }
1980
1981 strvec_clear(&argv);
builtin/gc.c
+32 -9
@@ -167,9 +167,11 @@ static void gc_config(void)
167 struct maintenance_run_opts;
168 static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)
169 {
170 - const char *argv[] = { "pack-refs", "--all", "--prune", NULL };
170 + struct child_process cmd = CHILD_PROCESS_INIT;
171
172 - return run_command_v_opt(argv, RUN_GIT_CMD);
172 + cmd.git_cmd = 1;
173 + strvec_pushl(&cmd.args, "pack-refs", "--all", "--prune", NULL);
174 + return run_command(&cmd);
175 }
176
177 static int too_many_loose_objects(void)
@@ -535,8 +537,14 @@ static void gc_before_repack(void)
537 if (pack_refs && maintenance_task_pack_refs(NULL))
538 die(FAILED_RUN, "pack-refs");
539
538 - if (prune_reflogs && run_command_v_opt(reflog.v, RUN_GIT_CMD))
539 - die(FAILED_RUN, reflog.v[0]);
540 + if (prune_reflogs) {
541 + struct child_process cmd = CHILD_PROCESS_INIT;
542 +
543 + cmd.git_cmd = 1;
544 + strvec_pushv(&cmd.args, reflog.v);
545 + if (run_command(&cmd))
546 + die(FAILED_RUN, reflog.v[0]);
547 + }
548 }
549
550 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)
558 int daemonized = 0;
559 int keep_largest_pack = -1;
560 timestamp_t dummy;
561 + struct child_process rerere_cmd = CHILD_PROCESS_INIT;
562
563 struct option builtin_gc_options[] = {
564 OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -671,11 +680,17 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
680 gc_before_repack();
681
682 if (!repository_format_precious_objects) {
674 - if (run_command_v_opt(repack.v,
675 - RUN_GIT_CMD | RUN_CLOSE_OBJECT_STORE))
683 + struct child_process repack_cmd = CHILD_PROCESS_INIT;
684 +
685 + repack_cmd.git_cmd = 1;
686 + repack_cmd.close_object_store = 1;
687 + strvec_pushv(&repack_cmd.args, repack.v);
688 + if (run_command(&repack_cmd))
689 die(FAILED_RUN, repack.v[0]);
690
691 if (prune_expire) {
692 + struct child_process prune_cmd = CHILD_PROCESS_INIT;
693 +
694 /* run `git prune` even if using cruft packs */
695 strvec_push(&prune, prune_expire);
696 if (quiet)
@@ -683,18 +698,26 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
698 if (has_promisor_remote())
699 strvec_push(&prune,
700 "--exclude-promisor-objects");
686 - if (run_command_v_opt(prune.v, RUN_GIT_CMD))
701 + prune_cmd.git_cmd = 1;
702 + strvec_pushv(&prune_cmd.args, prune.v);
703 + if (run_command(&prune_cmd))
704 die(FAILED_RUN, prune.v[0]);
705 }
706 }
707
708 if (prune_worktrees_expire) {
709 + struct child_process prune_worktrees_cmd = CHILD_PROCESS_INIT;
710 +
711 strvec_push(&prune_worktrees, prune_worktrees_expire);
693 - if (run_command_v_opt(prune_worktrees.v, RUN_GIT_CMD))
712 + prune_worktrees_cmd.git_cmd = 1;
713 + strvec_pushv(&prune_worktrees_cmd.args, prune_worktrees.v);
714 + if (run_command(&prune_worktrees_cmd))
715 die(FAILED_RUN, prune_worktrees.v[0]);
716 }
717
697 - if (run_command_v_opt(rerere.v, RUN_GIT_CMD))
718 + rerere_cmd.git_cmd = 1;
719 + strvec_pushv(&rerere_cmd.args, rerere.v);
720 + if (run_command(&rerere_cmd))
721 die(FAILED_RUN, rerere.v[0]);
722
723 report_garbage = report_pack_garbage;
builtin/merge-index.c
+3 -1
@@ -12,6 +12,7 @@ static int merge_entry(int pos, const char *path)
12 const char *arguments[] = { pgm, "", "", "", path, "", "", "", NULL };
13 char hexbuf[4][GIT_MAX_HEXSZ + 1];
14 char ownbuf[4][60];
15 + struct child_process cmd = CHILD_PROCESS_INIT;
16
17 if (pos >= active_nr)
18 die("git merge-index: %s not in the cache", path);
@@ -31,7 +32,8 @@ static int merge_entry(int pos, const char *path)
32 if (!found)
33 die("git merge-index: %s not in the cache", path);
34
34 - if (run_command_v_opt(arguments, 0)) {
35 + strvec_pushv(&cmd.args, arguments);
36 + if (run_command(&cmd)) {
37 if (one_shot)
38 err++;
39 else {
run-command.c
-15
@@ -1004,21 +1004,6 @@ int run_command(struct child_process *cmd)
1004 return finish_command(cmd);
1005 }
1006
1007 -int run_command_v_opt(const char **argv, int opt)
1008 -{
1009 - struct child_process cmd = CHILD_PROCESS_INIT;
1010 - strvec_pushv(&cmd.args, argv);
1011 - cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
1012 - cmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
1013 - cmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
1014 - cmd.silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;
1015 - cmd.use_shell = opt & RUN_USING_SHELL ? 1 : 0;
1016 - cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
1017 - cmd.wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0;
1018 - cmd.close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0;
1019 - return run_command(&cmd);
1020 -}
1021 -
1007 #ifndef NO_PTHREADS
1008 static pthread_t main_thread;
1009 static int main_thread_set;
run-command.h
+1 -22
@@ -150,8 +150,7 @@ struct child_process {
150 }
151
152 /**
153 - * The functions: start_command, finish_command, run_command,
154 - * run_command_v_opt do the following:
153 + * The functions: start_command, finish_command, run_command do the following:
154 *
155 * - If a system call failed, errno is set and -1 is returned. A diagnostic
156 * is printed.
@@ -223,26 +222,6 @@ int run_command(struct child_process *);
222 */
223 int run_auto_maintenance(int quiet);
224
226 -#define RUN_COMMAND_NO_STDIN (1<<0)
227 -#define RUN_GIT_CMD (1<<1)
228 -#define RUN_COMMAND_STDOUT_TO_STDERR (1<<2)
229 -#define RUN_SILENT_EXEC_FAILURE (1<<3)
230 -#define RUN_USING_SHELL (1<<4)
231 -#define RUN_CLEAN_ON_EXIT (1<<5)
232 -#define RUN_WAIT_AFTER_CLEAN (1<<6)
233 -#define RUN_CLOSE_OBJECT_STORE (1<<7)
234 -
235 -/**
236 - * Convenience function that encapsulates a sequence of
237 - * start_command() followed by finish_command(). The argument argv
238 - * specifies the program and its arguments. The argument opt is zero
239 - * or more of the flags `RUN_COMMAND_NO_STDIN`, `RUN_GIT_CMD`,
240 - * `RUN_COMMAND_STDOUT_TO_STDERR`, or `RUN_SILENT_EXEC_FAILURE`
241 - * that correspond to the members .no_stdin, .git_cmd,
242 - * .stdout_to_stderr, .silent_exec_failure of `struct child_process`.
243 - */
244 -int run_command_v_opt(const char **argv, int opt);
245 -
225 /**
226 * Execute the given command, sending "in" to its stdin, and capturing its
227 * stdout and stderr in the "out" and "err" strbufs. Any of the three may
sequencer.c
+4 -2
@@ -3555,11 +3555,13 @@ static int error_failed_squash(struct repository *r,
3555
3556 static int do_exec(struct repository *r, const char *command_line)
3557 {
3558 - const char *child_argv[] = { command_line, NULL };
3558 + struct child_process cmd = CHILD_PROCESS_INIT;
3559 int dirty, status;
3560
3561 fprintf(stderr, _("Executing: %s\n"), command_line);
3562 - status = run_command_v_opt(child_argv, RUN_USING_SHELL);
3562 + cmd.use_shell = 1;
3563 + strvec_push(&cmd.args, command_line);
3564 + status = run_command(&cmd);
3565
3566 /* force re-reading of the cache */
3567 if (discard_index(r->index) < 0 || repo_read_index(r) < 0)
shell.c
+12 -5
@@ -52,21 +52,24 @@ static void cd_to_homedir(void)
52 static void run_shell(void)
53 {
54 int done = 0;
55 - static const char *help_argv[] = { HELP_COMMAND, NULL };
55 + struct child_process help_cmd = CHILD_PROCESS_INIT;
56
57 if (!access(NOLOGIN_COMMAND, F_OK)) {
58 /* Interactive login disabled. */
59 - const char *argv[] = { NOLOGIN_COMMAND, NULL };
59 + struct child_process nologin_cmd = CHILD_PROCESS_INIT;
60 int status;
61
62 - status = run_command_v_opt(argv, 0);
62 + strvec_push(&nologin_cmd.args, NOLOGIN_COMMAND);
63 + status = run_command(&nologin_cmd);
64 if (status < 0)
65 exit(127);
66 exit(status);
67 }
68
69 /* Print help if enabled */
69 - run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
70 + help_cmd.silent_exec_failure = 1;
71 + strvec_push(&help_cmd.args, HELP_COMMAND);
72 + run_command(&help_cmd);
73
74 do {
75 const char *prog;
@@ -125,9 +128,13 @@ static void run_shell(void)
128 !strcmp(prog, "exit") || !strcmp(prog, "bye")) {
129 done = 1;
130 } else if (is_valid_cmd_name(prog)) {
131 + struct child_process cmd = CHILD_PROCESS_INIT;
132 +
133 full_cmd = make_cmd(prog);
134 argv[0] = full_cmd;
130 - code = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE);
135 + cmd.silent_exec_failure = 1;
136 + strvec_pushv(&cmd.args, argv);
137 + code = run_command(&cmd);
138 if (code == -1 && errno == ENOENT) {
139 fprintf(stderr, "unrecognized command '%s'\n", prog);
140 }
t/helper/test-trace2.c
+3 -1
@@ -132,6 +132,7 @@ static int ut_003error(int argc, const char **argv)
132 */
133 static int ut_004child(int argc, const char **argv)
134 {
135 + struct child_process cmd = CHILD_PROCESS_INIT;
136 int result;
137
138 /*
@@ -141,7 +142,8 @@ static int ut_004child(int argc, const char **argv)
142 if (!argc)
143 return 0;
144
144 - result = run_command_v_opt(argv, 0);
145 + strvec_pushv(&cmd.args, argv);
146 + result = run_command(&cmd);
147 exit(result);
148 }
149