From patchwork Thu Feb 9 00:02:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13133825 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 7AEB5C05027 for ; Thu, 9 Feb 2023 00:02:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230210AbjBIAC1 (ORCPT ); Wed, 8 Feb 2023 19:02:27 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48522 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230203AbjBIACX (ORCPT ); Wed, 8 Feb 2023 19:02:23 -0500 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 6D6151E9DD for ; Wed, 8 Feb 2023 16:02:19 -0800 (PST) Received: by mail-pg1-x54a.google.com with SMTP id k16-20020a635a50000000b0042986056df6so259351pgm.2 for ; Wed, 08 Feb 2023 16:02:19 -0800 (PST) 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=FxncT6tBVMpHpysI0Lok4V117zXff9hUB75vrzmQTBI=; b=p9sTyu5VBXtid+XdpJCTdd9FsR3vXkoxUhxIg9gcpRPMOk4cIQ5RUhHa3JN5ey5gwL KmHBkEFwG99dZOL5+EYreoJ+nlBfi+E1jFlxdjD8lws0z2bibM5YS8q7HsyMRzb/beBu rUakrUze1v8JGHfHktQ/3AocSoSFLzu3I6cMBFwZrlrzFiy+9ITrgR7jSJw8r1TTP7WB m8gcU4mSu/HHRJ3G0x3/KFU0dJAbtnajGE0l0XgkpyGHpU4U4oIfnsl0c4enM8spILs9 czrhKNabjvgMVr860KHVpfeMXpS7R8dJxwQFQpVSKZSZr3ISMhPo3AF8iTxCbxJZ36Jh L4hA== 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=FxncT6tBVMpHpysI0Lok4V117zXff9hUB75vrzmQTBI=; b=3s+5g5aMdt63J/4pV3h1yZIZ52Yu9iUwt5aYlADq2OM+9zPvrKsrnVxD5ifRiBb41t PDA46RSZBlt5jl57SGgecZfeArvexDWQUOhRtJCsMUaitg3AxYe4J3Dck5UGfW6obGpC Pllv/8fCOLlAiTIQAAzzWsR2FPx632ygwK5B3HVJLUuS0i+ysqeVcDf2i5bJ2PAUCmBw SXis2Q+wax91N8j9iKqoCaMWdwFouuBuT5Ls2ZkTBYwWB2Kwhg3ZfIm3/LgVc0vJAvN7 fQcHpBX+z9M9O7esWTBN/7Hm2/yPENdtAZK52+TmdOluGA4Rj7PnRQA2VCeMha4Kz0B5 BUFw== X-Gm-Message-State: AO0yUKUcJQMTqXxY1xEaAp/intk8ggxM6dySByv2NREDSkSMIdfV0fPr HqMBwfTwO2GXB1GRsZiMQX2bjnEJR9tuZl3aYdH4DiEKzzcK/7N19qXPjzdB2sHYWaiv6zgjNgY pq9AxZw5w0RkYfopVtAS5Vxi5kQElIzSWemCk0LhGjIdfqdpAWk3qkY9jhCEJsXzXuw== X-Google-Smtp-Source: AK7set8Hz8d4UP+N+HeuLJ2ishXd8x4CvtVVxYTo8Ikj8eoYqfyo6ijDHpNPcHwOD8fpTmvgTvZXrUWhw1pxDMU= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a63:8f41:0:b0:4fb:1449:8b3d with SMTP id r1-20020a638f41000000b004fb14498b3dmr1663348pgn.86.1675900938748; Wed, 08 Feb 2023 16:02:18 -0800 (PST) Date: Thu, 9 Feb 2023 00:02:07 +0000 In-Reply-To: <20230207181706.363453-1-calvinwan@google.com> Mime-Version: 1.0 References: <20230207181706.363453-1-calvinwan@google.com> X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Message-ID: <20230209000212.1892457-2-calvinwan@google.com> Subject: [PATCH v8 1/6] run-command: add duplicate_output_fn to run_processes_parallel_opts From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , avarab@gmail.com, chooglen@google.com, newren@gmail.com, jonathantanmy@google.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add duplicate_output_fn as an optionally set function in run_process_parallel_opts. If set, output from each child process is copied and passed to the callback function whenever output from the child process is buffered to allow for separate parsing. Fix two items in pp_buffer_stderr: * strbuf_read_once returns a ssize_t but the variable it is set to is an int so fix that. * Add missing brackets to "else if" statement The ungroup/duplicate_output incompatibility check is nested to prepare for future imcompatibles modes with ungroup. Signed-off-by: Calvin Wan --- run-command.c | 16 ++++++++++++--- run-command.h | 25 ++++++++++++++++++++++++ t/helper/test-run-command.c | 20 +++++++++++++++++++ t/t0061-run-command.sh | 39 +++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 3 deletions(-) diff --git a/run-command.c b/run-command.c index 756f1839aa..50f741f2ab 100644 --- a/run-command.c +++ b/run-command.c @@ -1526,6 +1526,11 @@ 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->ungroup) { + if (opts->duplicate_output) + BUG("duplicate_output and ungroup are incompatible with each other"); + } + CALLOC_ARRAY(pp->children, n); if (!opts->ungroup) CALLOC_ARRAY(pp->pfd, n); @@ -1645,14 +1650,19 @@ 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); + ssize_t n = strbuf_read_once(&pp->children[i].err, + pp->children[i].process.err, 0); if (n == 0) { close(pp->children[i].process.err); pp->children[i].state = GIT_CP_WAIT_CLEANUP; - } else if (n < 0) + } else if (n < 0) { if (errno != EAGAIN) die_errno("read"); + } else if (opts->duplicate_output) { + opts->duplicate_output(&pp->children[i].err, + pp->children[i].err.len - n, + opts->data, pp->children[i].data); + } } } } diff --git a/run-command.h b/run-command.h index 072db56a4d..0c16d7f251 100644 --- a/run-command.h +++ b/run-command.h @@ -408,6 +408,25 @@ typedef int (*start_failure_fn)(struct strbuf *out, void *pp_cb, void *pp_task_cb); +/** + * This callback is called whenever output from a child process is buffered + * + * See run_processes_parallel() below for a discussion of the "struct + * strbuf *out" parameter. + * + * The offset refers to the number of bytes originally in "out" before + * the output from the child process was buffered. Therefore, the buffer + * range, "out + buf" to the end of "out", would contain the buffer of + * the child process output. + * + * 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 (*duplicate_output_fn)(struct strbuf *out, size_t offset, + void *pp_cb, void *pp_task_cb); + /** * This callback is called on every child process that finished processing. * @@ -461,6 +480,12 @@ struct run_process_parallel_opts */ start_failure_fn start_failure; + /** + * duplicate_output: See duplicate_output_fn() above. Unless you need + * to capture output from child processes, leave this as NULL. + */ + duplicate_output_fn duplicate_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..4596ba68a8 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -52,6 +52,20 @@ static int no_job(struct child_process *cp, return 0; } +static void duplicate_output(struct strbuf *out, + size_t offset, + void *pp_cb UNUSED, + void *pp_task_cb UNUSED) +{ + struct string_list list = STRING_LIST_INIT_DUP; + struct string_list_item *item; + + string_list_split(&list, out->buf + offset, '\n', -1); + for_each_string_list_item(item, &list) + fprintf(stderr, "duplicate_output: %s\n", item->string); + string_list_clear(&list, 0); +} + static int task_finished(int result, struct strbuf *err, void *pp_cb, @@ -439,6 +453,12 @@ int cmd__run_command(int argc, const char **argv) opts.ungroup = 1; } + if (!strcmp(argv[1], "--duplicate-output")) { + argv += 1; + argc -= 1; + opts.duplicate_output = duplicate_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 e2411f6a9b..31f1db96fc 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -135,6 +135,15 @@ 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 --duplicate-output' ' + test-tool run-command --duplicate-output run-command-parallel 5 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test 4 = $(grep -c "duplicate_output: Hello" err) && + test 4 = $(grep -c "duplicate_output: World" err) && + sed "/duplicate_output/d" err >err1 && + test_cmp expect err1 +' + 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 && @@ -147,6 +156,15 @@ 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 --duplicate-output' ' + test-tool run-command --duplicate-output run-command-parallel 4 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test 4 = $(grep -c "duplicate_output: Hello" err) && + test 4 = $(grep -c "duplicate_output: World" err) && + sed "/duplicate_output/d" err >err1 && + test_cmp expect err1 +' + 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 && @@ -159,6 +177,15 @@ 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 --duplicate-output' ' + test-tool run-command --duplicate-output run-command-parallel 3 sh -c "printf \"%s\n%s\n\" Hello World" >out 2>err && + test_must_be_empty out && + test 4 = $(grep -c "duplicate_output: Hello" err) && + test 4 = $(grep -c "duplicate_output: World" err) && + sed "/duplicate_output/d" err >err1 && + test_cmp expect err1 +' + 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 && @@ -180,6 +207,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 --duplicate-output' ' + test-tool run-command --duplicate-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 && @@ -196,6 +229,12 @@ test_expect_success 'run_command outputs ' ' test_cmp expect actual ' +test_expect_success 'run_command outputs --duplicate-output' ' + test-tool run-command --duplicate-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 Feb 9 00:02:08 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13133826 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 43C8CC636D4 for ; Thu, 9 Feb 2023 00:02:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230225AbjBIAC3 (ORCPT ); Wed, 8 Feb 2023 19:02:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48466 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230221AbjBIACY (ORCPT ); Wed, 8 Feb 2023 19:02:24 -0500 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B4D951F5EC for ; Wed, 8 Feb 2023 16:02:21 -0800 (PST) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-5005ef73cf3so3204017b3.2 for ; Wed, 08 Feb 2023 16:02:21 -0800 (PST) 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=K46XHE2BLqdDbnyWSR0bU7t4XlcU/9xs6xJD00qC73Y=; b=oDnqvPKed0DuFaC0OU2OsbBP1SLR/R0STZfssp+5GzChtrX2r6nTKI5NIM8sGWsW9l BLdv5OWCv5eF9NUSNrQYFT6XZcPTjOCHjfD6MXuhzMS8j2tCq/OLQ+FvwXgAb+COb9/F KqfDmv5KZva7NeEFQZotK8ti9K4pJXfO2tNw+sK0p0rZJuynia4gu+cQLcT9Go+yFyMc F2onF/5DQqKvJy9LvY9Ik8UGbL6/9dhXPPNR7EWOia+UxX7ytvYGXeZ3IBSVyYqeUxtj UkE2LD+daByd2Uwc891jshz30C/bRFH9yHDRsf29qAYM8Fj7l9pWDFJrY8nus5GwgS8z IlJQ== 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=K46XHE2BLqdDbnyWSR0bU7t4XlcU/9xs6xJD00qC73Y=; b=hjmgNRmr+j1NTxCt6Wk0+P/LWgTTYLWS9DgBmO0W/xuLrSRTxK3ltf9zVZWAzxtEZR 8gozOYQiJuCF02qO8i4TDfuVMhSLi4vHXR0EkhFhu2wgGv1DwGOaUn+CHDFifQjyLW7O ijeRD4mV9FKxmkZMQZ4i4C/1gd8X6QG1jI2KZS8BZ+hjFf6cccrTURqVh8NPMhL37AAG w5iZwFXNTJgIc5QBFJoVLrvUJLDadA2aGWeN10PkkPzv7j8ws8dALDVpFZdcdF6cDO1s UQLaIy5jUoFu/sZbM88hA/5HUZwrFZEdE0fFlF6igJYLS5g+6yTKZZnEaYhP6bz9yIt8 lcdg== X-Gm-Message-State: AO0yUKWMYrm8w6XINtu92LVh+QNDuctALiBXXaTEDjfPRlZ+40FFJMMo WH9pPubecxamtjv7QwCZ7yMKm1BS5chcACgZooA57jhjE3T0m7gsMfIhvMqwhLNUMeJ4TqM5Td+ fQQjaI3syjpHmzFDazroQo78VzMgeMDrsjKkuy3RPcP8H5C6WEt4j6b/6+RCRjz81kA== X-Google-Smtp-Source: AK7set8bKina/gNC2LHiPpMysXlYLCrbnq0pRdfbV8Alt/RJpwu88D3g7Z9wie2kiKQLnle+HywQvdWZiUlYkMA= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a25:67c6:0:b0:8ad:5075:f78f with SMTP id b189-20020a2567c6000000b008ad5075f78fmr5ybc.0.1675900940267; Wed, 08 Feb 2023 16:02:20 -0800 (PST) Date: Thu, 9 Feb 2023 00:02:08 +0000 In-Reply-To: <20230207181706.363453-1-calvinwan@google.com> Mime-Version: 1.0 References: <20230207181706.363453-1-calvinwan@google.com> X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Message-ID: <20230209000212.1892457-3-calvinwan@google.com> Subject: [PATCH v8 2/6] submodule: strbuf variable rename From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , avarab@gmail.com, chooglen@google.com, newren@gmail.com, jonathantanmy@google.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 fae24ef34a..faf37c1101 100644 --- a/submodule.c +++ b/submodule.c @@ -1906,25 +1906,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 Feb 9 00:02:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13133827 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 21AEDC636D3 for ; Thu, 9 Feb 2023 00:02:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230236AbjBIACa (ORCPT ); Wed, 8 Feb 2023 19:02:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48496 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230208AbjBIACY (ORCPT ); Wed, 8 Feb 2023 19:02:24 -0500 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D1DF01F5ED for ; Wed, 8 Feb 2023 16:02:22 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id k15-20020a5b0a0f000000b007eba3f8e3baso357092ybq.4 for ; Wed, 08 Feb 2023 16:02:22 -0800 (PST) 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=xcCDJ/9InDiBfm3IVYXcLEw2c7g9o9VhW+yu1jBDgF0=; b=OVjPnV6zYLq0lYV86uuzkaNDCLIDK0OWSwJHmoahYzZ9dqygsVBxMlAhPnwKnXx5Z3 HcfFjEbEZSIOkeAEXnl+MeVelcPXptw495baipMRJgTd4FBFR5PCUGc3tyH+9wPdcMLO u34TBJcYN5ETyYKXMp6HKSxj8RfAH6jYDBUeQV7u/XfGlBZHffs+Fd8fd8FcfCIVrju6 EilsDd0JO8coKhlttoLJEApfiC9hGLEI19tyGm+kcRcCfUjuy1OyVE1PRodNdjLf44m8 jH/FVuZe5WXOuWTVkFdms9MsZU4uOBzectIlnnkijrQABKHxOOIg4S5fwms8xZzVzlvs +WLg== 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=xcCDJ/9InDiBfm3IVYXcLEw2c7g9o9VhW+yu1jBDgF0=; b=JNtrUzkYOlCn6QY5hHUA/lzx6cqxoXJ56h6ceUcihDHeLaS5tSv/kaplKmglRKA/aE N8Rxe4Def3E3qCgI6bdxFtC4ieuPUhCYDI0zrf5F3bKoCTtpsroNdSvL8A5xjvgkgaE+ taFlzgKimd3/bn8LEAPH0Nq2zGG5HWzcSwZ/2FSPmxrTG6LX9hGJdnXCOkAS+xqpe2Ny oYlbzdtzIPi15b+EcZ4fNUg312PxYk3YWDZU7MD1vAuwooGDUiWCz8ajwY4hThflrQfQ hyM7h1efIQ26bzi63amHyi1T8nxCfNO+FSdSBJa3cm+dwbn+lsx8PY9dpWtX2BELEs3h mrxg== X-Gm-Message-State: AO0yUKVh9lYlH+9usnsT4A+HQouYJyTKynb/a910L24j64/1+IB4EhTF yZaeOjzJ4rwash0wyIoQ3LN0p46NbWlpAhzvSy0p1nWk7boz7ARsRsw2twseaeRT7K/eE8lxa28 ZMYeq/1iJxkKsQyUlK0pc1EmTOCeBlQRhJIDWSQIS/NmDDDL7dbyT9Fx33cJy00WXPg== X-Google-Smtp-Source: AK7set/Njgs64aVhm84fyLt/0hRNVlt7FWE7Joqw3o6duoVYMfJBMvPsVTymu5yp1T7LQ/PiM3ANS7hF6NIlm40= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a05:6902:2c6:b0:801:f3b:524f with SMTP id w6-20020a05690202c600b008010f3b524fmr984657ybh.167.1675900942060; Wed, 08 Feb 2023 16:02:22 -0800 (PST) Date: Thu, 9 Feb 2023 00:02:09 +0000 In-Reply-To: <20230207181706.363453-1-calvinwan@google.com> Mime-Version: 1.0 References: <20230207181706.363453-1-calvinwan@google.com> X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Message-ID: <20230209000212.1892457-4-calvinwan@google.com> Subject: [PATCH v8 3/6] submodule: move status parsing into function From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , avarab@gmail.com, chooglen@google.com, newren@gmail.com, jonathantanmy@google.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 faf37c1101..768d4b4cd7 100644 --- a/submodule.c +++ b/submodule.c @@ -1870,6 +1870,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; @@ -1909,39 +1948,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 Feb 9 00:02:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13133828 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 71261C05027 for ; Thu, 9 Feb 2023 00:02:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230240AbjBIACd (ORCPT ); Wed, 8 Feb 2023 19:02:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48514 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230214AbjBIACZ (ORCPT ); Wed, 8 Feb 2023 19:02:25 -0500 Received: from mail-pj1-x104a.google.com (mail-pj1-x104a.google.com [IPv6:2607:f8b0:4864:20::104a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 33DB4977B for ; Wed, 8 Feb 2023 16:02:24 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id pb12-20020a17090b3c0c00b0023086f62807so164610pjb.2 for ; Wed, 08 Feb 2023 16:02:24 -0800 (PST) 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=NmBBXJHCXpbYnel3K3C7IJf0ydfzcb1xIWdlOlgOx3E=; b=QFaY5z50niXRJAD8lecsGXlIUJOIf/NU+qbKTq3Oa7Css0h7lh2gNf8XvN7DRvwnoe NZZqg8bMNx7TJ3iTI0NWhHxybMhlAY0l05AJDuuWgDvw/4MTvYr2R+5IkVlm2Gp5215o Yi2LVuafd8I4m+FAUpNvagOB3RPc0P/kj86WcyIKzePMF/BPuARgC6igTn5ObW4HGIC6 vkGOVwJ0gPx951BkBzYNh8u0qSg6wPbWu7QI4qiJCrY31ttPBikL30ssoM7sXi9HxNlM eQjLW6ZbScd0pzz855pbWBA1FA9ANEZJiITwLJvPdDWNVOdhQsoIHN6j2o9n78h9N9px t5qw== 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=NmBBXJHCXpbYnel3K3C7IJf0ydfzcb1xIWdlOlgOx3E=; b=wMfNxEcEhL4j1Hgku6Z0+4ZoI08+eyMV9AfhhIo1td7U4Op0QntrX2djWWzyzjCdHt 9I/eKQOXxCS0hSit6n/0EM0ZvqrHikKugXAqfcg81eJg5J4APmoONubZeHZfpCvd2kvb vktf+5ybdmtYWR3s1lvLFbwf2nUGy9vxEGoFELRqSIJIWlL6AWRy6j3rqM+M2MWs8i8m gO1dQA9ICw9rY9VmmiI1Fxmr8f++YO4lUmfYQ5laAQrRuwgOZ+tV5oDcxUQe1P57EO6J xbGZHqIgi1iTWTZY/rPZNjSxIHlMyo/sUnQl1HCywqgvy1HMegpVWPtIsu3TB0ywNw5l I3+A== X-Gm-Message-State: AO0yUKUD5BCugGmCoYbVFgfgNgQ6uE/bxcJ3e2sjJ6TTJ3bwA837rw5B 6ZPNi0dyDwWYaRq6ySudo8GYUHWKvcGD0fSl2vZ/m2q5YGTlozbUW13ZSwRrOEzhrIZptRS/WTd TYn5wrIRbT4Bl9tpSUscK9fJZhtaUA8/WRGepQ46K979VTySloLb86m3IWnkl2i4tUw== X-Google-Smtp-Source: AK7set/WcxVCszWIpM8w7J2xpl/j/RUCkEGQwgEMAbQdkd+62AZMXcvf0P4yUSS4wUkE4BKHKcdLR2pfimolnVc= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a17:903:258e:b0:192:63c3:7b5f with SMTP id jb14-20020a170903258e00b0019263c37b5fmr2332902plb.28.1675900943602; Wed, 08 Feb 2023 16:02:23 -0800 (PST) Date: Thu, 9 Feb 2023 00:02:10 +0000 In-Reply-To: <20230207181706.363453-1-calvinwan@google.com> Mime-Version: 1.0 References: <20230207181706.363453-1-calvinwan@google.com> X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Message-ID: <20230209000212.1892457-5-calvinwan@google.com> Subject: [PATCH v8 4/6] submodule: refactor is_submodule_modified() From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , avarab@gmail.com, chooglen@google.com, newren@gmail.com, jonathantanmy@google.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Refactor out submodule status logic and error messages that will be used in a future patch. Signed-off-by: Calvin Wan --- submodule.c | 65 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/submodule.c b/submodule.c index 768d4b4cd7..426074cebb 100644 --- a/submodule.c +++ b/submodule.c @@ -28,6 +28,10 @@ static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; static int initialized_fetch_ref_tips; static struct oid_array ref_tips_before_fetch; static struct oid_array ref_tips_after_fetch; +#define STATUS_PORCELAIN_START_ERROR \ + N_("could not run 'git status --porcelain=2' in submodule %s") +#define STATUS_PORCELAIN_FAIL_ERROR \ + N_("'git status --porcelain=2' failed in submodule %s") /* * Check if the .gitmodules file is unmerged. Parsing of the .gitmodules file @@ -1870,6 +1874,40 @@ int fetch_submodules(struct repository *r, return spf.result; } +static int verify_submodule_git_directory(const char *path) +{ + const char *git_dir; + struct strbuf buf = STRBUF_INIT; + + strbuf_addf(&buf, "%s/.git", path); + 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 */ + return 0; + } + strbuf_release(&buf); + return 1; +} + +static void prepare_status_porcelain(struct child_process *cp, + const char *path, int ignore_untracked) +{ + strvec_pushl(&cp->args, "status", "--porcelain=2", NULL); + if (ignore_untracked) + strvec_push(&cp->args, "-uno"); + + prepare_submodule_repo_env(&cp->env); + cp->git_cmd = 1; + cp->no_stdin = 1; + cp->out = -1; + cp->dir = path; +} + static int parse_status_porcelain(char *str, size_t len, unsigned *dirty_submodule, int ignore_untracked) @@ -1915,33 +1953,14 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) struct strbuf buf = STRBUF_INIT; FILE *fp; unsigned dirty_submodule = 0; - const char *git_dir; int ignore_cp_exit_code = 0; - strbuf_addf(&buf, "%s/.git", path); - 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 */ + if (!verify_submodule_git_directory(path)) return 0; - } - strbuf_reset(&buf); - - strvec_pushl(&cp.args, "status", "--porcelain=2", NULL); - if (ignore_untracked) - strvec_push(&cp.args, "-uno"); - prepare_submodule_repo_env(&cp.env); - cp.git_cmd = 1; - cp.no_stdin = 1; - cp.out = -1; - cp.dir = path; + prepare_status_porcelain(&cp, path, ignore_untracked); if (start_command(&cp)) - die(_("Could not run 'git status --porcelain=2' in submodule %s"), path); + die(_(STATUS_PORCELAIN_START_ERROR), path); fp = xfdopen(cp.out, "r"); while (strbuf_getwholeline(&buf, fp, '\n') != EOF) { @@ -1956,7 +1975,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) fclose(fp); if (finish_command(&cp) && !ignore_cp_exit_code) - die(_("'git status --porcelain=2' failed in submodule %s"), path); + die(_(STATUS_PORCELAIN_FAIL_ERROR), path); strbuf_release(&buf); return dirty_submodule; From patchwork Thu Feb 9 00:02:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13133829 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 C663CC636D3 for ; Thu, 9 Feb 2023 00:02:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230243AbjBIACe (ORCPT ); Wed, 8 Feb 2023 19:02:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48456 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230220AbjBIAC0 (ORCPT ); Wed, 8 Feb 2023 19:02:26 -0500 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 080C91BAD5 for ; Wed, 8 Feb 2023 16:02:26 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id 144-20020a621896000000b0059e73803cdcso131280pfy.12 for ; Wed, 08 Feb 2023 16:02:26 -0800 (PST) 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=ZYwZxXuZCKch3x+Vy3R4rbmwqBPB6ajawKXYoGOyBuA=; b=oMkO9Nws3ldwuNHj6uVGWLxlV49DJ8hRQa2bxXa7L39NHhDHOPsWXR5zKS4JWp53HL ywRqpshmZMk6PIp3jDu64gZQDvB9E7Fp+Okkziby9pnz7GCT0BAPXoXudD2apqP5zNMR YZhXcN1WWjbnLoLbJP9lQiXqIQEbENQl2U8Mfp047lAySY6mGJOXBH/xm/u4Fr2H5DfR Ow181OT6T8L8Wxz5A96ok1T85/xnAtIhLFzU38FS4JpN2CNgIlzJytSfEzW457xe7BW1 LIyG9faWIgxseL405PJFLHuIxVdbWdyoQZLyLYaWARzC6ttVJ4Zv4cz2Ck+X7OdwOFNi ukrQ== 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=ZYwZxXuZCKch3x+Vy3R4rbmwqBPB6ajawKXYoGOyBuA=; b=i+jSEEc0L4Uo6bwrPSWvHEc7mjhCMjbLaEbhjbZCk+tXS8GwHndl4pwTKrcq5ThyxE RKPNvgY8CHVFlFCX2TgskwgwgY/+9f0BZ/wdQ0iz7Yl+RUo96lONLqlsWteZkLeAYOoj 4OfjCZ19f5lIxxFSlTDwKBCmErXFPHxBpm7HRHbae6kfMVjpbegPpwaR9SaGWuBvXPGD XplcZtaOSg+cJZHJW53rirIzdqpWNTw1vdENGEr+WWRvEfnSK1blDMBMIPCvXI18oYQ2 3xzhVsEQU3dHml0zL6IphCLhkPQnnHcDlkvfldcGOwTyZ5Lr8pkf+LyAfGSJul9NAv33 +maw== X-Gm-Message-State: AO0yUKUxrWr++jI4lGJFqY9D3oBAOTRg7PwXlGm4s6n3w55NKiVYjeAh baFwmo2yBm5CqjnbxD2x5EFZ8pIBj5wkUgKBZrvy+EwivQMZnEGAVuDwvLOdGWJXyTBE9J3Hmit pQ1sNconV4E4bAxObD8O2dzviaTAHm74VJCUFhvgpDI46swImhLVqDajS5iDDHSFukw== X-Google-Smtp-Source: AK7set9TnznOiGpLV7etb3ZLoXDBfQGBV+/ayKzdm1dyv9BSqK2O0wkHXv3V9nr9sXdH3iicg9HZJ7hCIHJNpMY= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:a17:90a:8b84:b0:230:ee6f:28bc with SMTP id z4-20020a17090a8b8400b00230ee6f28bcmr8450pjn.1.1675900945027; Wed, 08 Feb 2023 16:02:25 -0800 (PST) Date: Thu, 9 Feb 2023 00:02:11 +0000 In-Reply-To: <20230207181706.363453-1-calvinwan@google.com> Mime-Version: 1.0 References: <20230207181706.363453-1-calvinwan@google.com> X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Message-ID: <20230209000212.1892457-6-calvinwan@google.com> Subject: [PATCH v8 5/6] diff-lib: refactor out diff_change logic From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , avarab@gmail.com, chooglen@google.com, newren@gmail.com, jonathantanmy@google.com, phillip.wood123@gmail.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Refactor out logic that sets up the diff_change call into a helper function for a future patch. Signed-off-by: Calvin Wan --- diff-lib.c | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index dec040c366..7101cfda3f 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -88,6 +88,31 @@ static int match_stat_with_submodule(struct diff_options *diffopt, return changed; } +static int diff_change_helper(struct diff_options *options, + unsigned newmode, unsigned dirty_submodule, + int changed, struct index_state *istate, + struct cache_entry *ce) +{ + unsigned int oldmode; + const struct object_id *old_oid, *new_oid; + + if (!changed && !dirty_submodule) { + ce_mark_uptodate(ce); + mark_fsmonitor_valid(istate, ce); + if (!options->flags.find_copies_harder) + return 1; + } + oldmode = ce->ce_mode; + old_oid = &ce->oid; + new_oid = changed ? null_oid() : &ce->oid; + diff_change(options, oldmode, newmode, + old_oid, new_oid, + !is_null_oid(old_oid), + !is_null_oid(new_oid), + ce->name, 0, dirty_submodule); + return 0; +} + int run_diff_files(struct rev_info *revs, unsigned int option) { int entries, i; @@ -105,11 +130,10 @@ int run_diff_files(struct rev_info *revs, unsigned int option) diff_unmerged_stage = 2; entries = istate->cache_nr; for (i = 0; i < entries; i++) { - unsigned int oldmode, newmode; + unsigned int newmode; struct cache_entry *ce = istate->cache[i]; int changed; unsigned dirty_submodule = 0; - const struct object_id *old_oid, *new_oid; if (diff_can_quit_early(&revs->diffopt)) break; @@ -245,21 +269,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option) newmode = ce_mode_from_stat(ce, st.st_mode); } - if (!changed && !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 = changed ? null_oid() : &ce->oid; - diff_change(&revs->diffopt, oldmode, newmode, - old_oid, new_oid, - !is_null_oid(old_oid), - !is_null_oid(new_oid), - ce->name, 0, dirty_submodule); - + if (diff_change_helper(&revs->diffopt, newmode, dirty_submodule, + changed, istate, ce)) + continue; } diffcore_std(&revs->diffopt); diff_flush(&revs->diffopt); From patchwork Thu Feb 9 00:02:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Wan X-Patchwork-Id: 13133830 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 D79E3C6379F for ; Thu, 9 Feb 2023 00:02:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230253AbjBIACg (ORCPT ); Wed, 8 Feb 2023 19:02:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230221AbjBIAC3 (ORCPT ); Wed, 8 Feb 2023 19:02:29 -0500 Received: from mail-pg1-x549.google.com (mail-pg1-x549.google.com [IPv6:2607:f8b0:4864:20::549]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5B0C01F5DD for ; Wed, 8 Feb 2023 16:02:27 -0800 (PST) Received: by mail-pg1-x549.google.com with SMTP id r126-20020a632b84000000b004393806c06eso259212pgr.4 for ; Wed, 08 Feb 2023 16:02:27 -0800 (PST) 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=fZ8P+iFvLwZz+A/MsVMwA/SDS9YJo2o6hfl3X1HRuok=; b=MV99hhytA9vHMbB+N9rQi/0OcfsA2UTm88dTzFs2ktRI6RdJeBwtAnpCCzoALpHTxC eVB7ovLODqFsIUzg/W8R75RiF3lZyw4bwXRCEyE/EJclMjdOXrnfgSFsgv/s+N7vfvV8 GFzG1oVIkohfqHFw+lUnW7iLuIrZGCINWv5CkBxCG5sFq4dGrBY9cOD7FGxyrvnJ9t5h kCzQIkX3iQzUeVXn2CELzxfzkrUwmK/3kZvCHbCvifIlDu4cnHxiSd7vUHgRK9xgmgm7 VZUWch4huU3qaN0Mgu8tAMguVjT0Zh1aALrdjCvddjtl3cHKnWvE/N1DyBtZKDUWmmM+ BUHw== 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=fZ8P+iFvLwZz+A/MsVMwA/SDS9YJo2o6hfl3X1HRuok=; b=r441WFSYov47mHXpCyUmHO5wO/znt1HJZJqSlYVyhBWl78MWsF3dWOhkNJQovlmvWr r3LAA3JccPMfJCuADXudBuXpu29Pa2vFiySuPMm6apVzVKS1/qWBBTYK2q+fqsay7vfn xpSxg/6QToqk0quwXfpLhHJBiyCBpTO+UcWS0g41B+BuPYIE+rSZb93/5drTzZ4qVfKo ob9LAHTsMjjVTdkFUUPYJ4GSvspmwziKHnLkec6vj9395yRGS2jhXA57uHgvERoMzXYE Z/TuC9GjX1MIetRNTFEOQxFX0GFKt8RUzJa3/3A6cGU9vbaWzM0iKFA0BW6ZBE/ubQpL 4r5A== X-Gm-Message-State: AO0yUKWYg9cRJc+NdMcpiDfNR0sHVOSwQYybJRXQOECB2TWVP9aZrIOE mSoTsMP8Y7FQqyCqmdP1mF8yAMXnveVc1W2OM0jho896FYQKDo75D5PcQRGAtnhwa5HWEHen8+/ 7uuT9tmOaOwu+eu/WCE0bg9azFcyBYukyabQhX862BTp4lvhrIDE2P9x9K0Fz1vDeqQ== X-Google-Smtp-Source: AK7set8PbrIC7pzJlSaCAUVHt/ZoN2glx+0+gf7XfCW4GKk+EqJ7OaiuwVIEtmweKX4t+zwU7DW1dn4fEfTBLqg= X-Received: from barleywine.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:3bd4]) (user=calvinwan job=sendgmr) by 2002:aa7:9d8e:0:b0:5a8:2b97:e92d with SMTP id f14-20020aa79d8e000000b005a82b97e92dmr962990pfq.36.1675900946707; Wed, 08 Feb 2023 16:02:26 -0800 (PST) Date: Thu, 9 Feb 2023 00:02:12 +0000 In-Reply-To: <20230207181706.363453-1-calvinwan@google.com> Mime-Version: 1.0 References: <20230207181706.363453-1-calvinwan@google.com> X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Message-ID: <20230209000212.1892457-7-calvinwan@google.com> Subject: [PATCH v8 6/6] diff-lib: parallelize run_diff_files for submodules From: Calvin Wan To: git@vger.kernel.org Cc: Calvin Wan , avarab@gmail.com, chooglen@google.com, newren@gmail.com, jonathantanmy@google.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 duplicated and passed to status_pipe_output which stores it to be parsed on completion of the subprocess. 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 | 91 ++++++++++++++++--- submodule.c | 140 +++++++++++++++++++++++++++++ submodule.h | 9 ++ t/t4027-diff-submodule.sh | 31 +++++++ t/t7506-status-submodule.sh | 25 ++++++ 6 files changed, 294 insertions(+), 14 deletions(-) diff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt index 6490527b45..3209eb8117 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 some reasonable default. + 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 7101cfda3f..2dde575ec6 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,26 +66,46 @@ 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); - 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)) + struct diff_flags orig_flags; + int defer = 0; + + 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)) { + 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); - diffopt->flags = orig_flags; + diffopt->flags.ignore_untracked_in_submodules); + } } +cleanup: + diffopt->flags = orig_flags; +ret: + if (defer_submodule_status) + *defer_submodule_status = defer; return changed; } @@ -121,6 +142,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/"); @@ -244,6 +266,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 = 1; changed = check_removed(istate, ce, &st); if (changed) { @@ -265,14 +289,53 @@ 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, + .path = ce->name, + }; + 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 (diff_change_helper(&revs->diffopt, newmode, dirty_submodule, changed, istate, ce)) continue; } + if (submodules.nr) { + unsigned long parallel_jobs; + struct string_list_item *item; + + if (git_config_get_ulong("submodule.diffjobs", ¶llel_jobs)) + parallel_jobs = 1; + else if (!parallel_jobs) + parallel_jobs = online_cpus(); + + if (get_submodules_status(&submodules, parallel_jobs)) + die(_("submodule status failed")); + for_each_string_list_item(item, &submodules) { + struct submodule_status_util *util = item->util; + + if (diff_change_helper(&revs->diffopt, util->newmode, + util->dirty_submodule, util->changed, + istate, util->ce)) + continue; + } + } + string_list_clear(&submodules, 1); diffcore_std(&revs->diffopt); diff_flush(&revs->diffopt); trace_performance_since(start, "diff-files"); @@ -320,7 +383,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 426074cebb..e175fb8d3f 100644 --- a/submodule.c +++ b/submodule.c @@ -1373,6 +1373,13 @@ 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 submodule_parallel_fetch { /* * The index of the last index entry processed by @@ -1455,6 +1462,12 @@ struct fetch_task { struct oid_array *commits; /* Ensure these commits are fetched */ }; +struct status_task { + const char *path; + struct strbuf out; + 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 @@ -1947,6 +1960,25 @@ static int parse_status_porcelain(char *str, size_t len, return 0; } +static void parse_status_porcelain_strbuf(struct strbuf *buf, + unsigned *dirty_submodule, + int ignore_untracked) +{ + struct string_list list = STRING_LIST_INIT_DUP; + struct string_list_item *item; + + string_list_split(&list, buf->buf, '\n', -1); + + for_each_string_list_item(item, &list) { + if (parse_status_porcelain(item->string, + strlen(item->string), + dirty_submodule, + ignore_untracked)) + break; + } + string_list_clear(&list, 0); +} + unsigned is_submodule_modified(const char *path, int ignore_untracked) { struct child_process cp = CHILD_PROCESS_INIT; @@ -1981,6 +2013,114 @@ 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; + struct status_task *task; + + if (!verify_submodule_git_directory(util->path)) + continue; + + task = xmalloc(sizeof(*task)); + task->path = util->path; + task->ignore_untracked = util->ignore_untracked; + strbuf_init(&task->out, 0); + 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); + prepare_status_porcelain(cp, task->path, task->ignore_untracked); + *task_cb = task; + return 1; +} + +static int status_start_failure(struct strbuf *err, + void *cb, void *task_cb) +{ + struct submodule_parallel_status *sps = cb; + struct status_task *task = task_cb; + + sps->result = 1; + strbuf_addf(err, _(STATUS_PORCELAIN_START_ERROR), task->path); + return 0; +} + +static void status_duplicate_output(struct strbuf *out, + size_t offset, + void *cb, void *task_cb) +{ + struct status_task *task = task_cb; + + strbuf_add(&task->out, out->buf + offset, out->len - offset); + strbuf_setlen(out, offset); +} + +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; + + if (retvalue) { + sps->result = 1; + strbuf_addf(err, _(STATUS_PORCELAIN_FAIL_ERROR), task->path); + } + + parse_status_porcelain_strbuf(&task->out, + &util->dirty_submodule, + util->ignore_untracked); + + strbuf_release(&task->out); + free(task); + + return 0; +} + +int get_submodules_status(struct string_list *submodules, + int max_parallel_jobs) +{ + struct submodule_parallel_status sps = { + .submodule_names = submodules, + }; + const struct run_process_parallel_opts opts = { + .tr2_category = "submodule", + .tr2_label = "parallel/status", + + .processes = max_parallel_jobs, + + .get_next_task = get_next_submodule_status, + .start_failure = status_start_failure, + .duplicate_output = status_duplicate_output, + .task_finished = status_finish, + .data = &sps, + }; + + 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 b52a4ff1e7..08d278a414 100644 --- a/submodule.h +++ b/submodule.h @@ -41,6 +41,13 @@ struct submodule_update_strategy { .type = SM_UPDATE_UNSPECIFIED, \ } +struct submodule_status_util { + int changed, ignore_untracked; + unsigned dirty_submodule, newmode; + struct cache_entry *ce; + const char *path; +}; + 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 +101,8 @@ int fetch_submodules(struct repository *r, int command_line_option, int default_option, int quiet, int max_parallel_jobs); +int get_submodules_status(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..1c747cc325 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 && @@ -70,6 +89,18 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree)' ' test_cmp expect.body actual.body ' +test_expect_success 'git diff HEAD with dirty submodule (work tree, parallel)' ' + ( + cd sub && + git reset --hard && + echo >>world + ) && + git -c submodule.diffJobs=8 diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev-dirty && + test_cmp expect.body actual.body +' + test_expect_success 'git diff HEAD with dirty submodule (index)' ' ( cd sub && diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index d050091345..7da64e4c4c 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -412,4 +412,29 @@ 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_expect_success 'status in superproject with submodules (parallel)' ' + git -C super status --porcelain >output && + git -C super -c submodule.diffJobs=8 status --porcelain >output_parallel && + diff output output_parallel +' + test_done