git/run-command.h, branch v2.40.2

Merge branch 'rs/no-more-run-command-v'

2022-11-08T22:15:12Z

Simplify the run-command API. * rs/no-more-run-command-v: replace and remove run_command_v_opt() replace and remove run_command_v_opt_cd_env_tr2() replace and remove run_command_v_opt_tr2() replace and remove run_command_v_opt_cd_env() use child_process members "args" and "env" directly use child_process member "args" instead of string array variable sequencer: simplify building argument list in do_exec() bisect--helper: factor out do_bisect_run() bisect: simplify building "checkout" argument list am: simplify building "show" argument list run-command: fix return value comment merge: remove always-the-same "verbose" arguments

replace and remove run_command_v_opt()

2022-10-30T18:04:51Z

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 Signed-off-by: René Scharfe Signed-off-by: Taylor Blau

replace and remove run_command_v_opt_cd_env_tr2()

2022-10-30T18:04:50Z

The convenience function run_command_v_opt_cd_env_tr2() has no external callers left. Inline it and remove it from the API. Signed-off-by: René Scharfe Signed-off-by: Taylor Blau

replace and remove run_command_v_opt_tr2()

2022-10-30T18:04:48Z

The convenience function run_command_v_opt_tr2() is only used by a single caller. Use struct child_process and run_command() directly instead and remove the underused function. Signed-off-by: René Scharfe Signed-off-by: Taylor Blau

replace and remove run_command_v_opt_cd_env()

2022-10-30T18:04:47Z

run_command_v_opt_cd_env() is only used in an example in a comment. Use the struct child_process member "env" and run_command() directly instead and then remove the unused convenience function. Signed-off-by: René Scharfe Signed-off-by: Taylor Blau

run-command: fix return value comment

2022-10-30T18:04:32Z

483bbd4e4c (run-command: introduce child_process_init(), 2014-08-19) and 2d71608ec0 (run-command: factor out child_process_clear(), 2015-10-24) added help texts about child_process_init() and child_process_clear() without updating the immediately following documentation of return codes that only applied to the preexisting functions. 4c4066d95d (run-command: move doc to run-command.h, 2019-11-17) started to list the functions explicitly that this paragraph applies to, but still wrongly included child_process_init() and child_process_clear(). Remove their names from that list. Suggested-by: Ævar Arnfjörð Bjarmason Signed-off-by: René Scharfe Signed-off-by: Taylor Blau

run-command API: move *_tr2() users to "run_processes_parallel()"

2022-10-12T21:12:41Z

Have the users of the "run_processes_parallel_tr2()" function use "run_processes_parallel()" instead. In preceding commits the latter was refactored to take a "struct run_process_parallel_opts" argument, since the only reason for "run_processes_parallel_tr2()" to exist was to take arguments that are now a part of that struct we can do away with it. See ee4512ed481 (trace2: create new combined trace facility, 2019-02-22) for the addition of the "*_tr2()" variant of the function, it was used by every caller except "t/helper/test-run-command.c".. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

run-command API: have run_process_parallel() take an "opts" struct

2022-10-12T21:12:41Z

As noted in fd3aaf53f71 (run-command: add an "ungroup" option to run_process_parallel(), 2022-06-07) which added the "ungroup" passing it to "run_process_parallel()" via the global "run_processes_parallel_ungroup" variable was a compromise to get the smallest possible regression fix for "maint" at the time. This follow-up to that is a start at passing that parameter and others via a new "struct run_process_parallel_opts", as the earlier version[1] of what became fd3aaf53f71 did. Since we need to change all of the occurrences of "n" to "opt->SOMETHING" let's take the opportunity and rename the terse "n" to "processes". We could also have picked "max_processes", "jobs", "threads" etc., but as the API is named "run_processes_parallel()" let's go with "processes". Since the new "run_processes_parallel()" function is able to take an optional "tr2_category" and "tr2_label" via the struct we can at this point migrate all of the users of "run_processes_parallel_tr2()" over to it. But let's not migrate all the API users yet, only the two users that passed the "ungroup" parameter via the "run_processes_parallel_ungroup" global 1. https://lore.kernel.org/git/cover-v2-0.8-00000000000-20220518T195858Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

run-command API: make "n" parameter a "size_t"

2022-10-12T21:12:40Z

Make the "n" variable added in c553c72eed6 (run-command: add an asynchronous parallel child processor, 2015-12-15) a "size_t". As we'll see in a subsequent commit we do pass "0" here, but never "jobs < 0". We could have made it an "unsigned int", but as we're having to change this let's not leave another case in the codebase where a size_t and "unsigned int" size differ on some platforms. In this case it's likely to never matter, but it's easier to not need to worry about it. After this and preceding changes: make run-command.o DEVOPTS=extra-all CFLAGS=-Wno-unused-parameter Only has one (and new) -Wsigned-compare warning relevant to a comparison about our "n" or "{nr,max}_processes": About using our "n" (size_t) in the same expression as online_cpus() (int). A subsequent commit will adjust & deal with online_cpus() and that warning. The only users of the "n" parameter are: * builtin/fetch.c: defaults to 1, reads from the "fetch.parallel" config. As seen in the code that parses the config added in d54dea77dba (fetch: let --jobs= parallelize --multiple, too, 2019-10-05) will die if the git_config_int() return value is < 0. It will however pass us n = 0, as we'll see in a subsequent commit. * submodule.c: defaults to 1, reads from "submodule.fetchJobs" config. Read via code originally added in a028a1930c6 (fetching submodules: respect `submodule.fetchJobs` config option, 2016-02-29). It now piggy-backs on the the submodule.fetchJobs code and validation added in f20e7c1ea24 (submodule: remove submodule.fetchjobs from submodule-config parsing, 2017-08-02). Like builtin/fetch.c it will die if the git_config_int() return value is < 0, but like builtin/fetch.c it will pass us n = 0. * builtin/submodule--helper.c: defaults to 1. Read via code originally added in 2335b870fa7 (submodule update: expose parallelism to the user, 2016-02-29). Since f20e7c1ea24 (submodule: remove submodule.fetchjobs from submodule-config parsing, 2017-08-02) it shares a config parser and semantics with the submodule.c caller. * hook.c: hardcoded to 1, see 96e7225b310 (hook: add 'run' subcommand, 2021-12-22). * t/helper/test-run-command.c: can be -1 after parsing the arguments, but will then be overridden to online_cpus() before passing it to this API. See be5d88e1128 (test-tool run-command: learn to run (parts of) the testsuite, 2019-10-04). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

run-command API: have "run_processes_parallel{,_tr2}()" return void

2022-10-12T21:12:40Z

Change the "run_processes_parallel{,_tr2}()" functions to return void, instead of int. Ever since c553c72eed6 (run-command: add an asynchronous parallel child processor, 2015-12-15) they have unconditionally returned 0. To get a "real" return value out of this function the caller needs to get it via the "task_finished_fn" callback, see the example in hook.c added in 96e7225b310 (hook: add 'run' subcommand, 2021-12-22). So the "result = " and "if (!result)" code added to "builtin/fetch.c" d54dea77dba (fetch: let --jobs= parallelize --multiple, too, 2019-10-05) has always been redundant, we always took that "if" path. Likewise the "ret =" in "t/helper/test-run-command.c" added in be5d88e1128 (test-tool run-command: learn to run (parts of) the testsuite, 2019-10-04) wasn't used, instead we got the return value from the "if (suite.failed.nr > 0)" block seen in the context. Subsequent commits will alter this API interface, getting rid of this always-zero return value makes it easier to understand those changes. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano