From patchwork Wed May 18 20:05:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854047 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D96B7C433EF for ; Wed, 18 May 2022 20:05:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242222AbiERUFe (ORCPT ); Wed, 18 May 2022 16:05:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53624 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242215AbiERUFd (ORCPT ); Wed, 18 May 2022 16:05:33 -0400 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F3DF02375DB for ; Wed, 18 May 2022 13:05:31 -0700 (PDT) Received: by mail-wm1-x32a.google.com with SMTP id c190-20020a1c35c7000000b0038e37907b5bso3823767wma.0 for ; Wed, 18 May 2022 13:05:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=k9GHGCuef0MIhLayTjOT1f61qHmpj35gpO2QY+Eq3Fw=; b=oE1Pe0q35UX6/zR1xecstaoc2EO3Y8WF6cOnzOPR9/FU5mJuRj8sYmg2/nDzSBo5vy 8HHMPWQ/CXjiSW3GIciwNeMKuFrSUWQoS5mLHK8p41vGYnNKv6cVPa/t2FTYDQEZHpHx dwY4Tm1SXVKhGCjf47MTu+kU1LrLq2gTmxOsQf8cTzp05N17r3VPhytiqUSC3DGv5Hl2 UBSfRz2zPIp9+Nx3EwkUrY6+1Lno1ZEsbWvtnfrajOPvahTD6ZilrZsl3z5wPGYCFvmb 3gNvVCrxcIq9yFeqFmCCGkmCwGFX9kiGOROoRj7RFgXPX8+eTlvRImnj+SMwre10slS7 daiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=k9GHGCuef0MIhLayTjOT1f61qHmpj35gpO2QY+Eq3Fw=; b=pUYtRGWBGvme2VlywWRm/FrKqUo2RrKRAWUOHRQMF0PwWEYnerV+rO8quWVzkmdDwj lI5CbblyEoFVXakB6bftpwiEMgPxMSdN9yVf6ZwToB5ZKtH/IXhNrNCo9VzjlK3+Oyc3 fR4z/PDf/VpJ66Q3EVhTQoQ9CVGrmH70hTHUOpgQCW+PVB4+RyJcx8xiEHTaNZKzRz9I 3SzteMvEr8h+RnC8OEg4l1dTGVPg3zlVn2NcxOYvTXra9hAtvPqZi8ZrcQFCALR+Z/S9 VLVeLBQk3geu/J8+6F6ZzJzGKV5Rgm3ptPYaiOWp8eP51OeUbTUj1TFpUPV+N7Q4+AYQ CX4g== X-Gm-Message-State: AOAM531GEKYKPr5+ur/xmfriR/RNGuOUXLqIH8UM/VwEPsnR6T/C7KWH EY/i5yjCPPCWdmu5QtPK9Hxx4GOPB/gSNw== X-Google-Smtp-Source: ABdhPJyc/Q+6KDBjRMU1pdFikJ7gX66uxko7nsFwaTyJoNqHlTMNzD9pNDUp18kVKKJIdbRbZDFvCA== X-Received: by 2002:a05:600c:4e13:b0:394:797e:3358 with SMTP id b19-20020a05600c4e1300b00394797e3358mr1403858wmq.30.1652904330052; Wed, 18 May 2022 13:05:30 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:29 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 1/8] run-command tests: change if/if/... to if/else if/else Date: Wed, 18 May 2022 22:05:17 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Refactor the code in cmd__run_command() to make a subsequent changes smaller by reducing duplication. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/helper/test-run-command.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index f3b90aa834a..bd98dd9624b 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -371,6 +371,8 @@ int cmd__run_command(int argc, const char **argv) { struct child_process proc = CHILD_PROCESS_INIT; int jobs; + get_next_task_fn next_fn = NULL; + task_finished_fn finished_fn = NULL; if (argc > 1 && !strcmp(argv[1], "testsuite")) exit(testsuite(argc - 1, argv + 1)); @@ -411,18 +413,18 @@ int cmd__run_command(int argc, const char **argv) strvec_clear(&proc.args); strvec_pushv(&proc.args, (const char **)argv + 3); - if (!strcmp(argv[1], "run-command-parallel")) - exit(run_processes_parallel(jobs, parallel_next, - NULL, NULL, &proc)); - - if (!strcmp(argv[1], "run-command-abort")) - exit(run_processes_parallel(jobs, parallel_next, - NULL, task_finished, &proc)); - - if (!strcmp(argv[1], "run-command-no-jobs")) - exit(run_processes_parallel(jobs, no_job, - NULL, task_finished, &proc)); + if (!strcmp(argv[1], "run-command-parallel")) { + next_fn = parallel_next; + } else if (!strcmp(argv[1], "run-command-abort")) { + next_fn = parallel_next; + finished_fn = task_finished; + } else if (!strcmp(argv[1], "run-command-no-jobs")) { + next_fn = no_job; + finished_fn = task_finished; + } else { + fprintf(stderr, "check usage\n"); + return 1; + } - fprintf(stderr, "check usage\n"); - return 1; + exit(run_processes_parallel(jobs, next_fn, NULL, finished_fn, &proc)); } From patchwork Wed May 18 20:05:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854048 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2AF6C433F5 for ; Wed, 18 May 2022 20:05:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242259AbiERUFf (ORCPT ); Wed, 18 May 2022 16:05:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53650 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242244AbiERUFe (ORCPT ); Wed, 18 May 2022 16:05:34 -0400 Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 853672375D2 for ; Wed, 18 May 2022 13:05:32 -0700 (PDT) Received: by mail-wm1-x336.google.com with SMTP id k126-20020a1ca184000000b003943fd07180so1599764wme.3 for ; Wed, 18 May 2022 13:05:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=zDIvb4GctMknH+vb94WMvGaJKCld3OdP+R0L/ci7OWI=; b=ce/rkdVjWJENDYIes+msEKqzOkDvnxw2ZIVYLQY0mlMpWEd2Xu7i+C72lsvPVDoDUJ wNc6fAeW/NfY8MdvGx2zweLUoASEXLwniSoIIKG/J5KeW3wc/qM+lMnDcbpa8BNK93GV ZYlZ6MPQqEBZhMW35MSTxqRCyvVyaViomANrxVs9QaEQDgMbNC5Fua0DSbjawhe7O5pS AQ6X5vPl0dpPoAP+xGEV3FevzDBCRuKTXGb/VsXFfR221c6SFeJMeB6HBxCiVBBymYTg 01sMC48fFsiTSPlleBdxZfz9DmJgmKKdt7TJwIVZTgFYx888R+WS92V0vxW/X3+YcgI/ iSUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=zDIvb4GctMknH+vb94WMvGaJKCld3OdP+R0L/ci7OWI=; b=mPJByfWt7RJJVuv7hFphOdTTBSaCXdo/vqOypg6+94a2mpFIvBjGePJsBFOvACpdIT y4HsuZbkhiGdPaE8qOdxhdqIxAioFyx2hIBA2o2QrPcno2Nw4kN8CKbV1IDUE7XWn13G xXxWHuvmtPFB2u2SgvKCwbJYNeWFILC4BGffc1DOkVeMFD+lWoLNHdB/wNUNNZgCMQi9 lU9M3X/zc5jXoJcQP5DQcBUZLGm9r+RXGtjwjfsWa528h6RX4cVdCTjKubfQnHdttgAP VUPV0ENgqAKGxdyNQL0G33zjkh60B23/X68O/2TGsaRuFkQVuw2mYnkjMqs7N7VUcgP1 brlg== X-Gm-Message-State: AOAM533+/qWZ1x5Q7hU9AFDb6IwnmoiqYHsdng9qWu4Gfseq9HhvZJFT XUBhOHLgO+WwT8nfuGa8Bc4s1qp3T3m7VA== X-Google-Smtp-Source: ABdhPJzuictw1yLcSl8ziECY0iu40V58FI0hgLKljPJgReiNy39sJRfYzIuTdX1K38nnO/wNMLzC/Q== X-Received: by 2002:a05:600c:382:b0:394:6172:59dc with SMTP id w2-20020a05600c038200b00394617259dcmr1400357wmd.120.1652904330841; Wed, 18 May 2022 13:05:30 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:30 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 2/8] run-command API: use "opts" struct for run_processes_parallel{,_tr2}() Date: Wed, 18 May 2022 22:05:18 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a new "struct run_process_parallel_opts" to replace the growing run_processes_parallel() and run_processes_parallel_tr2() argument lists. This refactoring makes it easier to add new options and parameters easier. The *_tr2() variant of the function was added in ee4512ed481 (trace2: create new combined trace facility, 2019-02-22), and has subsequently been used by every caller except t/helper/test-run-command.c. Signed-off-by: Ævar Arnfjörð Bjarmason Reviewed-by: Emily Shaffer --- builtin/fetch.c | 18 +++++++++----- builtin/submodule--helper.c | 15 ++++++++---- hook.c | 21 +++++++++------- run-command.c | 48 ++++++++++++++++--------------------- run-command.h | 35 ++++++++++++++++++++------- submodule.c | 18 +++++++++----- t/helper/test-run-command.c | 17 ++++++++++--- 7 files changed, 109 insertions(+), 63 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index e3791f09ed5..d85bf135e66 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1948,14 +1948,20 @@ static int fetch_multiple(struct string_list *list, int max_children) if (max_children != 1 && list->nr != 1) { struct parallel_fetch_state state = { argv.v, list, 0, 0 }; + struct run_process_parallel_opts run_opts = { + .tr2_category = "fetch", + .tr2_label = "parallel/fetch", + + .jobs = max_children, + + .get_next_task = &fetch_next_remote, + .start_failure = &fetch_failed_to_start, + .task_finished = &fetch_finished, + .data = &state, + }; strvec_push(&argv, "--end-of-options"); - result = run_processes_parallel_tr2(max_children, - &fetch_next_remote, - &fetch_failed_to_start, - &fetch_finished, - &state, - "fetch", "parallel/fetch"); + result = run_processes_parallel(&run_opts); if (!result) result = state.result; diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 1a8e5d06214..756807e965d 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2651,12 +2651,19 @@ static int update_submodules(struct update_data *update_data) { int i, res = 0; struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT; + struct run_process_parallel_opts run_opts = { + .tr2_category = "submodule", + .tr2_label = "parallel/update", + + .get_next_task = update_clone_get_next_task, + .start_failure = update_clone_start_failure, + .task_finished = update_clone_task_finished, + .data = &suc, + }; suc.update_data = update_data; - run_processes_parallel_tr2(suc.update_data->max_jobs, update_clone_get_next_task, - update_clone_start_failure, - update_clone_task_finished, &suc, "submodule", - "parallel/update"); + run_opts.jobs = suc.update_data->max_jobs; + run_processes_parallel(&run_opts); /* * We saved the output and put it out all at once now. diff --git a/hook.c b/hook.c index 1d51be3b77a..9aefccfc34a 100644 --- a/hook.c +++ b/hook.c @@ -121,8 +121,19 @@ int run_hooks_opt(const char *hook_name, struct run_hooks_opt *options) .options = options, }; const char *const hook_path = find_hook(hook_name); - int jobs = 1; + const int jobs = 1; int ret = 0; + struct run_process_parallel_opts run_opts = { + .tr2_category = "hook", + .tr2_label = hook_name, + + .jobs = jobs, + + .get_next_task = pick_next_hook, + .start_failure = notify_start_failure, + .task_finished = notify_hook_finished, + .data = &cb_data, + }; if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); @@ -144,13 +155,7 @@ int run_hooks_opt(const char *hook_name, struct run_hooks_opt *options) cb_data.hook_path = abs_path.buf; } - run_processes_parallel_tr2(jobs, - pick_next_hook, - notify_start_failure, - notify_hook_finished, - &cb_data, - "hook", - hook_name); + run_processes_parallel(&run_opts); ret = cb_data.rc; cleanup: strbuf_release(&abs_path); diff --git a/run-command.c b/run-command.c index a8501e38ceb..8c156fd080e 100644 --- a/run-command.c +++ b/run-command.c @@ -1533,13 +1533,14 @@ static void handle_children_on_signal(int signo) } static void pp_init(struct parallel_processes *pp, - int n, - get_next_task_fn get_next_task, - start_failure_fn start_failure, - task_finished_fn task_finished, - void *data) + struct run_process_parallel_opts *opts) { int i; + int n = opts->jobs; + void *data = opts->data; + get_next_task_fn get_next_task = opts->get_next_task; + start_failure_fn start_failure = opts->start_failure; + task_finished_fn task_finished = opts->task_finished; if (n < 1) n = online_cpus(); @@ -1738,18 +1739,23 @@ static int pp_collect_finished(struct parallel_processes *pp) return result; } -int run_processes_parallel(int n, - get_next_task_fn get_next_task, - start_failure_fn start_failure, - task_finished_fn task_finished, - void *pp_cb) +int run_processes_parallel(struct run_process_parallel_opts *opts) { int i, code; int output_timeout = 100; int spawn_cap = 4; struct parallel_processes pp; + const char *tr2_category = opts->tr2_category; + const char *tr2_label = opts->tr2_label; + const int do_trace2 = tr2_category && tr2_label; + const int n = opts->jobs; - pp_init(&pp, n, get_next_task, start_failure, task_finished, pp_cb); + if (do_trace2) + trace2_region_enter_printf(tr2_category, tr2_label, NULL, + "max:%d", ((n < 1) ? online_cpus() + : n)); + + pp_init(&pp, opts); while (1) { for (i = 0; i < spawn_cap && !pp.shutdown && @@ -1777,25 +1783,11 @@ int run_processes_parallel(int n, } pp_cleanup(&pp); - return 0; -} - -int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task, - start_failure_fn start_failure, - task_finished_fn task_finished, void *pp_cb, - const char *tr2_category, const char *tr2_label) -{ - int result; - trace2_region_enter_printf(tr2_category, tr2_label, NULL, "max:%d", - ((n < 1) ? online_cpus() : n)); + if (do_trace2) + trace2_region_leave(tr2_category, tr2_label, NULL); - result = run_processes_parallel(n, get_next_task, start_failure, - task_finished, pp_cb); - - trace2_region_leave(tr2_category, tr2_label, NULL); - - return result; + return 0; } int run_auto_maintenance(int quiet) diff --git a/run-command.h b/run-command.h index 5bd0c933e80..b0268ed3db1 100644 --- a/run-command.h +++ b/run-command.h @@ -458,6 +458,32 @@ typedef int (*task_finished_fn)(int result, void *pp_task_cb); /** + * Options to pass to run_processes_parallel(), { 0 }-initialized + * means no options. Fields: + * + * tr2_category & tr2_label: sets the trace2 category and label for + * logging. These must either be unset, or both of them must be set. + * + * jobs: see 'n' in run_processes_parallel() below. + * + * *_fn & data: see run_processes_parallel() below. + */ +struct run_process_parallel_opts +{ + const char *tr2_category; + const char *tr2_label; + + int jobs; + + get_next_task_fn get_next_task; + start_failure_fn start_failure; + task_finished_fn task_finished; + void *data; +}; + +/** + * Options are passed via the "struct run_process_parallel_opts" above. + * Runs up to n processes at the same time. Whenever a process can be * started, the callback get_next_task_fn is called to obtain the data * required to start another child process. @@ -469,14 +495,7 @@ typedef int (*task_finished_fn)(int result, * start_failure_fn and task_finished_fn can be NULL to omit any * special handling. */ -int run_processes_parallel(int n, - get_next_task_fn, - start_failure_fn, - task_finished_fn, - void *pp_cb); -int run_processes_parallel_tr2(int n, get_next_task_fn, start_failure_fn, - task_finished_fn, void *pp_cb, - const char *tr2_category, const char *tr2_label); +int run_processes_parallel(struct run_process_parallel_opts *opts); /** * Convenience function which prepares env_array for a command to be run in a diff --git a/submodule.c b/submodule.c index 86c8f0f89db..8cbcd3fce23 100644 --- a/submodule.c +++ b/submodule.c @@ -1817,6 +1817,17 @@ int fetch_submodules(struct repository *r, { int i; struct submodule_parallel_fetch spf = SPF_INIT; + struct run_process_parallel_opts run_opts = { + .tr2_category = "submodule", + .tr2_label = "parallel/fetch", + + .jobs = max_parallel_jobs, + + .get_next_task = get_next_submodule, + .start_failure = fetch_start_failure, + .task_finished = fetch_finish, + .data = &spf, + }; spf.r = r; spf.command_line_option = command_line_option; @@ -1838,12 +1849,7 @@ int fetch_submodules(struct repository *r, calculate_changed_submodule_paths(r, &spf.changed_submodule_names); string_list_sort(&spf.changed_submodule_names); - run_processes_parallel_tr2(max_parallel_jobs, - get_next_submodule, - fetch_start_failure, - fetch_finish, - &spf, - "submodule", "parallel/fetch"); + run_processes_parallel(&run_opts); if (spf.submodules_with_errors.len > 0) fprintf(stderr, _("Errors during submodule fetch:\n%s"), diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index bd98dd9624b..56a806f228b 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -142,6 +142,12 @@ static int testsuite(int argc, const char **argv) "write JUnit-style XML files"), OPT_END() }; + struct run_process_parallel_opts run_opts = { + .get_next_task = next_test, + .start_failure = test_failed, + .task_finished = test_finished, + .data = &suite, + }; argc = parse_options(argc, argv, NULL, options, testsuite_usage, PARSE_OPT_STOP_AT_NON_OPTION); @@ -181,9 +187,9 @@ static int testsuite(int argc, const char **argv) fprintf(stderr, "Running %"PRIuMAX" tests (%d at a time)\n", (uintmax_t)suite.tests.nr, max_jobs); + run_opts.jobs = max_jobs; - ret = run_processes_parallel(max_jobs, next_test, test_failed, - test_finished, &suite); + ret = run_processes_parallel(&run_opts); if (suite.failed.nr > 0) { ret = 1; @@ -373,6 +379,7 @@ int cmd__run_command(int argc, const char **argv) int jobs; get_next_task_fn next_fn = NULL; task_finished_fn finished_fn = NULL; + struct run_process_parallel_opts opts = { 0 }; if (argc > 1 && !strcmp(argv[1], "testsuite")) exit(testsuite(argc - 1, argv + 1)); @@ -412,6 +419,8 @@ int cmd__run_command(int argc, const char **argv) jobs = atoi(argv[2]); strvec_clear(&proc.args); strvec_pushv(&proc.args, (const char **)argv + 3); + opts.jobs = jobs; + opts.data = &proc; if (!strcmp(argv[1], "run-command-parallel")) { next_fn = parallel_next; @@ -426,5 +435,7 @@ int cmd__run_command(int argc, const char **argv) return 1; } - exit(run_processes_parallel(jobs, next_fn, NULL, finished_fn, &proc)); + opts.get_next_task = next_fn; + opts.task_finished = finished_fn; + exit(run_processes_parallel(&opts)); } From patchwork Wed May 18 20:05:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854049 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6393FC433FE for ; Wed, 18 May 2022 20:05:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242267AbiERUFf (ORCPT ); Wed, 18 May 2022 16:05:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53662 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242215AbiERUFe (ORCPT ); Wed, 18 May 2022 16:05:34 -0400 Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7A99E2375C2 for ; Wed, 18 May 2022 13:05:33 -0700 (PDT) Received: by mail-wr1-x432.google.com with SMTP id j25so4136240wrc.9 for ; Wed, 18 May 2022 13:05:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=pH+9sYH/nHev4eul/h9aNVoyU7/+f8iUHy/fDXuV5bg=; b=KVm8CJ7Hbjq6T2XO8KmeS+FlBslCyvU/Ls+5lDnBsuNO6bhsGQvUaIN8KEnLfn6HYL eYG5Ry2PWlSmM9lMFu5dyu6Iy+UqfQzHUhJ1Ql20EJaUoGFHgqD8rfTXh+ZiCMalyAch Ggh+dhglemkyHGS9SXvD/obDt4HECEeg5Q7nnFKkokcIWVxIURIqRHaJ30DKb7LwyJPP sUhcVVgl5NtniuVOE+OQXvI7xPf2xl/QpkybX0odtVsSmKzaga3bwCoKz3mJ+WaNPrRL C+aHG6neGRW4NXcqFV7fCoFdO8wQCPiF1NStRVsG9mLE2Wvx9vz8jXVdgDDtSvBdJBHw z1zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=pH+9sYH/nHev4eul/h9aNVoyU7/+f8iUHy/fDXuV5bg=; b=zul/35hTXH9bz86pF0oxeIZVq5k4dxk3uDBU9gjL6HFItXkL61cd8HbAR1N+47vZ43 8+2J/E0eM6IP6IZDsd2fF8S3SlexkDc2r1RDDg+ZycgHi9MgYJpyaiY41qtUIq/QwU9K O3rzSwS/ErF9mdaI7ezfKh/nu/Pabt9wrrsxLvK2jeYloiNq6/y3ssyQOpF2oyw3SCQ5 1GwVDiVlkTfMtVxaBLua6FZ8CKT0I3OjpTU1gaHEndyoWDh2nP5FyAUuR6+FmbZMaJyx l1C3oYH5ebFew0kIfPXGA0TswXqEHOIvQmc9wo5YfnaqoiZEnfhKBZqvnlNdDnWekv5t m3jQ== X-Gm-Message-State: AOAM532yy/c0Mq6q+TL0w24aquQyllytshmju3diVYSZZo9NiqQfyBYl WiuWJ330Fx5+4xbSqwWxg7EBAL0G7oH6Zg== X-Google-Smtp-Source: ABdhPJyEoQheDasbQ+qEGm2rWGns5AfQR/CZ53v/pqjQPvpkJkbforWGD3BOuDCFaChyA7JWztY1Hw== X-Received: by 2002:a5d:4f8c:0:b0:20e:5f7c:db32 with SMTP id d12-20020a5d4f8c000000b0020e5f7cdb32mr1082472wru.596.1652904331644; Wed, 18 May 2022 13:05:31 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:31 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 3/8] run-command tests: test stdout of run_command_parallel() Date: Wed, 18 May 2022 22:05:19 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Extend the tests added in c553c72eed6 (run-command: add an asynchronous parallel child processor, 2015-12-15) to test stdout in addition to stderr. A subsequent commit will add additional related tests for a new feature, making it obvious how the output of the two compares on both stdout and stderr will make this easier to reason about. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/t0061-run-command.sh | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index ee281909bc3..7d00f3cc2af 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -130,18 +130,21 @@ World EOF test_expect_success 'run_command runs in parallel with more jobs available than tasks' ' - test-tool run-command run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && - test_cmp expect actual + test-tool run-command run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_cmp expect err ' test_expect_success 'run_command runs in parallel with as many jobs as tasks' ' - test-tool run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && - test_cmp expect actual + test-tool run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_cmp expect err ' test_expect_success 'run_command runs in parallel with more tasks than jobs available' ' - test-tool run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && - test_cmp expect actual + test-tool run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_cmp expect err ' cat >expect <<-EOF @@ -154,8 +157,9 @@ asking for a quick stop EOF test_expect_success 'run_command is asked to abort gracefully' ' - test-tool run-command run-command-abort 3 false 2>actual && - test_cmp expect actual + test-tool run-command run-command-abort 3 false >out 2>err && + test_must_be_empty out && + test_cmp expect err ' cat >expect <<-EOF @@ -163,8 +167,9 @@ no further jobs available EOF test_expect_success 'run_command outputs ' ' - test-tool run-command run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && - test_cmp expect actual + test-tool run-command run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_cmp expect err ' test_trace () { From patchwork Wed May 18 20:05:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854051 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4563BC433FE for ; Wed, 18 May 2022 20:06:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242307AbiERUFv (ORCPT ); Wed, 18 May 2022 16:05:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53682 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242264AbiERUFf (ORCPT ); Wed, 18 May 2022 16:05:35 -0400 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 273FB2375C4 for ; Wed, 18 May 2022 13:05:34 -0700 (PDT) Received: by mail-wr1-x42f.google.com with SMTP id k30so4152825wrd.5 for ; Wed, 18 May 2022 13:05:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=a/u9MXgIjBzGCtIK8vpp0tte2/yRO94eKu3/TNl5XG8=; b=RqbA5xkqtZspcEcoMQUjePa+vA2n0LL72zT7seuMRvJnLai6hmySOMi8MfUnktUZgW UH4g7M8sZLm+iB6d/SBuVYPjJNN0X40li67YkmBJQtLeyA2a3RdLaTVmKRjos6VNG8Xf pqGj4zYXU/NPKN8KE7d243/JYj7qoDMGHI/kv+tHKpm1MdT7712r/mx1b2ZkZ08nf/o2 lIbv7S9Jb5cf56y7Yz/Af/vEUBxI46WdUUn8oxgWN2ozLRalR2ofaJ2eH40U0MCaS/7E F4LgqYgh/v9JpXtgZ41uGSDYMCFO4WeddeXgQhQ9hXOB2mByJGKmJIKm9tblYQBplx9g Dyhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=a/u9MXgIjBzGCtIK8vpp0tte2/yRO94eKu3/TNl5XG8=; b=pL6c+Mdd8Ie6wd2AxOYWQAATC7qtJrja5FN6JDvWDIfFRMJ1gFN+/lFVy1uMM7Ap6T yPRgoVv6tFLefNL8J75goQm8pedAAG1g+uwmA0/sdWev1qAJhYofyi/meHyvDLIrGfBd d3GIR8Pv105nE+XKqqE0xKVFS3muRzFCroFP6R0rcpnCzK/HQvPacSXnqd16reH5ePei 0o3QRY/Vra3m9dyvt9m7ERcHt2SwcPXOEkQSgL//1M4L87h25VNs/CfUAmw2Yo3aYkbz tw6HYBSkbPLVLrbB3r59/W2MDN76PgyyNbX/6MBIdCjAM9T4NZjMxev+yz9W6U8mFc2+ VR8A== X-Gm-Message-State: AOAM532cwXLC7WGktXvTGqqfSecHCXpcwaNozT9EvSZKCgXMHP+rEmVH 33Df2Tnz93+o5KRHSf8IIpdIXbtpGoUpKA== X-Google-Smtp-Source: ABdhPJz3c9e4KnPQbDG53JwKG/RYi1KC5O6ia6LuJSOaGRNr33mTvZFEaOEHNtvON5HmozcIsOB0Sg== X-Received: by 2002:adf:f085:0:b0:20d:e4d:1fe with SMTP id n5-20020adff085000000b0020d0e4d01femr1114181wro.42.1652904332466; Wed, 18 May 2022 13:05:32 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:31 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 4/8] run-command.c: add an initializer for "struct parallel_processes" Date: Wed, 18 May 2022 22:05:20 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a PARALLEL_PROCESSES_INIT macro for "struct parallel_processes", this allows us to do away with a call to strbuf_init(), in subsequent commits we'll be able to rely on other fields being NULL'd. Signed-off-by: Ævar Arnfjörð Bjarmason --- run-command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/run-command.c b/run-command.c index 8c156fd080e..839c85d12e5 100644 --- a/run-command.c +++ b/run-command.c @@ -1498,6 +1498,9 @@ struct parallel_processes { int output_owner; struct strbuf buffered_output; /* of finished children */ }; +#define PARALLEL_PROCESSES_INIT { \ + .buffered_output = STRBUF_INIT, \ +} static int default_start_failure(struct strbuf *out, void *pp_cb, @@ -1562,7 +1565,6 @@ static void pp_init(struct parallel_processes *pp, pp->shutdown = 0; CALLOC_ARRAY(pp->children, n); CALLOC_ARRAY(pp->pfd, n); - strbuf_init(&pp->buffered_output, 0); for (i = 0; i < n; i++) { strbuf_init(&pp->children[i].err, 0); @@ -1744,7 +1746,7 @@ int run_processes_parallel(struct run_process_parallel_opts *opts) int i, code; int output_timeout = 100; int spawn_cap = 4; - struct parallel_processes pp; + struct parallel_processes pp = PARALLEL_PROCESSES_INIT; const char *tr2_category = opts->tr2_category; const char *tr2_label = opts->tr2_label; const int do_trace2 = tr2_category && tr2_label; From patchwork Wed May 18 20:05:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854055 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C14CC4332F for ; Wed, 18 May 2022 20:06:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242330AbiERUF6 (ORCPT ); Wed, 18 May 2022 16:05:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53780 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242284AbiERUFh (ORCPT ); Wed, 18 May 2022 16:05:37 -0400 Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 54C382375DE for ; Wed, 18 May 2022 13:05:35 -0700 (PDT) Received: by mail-wr1-x443.google.com with SMTP id u27so3321498wru.8 for ; Wed, 18 May 2022 13:05:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=npILAX0xc/pUxJHt75lydxvmgqWooBCDAuTTf463inM=; b=CYSnaTVFgJrXG7UNcfEo962BGBUUpYLsWrAi4egqHJMHj+huuZth7j38f5VsdmqL5x 18TXgRl7D+Tb5Bfv+qWXshqPGudrtorgY80Bgag/IvL6E+GFvCgfGnmnkYBgsP31PuFU evO3+zRxUQ5IwN5LKunSoa9Cbmn6d27f746hq96xlVobiUl71fGhjFGmObkkTFaSY/yu rfer9ItgmYXtKU6FjrRZ0tnnU4p1/z85pKEepLcuP2jnDgGwNDS2l6WyVPUOcOK83iek cZ2foBK+I8A5jxCZpGuBovD2EYZ4xgpmVTlLbe3RncKZUmt6emDuVTY/RLt6BmueygI7 OVpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=npILAX0xc/pUxJHt75lydxvmgqWooBCDAuTTf463inM=; b=JWAvsugqydo9K124CznpP/H+M0zGUGjsfQJF389wOKurT6bN+hVE+kLDG274L7BDT7 vTjC//DinD2YIbFJYcwJVb4smusCjgSBPEfitxSeCVHVHE5v8YCTyfGNFaOG7VA3xcf0 8Yuce176ptm+E30yGqRa1rEtI6ruufMN7J2uKN5/MRKZjsRIRp1TA4jTH+s8WEmgM7PK Oc9N7AC+IcmDCbtyMktEyjVm7ryVCQnZoRqTBwQOtkxvodeVhTpitypn1Ap6QZdvEc1o bnqq3L4L5eXCRqQSCubFHwL7jPPGnD53jss7MwUZJHdaklzEc0p1AQWldTxuAQSz1SU7 Pv2w== X-Gm-Message-State: AOAM5317evtSgCUdN3pEt7nVll9ZVzxStHwHe6TvbmszcGPZBf6hibkn DLyeEurMzTD5UkVY0Nc3E0ySGsVozCcufg== X-Google-Smtp-Source: ABdhPJwidNWcUJKTaiSbCowSjfhtNBbVQXDmF0lcWQhWtpvbW6nyx76bli/qSMlSg5WBAb52czUT9Q== X-Received: by 2002:a05:6000:144f:b0:20c:6090:3040 with SMTP id v15-20020a056000144f00b0020c60903040mr1050033wrx.479.1652904333474; Wed, 18 May 2022 13:05:33 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:32 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 5/8] run-command: add an "ungroup" option to run_process_parallel() Date: Wed, 18 May 2022 22:05:21 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Extend the parallel execution API added in c553c72eed6 (run-command: add an asynchronous parallel child processor, 2015-12-15) to support a mode where the stdout and stderr of the processes isn't captured and output in a deterministic order, instead we'll leave it to the kernel and stdio to sort it out. This gives the API same functionality as GNU parallel's --ungroup option. As we'll see in a subsequent commit the main reason to want this is to support stdout and stderr being connected to the TTY in the case of jobs=1, demonstrated here with GNU parallel: $ parallel --ungroup 'test -t {} && echo TTY || echo NTTY' ::: 1 2 TTY TTY $ parallel 'test -t {} && echo TTY || echo NTTY' ::: 1 2 NTTY NTTY Another is as GNU parallel's documentation notes a potential for optimization. Our results will be a bit different, but in cases where you want to run processes in parallel where the exact order isn't important this can be a lot faster: $ hyperfine -r 3 -L o ,--ungroup 'parallel {o} seq ::: 10000000 >/dev/null ' Benchmark 1: parallel seq ::: 10000000 >/dev/null Time (mean ± σ): 220.2 ms ± 9.3 ms [User: 124.9 ms, System: 96.1 ms] Range (min … max): 212.3 ms … 230.5 ms 3 runs Benchmark 2: parallel --ungroup seq ::: 10000000 >/dev/null Time (mean ± σ): 154.7 ms ± 0.9 ms [User: 136.2 ms, System: 25.1 ms] Range (min … max): 153.9 ms … 155.7 ms 3 runs Summary 'parallel --ungroup seq ::: 10000000 >/dev/null ' ran 1.42 ± 0.06 times faster than 'parallel seq ::: 10000000 >/dev/null ' A large part of the juggling in the API is to make the API safer for its maintenance and consumers alike. For the maintenance of the API we e.g. avoid malloc()-ing the "pp->pfd", ensuring that SANITIZE=address and other similar tools will catch any unexpected misuse. For API consumers we take pains to never pass the non-NULL "out" buffer to an API user that provided the "ungroup" option. The resulting code in t/helper/test-run-command.c isn't typical of such a user, i.e. they'd typically use one mode or the other, and would know whether they'd provided "ungroup" or not. Signed-off-by: Ævar Arnfjörð Bjarmason --- run-command.c | 76 ++++++++++++++++++++++++++++--------- run-command.h | 31 +++++++++++---- t/helper/test-run-command.c | 26 ++++++++++--- t/t0061-run-command.sh | 30 +++++++++++++++ 4 files changed, 132 insertions(+), 31 deletions(-) diff --git a/run-command.c b/run-command.c index 839c85d12e5..39e09ee39fc 100644 --- a/run-command.c +++ b/run-command.c @@ -1468,7 +1468,7 @@ int pipe_command(struct child_process *cmd, enum child_state { GIT_CP_FREE, GIT_CP_WORKING, - GIT_CP_WAIT_CLEANUP, + GIT_CP_WAIT_CLEANUP, /* only for !ungroup */ }; struct parallel_processes { @@ -1494,6 +1494,7 @@ struct parallel_processes { struct pollfd *pfd; unsigned shutdown : 1; + unsigned ungroup : 1; int output_owner; struct strbuf buffered_output; /* of finished children */ @@ -1563,12 +1564,16 @@ static void pp_init(struct parallel_processes *pp, pp->nr_processes = 0; pp->output_owner = 0; pp->shutdown = 0; + pp->ungroup = opts->ungroup; CALLOC_ARRAY(pp->children, n); - CALLOC_ARRAY(pp->pfd, n); + if (!pp->ungroup) + CALLOC_ARRAY(pp->pfd, n); for (i = 0; i < n; i++) { strbuf_init(&pp->children[i].err, 0); child_process_init(&pp->children[i].process); + if (!pp->pfd) + continue; pp->pfd[i].events = POLLIN | POLLHUP; pp->pfd[i].fd = -1; } @@ -1594,7 +1599,8 @@ static void pp_cleanup(struct parallel_processes *pp) * When get_next_task added messages to the buffer in its last * iteration, the buffered output is non empty. */ - strbuf_write(&pp->buffered_output, stderr); + if (!pp->ungroup) + strbuf_write(&pp->buffered_output, stderr); strbuf_release(&pp->buffered_output); sigchain_pop_common(); @@ -1609,6 +1615,7 @@ static void pp_cleanup(struct parallel_processes *pp) */ static int pp_start_one(struct parallel_processes *pp) { + const int ungroup = pp->ungroup; int i, code; for (i = 0; i < pp->max_processes; i++) @@ -1618,24 +1625,30 @@ static int pp_start_one(struct parallel_processes *pp) BUG("bookkeeping is hard"); code = pp->get_next_task(&pp->children[i].process, - &pp->children[i].err, + ungroup ? NULL : &pp->children[i].err, pp->data, &pp->children[i].data); if (!code) { - strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); - strbuf_reset(&pp->children[i].err); + if (!ungroup) { + strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); + strbuf_reset(&pp->children[i].err); + } return 1; } - pp->children[i].process.err = -1; - pp->children[i].process.stdout_to_stderr = 1; + if (!ungroup) { + pp->children[i].process.err = -1; + pp->children[i].process.stdout_to_stderr = 1; + } pp->children[i].process.no_stdin = 1; if (start_command(&pp->children[i].process)) { - code = pp->start_failure(&pp->children[i].err, + code = pp->start_failure(ungroup ? NULL : &pp->children[i].err, pp->data, pp->children[i].data); - strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); - strbuf_reset(&pp->children[i].err); + if (!ungroup) { + strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); + strbuf_reset(&pp->children[i].err); + } if (code) pp->shutdown = 1; return code; @@ -1643,14 +1656,26 @@ static int pp_start_one(struct parallel_processes *pp) pp->nr_processes++; pp->children[i].state = GIT_CP_WORKING; - pp->pfd[i].fd = pp->children[i].process.err; + if (pp->pfd) + pp->pfd[i].fd = pp->children[i].process.err; return 0; } +static void pp_mark_working_for_cleanup(struct parallel_processes *pp) +{ + int i; + + for (i = 0; i < pp->max_processes; i++) + if (pp->children[i].state == GIT_CP_WORKING) + pp->children[i].state = GIT_CP_WAIT_CLEANUP; +} + static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout) { int i; + assert(!pp->ungroup); + while ((i = poll(pp->pfd, pp->max_processes, output_timeout)) < 0) { if (errno == EINTR) continue; @@ -1677,6 +1702,9 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout) static void pp_output(struct parallel_processes *pp) { int i = pp->output_owner; + + assert(!pp->ungroup); + if (pp->children[i].state == GIT_CP_WORKING && pp->children[i].err.len) { strbuf_write(&pp->children[i].err, stderr); @@ -1686,10 +1714,15 @@ static void pp_output(struct parallel_processes *pp) static int pp_collect_finished(struct parallel_processes *pp) { + const int ungroup = pp->ungroup; int i, code; int n = pp->max_processes; int result = 0; + if (ungroup) + for (i = 0; i < pp->max_processes; i++) + pp->children[i].state = GIT_CP_WAIT_CLEANUP; + while (pp->nr_processes > 0) { for (i = 0; i < pp->max_processes; i++) if (pp->children[i].state == GIT_CP_WAIT_CLEANUP) @@ -1700,8 +1733,8 @@ static int pp_collect_finished(struct parallel_processes *pp) code = finish_command(&pp->children[i].process); code = pp->task_finished(code, - &pp->children[i].err, pp->data, - pp->children[i].data); + ungroup ? NULL : &pp->children[i].err, + pp->data, pp->children[i].data); if (code) result = code; @@ -1710,10 +1743,13 @@ static int pp_collect_finished(struct parallel_processes *pp) pp->nr_processes--; pp->children[i].state = GIT_CP_FREE; - pp->pfd[i].fd = -1; + if (pp->pfd) + pp->pfd[i].fd = -1; child_process_init(&pp->children[i].process); - if (i != pp->output_owner) { + if (ungroup) { + ; /* no strbuf_*() work to do here */ + } else if (i != pp->output_owner) { strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); strbuf_reset(&pp->children[i].err); } else { @@ -1774,8 +1810,12 @@ int run_processes_parallel(struct run_process_parallel_opts *opts) } if (!pp.nr_processes) break; - pp_buffer_stderr(&pp, output_timeout); - pp_output(&pp); + if (opts->ungroup) { + pp_mark_working_for_cleanup(&pp); + } else { + pp_buffer_stderr(&pp, output_timeout); + pp_output(&pp); + } code = pp_collect_finished(&pp); if (code) { pp.shutdown = 1; diff --git a/run-command.h b/run-command.h index b0268ed3db1..dcb6ded4b55 100644 --- a/run-command.h +++ b/run-command.h @@ -405,6 +405,10 @@ void check_pipe(int err); * pp_cb is the callback cookie as passed to run_processes_parallel. * You can store a child process specific callback cookie in pp_task_cb. * + * The "struct strbuf *err" parameter is either a pointer to a string + * to write errors to, or NULL if the "ungroup" option was + * provided. See run_processes_parallel() below. + * * Even after returning 0 to indicate that there are no more processes, * this function will be called again until there are no more running * child processes. @@ -423,9 +427,9 @@ typedef int (*get_next_task_fn)(struct child_process *cp, * This callback is called whenever there are problems starting * a new process. * - * You must not write to stdout or stderr in this function. Add your - * message to the strbuf out instead, which will be printed without - * messing up the output of the other parallel processes. + * The "struct strbuf *err" parameter is either a pointer to a string + * to write errors to, or NULL if the "ungroup" option was + * provided. See run_processes_parallel() below. * * pp_cb is the callback cookie as passed into run_processes_parallel, * pp_task_cb is the callback cookie as passed into get_next_task_fn. @@ -441,9 +445,9 @@ typedef int (*start_failure_fn)(struct strbuf *out, /** * This callback is called on every child process that finished processing. * - * You must not write to stdout or stderr in this function. Add your - * message to the strbuf out instead, which will be printed without - * messing up the output of the other parallel processes. + * The "struct strbuf *err" parameter is either a pointer to a string + * to write errors to, or NULL if the "ungroup" option was + * provided. See run_processes_parallel() below. * * pp_cb is the callback cookie as passed into run_processes_parallel, * pp_task_cb is the callback cookie as passed into get_next_task_fn. @@ -466,6 +470,10 @@ typedef int (*task_finished_fn)(int result, * * jobs: see 'n' in run_processes_parallel() below. * + * ungroup: Ungroup output. Output is printed as soon as possible and + * bypasses run-command's internal processing. This may cause output + * from different commands to be mixed. + * * *_fn & data: see run_processes_parallel() below. */ struct run_process_parallel_opts @@ -474,6 +482,7 @@ struct run_process_parallel_opts const char *tr2_label; int jobs; + unsigned int ungroup:1; get_next_task_fn get_next_task; start_failure_fn start_failure; @@ -490,10 +499,18 @@ struct run_process_parallel_opts * * The children started via this function run in parallel. Their output * (both stdout and stderr) is routed to stderr in a manner that output - * from different tasks does not interleave. + * from different tasks does not interleave (but see "ungroup" above). * * start_failure_fn and task_finished_fn can be NULL to omit any * special handling. + * + * If the "ungroup" option isn't specified the callbacks will get a + * pointer to a "struct strbuf *out", and must not write to stdout or + * stderr as such output will mess up the output of the other parallel + * processes. If "ungroup" option is specified callbacks will get a + * NULL "struct strbuf *out" parameter, and are responsible for + * emitting their own output, including dealing with any race + * conditions due to writing in parallel to stdout and stderr. */ int run_processes_parallel(struct run_process_parallel_opts *opts); diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 56a806f228b..986acbce5f2 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -31,7 +31,11 @@ static int parallel_next(struct child_process *cp, return 0; strvec_pushv(&cp->args, d->args.v); - strbuf_addstr(err, "preloaded output of a child\n"); + if (err) + strbuf_addstr(err, "preloaded output of a child\n"); + else + fprintf(stderr, "preloaded output of a child\n"); + number_callbacks++; return 1; } @@ -41,7 +45,10 @@ static int no_job(struct child_process *cp, void *cb, void **task_cb) { - strbuf_addstr(err, "no further jobs available\n"); + if (err) + strbuf_addstr(err, "no further jobs available\n"); + else + fprintf(stderr, "no further jobs available\n"); return 0; } @@ -50,7 +57,10 @@ static int task_finished(int result, void *pp_cb, void *pp_task_cb) { - strbuf_addstr(err, "asking for a quick stop\n"); + if (err) + strbuf_addstr(err, "asking for a quick stop\n"); + else + fprintf(stderr, "asking for a quick stop\n"); return 1; } @@ -422,12 +432,15 @@ int cmd__run_command(int argc, const char **argv) opts.jobs = jobs; opts.data = &proc; - if (!strcmp(argv[1], "run-command-parallel")) { + if (!strcmp(argv[1], "run-command-parallel") || + !strcmp(argv[1], "run-command-parallel-ungroup")) { next_fn = parallel_next; - } else if (!strcmp(argv[1], "run-command-abort")) { + } else if (!strcmp(argv[1], "run-command-abort") || + !strcmp(argv[1], "run-command-abort-ungroup")) { next_fn = parallel_next; finished_fn = task_finished; - } else if (!strcmp(argv[1], "run-command-no-jobs")) { + } else if (!strcmp(argv[1], "run-command-no-jobs") || + !strcmp(argv[1], "run-command-no-jobs-ungroup")) { next_fn = no_job; finished_fn = task_finished; } else { @@ -435,6 +448,7 @@ int cmd__run_command(int argc, const char **argv) return 1; } + opts.ungroup = ends_with(argv[1], "-ungroup"); opts.get_next_task = next_fn; opts.task_finished = finished_fn; exit(run_processes_parallel(&opts)); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 7d00f3cc2af..3628719a06d 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -135,18 +135,36 @@ test_expect_success 'run_command runs in parallel with more jobs available than test_cmp expect err ' +test_expect_success 'run_command runs ungrouped in parallel with more jobs available than tasks' ' + test-tool run-command run-command-parallel-ungroup 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_line_count = 8 out && + test_line_count = 4 err +' + test_expect_success 'run_command runs in parallel with as many jobs as tasks' ' test-tool run-command run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && test_must_be_empty out && test_cmp expect err ' +test_expect_success 'run_command runs ungrouped in parallel with as many jobs as tasks' ' + test-tool run-command run-command-parallel-ungroup 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_line_count = 8 out && + test_line_count = 4 err +' + test_expect_success 'run_command runs in parallel with more tasks than jobs available' ' test-tool run-command run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && test_must_be_empty out && test_cmp expect err ' +test_expect_success 'run_command runs ungrouped in parallel with more tasks than jobs available' ' + test-tool run-command run-command-parallel-ungroup 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_line_count = 8 out && + test_line_count = 4 err +' + cat >expect <<-EOF preloaded output of a child asking for a quick stop @@ -162,6 +180,12 @@ test_expect_success 'run_command is asked to abort gracefully' ' test_cmp expect err ' +test_expect_success 'run_command is asked to abort gracefully (ungroup)' ' + test-tool run-command run-command-abort-ungroup 3 false >out 2>err && + test_must_be_empty out && + test_line_count = 6 err +' + cat >expect <<-EOF no further jobs available EOF @@ -172,6 +196,12 @@ test_expect_success 'run_command outputs ' ' test_cmp expect err ' +test_expect_success 'run_command outputs (ungroup) ' ' + test-tool run-command run-command-no-jobs-ungroup 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_cmp expect err +' + test_trace () { expect="$1" shift From patchwork Wed May 18 20:05:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854053 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 664A8C433EF for ; Wed, 18 May 2022 20:06:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242319AbiERUFy (ORCPT ); Wed, 18 May 2022 16:05:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53750 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242276AbiERUFg (ORCPT ); Wed, 18 May 2022 16:05:36 -0400 Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 523682375DD for ; Wed, 18 May 2022 13:05:35 -0700 (PDT) Received: by mail-wr1-x432.google.com with SMTP id j25so4136240wrc.9 for ; Wed, 18 May 2022 13:05:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=SwXw+sCZFdt6p3ZdqeUNdkO/ZtGUIZ5Q446Ul4WIZ88=; b=h/9kwfeMm81MVki6ircq2R3S2ZidR/L6KYsdEosV4zWUyctscbu7PcNAfuphp25TYG xeO2ayeuuzElLuWLC2o9p19VDC1l3rg6ODclOo3cbgLi2TIYvLJpV0b70BcmpvuMna3S AtlVJQTRz+aOrCG5vOQXfmkxYuObxPl9hbRMNzIiqHMHHf4j+/udbdhCl+kSsaGSzbCJ Gt6UdUFBOsxUz/WfD4VqCtHKuElZLr7fwzfobR++THmTCRJ64OrgF4Q9VhXInlHRdXu7 PMY/DE6M1fsbazlfuLnkYku+sALKYLdngKeov0Jae/xPT6baVbf8ip3GrxRO8fpmmtQ7 Efug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=SwXw+sCZFdt6p3ZdqeUNdkO/ZtGUIZ5Q446Ul4WIZ88=; b=5WonD0wZ108CE5voUKuYRDET4C3BLnTkmLzSb9tD06LgzZ+kkw+oB6UfDio7kMrENu mmDeyICA+7lDPPptidqvKfwyWsUrqcVf3Na3g3huVdwh6fGpMYqFKD64IXEhzEQ0XrLV 76u6pbt19A5hUocomt92sULE62lmUxJG8iG9VmqijU0Z0IVRzEfUYl68Z5NOQZzn/mHZ Uewq0ZO0PHFNxS2nxFo/VA5J1ujgNJ5IkNU8x0s43/8oHKB3g99neNGjsD1XIJsc4nR/ +GHN4eMl3/QI4lg876k0nDZpwzqSOhqwVbOSD7gm7zwPK3xIN+zd02i2D/dq09JZ71Zv 3uyw== X-Gm-Message-State: AOAM531fV0oR1Fsc62AGLuQStcrFIBf27R5oSsEkOiml7VhAHQ2axSo2 CCVWK0HytA5JnP9MjtGbIfYRXr5WkCxk1g== X-Google-Smtp-Source: ABdhPJza45FbJsS+w3SamKhrQcB01urmT58GeOShjgGfmE98MeXLUZv/s3CvHvT+UjXCzS69sfuZwg== X-Received: by 2002:a5d:4ed1:0:b0:20a:e375:35f0 with SMTP id s17-20020a5d4ed1000000b0020ae37535f0mr1092172wrv.94.1652904334551; Wed, 18 May 2022 13:05:34 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:33 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 6/8] hook tests: fix redirection logic error in 96e7225b310 Date: Wed, 18 May 2022 22:05:22 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The tests added in 96e7225b310 (hook: add 'run' subcommand, 2021-12-22) were redirecting to "actual" both in the body of the hook itself and in the testing code below. The net result was that the "2>>actual" redirection later in the test wasn't doing anything. Let's have those redirection do what it looks like they're doing. Signed-off-by: Ævar Arnfjörð Bjarmason --- t/t1800-hook.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 26ed5e11bc8..1e4adc3d53e 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -94,7 +94,7 @@ test_expect_success 'git hook run -- out-of-repo runs excluded' ' test_expect_success 'git -c core.hooksPath= hook run' ' mkdir my-hooks && write_script my-hooks/test-hook <<-\EOF && - echo Hook ran $1 >>actual + echo Hook ran $1 EOF cat >expect <<-\EOF && From patchwork Wed May 18 20:05:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854052 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A328AC433F5 for ; Wed, 18 May 2022 20:06:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242337AbiERUGA (ORCPT ); Wed, 18 May 2022 16:06:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53826 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242297AbiERUFi (ORCPT ); Wed, 18 May 2022 16:05:38 -0400 Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 55C712375C2 for ; Wed, 18 May 2022 13:05:37 -0700 (PDT) Received: by mail-wm1-x334.google.com with SMTP id r188-20020a1c44c5000000b003946c466c17so3783017wma.4 for ; Wed, 18 May 2022 13:05:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=VwDPmVX+XkHArFj69SFSq4Kj+fFOLyuvER1lDFiQKco=; b=Wunrv0iDR/MrwYHZ9cYWT7VwPgsBwA7sEF75Xz4tClTp/bSJyL9xBXe01Aw03kYv/4 yhARVk+dfrsxLKTbfw4gAnAPL1htTmFdvmAs7xG/ZCduImJiPxm/uUEpgUnb9u14XzL9 C75DHhkKtkscLApS0f5zAmwar592Wq7IZnZhG5QxnP3PA8UYfMM26vy/p50i7SeW/rwu Oz+slzVkUQsmuLC6/oWVnegPKlQsV0spxF2aQmxhjSCJy/LgzjEX4ZJX7LrsPbWf+u5l 7VN9CMHTFBnJlNQm8qvi7WQGjTvNB1HqJZw6nEUvVmbiuKBmV00SzOIn8C0llfYVv1u0 gCbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=VwDPmVX+XkHArFj69SFSq4Kj+fFOLyuvER1lDFiQKco=; b=x2CndpdPVVOxkSJNjTpeYtQoxVx1NpWGK0kGtFwQXcWNjBgQUEmEYKEjUmodSPKrVT Zb3JU5yHQjgORfOxKvEyYQwMVuesZNND0qEi8ENGrFcI325RQRB3C9xdJ3D7oFhguKPH rW7dvfrOWXG9b08el2dLAoZ1cvulZaFVMbr2kMJNByVNQ2ei5bCHDmYoGnkfpJ5R1VIG IoEONzOiVRKrdW+1Jq2lA7LkRlfNGcTpHO7tP7bpR9l+F0+x8D8HAMNO/CPm+5iUdsJd 9eR4RClC0LuaPolUfJTYCTgMzp+Hr/GZluwKt99kjUZP2Z7PS+7OH54FknLTkWHviD06 AZVQ== X-Gm-Message-State: AOAM531UtCZfdSeYenxAwkHD68v4Hp5hNXEU2UTQmYyVxtAv2d/armyb f+J7vAze3Tquc4fm0LCAg47UXdBqs9zVew== X-Google-Smtp-Source: ABdhPJw/WapFfLjcRmEtewv2hoPnPZwMHJall8l2VAyHt0L9qp2qyIJcdLy9g0qe24ISP4NTcxG33w== X-Received: by 2002:a05:600c:1986:b0:394:867f:984c with SMTP id t6-20020a05600c198600b00394867f984cmr1410331wmq.20.1652904335507; Wed, 18 May 2022 13:05:35 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:34 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 7/8] hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr" Date: Wed, 18 May 2022 22:05:23 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Amend code added in 96e7225b310 (hook: add 'run' subcommand, 2021-12-22) to stop setting these two flags. We use the run_process_parallel() API added in c553c72eed6 (run-command: add an asynchronous parallel child processor, 2015-12-15), which always sets these in pp_start_one() (in addition to setting .err = -1). Note that an assert() to check that these values are already what we're setting them to here would fail. That's because in pp_start_one() we'll set these after calling this "get_next_task" callback (which we call pick_next_hook()). But the only case where we weren't setting these just after returning from this function was if we took the "return 0" path here, in which case we wouldn't have set these. So while this code wasn't wrong, it was entirely redundant. The run_process_parallel() also can't work with a generic "struct child_process", it needs one that's behaving in a way that it expects when it comes to stderr/stdout. So we shouldn't be changing these values, or in this case keeping around code that gives the impression that doing in the general case is OK. Signed-off-by: Ævar Arnfjörð Bjarmason --- hook.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hook.c b/hook.c index 9aefccfc34a..dc498ef5c39 100644 --- a/hook.c +++ b/hook.c @@ -53,9 +53,7 @@ static int pick_next_hook(struct child_process *cp, if (!hook_path) return 0; - cp->no_stdin = 1; strvec_pushv(&cp->env_array, hook_cb->options->env.v); - cp->stdout_to_stderr = 1; cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; From patchwork Wed May 18 20:05:24 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12854054 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B2266C43219 for ; Wed, 18 May 2022 20:06:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242346AbiERUGC (ORCPT ); Wed, 18 May 2022 16:06:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54664 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242306AbiERUFu (ORCPT ); Wed, 18 May 2022 16:05:50 -0400 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6A1592375E9 for ; Wed, 18 May 2022 13:05:38 -0700 (PDT) Received: by mail-wm1-x332.google.com with SMTP id a14-20020a7bc1ce000000b00393fb52a386so3797842wmj.1 for ; Wed, 18 May 2022 13:05:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=qstmnqVssfnKoI8gpTKL61nuXl96/A+PoHUHtWKmtqI=; b=DiCMfqoyE9+2CNfyLEIRlfF955ddyN5WBHWy2uV1EqLqXmxqb07v8KSu3BHcfJM3lc WIsuQWp5WBEaRiOfBUZcxExyD8MOm+/tF7zm9OtVU2Z+i3hCHny+2oC3SkJfKGg9U9sO 8n6gS+Nyzx6RtlHZ30/qNyZVLRVXubzaL5etQeOc2vuV+PfANIetEHYwCOePp9ef7LjI GdnHJnnc4nM6uU+V+NnBBDg70WOM2YuIH3qvSAnS72M0IJAL4GL/4J++GGH4C0fLSPT2 k2rmL6rrxuc1WMUUYBZbvBEdNU7D3fLwLBc1XlUG1FmAMWyVoc/1IfL2NTIEKwLt4z+1 4ZhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=qstmnqVssfnKoI8gpTKL61nuXl96/A+PoHUHtWKmtqI=; b=HdQ/XkGwq4DmjWKp/wb3FXyEDb0wQjsHfFbnKH9SbbGpMGEvjz3wJLeG1MQnydfL8w ycKMsKGhcu0r4otAyrBbiHSFyW3Z+Dt2H/UDXCFUcJiit8dKKBsh36mEj6bXKM03zQZ2 u1H+sAwmgvQs+KobudS+oq/0thO+u6ZK5n+s8TkLiHaORgy0cAiOmrU+iVwnkHsXX58r U1dSNqeXMaxCMyoEE6hO6WxMVxA+cnkCc9caeuAI/3Qu/P4eeawdV2TyUgG8G/+stbMo y8fv9KQpdNwa4Km9yz3bRQ0zIUNqjJT9EANtO2bJ4Gv+kiiHFJR2OoT/vVQJsQ37ShT1 qYZQ== X-Gm-Message-State: AOAM532saCzLJ0SPRl8ALI9KEYKuU0BFKvNw+d84oawvfrUgfjloZvn4 4xFjiRdfHuBWyQVnhspAGyAie0QeicUSig== X-Google-Smtp-Source: ABdhPJzIkRVRaJmq/YPZNUR1QkEghc1AY2M02eA1PdEK5Zia0JiztJxJLmOEDYNqQM/3R2i6WycoSA== X-Received: by 2002:a05:600c:288:b0:394:31f9:68f with SMTP id 8-20020a05600c028800b0039431f9068fmr1349631wmk.57.1652904336587; Wed, 18 May 2022 13:05:36 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id f18-20020adfb612000000b0020d00174eabsm2674441wre.94.2022.05.18.13.05.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 13:05:35 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Anthony Sottile , Emily Shaffer , Phillip Wood , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [PATCH v2 8/8] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Date: Wed, 18 May 2022 22:05:24 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.952.g0ae626f6cd7 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Fix a regression reported[1] in f443246b9f2 (commit: convert {pre-commit,prepare-commit-msg} hook to hook.h, 2021-12-22): Due to using the run_process_parallel() API in the earlier 96e7225b310 (hook: add 'run' subcommand, 2021-12-22) we'd capture the hook's stderr and stdout, and thus lose the connection to the TTY in the case of e.g. the "pre-commit" hook. As a preceding commit notes GNU parallel's similar --ungroup option also has it emit output faster. While we're unlikely to have hooks that emit truly massive amounts of output (or where the performance thereof matters) it's still informative to measure the overhead. In a similar "seq" test we're now ~30% faster: $ cat .git/hooks/seq-hook; git hyperfine -L rev origin/master,HEAD~0 -s 'make CFLAGS=-O3' './git hook run seq-hook' #!/bin/sh seq 100000000 Benchmark 1: ./git hook run seq-hook' in 'origin/master Time (mean ± σ): 787.1 ms ± 13.6 ms [User: 701.6 ms, System: 534.4 ms] Range (min … max): 773.2 ms … 806.3 ms 10 runs Benchmark 2: ./git hook run seq-hook' in 'HEAD~0 Time (mean ± σ): 603.4 ms ± 1.6 ms [User: 573.1 ms, System: 30.3 ms] Range (min … max): 601.0 ms … 606.2 ms 10 runs Summary './git hook run seq-hook' in 'HEAD~0' ran 1.30 ± 0.02 times faster than './git hook run seq-hook' in 'origin/master' In the preceding commit we removed the "stdout_to_stderr=1" assignment as being redundant. This change brings it back as with ".ungroup=1" the run_process_parallel() function doesn't provide them for us implicitly. As an aside omitting the stdout_to_stderr=1 here would have all tests pass, except those that test "git hook run" itself in t1800-hook.sh. But our tests passing is the result of another test blind spot, as was the case with the regression being fixed here. The "stdout_to_stderr=1" for hooks is long-standing behavior, see e.g. 1d9e8b56fe3 (Split back out update_hook handling in receive-pack, 2007-03-10) and other follow-up commits (running "git log" with "--reverse -p -Gstdout_to_stderr" is a good start). 1. https://lore.kernel.org/git/CA+dzEBn108QoMA28f0nC8K21XT+Afua0V2Qv8XkR8rAeqUCCZw@mail.gmail.com/ Reported-by: Anthony Sottile Signed-off-by: Ævar Arnfjörð Bjarmason --- hook.c | 5 +++++ t/t1800-hook.sh | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/hook.c b/hook.c index dc498ef5c39..5f31b60384a 100644 --- a/hook.c +++ b/hook.c @@ -54,6 +54,7 @@ static int pick_next_hook(struct child_process *cp, return 0; strvec_pushv(&cp->env_array, hook_cb->options->env.v); + cp->stdout_to_stderr = 1; /* because of .ungroup = 1 */ cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; @@ -126,6 +127,7 @@ int run_hooks_opt(const char *hook_name, struct run_hooks_opt *options) .tr2_label = hook_name, .jobs = jobs, + .ungroup = jobs == 1, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, @@ -136,6 +138,9 @@ int run_hooks_opt(const char *hook_name, struct run_hooks_opt *options) if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); + if (jobs != 1 || !run_opts.ungroup) + BUG("TODO: think about & document order & interleaving of parallel hook output"); + if (options->invoked_hook) *options->invoked_hook = 0; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 1e4adc3d53e..f22754deccc 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -4,6 +4,7 @@ test_description='git-hook command' TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-terminal.sh test_expect_success 'git hook usage' ' test_expect_code 129 git hook && @@ -120,4 +121,40 @@ test_expect_success 'git -c core.hooksPath= hook run' ' test_cmp expect actual ' +test_hook_tty() { + local fd="$1" && + + cat >expect && + + test_when_finished "rm -rf repo" && + git init repo && + + test_hook -C repo pre-commit <<-EOF && + { + test -t 1 && echo >&$fd STDOUT TTY || echo >&$fd STDOUT NO TTY && + test -t 2 && echo >&$fd STDERR TTY || echo >&$fd STDERR NO TTY + } $fd>actual + EOF + + test_commit -C repo A && + test_commit -C repo B && + git -C repo reset --soft HEAD^ && + test_terminal git -C repo commit -m"B.new" && + test_cmp expect repo/actual +} + +test_expect_success TTY 'git hook run: stdout and stderr are connected to a TTY: STDOUT redirect' ' + test_hook_tty 1 <<-\EOF + STDOUT NO TTY + STDERR TTY + EOF +' + +test_expect_success TTY 'git hook run: stdout and stderr are connected to a TTY: STDERR redirect' ' + test_hook_tty 2 <<-\EOF + STDOUT TTY + STDERR NO TTY + EOF +' + test_done