From patchwork Thu Oct 20 23:25:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13014148 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 74BDAC433FE for ; Thu, 20 Oct 2022 23:25:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229736AbiJTXZz (ORCPT ); Thu, 20 Oct 2022 19:25:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33324 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229788AbiJTXZv (ORCPT ); Thu, 20 Oct 2022 19:25:51 -0400 Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3C94FFF8FC for ; Thu, 20 Oct 2022 16:25:50 -0700 (PDT) Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-368e6c449f2so8680437b3.5 for ; Thu, 20 Oct 2022 16:25:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=CLtMtz1HzG6FmW7t66EFkPPB0mWkMRURetxLgwhmnqM=; b=QIB5jA5NTo3ANZ6yO8zkONQPpx0REVByiW3MgMEsxyiZ/BU1VHtBra9AHCfFXLFTRv rqDuQl0JGj6If9ms3v57+a/pP4Ko1n6oEI4XNgihnnaXJafWXmo3akEx9aTY1uSyE1eG wFKGcwUuKvohjmW2dC44fqSTpNLQZdBbP4SCllfpx2lldH2cDdWUFppQFpHKLALj37tj gTmQ05GL//uRkotChTItVOcpVicd7WH4n8rTXN2GTmX33kDBhxHJx9qYqRN3esRl/65W HOiZq/wfHUeOzq0+7Z7nGFK1N+VcrRu8AEUxpgf5r6dk1sM7ITljD2VCEfkRlGMDxyGy UoWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=CLtMtz1HzG6FmW7t66EFkPPB0mWkMRURetxLgwhmnqM=; b=DEjT6M7WlUt1+PP3q6eIwJ+MHfgXIO/oKzkv5M9fCrACJ2gfgmbbkvMmqASsTprsDv +qB1asj9bxDyq1F5bEEzsVNXn6mE0Ab/AWqIAVV6SObmKVa7GAkLhje7EhHEv9mxAEQy e7PGXby9dX/ZClcZk2G7vOhn+VJNwLGRxcjddg4eA16RUcQbtN3AeLioIXIL2N/3s8hz fgoXu57k+Kmv7ItDj0x4q48Hf6Nnr630rhA/un4JrGxlmnqtAFcsuLTzkMwkw972f1+z 9MOi0ydizWFYxHIckPKjL6YolEbgDKadhHqr2wwR72KdSIIMJOzT+pX9iXSl5sMpnyhj 8ZrQ== X-Gm-Message-State: ACrzQf2s+1nb+7Tttg8fq5fR7pC7hQybQ42KjDHValG7WiAB91anehU6 HxFYUkGuY8rIa1XXCVGOH99J1REBA8dJCU7LOotXYT1PpWV+ge3/E0ucs3tkj7QGNCOXvJojKr1 UgB72Ug9gLDRPbxBjU8uFbJEZsG8aUMY6LapzGbehEDRm97wXeZpZw4SepW7nqJ7OlQ== X-Google-Smtp-Source: AMsMyM7LFtfkGDVlkzqyYDuptqImIL0nwtA9ELmIEkRxsIjaja6OCTVwk7vHyTLb+yZf7QDFoilNbejfuEbk8zo= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a05:6902:349:b0:695:9d03:e009 with SMTP id e9-20020a056902034900b006959d03e009mr12835369ybs.588.1666308349522; Thu, 20 Oct 2022 16:25:49 -0700 (PDT) Date: Thu, 20 Oct 2022 23:25:27 +0000 In-Reply-To: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.38.0.135.g90850a2211-goog Message-ID: <20221020232532.1128326-2-calvinwan@google.com> Subject: [PATCH v3 1/6] run-command: add pipe_output_fn to run_processes_parallel_opts From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , emilyshaffer@google.com, avarab@gmail.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add pipe_output_fn as an optionally set function in run_process_parallel_opts. If set, output from each child process is first separately stored in 'out' and then piped to the callback function when the child process finishes to allow for separate parsing. Two of the tests check for line count rather than an exact match since the interleaved output order is not guaranteed to be exactly the same every run through. Signed-off-by: Calvin Wan --- run-command.c | 21 +++++++++++++++++++-- run-command.h | 21 +++++++++++++++++++++ t/helper/test-run-command.c | 13 +++++++++++++ t/t0061-run-command.sh | 30 ++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/run-command.c b/run-command.c index c772acd743..03787bc7f5 100644 --- a/run-command.c +++ b/run-command.c @@ -1503,6 +1503,7 @@ struct parallel_processes { enum child_state state; struct child_process process; struct strbuf err; + struct strbuf out; void *data; } *children; /* @@ -1560,6 +1561,9 @@ static void pp_init(struct parallel_processes *pp, if (!opts->get_next_task) BUG("you need to specify a get_next_task function"); + + if (opts->pipe_output && opts->ungroup) + BUG("pipe_output and ungroup are incompatible with each other"); CALLOC_ARRAY(pp->children, n); if (!opts->ungroup) @@ -1567,6 +1571,8 @@ static void pp_init(struct parallel_processes *pp, for (size_t i = 0; i < n; i++) { strbuf_init(&pp->children[i].err, 0); + if (opts->pipe_output) + strbuf_init(&pp->children[i].out, 0); child_process_init(&pp->children[i].process); if (pp->pfd) { pp->pfd[i].events = POLLIN | POLLHUP; @@ -1586,6 +1592,7 @@ static void pp_cleanup(struct parallel_processes *pp, trace_printf("run_processes_parallel: done"); for (size_t i = 0; i < opts->processes; i++) { strbuf_release(&pp->children[i].err); + strbuf_release(&pp->children[i].out); child_process_clear(&pp->children[i].process); } @@ -1680,8 +1687,12 @@ static void pp_buffer_stderr(struct parallel_processes *pp, for (size_t i = 0; i < opts->processes; i++) { if (pp->children[i].state == GIT_CP_WORKING && pp->pfd[i].revents & (POLLIN | POLLHUP)) { - int n = strbuf_read_once(&pp->children[i].err, - pp->children[i].process.err, 0); + struct strbuf buf = STRBUF_INIT; + int n = strbuf_read_once(&buf, pp->children[i].process.err, 0); + strbuf_addbuf(&pp->children[i].err, &buf); + if (opts->pipe_output) + strbuf_addbuf(&pp->children[i].out, &buf); + strbuf_release(&buf); if (n == 0) { close(pp->children[i].process.err); pp->children[i].state = GIT_CP_WAIT_CLEANUP; @@ -1717,6 +1728,12 @@ static int pp_collect_finished(struct parallel_processes *pp, if (i == opts->processes) break; + if (opts->pipe_output) { + opts->pipe_output(&pp->children[i].out, opts->data, + pp->children[i].data); + strbuf_reset(&pp->children[i].out); + } + code = finish_command(&pp->children[i].process); if (opts->task_finished) diff --git a/run-command.h b/run-command.h index e3e1ea01ad..b4584c3698 100644 --- a/run-command.h +++ b/run-command.h @@ -440,6 +440,21 @@ typedef int (*start_failure_fn)(struct strbuf *out, void *pp_cb, void *pp_task_cb); +/** + * This callback is called on every child process that finished processing. + * + * "struct strbuf *process_out" contains the output from the finished child + * process. + * + * 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. + * + * This function is incompatible with "ungroup" + */ +typedef void (*pipe_output_fn)(struct strbuf *process_out, + void *pp_cb, + void *pp_task_cb); + /** * This callback is called on every child process that finished processing. * @@ -493,6 +508,12 @@ struct run_process_parallel_opts */ start_failure_fn start_failure; + /** + * pipe_output: See pipe_output_fn() above. This should be + * NULL unless process specific output is needed + */ + pipe_output_fn pipe_output; + /** * task_finished: See task_finished_fn() above. This can be * NULL to omit any special handling. diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 3ecb830f4a..e9b41419a0 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -52,6 +52,13 @@ static int no_job(struct child_process *cp, return 0; } +static void pipe_output(struct strbuf *process_out, + void *pp_cb, + void *pp_task_cb) +{ + fprintf(stderr, "%s", process_out->buf); +} + static int task_finished(int result, struct strbuf *err, void *pp_cb, @@ -439,6 +446,12 @@ int cmd__run_command(int argc, const char **argv) opts.ungroup = 1; } + if (!strcmp(argv[1], "--pipe-output")) { + argv += 1; + argc -= 1; + opts.pipe_output = pipe_output; + } + jobs = atoi(argv[2]); strvec_clear(&proc.args); strvec_pushv(&proc.args, (const char **)argv + 3); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 7b5423eebd..e50e57db89 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -134,6 +134,12 @@ test_expect_success 'run_command runs in parallel with more jobs available than test_cmp expect actual ' +test_expect_success 'run_command runs in parallel with more jobs available than tasks --pipe-output' ' + test-tool run-command --pipe-output run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_line_count = 20 err +' + test_expect_success 'run_command runs ungrouped in parallel with more jobs available than tasks' ' test-tool run-command --ungroup run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && test_line_count = 8 out && @@ -145,6 +151,12 @@ test_expect_success 'run_command runs in parallel with as many jobs as tasks' ' test_cmp expect actual ' +test_expect_success 'run_command runs in parallel with as many jobs as tasks --pipe-output' ' + test-tool run-command --pipe-output run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_line_count = 20 err +' + test_expect_success 'run_command runs ungrouped in parallel with as many jobs as tasks' ' test-tool run-command --ungroup run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && test_line_count = 8 out && @@ -156,6 +168,12 @@ test_expect_success 'run_command runs in parallel with more tasks than jobs avai test_cmp expect actual ' +test_expect_success 'run_command runs in parallel with more tasks than jobs available --pipe-output' ' + test-tool run-command --pipe-output run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_line_count = 20 err +' + test_expect_success 'run_command runs ungrouped in parallel with more tasks than jobs available' ' test-tool run-command --ungroup run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && test_line_count = 8 out && @@ -176,6 +194,12 @@ test_expect_success 'run_command is asked to abort gracefully' ' test_cmp expect actual ' +test_expect_success 'run_command is asked to abort gracefully --pipe-output' ' + test-tool run-command --pipe-output run-command-abort 3 false >out 2>err && + test_must_be_empty out && + test_cmp expect err +' + test_expect_success 'run_command is asked to abort gracefully (ungroup)' ' test-tool run-command --ungroup run-command-abort 3 false >out 2>err && test_must_be_empty out && @@ -191,6 +215,12 @@ test_expect_success 'run_command outputs ' ' test_cmp expect actual ' +test_expect_success 'run_command outputs --pipe-output' ' + test-tool run-command --pipe-output 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_expect_success 'run_command outputs (ungroup) ' ' test-tool run-command --ungroup run-command-no-jobs 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && test_must_be_empty out && From patchwork Thu Oct 20 23:25:28 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13014149 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 766D3C433FE for ; Thu, 20 Oct 2022 23:25:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229810AbiJTXZ6 (ORCPT ); Thu, 20 Oct 2022 19:25:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33724 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229779AbiJTXZw (ORCPT ); Thu, 20 Oct 2022 19:25:52 -0400 Received: from mail-pl1-x649.google.com (mail-pl1-x649.google.com [IPv6:2607:f8b0:4864:20::649]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C8D11114DD1 for ; Thu, 20 Oct 2022 16:25:51 -0700 (PDT) Received: by mail-pl1-x649.google.com with SMTP id s13-20020a170902ea0d00b00183243c7a0fso472324plg.3 for ; Thu, 20 Oct 2022 16:25:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=QIdV0eGQf08L4kmatnCvCzSClEB7YL7MPWJoBttP+6s=; b=XJ7DQZQgkVdkHwgnyjQe/O+CNg5av+GZ7aQhwQhqq/QE5JN4/mBgYwAcrsSsDodDsW BgMCmKkL15rctFnPYTkAaSq3/ufWNbN3sHjRMWs3xMvpi7oSzRZ8nXOueLOx5GSqasBk Fm8Sw3ypzBdwecPJI2nF6pcitJmYNS7vEKYqIQwgIOiM1KX2AmENayNAqY30zKUhX2Ec Y+si5s9eWgHZBMz1QlCXig1ElqLFli+not3cYktpxfoUEdvDinfjCOewB4SNJ2eUxoV0 xQ8e/5rhWw13S9lyVJSsI8f3YQaHZ8JxS8sg/8VtIBEKHbyZoOOf6YxhwLfT6KI0FJc9 ANWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QIdV0eGQf08L4kmatnCvCzSClEB7YL7MPWJoBttP+6s=; b=4oOuUCeyFigBjDOwH6joJyfqKky62V9JlSdpnZBoMTKwQHHB0a1Khtu7d4kBld0vDr ajBP7ClS6Ah7Vf9VLRaG+yTTXkA5XHhIjJLmo/GNT2Ubiyx302I5lI7wLDR9GbUMOwmQ HoZ1DeLlc4uTJLLywoJTL9BnVvNExmCa8RjEWLaMCl8jCyxUz52/5+XpTnQLi/hnjd7U hM5QVvqUQDYdumfWnP3wEkWgVklOvVae4V2+5Os1ygQFH0kbOlIOnEPyEPW8Pq4d1c3E rz7zvomtOxbhJNcrPOijzDit3+1umrXX2360r6CTl/ApQ4xagGyLR+wMxeNiPkPnTdMo ujXQ== X-Gm-Message-State: ACrzQf17xfir8dosDcD9Br3TLOh0C82Hc3uZIScIl9VuRzKCpvr0ZFtk VCLisj4n9mubskB9LX/UucZ5o3oKYD4VugWF0Jod8enW9WNeuKqbL6XT2tVNeIyI0hFsAHj1ieo ljdk03m4u4JbeZXWApHn7xm5OjNQVKbUtc500sKheZ+PGFDwHUNYu9HQAI8lL8VCb6g== X-Google-Smtp-Source: AMsMyM7Jt1HiD27ufMKpabtrOe8AYgz5j71YwQe8bGbYP4Iwffn7F5VMBhVLUCpK+5OxQlpza56KOJyBX2rDDQQ= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a17:902:ef4c:b0:186:6399:6b48 with SMTP id e12-20020a170902ef4c00b0018663996b48mr5574039plx.128.1666308351227; Thu, 20 Oct 2022 16:25:51 -0700 (PDT) Date: Thu, 20 Oct 2022 23:25:28 +0000 In-Reply-To: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.38.0.135.g90850a2211-goog Message-ID: <20221020232532.1128326-3-calvinwan@google.com> Subject: [PATCH v3 2/6] run-command: add hide_output to run_processes_parallel_opts From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , emilyshaffer@google.com, avarab@gmail.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Output from child processes and callbacks may not be necessary to print out for every invoker of run_processes_parallel. Add hide_output as an option to not print said output. Signed-off-by: Calvin Wan --- run-command.c | 8 +++++--- run-command.h | 9 +++++++++ t/helper/test-run-command.c | 6 ++++++ t/t0061-run-command.sh | 6 ++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/run-command.c b/run-command.c index 03787bc7f5..3aa28ad6dc 100644 --- a/run-command.c +++ b/run-command.c @@ -1603,7 +1603,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 (!opts->hide_output) + strbuf_write(&pp->buffered_output, stderr); strbuf_release(&pp->buffered_output); sigchain_pop_common(); @@ -1754,7 +1755,7 @@ static int pp_collect_finished(struct parallel_processes *pp, pp->pfd[i].fd = -1; child_process_init(&pp->children[i].process); - if (opts->ungroup) { + if (opts->ungroup || opts->hide_output) { ; /* no strbuf_*() work to do here */ } else if (i != pp->output_owner) { strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); @@ -1826,7 +1827,8 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) pp.children[i].state = GIT_CP_WAIT_CLEANUP; } else { pp_buffer_stderr(&pp, opts, output_timeout); - pp_output(&pp); + if (!opts->hide_output) + pp_output(&pp); } code = pp_collect_finished(&pp, opts); if (code) { diff --git a/run-command.h b/run-command.h index b4584c3698..4570812c08 100644 --- a/run-command.h +++ b/run-command.h @@ -496,6 +496,11 @@ struct run_process_parallel_opts */ unsigned int ungroup:1; + /** + * hide_output: see 'hide_output' in run_processes_parallel() below. + */ + unsigned int hide_output:1; + /** * get_next_task: See get_next_task_fn() above. This must be * specified. @@ -547,6 +552,10 @@ struct run_process_parallel_opts * 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. + * + * If the "hide_output" option is set, any output in the "struct strbuf + * *out" parameter will not be printed by this function. This includes + * output from child processes as well as callbacks. */ void run_processes_parallel(const struct run_process_parallel_opts *opts); diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index e9b41419a0..1af443db4d 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -446,6 +446,12 @@ int cmd__run_command(int argc, const char **argv) opts.ungroup = 1; } + if (!strcmp(argv[1], "--hide-output")) { + argv += 1; + argc -= 1; + opts.hide_output = 1; + } + if (!strcmp(argv[1], "--pipe-output")) { argv += 1; argc -= 1; diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index e50e57db89..a0219f6093 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -180,6 +180,12 @@ test_expect_success 'run_command runs ungrouped in parallel with more tasks than test_line_count = 4 err ' +test_expect_success 'run_command hides output when run in parallel' ' + test-tool run-command --hide-output run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test_must_be_empty err +' + cat >expect <<-EOF preloaded output of a child asking for a quick stop From patchwork Thu Oct 20 23:25:29 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13014150 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 8E5D5C4332F for ; Thu, 20 Oct 2022 23:26:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229755AbiJTX0A (ORCPT ); Thu, 20 Oct 2022 19:26:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34964 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229740AbiJTXZz (ORCPT ); Thu, 20 Oct 2022 19:25:55 -0400 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4D3C5123469 for ; Thu, 20 Oct 2022 16:25:53 -0700 (PDT) Received: by mail-pg1-x54a.google.com with SMTP id l185-20020a6388c2000000b004610d11faddso442027pgd.1 for ; Thu, 20 Oct 2022 16:25:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=n2hotPdrGrYF4F+VcmnLcsvSNqVOiwHxa9vMAL4jhHk=; b=GhTiT3D+CApSwbc6GqfiaCDRTJTZT84BCqWdVOb/2ObtB9gTDC/F/T95Yr8bNQQJ+U A7krETtP61FHjKqkpL8McQ/CzDTyRB6MEzzBEjD2uD88V2SLwkyG366uptv29OmhSCTc GD4biI8Uy6gPGUHW5maOmAo6vtwFfJaii/xv/1cALcG3Ny12atw2LnGdYIasP3x+5eZD R8kLbGhMkHrJjxqcJRlpRHki1BDCl636aSegPfT2CAMs+BFfDGtKMXbGZkm4kWGjiNRj qRlP9tLMaXisVmyWgW54DuXzbZmgkV4w5PPU66nsboLIpcDWIkt8P37PhgqwROhpUhwN ZkiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=n2hotPdrGrYF4F+VcmnLcsvSNqVOiwHxa9vMAL4jhHk=; b=VcbOLYYALqkq+M3hvTh8X+Tm1Ca/8v5aTkOlXIIOQYBneED52F4UBb9g+Z9EZpaKCw bOMPmyd81BCN7KJyQ/m9fk5QD0O6YNotc41jiAc7U097Dua8/+ZdVUbGhDoRzihK3SPk tQC4Gi5r+aIjW0b1Y4xgAYE3r2i3le1rsiK3gn6P4sD5E2lRpS23JrXsupzxVkUgpE7e 10DXNPTDnreqPSh7ISrA2P0HK7eEwBibnT1Dexlg0/imiQoeErMMrTXWOu6s7yGZTQcc l8gImc0duimi2YDRk9e52Rad9uki9K78vDvaN+CKUW3+ENTn0H0a+794AsVCyaasPXSu qzFw== X-Gm-Message-State: ACrzQf1w8WvldUB2YD8rrc+G6hvJWqgyKUtdNMzT1TNQp2Y82gvri4Rl JW0keCV82Yb7AvHTB7ZlROvSFQ/h5GqZHEO6QTFV3GhXm8p9BChz4U2lQkzwueYzXIRAgaTMQsi qntIzN2fxqqMuvSF0qvIHV1M++rcdiLPTwI1VBfHkRPz03hwcaDRZtHBOHOj/6f0aig== X-Google-Smtp-Source: AMsMyM7POoLZgJ0iiyxp/rYwBN3R1eYgx9f8suId/16Nl5Lyg69rR7OW+hG4CEaIJbxGDgW92rplx25KtrXqOX8= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a05:6a00:1596:b0:563:9a1a:b5b0 with SMTP id u22-20020a056a00159600b005639a1ab5b0mr15755486pfk.38.1666308353072; Thu, 20 Oct 2022 16:25:53 -0700 (PDT) Date: Thu, 20 Oct 2022 23:25:29 +0000 In-Reply-To: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.38.0.135.g90850a2211-goog Message-ID: <20221020232532.1128326-4-calvinwan@google.com> Subject: [PATCH v3 3/6] submodule: strbuf variable rename From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , emilyshaffer@google.com, avarab@gmail.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A prepatory change for a future patch that moves the status parsing logic to a separate function. Signed-off-by: Calvin Wan --- submodule.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/submodule.c b/submodule.c index f7c71f1f4b..ac214f250d 100644 --- a/submodule.c +++ b/submodule.c @@ -1900,25 +1900,28 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) fp = xfdopen(cp.out, "r"); while (strbuf_getwholeline(&buf, fp, '\n') != EOF) { + char *str = buf.buf; + const size_t len = buf.len; + /* regular untracked files */ - if (buf.buf[0] == '?') + if (str[0] == '?') dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; - if (buf.buf[0] == 'u' || - buf.buf[0] == '1' || - buf.buf[0] == '2') { + if (str[0] == 'u' || + str[0] == '1' || + str[0] == '2') { /* T = line type, XY = status, SSSS = submodule state */ - if (buf.len < strlen("T XY SSSS")) + if (len < strlen("T XY SSSS")) BUG("invalid status --porcelain=2 line %s", - buf.buf); + str); - if (buf.buf[5] == 'S' && buf.buf[8] == 'U') + if (str[5] == 'S' && str[8] == 'U') /* nested untracked file */ dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; - if (buf.buf[0] == 'u' || - buf.buf[0] == '2' || - memcmp(buf.buf + 5, "S..U", 4)) + if (str[0] == 'u' || + str[0] == '2' || + memcmp(str + 5, "S..U", 4)) /* other change */ dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; } From patchwork Thu Oct 20 23:25:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13014151 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 4526BC433FE for ; Thu, 20 Oct 2022 23:26:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229850AbiJTX0H (ORCPT ); Thu, 20 Oct 2022 19:26:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35426 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229817AbiJTXZ6 (ORCPT ); Thu, 20 Oct 2022 19:25:58 -0400 Received: from mail-ot1-x349.google.com (mail-ot1-x349.google.com [IPv6:2607:f8b0:4864:20::349]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F2878129741 for ; Thu, 20 Oct 2022 16:25:55 -0700 (PDT) Received: by mail-ot1-x349.google.com with SMTP id a22-20020a0568300b9600b0065c0cef3662so610180otv.14 for ; Thu, 20 Oct 2022 16:25:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ZCueAvPmcQ8WV8lVsi1yzTc2SSQu9FZapWvIkvQekMM=; b=sptY6Qdrow5qD36MkeFUjM9CpYV8GleDvFRFrDLR8U42HDBEGaPzGKB3uCMqcrmnNb tYXkBZscNjDN07s15qNQyN6vzZ9qxP4f5qbeNuT1YcJKd+8Kbaj0s9oEjkbg+qXt0AzF 4ut/7c9JYkOXRNSbpwGfCdaGsZIicsRcJbBrzuP5F7Zk3NKYT2wapbFrIi0NGeGVi9kk S+rXgBDi5EUwxUrgSKpA3P2VIiiw0HFx0kPjn0Ez5pFR8KLjKGex5WPMPSFVuY0TOyEW 486mF2KsU4XlmY3bBAmhKygQ4B/o2hibsDNgJ+ANPXVJTBoJSjAQhNuB+8Q094LZi+Q7 bn+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ZCueAvPmcQ8WV8lVsi1yzTc2SSQu9FZapWvIkvQekMM=; b=GqfuryyjlzYgSylbT0I6/ICJn10gOsxe2cWEuv0Quz7gTRCgtVGD1+qn1+V7aOsCwR ML2dLfLashJ1sqhrlDyd3goTFpzI8yKqv2knTDiCq7Ub/2ake6NiFO3UVAs312Fr1AnL vSYbqWeuDrMj5+N1dwslmKNw26/3UVinXSOTOOjBUfLD+iMBvPOVvRxyvFgNPxo8ea6B Du/yNWco3eYLfE3Abm20YllB8M1P6D69XsIpG9m78mtCa9f2Qg1y6GHYmGbWUSbTM8ss Q931ffNIHDBsW4EyVhU49wBYI/fqAWHEkgvzIG11DJk3Q6c1ZadUflziJhgDB4VEnvdb 0xfg== X-Gm-Message-State: ACrzQf3M/R+WbMiT72LBjksLABC5FP1zJGz7mfYB2mtvxlRGI50qmIfN +OghWvTG/gDn45aDAS4SAzk2NpwaiClutljeMdAKflBOjWwg7ENDpP6sbPcLAvL51hCbGgfiH3s i+TXl/DCIEYisWvLfZcS2raS9PrFAoPSXcJdBz5TQOn2Qef/dYPxDz3zKEoG7KkafAA== X-Google-Smtp-Source: AMsMyM4ZFY5HAx+EjwQ23abqGLko4zGRmqI7LNOiHZASiKHI5e37mwdeyTPfE6ohI9fdBiQmvFI7Dz5YjpkrhNg= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a05:6870:558d:b0:136:60bd:1944 with SMTP id n13-20020a056870558d00b0013660bd1944mr10406538oao.100.1666308354880; Thu, 20 Oct 2022 16:25:54 -0700 (PDT) Date: Thu, 20 Oct 2022 23:25:30 +0000 In-Reply-To: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.38.0.135.g90850a2211-goog Message-ID: <20221020232532.1128326-5-calvinwan@google.com> Subject: [PATCH v3 4/6] submodule: move status parsing into function From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , emilyshaffer@google.com, avarab@gmail.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A future patch requires the ability to parse the output of git status --porcelain=2. Move parsing code from is_submodule_modified to parse_status_porcelain. Signed-off-by: Calvin Wan --- submodule.c | 74 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/submodule.c b/submodule.c index ac214f250d..289be0fb93 100644 --- a/submodule.c +++ b/submodule.c @@ -1864,6 +1864,45 @@ int fetch_submodules(struct repository *r, return spf.result; } +static int parse_status_porcelain(char *str, size_t len, + unsigned *dirty_submodule, + int ignore_untracked) +{ + /* regular untracked files */ + if (str[0] == '?') + *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; + + if (str[0] == 'u' || + str[0] == '1' || + str[0] == '2') { + /* T = line type, XY = status, SSSS = submodule state */ + if (len < strlen("T XY SSSS")) + BUG("invalid status --porcelain=2 line %s", + str); + + if (str[5] == 'S' && str[8] == 'U') + /* nested untracked file */ + *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; + + if (str[0] == 'u' || + str[0] == '2' || + memcmp(str + 5, "S..U", 4)) + /* other change */ + *dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; + } + + if ((*dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && + ((*dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || + ignore_untracked)) { + /* + * We're not interested in any further information from + * the child any more, neither output nor its exit code. + */ + return 1; + } + return 0; +} + unsigned is_submodule_modified(const char *path, int ignore_untracked) { struct child_process cp = CHILD_PROCESS_INIT; @@ -1903,39 +1942,10 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) char *str = buf.buf; const size_t len = buf.len; - /* regular untracked files */ - if (str[0] == '?') - dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; - - if (str[0] == 'u' || - str[0] == '1' || - str[0] == '2') { - /* T = line type, XY = status, SSSS = submodule state */ - if (len < strlen("T XY SSSS")) - BUG("invalid status --porcelain=2 line %s", - str); - - if (str[5] == 'S' && str[8] == 'U') - /* nested untracked file */ - dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED; - - if (str[0] == 'u' || - str[0] == '2' || - memcmp(str + 5, "S..U", 4)) - /* other change */ - dirty_submodule |= DIRTY_SUBMODULE_MODIFIED; - } - - if ((dirty_submodule & DIRTY_SUBMODULE_MODIFIED) && - ((dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) || - ignore_untracked)) { - /* - * We're not interested in any further information from - * the child any more, neither output nor its exit code. - */ - ignore_cp_exit_code = 1; + ignore_cp_exit_code = parse_status_porcelain(str, len, &dirty_submodule, + ignore_untracked); + if (ignore_cp_exit_code) break; - } } fclose(fp); From patchwork Thu Oct 20 23:25:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13014152 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 BEDA9C4332F for ; Thu, 20 Oct 2022 23:26:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229894AbiJTX0I (ORCPT ); Thu, 20 Oct 2022 19:26:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35366 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229788AbiJTXZ6 (ORCPT ); Thu, 20 Oct 2022 19:25:58 -0400 Received: from mail-pf1-x44a.google.com (mail-pf1-x44a.google.com [IPv6:2607:f8b0:4864:20::44a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 50A9A12B37D for ; Thu, 20 Oct 2022 16:25:57 -0700 (PDT) Received: by mail-pf1-x44a.google.com with SMTP id p1-20020aa78601000000b00565a29d32e5so418014pfn.5 for ; Thu, 20 Oct 2022 16:25:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=T+YOhlbdI4P6WDMl9yaFzGuIV/YOZxL10ETZHK3KJbk=; b=Exau4LnAqchT++8zN4FO4R3k0JuCHtmCYbTXZSZVneJr69lAPVXCpzr7oyb3pmt9KU sNmW4O3fRyxMmpIlUNYaIK5GIDDL+SLfLpgc2rZj3i2D7oFHB1+K43Fyh0+9wibAzSkY exmnm/n6ph4YoPGQIKUCOth9JjyaP1fj0Zf/YGIMNlalv5FHqiEdclSHsi61Of1YeNTf lqyktYFvykOHlOCtKxZm9hHsOO2Ju4s8ArfkKoo1rma/vrjNO9QsNlHD12y7Wt/CjiGe cNeBbdK961xxoRdoEu3fsWVRZ9sJc+2M9/jurd6roQ0MBkfRygsw/qYNFWqs5L/lyDzz Jr6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=T+YOhlbdI4P6WDMl9yaFzGuIV/YOZxL10ETZHK3KJbk=; b=aCDk9yjuNyMicrejTsPBRpB9mYboYo14xQa89p6A2i+91XgqVkvMzQpmGgO6PDUNwk n/jYGj4J7fQ5cZnmAifZ3v24BIok+s2juF4CiK893mw+nT4KK4Z1vH0P53gvCmoyui3h +a9PFg+Hb3rWdel/fYAqDxVdX5w8988ku4zyClN10O/t8IiiG7+1+xaxG+Kz9TIJzBl+ GmYhrvG4FdyyT90XhAH5WPH59d+hmiWvDCOoZ5yMhkNv8IxyN8AtIFhmmqo+Fai//n0J GTUIpCVuT7OGuU72+Cw1+wYgPjjpJjMKquPucmgkTfftHa5ldZm2pEkU/KWlPnb9rr+r nKkw== X-Gm-Message-State: ACrzQf1Xk/J5rJ3NULZ1Oc/p8c7Wglf1qUGdjn2Ok3YephMtsgYEAXcq lHPptLD716YYcOcfXpR1aV8R7gLAQtrIUIiQHHOMD3fIovh3W3AoIrFRGTz3ZYmmBchkwUBG8RU TLGSFkjYqx50gWS9+KGe9HoobAI6kIq6aR0OHWXc28oR/Jx9gBa+Hc46WLKA+wwpFEw== X-Google-Smtp-Source: AMsMyM4CkvtdeD0MaDJMnPg2be6vnQA4x5Fre5RSFir5LgOmLthnJbdbFp2GJlc1Ll5ozZexJisKZJwKjmDSq5o= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:aa7:979a:0:b0:563:a69:96c7 with SMTP id o26-20020aa7979a000000b005630a6996c7mr16021346pfp.29.1666308356797; Thu, 20 Oct 2022 16:25:56 -0700 (PDT) Date: Thu, 20 Oct 2022 23:25:31 +0000 In-Reply-To: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.38.0.135.g90850a2211-goog Message-ID: <20221020232532.1128326-6-calvinwan@google.com> Subject: [PATCH v3 5/6] diff-lib: refactor match_stat_with_submodule From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , emilyshaffer@google.com, avarab@gmail.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Flatten out the if statements in match_stat_with_submodule so the logic is more readable and easier for future patches to add to. orig_flags didn't need to be set if the cache entry wasn't a GITLINK so defer setting it. Signed-off-by: Calvin Wan --- diff-lib.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 2edea41a23..f5257c0c71 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -73,18 +73,25 @@ static int match_stat_with_submodule(struct diff_options *diffopt, unsigned *dirty_submodule) { int changed = ie_match_stat(diffopt->repo->index, ce, st, ce_option); - if (S_ISGITLINK(ce->ce_mode)) { - struct diff_flags orig_flags = diffopt->flags; - if (!diffopt->flags.override_submodule_config) - set_diffopt_flags_from_submodule_config(diffopt, ce->name); - if (diffopt->flags.ignore_submodules) - changed = 0; - else if (!diffopt->flags.ignore_dirty_submodules && - (!changed || diffopt->flags.dirty_submodules)) - *dirty_submodule = is_submodule_modified(ce->name, - diffopt->flags.ignore_untracked_in_submodules); - diffopt->flags = orig_flags; + struct diff_flags orig_flags; + + if (!S_ISGITLINK(ce->ce_mode)) + goto ret; + + orig_flags = diffopt->flags; + if (!diffopt->flags.override_submodule_config) + set_diffopt_flags_from_submodule_config(diffopt, ce->name); + if (diffopt->flags.ignore_submodules) { + changed = 0; + goto cleanup; } + if (!diffopt->flags.ignore_dirty_submodules && + (!changed || diffopt->flags.dirty_submodules)) + *dirty_submodule = is_submodule_modified(ce->name, + diffopt->flags.ignore_untracked_in_submodules); +cleanup: + diffopt->flags = orig_flags; +ret: return changed; } From patchwork Thu Oct 20 23:25:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13014153 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 771CDC433FE for ; Thu, 20 Oct 2022 23:26:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229987AbiJTX0R (ORCPT ); Thu, 20 Oct 2022 19:26:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36336 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229768AbiJTX0E (ORCPT ); Thu, 20 Oct 2022 19:26:04 -0400 Received: from mail-pf1-x449.google.com (mail-pf1-x449.google.com [IPv6:2607:f8b0:4864:20::449]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3F6391285FC for ; Thu, 20 Oct 2022 16:25:59 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id p29-20020a056a0026dd00b00562f01c165cso402069pfw.20 for ; Thu, 20 Oct 2022 16:25:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=Jbwnq2kpTZFUte7SmCsp2VL4OUUaUxEPqKHBUJAZ7Ww=; b=AOgO6J3oWz0yo1zrTOXp92QPeEyAg3eCyivnbowpx4MZtGiPjka5Ehye8M8mO/7c1b VindLPv/E7xhAVf6TLD4kDHnulwrysjgcwnISffBxhvGDAkmdZn40293FnDQocNs3vL9 lTXTIhBi59Ne88PyQGw9pmn1TdoUIzCSCZZ7EW+gYlDD8MQSE/U0Jbc9AYF7mV0pI1v2 bSSf9eur5SAlaWVGkPYXcGGfV+oX3tK9td1s5pVIWug8HZia4mrp+VjHuSP7/HAM7lsJ xo4D3jQSm5ieeBCFwn/1vIf82swksa1ixRi5ZxPhIYmJU8IOKPX2rJEFY5dinC90c+ak bO2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Jbwnq2kpTZFUte7SmCsp2VL4OUUaUxEPqKHBUJAZ7Ww=; b=3zCxko1esuAuEx/5jG1i0ffRjpV6lp5lX3SOx8Fv8UlcZqVADZ6VLeJvXeL8plTP9B ZvEKkdSBPlW8fHe/wT5V2mNfeWYVa12vS78B21EmrIABibveYX22Uzq2BMveYA5InJ0o XMK/UyfZzORvUByj6IT1zH9nfnd4cx3GamtCtWkipDsr18vRk+tW287Ij0fmlrT7betJ axcmYD2C9nsYCRG0FMSe7hwebgmsJAAM0J3M2mMuaEP5+5QuyD8C070cGDYHS5aNLtrz DqleUbcn7Ix3UKD3BOOrNP6USogghJAXYWH/PhZLz01qn6f1aIcexu0GNntk0Qyf/iGt +COQ== X-Gm-Message-State: ACrzQf0VTx7HP5XFja6Gjfj9AW/VVYaIj6sPbOtBMrjWeGP0NXGCYuUZ oqLhJ7he63BDvPJ6Mf+/sAmMnfg/B9jnEeyOy9l0zEi4NsTPuF3IdYuCeNHZUv7jvdKxwlCYEe4 1X6b6cUyYix+tueDzWl9sf9AMP0XDZUsG4HnvC9lGiiCnvtlgFqNRFONenEICANaHCQ== X-Google-Smtp-Source: AMsMyM70jcsKZHlctdtzEnO1gc7N2fIe9En5tIJCXGMWF0WNVOQKtSIyLtsb5WzIAJ6B8dNutBcdmJcODYpgrjw= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a17:902:ec81:b0:185:3cea:6326 with SMTP id x1-20020a170902ec8100b001853cea6326mr16460195plg.24.1666308358231; Thu, 20 Oct 2022 16:25:58 -0700 (PDT) Date: Thu, 20 Oct 2022 23:25:32 +0000 In-Reply-To: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.38.0.135.g90850a2211-goog Message-ID: <20221020232532.1128326-7-calvinwan@google.com> Subject: [PATCH v3 6/6] diff-lib: parallelize run_diff_files for submodules From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , emilyshaffer@google.com, avarab@gmail.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org During the iteration of the index entries in run_diff_files, whenever a submodule is found and needs its status checked, a subprocess is spawned for it. Instead of spawning the subprocess immediately and waiting for its completion to continue, hold onto all submodules and relevant information in a list. Then use that list to create tasks for run_processes_parallel. Subprocess output is piped to status_pipe_output which parses it. Add config option submodule.diffJobs to set the maximum number of parallel jobs. The option defaults to 1 if unset. If set to 0, the number of jobs is set to online_cpus(). Since run_diff_files is called from many different commands, I chose to grab the config option in the function rather than adding variables to every git command and then figuring out how to pass them all in. Signed-off-by: Calvin Wan --- Documentation/config/submodule.txt | 12 +++ diff-lib.c | 79 +++++++++++++-- submodule.c | 157 +++++++++++++++++++++++++++++ submodule.h | 9 ++ t/t4027-diff-submodule.sh | 19 ++++ t/t7506-status-submodule.sh | 19 ++++ 6 files changed, 289 insertions(+), 6 deletions(-) diff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt index 6490527b45..1144a5ad74 100644 --- a/Documentation/config/submodule.txt +++ b/Documentation/config/submodule.txt @@ -93,6 +93,18 @@ submodule.fetchJobs:: in parallel. A value of 0 will give some reasonable default. If unset, it defaults to 1. +submodule.diffJobs:: + Specifies how many submodules are diffed at the same time. A + positive integer allows up to that number of submodules diffed + in parallel. A value of 0 will give the number of logical cores. + If unset, it defaults to 1. The diff operation is used by many + other git commands such as add, merge, diff, status, stash and + more. Note that the expensive part of the diff operation is + reading the index from cache or memory. Therefore multiple jobs + may be detrimental to performance if your hardware does not + support parallel reads or if the number of jobs greatly exceeds + the amount of supported reads. + submodule.alternateLocation:: Specifies how the submodules obtain alternates when submodules are cloned. Possible values are `no`, `superproject`. diff --git a/diff-lib.c b/diff-lib.c index f5257c0c71..0f30b4e478 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -14,6 +14,7 @@ #include "dir.h" #include "fsmonitor.h" #include "commit-reach.h" +#include "config.h" /* * diff-files @@ -65,15 +66,20 @@ static int check_removed(const struct index_state *istate, const struct cache_en * Return 1 when changes are detected, 0 otherwise. If the DIRTY_SUBMODULES * option is set, the caller does not only want to know if a submodule is * modified at all but wants to know all the conditions that are met (new - * commits, untracked content and/or modified content). + * commits, untracked content and/or modified content). If + * defer_submodule_status bit is set, dirty_submodule will be left to the + * caller to set. defer_submodule_status can also be set to 0 in this + * function if there is no need to check if the submodule is modified. */ static int match_stat_with_submodule(struct diff_options *diffopt, const struct cache_entry *ce, struct stat *st, unsigned ce_option, - unsigned *dirty_submodule) + unsigned *dirty_submodule, int *defer_submodule_status, + unsigned *ignore_untracked) { int changed = ie_match_stat(diffopt->repo->index, ce, st, ce_option); struct diff_flags orig_flags; + int defer = 0; if (!S_ISGITLINK(ce->ce_mode)) goto ret; @@ -86,12 +92,20 @@ static int match_stat_with_submodule(struct diff_options *diffopt, goto cleanup; } if (!diffopt->flags.ignore_dirty_submodules && - (!changed || diffopt->flags.dirty_submodules)) - *dirty_submodule = is_submodule_modified(ce->name, + (!changed || diffopt->flags.dirty_submodules)) { + if (defer_submodule_status && *defer_submodule_status) { + defer = 1; + *ignore_untracked = diffopt->flags.ignore_untracked_in_submodules; + } else { + *dirty_submodule = is_submodule_modified(ce->name, diffopt->flags.ignore_untracked_in_submodules); + } + } cleanup: diffopt->flags = orig_flags; ret: + if (defer_submodule_status) + *defer_submodule_status = defer; return changed; } @@ -103,6 +117,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) ? CE_MATCH_RACY_IS_DIRTY : 0); uint64_t start = getnanotime(); struct index_state *istate = revs->diffopt.repo->index; + struct string_list submodules = STRING_LIST_INIT_NODUP; diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/"); @@ -227,6 +242,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option) newmode = ce->ce_mode; } else { struct stat st; + unsigned ignore_untracked = 0; + int defer_submodule_status = !!revs->repo; changed = check_removed(istate, ce, &st); if (changed) { @@ -248,8 +265,24 @@ int run_diff_files(struct rev_info *revs, unsigned int option) } changed = match_stat_with_submodule(&revs->diffopt, ce, &st, - ce_option, &dirty_submodule); + ce_option, &dirty_submodule, + &defer_submodule_status, + &ignore_untracked); newmode = ce_mode_from_stat(ce, st.st_mode); + if (defer_submodule_status) { + struct submodule_status_util tmp = { + .changed = changed, + .dirty_submodule = 0, + .ignore_untracked = ignore_untracked, + .newmode = newmode, + .ce = ce + }; + struct string_list_item *item; + item = string_list_append(&submodules, ce->name); + item->util = xmalloc(sizeof(tmp)); + memcpy(item->util, &tmp, sizeof(tmp)); + continue; + } } if (!changed && !dirty_submodule) { @@ -268,6 +301,40 @@ int run_diff_files(struct rev_info *revs, unsigned int option) ce->name, 0, dirty_submodule); } + if (submodules.nr > 0) { + int parallel_jobs; + if (git_config_get_int("submodule.diffjobs", ¶llel_jobs)) + parallel_jobs = 1; + else if (!parallel_jobs) + parallel_jobs = online_cpus(); + else if (parallel_jobs < 0) + die(_("submodule.diffjobs cannot be negative")); + + if (get_submodules_status(revs->repo, &submodules, parallel_jobs)) + BUG("submodule status failed"); + for (size_t i = 0; i < submodules.nr; i++) { + struct submodule_status_util *util = submodules.items[i].util; + struct cache_entry *ce = util->ce; + unsigned int oldmode; + const struct object_id *old_oid, *new_oid; + + if (!util->changed && !util->dirty_submodule) { + ce_mark_uptodate(ce); + mark_fsmonitor_valid(istate, ce); + if (!revs->diffopt.flags.find_copies_harder) + continue; + } + oldmode = ce->ce_mode; + old_oid = &ce->oid; + new_oid = util->changed ? null_oid() : &ce->oid; + diff_change(&revs->diffopt, oldmode, util->newmode, + old_oid, new_oid, + !is_null_oid(old_oid), + !is_null_oid(new_oid), + ce->name, 0, util->dirty_submodule); + } + } + string_list_clear(&submodules, 1); diffcore_std(&revs->diffopt); diff_flush(&revs->diffopt); trace_performance_since(start, "diff-files"); @@ -315,7 +382,7 @@ static int get_stat_data(const struct index_state *istate, return -1; } changed = match_stat_with_submodule(diffopt, ce, &st, - 0, dirty_submodule); + 0, dirty_submodule, NULL, NULL); if (changed) { mode = ce_mode_from_stat(ce, st.st_mode); oid = null_oid(); diff --git a/submodule.c b/submodule.c index 289be0fb93..9aa1723a3b 100644 --- a/submodule.c +++ b/submodule.c @@ -1363,6 +1363,20 @@ int submodule_touches_in_range(struct repository *r, return ret; } +struct submodule_parallel_status { + size_t index_count; + int result; + + struct string_list *submodule_names; + struct repository *r; + + /* Pending statuses by OIDs */ + struct status_task **oid_status_tasks; + int oid_status_tasks_nr, oid_status_tasks_alloc; +}; + +#define SPS_INIT { 0 } + struct submodule_parallel_fetch { /* * The index of the last index entry processed by @@ -1445,6 +1459,13 @@ struct fetch_task { struct oid_array *commits; /* Ensure these commits are fetched */ }; +struct status_task { + struct repository *repo; + const char *path; + unsigned dirty_submodule; + int ignore_untracked; +}; + /** * When a submodule is not defined in .gitmodules, we cannot access it * via the regular submodule-config. Create a fake submodule, which we can @@ -1956,6 +1977,142 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) return dirty_submodule; } +static struct status_task * +get_status_task_from_index(struct submodule_parallel_status *sps, + struct strbuf *err) +{ + for (; sps->index_count < sps->submodule_names->nr; sps->index_count++) { + struct submodule_status_util *util = sps->submodule_names->items[sps->index_count].util; + const struct cache_entry *ce = util->ce; + struct status_task *task; + struct strbuf buf = STRBUF_INIT; + const char *git_dir; + + strbuf_addf(&buf, "%s/.git", ce->name); + git_dir = read_gitfile(buf.buf); + if (!git_dir) + git_dir = buf.buf; + if (!is_git_directory(git_dir)) { + if (is_directory(git_dir)) + die(_("'%s' not recognized as a git repository"), git_dir); + strbuf_release(&buf); + /* The submodule is not checked out, so it is not modified */ + util->dirty_submodule = 0; + continue; + } + strbuf_release(&buf); + + task = xmalloc(sizeof(*task)); + memset(task, 0, sizeof(*task)); + task->path = ce->name; + task->dirty_submodule = util->dirty_submodule; + task->ignore_untracked = util->ignore_untracked; + sps->index_count++; + return task; + } + return NULL; +} + + +static int get_next_submodule_status(struct child_process *cp, + struct strbuf *err, + void *data, void **task_cb) +{ + struct submodule_parallel_status *sps = data; + struct status_task *task = get_status_task_from_index(sps, err); + + if (!task) { + return 0; + } + + child_process_init(cp); + prepare_submodule_repo_env_in_gitdir(&cp->env); + + strvec_init(&cp->args); + strvec_pushl(&cp->args, "status", "--porcelain=2", NULL); + if (task->ignore_untracked) + strvec_push(&cp->args, "-uno"); + + prepare_submodule_repo_env(&cp->env); + cp->git_cmd = 1; + cp->no_stdin = 1; + cp->dir = task->path; + *task_cb = task; + return 1; +} + +static int status_start_failure(struct strbuf *err, + void *cb, void *task_cb) +{ + struct submodule_parallel_status *sps = cb; + + sps->result = 1; + return 0; +} + +static void status_pipe_output(struct strbuf *process_out, + void *cb, void *task_cb) +{ + struct status_task *task = task_cb; + struct string_list list = STRING_LIST_INIT_DUP; + + string_list_split(&list, process_out->buf, '\n', -1); + + for (size_t i = 0; i < list.nr; i++) { + if (parse_status_porcelain(list.items[i].string, + strlen(list.items[i].string), + &task->dirty_submodule, task->ignore_untracked)) + break; + } + string_list_clear(&list, 0); +} + +static int status_finish(int retvalue, struct strbuf *err, + void *cb, void *task_cb) +{ + struct submodule_parallel_status *sps = cb; + struct status_task *task = task_cb; + struct string_list_item *it = + string_list_lookup(sps->submodule_names, task->path); + struct submodule_status_util *util = it->util; + + util->dirty_submodule = task->dirty_submodule; + free(task); + + return 0; +} + +int get_submodules_status(struct repository *r, + struct string_list *submodules, + int max_parallel_jobs) +{ + struct submodule_parallel_status sps = SPS_INIT; + const struct run_process_parallel_opts opts = { + .tr2_category = "submodule", + .tr2_label = "parallel/status", + + .processes = max_parallel_jobs, + .hide_output = 1, + + .get_next_task = get_next_submodule_status, + .start_failure = status_start_failure, + .pipe_output = status_pipe_output, + .task_finished = status_finish, + .data = &sps, + }; + + sps.r = r; + + if (repo_read_index(r) < 0) + die(_("index file corrupt")); + + sps.submodule_names = submodules; + string_list_sort(sps.submodule_names); + run_processes_parallel(&opts); + + return sps.result; +} + int submodule_uses_gitfile(const char *path) { struct child_process cp = CHILD_PROCESS_INIT; diff --git a/submodule.h b/submodule.h index 6a9fec6de1..cbb7231a5d 100644 --- a/submodule.h +++ b/submodule.h @@ -41,6 +41,12 @@ struct submodule_update_strategy { .type = SM_UPDATE_UNSPECIFIED, \ } +struct submodule_status_util { + int changed, ignore_untracked; + unsigned dirty_submodule, newmode; + struct cache_entry *ce; +}; + int is_gitmodules_unmerged(struct index_state *istate); int is_writing_gitmodules_ok(void); int is_staging_gitmodules_ok(struct index_state *istate); @@ -94,6 +100,9 @@ int fetch_submodules(struct repository *r, int command_line_option, int default_option, int quiet, int max_parallel_jobs); +int get_submodules_status(struct repository *r, + struct string_list *submodules, + int max_parallel_jobs); unsigned is_submodule_modified(const char *path, int ignore_untracked); int submodule_uses_gitfile(const char *path); diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh index 40164ae07d..e08ee315a7 100755 --- a/t/t4027-diff-submodule.sh +++ b/t/t4027-diff-submodule.sh @@ -34,6 +34,25 @@ test_expect_success setup ' subtip=$3 subprev=$2 ' +test_expect_success 'diff in superproject with submodules respects parallel settings' ' + test_when_finished "rm -f trace.out" && + ( + GIT_TRACE=$(pwd)/trace.out git diff && + grep "1 tasks" trace.out && + >trace.out && + + git config submodule.diffJobs 8 && + GIT_TRACE=$(pwd)/trace.out git diff && + grep "8 tasks" trace.out && + >trace.out && + + GIT_TRACE=$(pwd)/trace.out git -c submodule.diffJobs=0 diff && + grep "preparing to run up to [0-9]* tasks" trace.out && + ! grep "up to 0 tasks" trace.out && + >trace.out + ) +' + test_expect_success 'git diff --raw HEAD' ' hexsz=$(test_oid hexsz) && git diff --raw --abbrev=$hexsz HEAD >actual && diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index f5426a8e58..dfdfb58bfc 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -410,4 +410,23 @@ test_expect_success 'status with added file in nested submodule (short)' ' EOF ' +test_expect_success 'status in superproject with submodules respects parallel settings' ' + test_when_finished "rm -f trace.out" && + ( + GIT_TRACE=$(pwd)/trace.out git status && + grep "1 tasks" trace.out && + >trace.out && + + git config submodule.diffJobs 8 && + GIT_TRACE=$(pwd)/trace.out git status && + grep "8 tasks" trace.out && + >trace.out && + + GIT_TRACE=$(pwd)/trace.out git -c submodule.diffJobs=0 status && + grep "preparing to run up to [0-9]* tasks" trace.out && + ! grep "up to 0 tasks" trace.out && + >trace.out + ) +' + test_done