From patchwork Tue Feb 15 17:23:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747412 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 1A99CC433FE for ; Tue, 15 Feb 2022 17:23:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242470AbiBORYC (ORCPT ); Tue, 15 Feb 2022 12:24:02 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60354 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242465AbiBORYA (ORCPT ); Tue, 15 Feb 2022 12:24:00 -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 EA38CDA85E for ; Tue, 15 Feb 2022 09:23:49 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id i16-20020aa78d90000000b004be3e88d746so14424232pfr.13 for ; Tue, 15 Feb 2022 09:23:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=66KfgZMamYnH+VV69MTVJzWXHjEOzcYsC2QaLw8JH38=; b=GzFC1FaWzqG3n9uLYEj3b4USKsZC3IumihTwWXNDyCsZZqLU7TaczGANwl4MnhSle9 8ygBVxlAWCxJaQjOYeKcZ/EH1gbvF6dF5CK+7qKlInm6uQC03XkQnn0xufABrdWXzUIT UCsHdnhIAjjy+Hwjh1FB8KxmByQ/ipiHQLdCFjeM+pKdpcJhjYft9w/eCLxiIrpTv38t KGIRqaUXKkbsG4gi5xHpku3qB6laY/pOXdAjRk8lad4of4MjuIEyCSa/Aht9wpM1uUV7 WHVBCQYajP8jmazen5pj3M2jyCnpOLxaMhhnRX2UnfCR2UlXIWM8VpgjZ/eblGkIQAWP VxAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=66KfgZMamYnH+VV69MTVJzWXHjEOzcYsC2QaLw8JH38=; b=8Kzb9EwkHpDdNK1HrRHNRfjEu93Ex9ugNtpnVo4grxvPaZv90adfgjQWNI9KgIQLoM /aeHx3gJDQQyADchhdTZhBQsMJFBIjYoR/TJWTWu2AgsaTDbc8fn0UcDozDhtXuHeA9U BIrqKY5eHH5Df9E+2P2qHDvmCBO5K9sqg38eEoyRAvK4ByIvOanYaDEnbSxM8H0J79WM 48o7/if4pA0Q+fnhLO5hwjp3QtTqJOHTDjhUZRcPrTURSavvx1K9T3Rqkg0yIpmQzO3c EIPIhhD5Mqpix/1QUrqvlHw+QsbuvGDddTIXtI0FwYsTGqHtCna82z5JVU6d4nh+WUz0 n2XA== X-Gm-Message-State: AOAM532ZQhV1LLEMDWtM4s01Zo2F3PCBmgdYXOXH8zUgb3uthZZQdepf 0QCmHOB0w2Lwmqvo+hmL9OEfbHEIVJgxJPhFz2lk+t7atKFgTfgTR59o0BQLVdgaGGudAVwgnUI 27bF/fuLu/0Yi8Cynlrt9YfPLhrFD6aYhcY2SmutbsCfciLsdEyua3FgLTjqTBQk= X-Google-Smtp-Source: ABdhPJzGPXY2R1pEsDQiR7Gqt7njTTx+CM9oM6gvzN1a41MgUvkOTm260s/fcVN5NRZpAMjS1GHhhig5J4UREQ== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:b83:: with SMTP id g3mr5433558pfj.58.1644945829285; Tue, 15 Feb 2022 09:23:49 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:10 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-2-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 1/9] t5526: introduce test helper to assert on fetches From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A future commit will change the stderr of "git fetch --recurse-submodules" and add new tests to t/t5526-fetch-submodules.sh. This poses two challenges: * The tests use test_cmp to assert on the stderr, which will fail on the future test because the stderr changes slightly, even though it still contains the information we expect. * The expect.err file is constructed by the add_upstream_commit() helper as input into test_cmp, but most tests fetch a different combination of repos from expect.err. This results in noisy tests that modify parts of that expect.err to generate the expected output. To address both of these issues, introduce a verify_fetch_result() helper to t/t5526-fetch-submodules.sh that asserts on the output of "git fetch --recurse-submodules" and handles the ordering of expect.err. As a result, the tests no longer construct expect.err manually. test_cmp is still invoked by verify_fetch_result(), but that will be replaced in a later commit. Signed-off-by: Glen Choo --- t/t5526-fetch-submodules.sh | 136 +++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 55 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 2dc75b80db..0e93df1665 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -13,6 +13,10 @@ export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB pwd=$(pwd) +# For each submodule in the test setup, this creates a commit and writes +# a file that contains the expected err if that new commit were fetched. +# These output files get concatenated in the right order by +# verify_fetch_result(). add_upstream_commit() { ( cd submodule && @@ -22,9 +26,9 @@ add_upstream_commit() { git add subfile && git commit -m new subfile && head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule" > ../expect.err && - echo "From $pwd/submodule" >> ../expect.err && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err + echo "Fetching submodule submodule" > ../expect.err.sub && + echo "From $pwd/submodule" >> ../expect.err.sub && + echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub ) && ( cd deepsubmodule && @@ -34,12 +38,33 @@ add_upstream_commit() { git add deepsubfile && git commit -m new deepsubfile && head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule/subdir/deepsubmodule" >> ../expect.err - echo "From $pwd/deepsubmodule" >> ../expect.err && - echo " $head1..$head2 deep -> origin/deep" >> ../expect.err + echo "Fetching submodule submodule/subdir/deepsubmodule" > ../expect.err.deep + echo "From $pwd/deepsubmodule" >> ../expect.err.deep && + echo " $head1..$head2 deep -> origin/deep" >> ../expect.err.deep ) } +# Verifies that the expected repositories were fetched. This is done by +# concatenating the files expect.err.[super|sub|deep] in the correct +# order and comparing it to the actual stderr. +# +# If a repo should not be fetched in the test, its corresponding +# expect.err file should be rm-ed. +verify_fetch_result() { + ACTUAL_ERR=$1 && + rm -f expect.err.combined && + if [ -f expect.err.super ]; then + cat expect.err.super >>expect.err.combined + fi && + if [ -f expect.err.sub ]; then + cat expect.err.sub >>expect.err.combined + fi && + if [ -f expect.err.deep ]; then + cat expect.err.deep >>expect.err.combined + fi && + test_cmp expect.err.combined $ACTUAL_ERR +} + test_expect_success setup ' mkdir deepsubmodule && ( @@ -77,7 +102,7 @@ test_expect_success "fetch --recurse-submodules recurses into submodules" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "submodule.recurse option triggers recursive fetch" ' @@ -87,7 +112,7 @@ test_expect_success "submodule.recurse option triggers recursive fetch" ' git -c submodule.recurse fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "fetch --recurse-submodules -j2 has the same output behaviour" ' @@ -97,7 +122,7 @@ test_expect_success "fetch --recurse-submodules -j2 has the same output behaviou GIT_TRACE="$TRASH_DIRECTORY/trace.out" git fetch --recurse-submodules -j2 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err && + verify_fetch_result actual.err && grep "2 tasks" trace.out ' @@ -127,7 +152,7 @@ test_expect_success "using fetchRecurseSubmodules=true in .gitmodules recurses i git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--no-recurse-submodules overrides .gitmodules config" ' @@ -158,7 +183,7 @@ test_expect_success "--recurse-submodules overrides fetchRecurseSubmodules setti git config --unset submodule.submodule.fetchRecurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--quiet propagates to submodules" ' @@ -186,7 +211,7 @@ test_expect_success "--dry-run propagates to submodules" ' git fetch --recurse-submodules --dry-run >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "Without --dry-run propagates to submodules" ' @@ -195,7 +220,7 @@ test_expect_success "Without --dry-run propagates to submodules" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "recurseSubmodules=true propagates into submodules" ' @@ -206,7 +231,7 @@ test_expect_success "recurseSubmodules=true propagates into submodules" ' git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--recurse-submodules overrides config in submodule" ' @@ -220,7 +245,7 @@ test_expect_success "--recurse-submodules overrides config in submodule" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--no-recurse-submodules overrides config setting" ' @@ -253,14 +278,14 @@ test_expect_success "Recursion stops when no new submodule commits are fetched" git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.sub && - echo " $head1..$head2 super -> origin/super" >>expect.err.sub && - head -3 expect.err >> expect.err.sub && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && - test_cmp expect.err.sub actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -271,14 +296,16 @@ test_expect_success "Recursion doesn't happen when new superproject commits don' git add file && git commit -m "new file" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.file && - echo " $head1..$head2 super -> origin/super" >> expect.err.file && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + rm expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err.file actual.err + verify_fetch_result actual.err ' test_expect_success "Recursion picks up config in submodule" ' @@ -295,9 +322,8 @@ test_expect_success "Recursion picks up config in submodule" ' git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.sub && - echo " $head1..$head2 super -> origin/super" >> expect.err.sub && - cat expect.err >> expect.err.sub && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && ( cd downstream && git fetch >../actual.out 2>../actual.err && @@ -306,7 +332,7 @@ test_expect_success "Recursion picks up config in submodule" ' git config --unset fetch.recurseSubmodules ) ) && - test_cmp expect.err.sub actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -331,15 +357,13 @@ test_expect_success "Recursion picks up all submodules when necessary" ' git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >> expect.err.2 && - cat expect.err.sub >> expect.err.2 && - tail -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && - test_cmp expect.err.2 actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -375,11 +399,8 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - tail -3 expect.err > expect.err.deepsub && - echo "From $pwd/." > expect.err && - echo " $head1..$head2 super -> origin/super" >>expect.err && - cat expect.err.sub >> expect.err && - cat expect.err.deepsub >> expect.err && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && ( cd downstream && git config fetch.recurseSubmodules false && @@ -395,7 +416,7 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess ) ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' @@ -405,14 +426,16 @@ test_expect_success "'--recurse-submodules=on-demand' stops when no new submodul git add file && git commit -m "new file" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.file && - echo " $head1..$head2 super -> origin/super" >> expect.err.file && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + rm expect.err.deep && ( cd downstream && git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err.file actual.err + verify_fetch_result actual.err ' test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config" ' @@ -426,9 +449,9 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git config fetch.recurseSubmodules on-demand && @@ -440,7 +463,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config git config --unset fetch.recurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err + verify_fetch_result actual.err ' test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' overrides fetch.recurseSubmodules" ' @@ -454,9 +477,9 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git config submodule.submodule.fetchRecurseSubmodules on-demand && @@ -468,7 +491,7 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override git config --unset submodule.submodule.fetchRecurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err + verify_fetch_result actual.err ' test_expect_success "don't fetch submodule when newly recorded commits are already present" ' @@ -480,14 +503,17 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea git add submodule && git commit -m "submodule rewound" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err && - echo " $head1..$head2 super -> origin/super" >> expect.err && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + # This file does not exist, but rm -f for readability + rm -f expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err && + verify_fetch_result actual.err && ( cd submodule && git checkout -q sub @@ -505,9 +531,9 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git rm .gitmodules && git commit -m "new submodule without .gitmodules" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." >expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >>expect.err.2 && + echo "From $pwd/." >expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && rm .gitmodules && @@ -523,7 +549,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git reset --hard ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err && + verify_fetch_result actual.err && git checkout HEAD^ -- .gitmodules && git add .gitmodules && git commit -m "new submodule restored .gitmodules" From patchwork Tue Feb 15 17:23:11 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747414 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 79E85C433EF for ; Tue, 15 Feb 2022 17:24:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242475AbiBORYH (ORCPT ); Tue, 15 Feb 2022 12:24:07 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60372 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242465AbiBORYC (ORCPT ); Tue, 15 Feb 2022 12:24:02 -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 36BDCDA85E for ; Tue, 15 Feb 2022 09:23:52 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id gj12-20020a17090b108c00b001b89b5f3dd4so2894544pjb.6 for ; Tue, 15 Feb 2022 09:23:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=aYnNqElXZZv/TrOWzr/fEOBK7uQ0ce/iC+53j9ReqQw=; b=WtoqbW6v20058XfIOJ7oKCMUwiMPMuC9mVPNfKiqw8TK/6yx+GdOeerlENFEYz73UE GoAWONb7GwVU3v4IaiE8x+Us1OhEqVByvDD3Z9j6h2A3NOv2kPg5mMpIL32qh1M1HzFS lQoFsQrMVRCAaCQANNYpN4VnB5jcDXhhruGYS/J3tuOzmirDsNTJlsbFnv5vyjFLE7jE L6CSDqMVkppqfpDPbPjctsi75nlSNqIeOGyCD56FGzwknQeggfj1cw/rTAv63SaLmGxF VwRZwMj9ehsXGqMTRETDO03ZWTGCiNq6rUNAS916IwQOcntoyN4jXWEc5EfMG3zkLNAv gxSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=aYnNqElXZZv/TrOWzr/fEOBK7uQ0ce/iC+53j9ReqQw=; b=BmpqERnRXzpRLlgrKRN3LSOBDg1eHj+fhIUX9buP6/8RDxgVoW904TrNpwArcKESf7 dGXsq+JYBMMLUUHmqGVcOCrAdMjLihaTTIF3Azz7uMBs9IDSPTSL/RXMgmJXe4Ttrwbf i5qHIszqENiLMkhMndtgg4UfK6nr2wSd1zOBLfRGf89iiOYEjLuZ8lpakSkT2LypqXNV AQaGlI714tWCq10F8Ziy7ICqZZm1QO2hWiOWXjS4qt6or8bDfnjtHuGJeOWaCBXl4Biy d+ESZy1tGmA97NthLBTOMKAH0lVYg3Rfh1FhPn8ltP2Eh4K4iLj0oWkl/XDd/HdB//lq T/LA== X-Gm-Message-State: AOAM533xqNNqztfd7C3+OD2qTTesI9hMpzKIJSJeHdsp0me08+ev9Q1K x6YLTeolhAP3f9rQaW6Df2o8UpJ17G/MMtYCCGCUfVu3BVDskwIgLpEmSIdsDsPH77Ec+EKmcd8 FfS6Wt13mx9HPDBKwd+IDtQNwqvVEg/CHpI4ke4Jlw7Y/C4DZyEw18zbf0O+/YNo= X-Google-Smtp-Source: ABdhPJx5aI7zZbj6aSVvgKhoKTG2o9ZFZyr2IWFQSrj416LMPfCvWacWfUyKDLV0lLpsFUKiS0q3wbG6VrePOA== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:2449:: with SMTP id d9mr2170129pfj.70.1644945831608; Tue, 15 Feb 2022 09:23:51 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:11 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-3-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 2/9] t5526: use grep to assert on fetches From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In a previous commit, we replaced test_cmp invocations with verify_fetch_result(). Finish the process of removing test_cmp by using grep in verify_fetch_result() instead. This makes the tests less sensitive to changes because, instead of checking the whole stderr, we only grep for the lines of the form * "..\s+branch\s+-> origin/branch" * "Fetching submodule " (if fetching a submodule) when we expect the repo to have fetched. If we expect the repo to not have fetched, grep to make sure the lines are absent. Also, simplify the assertions by using grep patterns that match only the relevant pieces of information, e.g. is irrelevant because we only want to know if the fetch was performed, so we don't need to know where the branch was before the fetch. Signed-off-by: Glen Choo --- t/t5526-fetch-submodules.sh | 131 +++++++++++++----------------------- 1 file changed, 48 insertions(+), 83 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 0e93df1665..cb18f0ac21 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -20,49 +20,52 @@ pwd=$(pwd) add_upstream_commit() { ( cd submodule && - head1=$(git rev-parse --short HEAD) && echo new >> subfile && test_tick && git add subfile && git commit -m new subfile && - head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule" > ../expect.err.sub && - echo "From $pwd/submodule" >> ../expect.err.sub && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub + git rev-parse --short HEAD >../subhead ) && ( cd deepsubmodule && - head1=$(git rev-parse --short HEAD) && echo new >> deepsubfile && test_tick && git add deepsubfile && git commit -m new deepsubfile && - head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule/subdir/deepsubmodule" > ../expect.err.deep - echo "From $pwd/deepsubmodule" >> ../expect.err.deep && - echo " $head1..$head2 deep -> origin/deep" >> ../expect.err.deep + git rev-parse --short HEAD >../deephead ) } # Verifies that the expected repositories were fetched. This is done by -# concatenating the files expect.err.[super|sub|deep] in the correct -# order and comparing it to the actual stderr. +# checking that the branches of [super|sub|deep] were updated to +# [super|sub|deep]head if the corresponding file exists. # -# If a repo should not be fetched in the test, its corresponding -# expect.err file should be rm-ed. +# If the [super|sub|deep] head file does not exist, this verifies that +# the corresponding repo was not fetched. Thus, if a repo should not be +# fetched in the test, its corresponding head file should be +# rm-ed. verify_fetch_result() { ACTUAL_ERR=$1 && - rm -f expect.err.combined && - if [ -f expect.err.super ]; then - cat expect.err.super >>expect.err.combined + # Each grep pattern is guaranteed to match the correct repo + # because each repo uses a different name for their branch i.e. + # "super", "sub" and "deep". + if [ -f superhead ]; then + grep -E "\.\.$(cat superhead)\s+super\s+-> origin/super" $ACTUAL_ERR + else + ! grep "super" $ACTUAL_ERR fi && - if [ -f expect.err.sub ]; then - cat expect.err.sub >>expect.err.combined + if [ -f subhead ]; then + grep "Fetching submodule submodule" $ACTUAL_ERR && + grep -E "\.\.$(cat subhead)\s+sub\s+-> origin/sub" $ACTUAL_ERR + else + ! grep "Fetching submodule submodule" $ACTUAL_ERR fi && - if [ -f expect.err.deep ]; then - cat expect.err.deep >>expect.err.combined - fi && - test_cmp expect.err.combined $ACTUAL_ERR + if [ -f deephead ]; then + grep "Fetching submodule submodule/subdir/deepsubmodule" $ACTUAL_ERR && + grep -E "\.\.$(cat deephead)\s+deep\s+-> origin/deep" $ACTUAL_ERR + else + ! grep "Fetching submodule submodule/subdir/deepsubmodule" $ACTUAL_ERR + fi } test_expect_success setup ' @@ -274,13 +277,10 @@ test_expect_success "Recursion doesn't happen when no new commits are fetched in ' test_expect_success "Recursion stops when no new submodule commits are fetched" ' - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -291,15 +291,12 @@ test_expect_success "Recursion stops when no new submodule commits are fetched" test_expect_success "Recursion doesn't happen when new superproject commits don't change any submodules" ' add_upstream_commit && - head1=$(git rev-parse --short HEAD) && echo a > file && git add file && git commit -m "new file" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && - rm expect.err.sub && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm subhead && + rm deephead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -318,12 +315,9 @@ test_expect_success "Recursion picks up config in submodule" ' ) ) && add_upstream_commit && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && + git rev-parse --short HEAD >superhead && ( cd downstream && git fetch >../actual.out 2>../actual.err && @@ -345,20 +339,13 @@ test_expect_success "Recursion picks up all submodules when necessary" ' git fetch && git checkout -q FETCH_HEAD ) && - head1=$(git rev-parse --short HEAD^) && git add subdir/deepsubmodule && git commit -m "new deepsubmodule" && - head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule" > ../expect.err.sub && - echo "From $pwd/submodule" >> ../expect.err.sub && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub + git rev-parse --short HEAD >../subhead ) && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && + git rev-parse --short HEAD >superhead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -376,13 +363,9 @@ test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no ne git fetch && git checkout -q FETCH_HEAD ) && - head1=$(git rev-parse --short HEAD^) && git add subdir/deepsubmodule && git commit -m "new deepsubmodule" && - head2=$(git rev-parse --short HEAD) && - echo Fetching submodule submodule > ../expect.err.sub && - echo "From $pwd/submodule" >> ../expect.err.sub && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub + git rev-parse --short HEAD >../subhead ) && ( cd downstream && @@ -395,12 +378,9 @@ test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no ne ' test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necessary (and ignores config)" ' - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && + git rev-parse --short HEAD >superhead && ( cd downstream && git config fetch.recurseSubmodules false && @@ -421,15 +401,12 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' add_upstream_commit && - head1=$(git rev-parse --short HEAD) && echo a >> file && git add file && git commit -m "new file" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && - rm expect.err.sub && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm subhead && + rm deephead && ( cd downstream && git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err @@ -445,13 +422,10 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config ) && add_upstream_commit && git config --global fetch.recurseSubmodules false && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && git config fetch.recurseSubmodules on-demand && @@ -473,13 +447,10 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override ) && add_upstream_commit && git config fetch.recurseSubmodules false && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && git config submodule.submodule.fetchRecurseSubmodules on-demand && @@ -499,15 +470,12 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea cd submodule && git checkout -q HEAD^^ ) && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "submodule rewound" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && - rm expect.err.sub && + git rev-parse --short HEAD >superhead && + rm subhead && # This file does not exist, but rm -f for readability - rm -f expect.err.deep && + rm -f deephead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -526,14 +494,11 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git fetch --recurse-submodules ) && add_upstream_commit && - head1=$(git rev-parse --short HEAD) && git add submodule && git rm .gitmodules && git commit -m "new submodule without .gitmodules" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." >expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && rm .gitmodules && From patchwork Tue Feb 15 17:23:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747415 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 F009AC433F5 for ; Tue, 15 Feb 2022 17:24:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242481AbiBORYK (ORCPT ); Tue, 15 Feb 2022 12:24:10 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60408 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242476AbiBORYF (ORCPT ); Tue, 15 Feb 2022 12:24:05 -0500 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A8CE4DB4BD for ; Tue, 15 Feb 2022 09:23:54 -0800 (PST) Received: by mail-yb1-xb4a.google.com with SMTP id i205-20020a2522d6000000b00622c778ac7cso14651554ybi.3 for ; Tue, 15 Feb 2022 09:23:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=3zCaOJUSDmEdWokesmEQSHK4M8N82lefIJd6zevwVPs=; b=OpRBRe+HHmRvSGg+JydPLjAgsYkvK9IxI6Svs7KO44rO/tApCglpD6IVtYMm77a19X Q1J4JSpOngEQyEp+h3bEAvi0VGxiJSXlZjCAPxMhJuxQ0jJxUGJTOALLylKC68ivdXS4 QD4JOr5AH/CzwgRYiWvmSZrHUjMUXuZMnsJ0c+7pxhP2UfmebDxUQ2JRzvdDQhCK23nU r4ecYrOw5R2EElQDUXv4jrZF1vF8ORJ7pJGKmGB9/d8ddWbmTtlyTB1XHcacXfXWNlxJ LOssJflq+yiaF+zOFsKI4PXQHVcdsRkclSYU49icDXD68hIeadj5e5gP8nEGbwVrjvyp 4D4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=3zCaOJUSDmEdWokesmEQSHK4M8N82lefIJd6zevwVPs=; b=un/EsbAoUg8mIkjLZEopH0X+HBgj6wPbOyCJ/rm4WrBXd96rId0mQ5HKV6rBhT/7SZ Jlw2JgQeaY/Ni0KhAHv85+b3wB8XpwtjhGYVKWKCHLut6YgYk3Cx2vlm8KmSRk9W7R39 opx7NwiZNEkzQyavAY4/s00BB+fxbKn4PF/RhG2wymyHn51DofxT+0cBfWfNw/SsNUsC RAKCYYvQkH5QqoWT8a0rCPvYGvJ4HyAV0mPopLQcZ/GqufVH2HTouixfcaCaK6Kucl15 Mum2h1AdLAQ+6O4ppqG8lAzmRwo3uZfjG3Wg6+fAsnKAbD2h0QDPZ/c26Z2S7mE/Qwzv Uv+A== X-Gm-Message-State: AOAM5319wsswhhJ3yf/9BJOBaYN1yxXmRbNPSdHOYsuM0ou5d60Kinay NM7L1CsIsKOkFXP6ARD9FJnDOTQ14xsTQ9ONymqJniszSnT39uzKjr5tBnIr8JvxIWzTLeZS6w2 SV8w23hpER/UuyWYZ6R89G1LxkaSUfxaEz2pFUadUtuYnlaOPfF/0uw+zjoxqk3Y= X-Google-Smtp-Source: ABdhPJyGor97RmG8WpOK0FffZcOX5UT8V+aGrOY1MCulAWs+BqBxtlrLT5x4u8uXScd9vs0jzjUQI/icanIoRA== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a25:904:: with SMTP id 4mr2243422ybj.326.1644945833843; Tue, 15 Feb 2022 09:23:53 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:12 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-4-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 3/9] submodule: make static functions read submodules from commits From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A future commit will teach "fetch --recurse-submodules" to fetch unpopulated submodules. To prepare for this, teach the necessary static functions how to read submodules from superproject commits using a "treeish_name" argument (instead of always reading from the index and filesystem) but do not actually change where submodules are read from. Submodules will be read from commits when we fetch unpopulated submodules. The changed function signatures follow repo_submodule_init()'s argument order, i.e. "path" then "treeish_name". Where needed, reorder the arguments of functions that already take "path" and "treeish_name" to be consistent with this convention. Signed-off-by: Glen Choo --- submodule.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/submodule.c b/submodule.c index 5ace18a7d9..7032dcabb8 100644 --- a/submodule.c +++ b/submodule.c @@ -932,6 +932,7 @@ struct has_commit_data { struct repository *repo; int result; const char *path; + const struct object_id *super_oid; }; static int check_has_commit(const struct object_id *oid, void *data) @@ -940,7 +941,7 @@ static int check_has_commit(const struct object_id *oid, void *data) struct repository subrepo; enum object_type type; - if (repo_submodule_init(&subrepo, cb->repo, cb->path, null_oid())) { + if (repo_submodule_init(&subrepo, cb->repo, cb->path, cb->super_oid)) { cb->result = 0; goto cleanup; } @@ -968,9 +969,10 @@ static int check_has_commit(const struct object_id *oid, void *data) static int submodule_has_commits(struct repository *r, const char *path, + const struct object_id *super_oid, struct oid_array *commits) { - struct has_commit_data has_commit = { r, 1, path }; + struct has_commit_data has_commit = { r, 1, path, super_oid }; /* * Perform a cheap, but incorrect check for the existence of 'commits'. @@ -1017,7 +1019,7 @@ static int submodule_needs_pushing(struct repository *r, const char *path, struct oid_array *commits) { - if (!submodule_has_commits(r, path, commits)) + if (!submodule_has_commits(r, path, null_oid(), commits)) /* * NOTE: We do consider it safe to return "no" here. The * correct answer would be "We do not know" instead of @@ -1277,7 +1279,7 @@ static void calculate_changed_submodule_paths(struct repository *r, if (!path) continue; - if (submodule_has_commits(r, path, commits)) { + if (submodule_has_commits(r, path, null_oid(), commits)) { oid_array_clear(commits); *name->string = '\0'; } @@ -1402,12 +1404,13 @@ static const struct submodule *get_non_gitmodules_submodule(const char *path) } static struct fetch_task *fetch_task_create(struct repository *r, - const char *path) + const char *path, + const struct object_id *treeish_name) { struct fetch_task *task = xmalloc(sizeof(*task)); memset(task, 0, sizeof(*task)); - task->sub = submodule_from_path(r, null_oid(), path); + task->sub = submodule_from_path(r, treeish_name, path); if (!task->sub) { /* * No entry in .gitmodules? Technically not a submodule, @@ -1439,11 +1442,12 @@ static void fetch_task_release(struct fetch_task *p) } static struct repository *get_submodule_repo_for(struct repository *r, - const char *path) + const char *path, + const struct object_id *treeish_name) { struct repository *ret = xmalloc(sizeof(*ret)); - if (repo_submodule_init(ret, r, path, null_oid())) { + if (repo_submodule_init(ret, r, path, treeish_name)) { free(ret); return NULL; } @@ -1464,7 +1468,7 @@ static int get_next_submodule(struct child_process *cp, if (!S_ISGITLINK(ce->ce_mode)) continue; - task = fetch_task_create(spf->r, ce->name); + task = fetch_task_create(spf->r, ce->name, null_oid()); if (!task) continue; @@ -1487,7 +1491,7 @@ static int get_next_submodule(struct child_process *cp, continue; } - task->repo = get_submodule_repo_for(spf->r, task->sub->path); + task->repo = get_submodule_repo_for(spf->r, task->sub->path, null_oid()); if (task->repo) { struct strbuf submodule_prefix = STRBUF_INIT; child_process_init(cp); From patchwork Tue Feb 15 17:23:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747416 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 A4600C433EF for ; Tue, 15 Feb 2022 17:24:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242477AbiBORYM (ORCPT ); Tue, 15 Feb 2022 12:24:12 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60416 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242474AbiBORYG (ORCPT ); Tue, 15 Feb 2022 12:24:06 -0500 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 568CAD3AD7 for ; Tue, 15 Feb 2022 09:23:56 -0800 (PST) Received: by mail-pf1-x44a.google.com with SMTP id z20-20020aa791d4000000b004bd024eaf19so14386137pfa.16 for ; Tue, 15 Feb 2022 09:23:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=xlGaBHHWVtlrI2fnEJ7RzLe0w2FXgQlueRLhihQyHLA=; b=e2yc/bsFFhkMVLGTtyxDNMnt0fAuriaL3yorpDPiEU2GL5Q0/uL0X2u0ldr4Y78u9e TLmiMnEKdTWRjtBgyTxYHBwtXe9GMVUOIvUtHzd2fnEA/LETo83net1aGK5U2ZZUMIvi lRPNBpJxieVPIWA5nVAUYKJGVUB6o4cC8QfcToIIzhxroNzJ2+mmQCpKvaTVQBaobRTQ X8hnK2xLsDeuZxMaoB3BPlKsDPi4jgfRhhztZbQnLuxOwI1Vu48GTeQPDYl2zRdKuCdv ssRdwGJjxdr0XDFMo26VxPqlM6vD0eXhw95vxa742HFKbaI07OLXt8sYlDuAT0dzMlj5 bIiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=xlGaBHHWVtlrI2fnEJ7RzLe0w2FXgQlueRLhihQyHLA=; b=BpT1FQRwbW4e9+2VoCpaBgy8tAZnJzGC1LcQ8Bt9+gTfkLZGxpSjOrX8Vl52tlKi5b ARNorxJCW3fKFN+pdG3eFgCPAUS2aNHklveT3s9pEmPqB15hifdnSY5zuYe5x5AN08KK M+T2qSpN5rMW+q8oJuf3hu7AVbdK6HdsxI0Zya2hp8n9KYgdWZNvTZNRxFwV0vwtRLgw xQF2nNB623kEHUjojIo1owV6KartlyQ2EuNmvwCjNyBRcgEYukILDpNDcXhD5dTPKnEs 4fPYy1kWE5tdhOGDEnOlEAT//Ctbz9SZzjSsQvuQCxi7vja3BbI/WDCta4qPiWjwbjjZ 7mJw== X-Gm-Message-State: AOAM530MBx3cYH14VUCAKwI5uA2mSTVNDKCDrMubuDuhffrZXbwT4R2a iaNeUiSC7Xv6FuHZ8l/onf7K3vc24y79StfoSJ2aaBTMnDEEsjyGaDxkoVuYgjf3rU/tNSAtTK4 zNY+WE5uPe4OPvHoW3ahv5MN8RQWTlLzliALkvA1PCWn0zZ8yU7Uun7JZGX56j74= X-Google-Smtp-Source: ABdhPJyLZRZp0w4x8YDfsW0Do0ZIlwJnryTtTj1z6P1GWe51hz2UnXu99dz9PL0qKs3Da38ba7J7yTE91KiJaQ== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:902:c409:: with SMTP id k9mr5140928plk.91.1644945835774; Tue, 15 Feb 2022 09:23:55 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:13 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-5-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 4/9] submodule: inline submodule_commits() into caller From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When collecting the string_list of changed submodule names, the new submodules commits are stored in the string_list_item.util as an oid_array. A subsequent commit will replace the oid_array with a struct that has more information. Prepare for this change by inlining submodule_commits() (which inserts into the string_list and initializes the string_list_item.util) into its only caller so that the code is easier to refactor later. Signed-off-by: Glen Choo --- submodule.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/submodule.c b/submodule.c index 7032dcabb8..7fdf7793fb 100644 --- a/submodule.c +++ b/submodule.c @@ -782,19 +782,6 @@ const struct submodule *submodule_from_ce(const struct cache_entry *ce) return submodule_from_path(the_repository, null_oid(), ce->name); } -static struct oid_array *submodule_commits(struct string_list *submodules, - const char *name) -{ - struct string_list_item *item; - - item = string_list_insert(submodules, name); - if (item->util) - return (struct oid_array *) item->util; - - /* NEEDSWORK: should we have oid_array_init()? */ - item->util = xcalloc(1, sizeof(struct oid_array)); - return (struct oid_array *) item->util; -} struct collect_changed_submodules_cb_data { struct repository *repo; @@ -830,9 +817,9 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - struct oid_array *commits; const struct submodule *submodule; const char *name; + struct string_list_item *item; if (!S_ISGITLINK(p->two->mode)) continue; @@ -859,8 +846,11 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, if (!name) continue; - commits = submodule_commits(changed, name); - oid_array_append(commits, &p->two->oid); + item = string_list_insert(changed, name); + if (!item->util) + /* NEEDSWORK: should we have oid_array_init()? */ + item->util = xcalloc(1, sizeof(struct oid_array)); + oid_array_append(item->util, &p->two->oid); } } From patchwork Tue Feb 15 17:23:14 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747417 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 38DA4C433F5 for ; Tue, 15 Feb 2022 17:24:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242484AbiBORYO (ORCPT ); Tue, 15 Feb 2022 12:24:14 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60442 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242479AbiBORYJ (ORCPT ); Tue, 15 Feb 2022 12:24:09 -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 B636BD997C for ; Tue, 15 Feb 2022 09:23:58 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id z12-20020a17090a1fcc00b001b63e477f9fso13464935pjz.3 for ; Tue, 15 Feb 2022 09:23:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=cmghuhzjuIPy5ZcOGB9ii1Itvf2r+QNug7p5xRGRrv8=; b=Lp4kipL5bw9GFGwpd//aK2aXWj01pfYyGvPAQxot4EcoXYH2B+v8p4+MpB0D68Sa3A QJLoxStZfusLuKjwM97/6vaL4ljd2IFuteP+/RIC6pcuy94vaRUA0QS/E8WnOcdIfC8j L9buBSqqvIoHZhB6sJoU9UmI7lAYY3mCPXOaXmvGBvzZL6OIKNLAcQ0NVnWriJ9xH8G3 s7RYhM9NUL3mKhQIsMvzMpRwkqLYGDrqa3NGwzqdd6bdpDw8uSoV+pFCXqAxO3DMu65d EHaFAVSxRcrH4Mq1IwDSEeYaxBC2cd3ERkkMF+lvY99SVgzwRK8/Cf1SDCWOKBm+NAsN uTqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=cmghuhzjuIPy5ZcOGB9ii1Itvf2r+QNug7p5xRGRrv8=; b=z2k0xtb18PZ6ad7PGtzAjVI6be6rWtClMAr/s3wXtD1w+Ev1YNiTa7qGJJWvVbSh4k lPQGTNuWLgl6PsXwmKPAnannB95RTXHVZIzeTAYwwSF8lfkRpZMbaXhFM+60Rf77rfW4 T47zuAmGTbZ4Zz/WXCiF/tc+h4t0H2OOS7xUZ4mky1BCXSpv4ZgFGHVsEigUgWV22abv zUGQWcaTrqdkfVxr/jOIp8LuBsQUJDvYHdhKX6KM8aW85dlo1IdnaXMFCQ9rgGxvXAL4 VfQIyWyf362COO6dCGucUHOFSfGLujfx6GohmPMDipv6nNf8qAPyr3D/MRcuoerON5+G uOXg== X-Gm-Message-State: AOAM531JQ1Ohkgy4vzNdsdoRzVJIgGMh6mFXIw0v2+Kv7f2c1WR5gGly EQhoQ05c5tQ1RvWZNZBIfClbRhgeGJIrdhfUn92heb06jLkEs/sggUQGlT9sZaQQVY2BX3SYdau WjZd8H4T3P77eLX0++tqRaRR/A0f5iff9iNVV4DPZztn4zvt5+AlRXqkkIiREI6c= X-Google-Smtp-Source: ABdhPJwf8kpnpGzV5qBYqpRoFkfY15aaLEuW+gg0Uius2eGqvYxAqK7ihAlcsBnjnd+AlEb2RuXT7kakZfJzPQ== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:e616:: with SMTP id j22mr5461858pjy.45.1644945838031; Tue, 15 Feb 2022 09:23:58 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:14 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-6-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 5/9] submodule: store new submodule commits oid_array in a struct From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org This commit prepares for a future commit that will teach `git fetch --recurse-submodules` how to fetch submodules that are present in /modules, but are not populated. To do this, we need to store more information about the changed submodule so that we can read the submodule configuration from the superproject commit instead of the filesystem. Refactor the changed submodules string_list.util to hold a struct instead of an oid_array. This struct only holds the new_commits oid_array for now; more information will be added later. Signed-off-by: Glen Choo --- submodule.c | 54 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/submodule.c b/submodule.c index 7fdf7793fb..5b1aa3fbe8 100644 --- a/submodule.c +++ b/submodule.c @@ -806,6 +806,20 @@ static const char *default_name_or_path(const char *path_or_name) return path_or_name; } +/* + * Holds relevant information for a changed submodule. Used as the .util + * member of the changed submodule string_list_item. + */ +struct changed_submodule_data { + /* The submodule commits that have changed in the rev walk. */ + struct oid_array new_commits; +}; + +static void changed_submodule_data_clear(struct changed_submodule_data *cs_data) +{ + oid_array_clear(&cs_data->new_commits); +} + static void collect_changed_submodules_cb(struct diff_queue_struct *q, struct diff_options *options, void *data) @@ -820,6 +834,7 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, const struct submodule *submodule; const char *name; struct string_list_item *item; + struct changed_submodule_data *cs_data; if (!S_ISGITLINK(p->two->mode)) continue; @@ -848,9 +863,9 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, item = string_list_insert(changed, name); if (!item->util) - /* NEEDSWORK: should we have oid_array_init()? */ - item->util = xcalloc(1, sizeof(struct oid_array)); - oid_array_append(item->util, &p->two->oid); + item->util = xcalloc(1, sizeof(struct changed_submodule_data)); + cs_data = item->util; + oid_array_append(&cs_data->new_commits, &p->two->oid); } } @@ -897,11 +912,12 @@ static void collect_changed_submodules(struct repository *r, reset_revision_walk(); } -static void free_submodules_oids(struct string_list *submodules) +static void free_submodules_data(struct string_list *submodules) { struct string_list_item *item; - for_each_string_list_item(item, submodules) - oid_array_clear((struct oid_array *) item->util); + for_each_string_list_item(item, submodules) { + changed_submodule_data_clear(item->util); + } string_list_clear(submodules, 1); } @@ -1069,7 +1085,7 @@ int find_unpushed_submodules(struct repository *r, collect_changed_submodules(r, &submodules, &argv); for_each_string_list_item(name, &submodules) { - struct oid_array *commits = name->util; + struct changed_submodule_data *cs_data = name->util; const struct submodule *submodule; const char *path = NULL; @@ -1082,11 +1098,11 @@ int find_unpushed_submodules(struct repository *r, if (!path) continue; - if (submodule_needs_pushing(r, path, commits)) + if (submodule_needs_pushing(r, path, &cs_data->new_commits)) string_list_insert(needs_pushing, path); } - free_submodules_oids(&submodules); + free_submodules_data(&submodules); strvec_clear(&argv); return needs_pushing->nr; @@ -1256,7 +1272,7 @@ static void calculate_changed_submodule_paths(struct repository *r, collect_changed_submodules(r, changed_submodule_names, &argv); for_each_string_list_item(name, changed_submodule_names) { - struct oid_array *commits = name->util; + struct changed_submodule_data *cs_data = name->util; const struct submodule *submodule; const char *path = NULL; @@ -1269,8 +1285,8 @@ static void calculate_changed_submodule_paths(struct repository *r, if (!path) continue; - if (submodule_has_commits(r, path, null_oid(), commits)) { - oid_array_clear(commits); + if (submodule_has_commits(r, path, null_oid(), &cs_data->new_commits)) { + changed_submodule_data_clear(cs_data); *name->string = '\0'; } } @@ -1307,7 +1323,7 @@ int submodule_touches_in_range(struct repository *r, strvec_clear(&args); - free_submodules_oids(&subs); + free_submodules_data(&subs); return ret; } @@ -1591,7 +1607,7 @@ static int fetch_finish(int retvalue, struct strbuf *err, struct fetch_task *task = task_cb; struct string_list_item *it; - struct oid_array *commits; + struct changed_submodule_data *cs_data; if (!task || !task->sub) BUG("callback cookie bogus"); @@ -1619,14 +1635,14 @@ static int fetch_finish(int retvalue, struct strbuf *err, /* Could be an unchanged submodule, not contained in the list */ goto out; - commits = it->util; - oid_array_filter(commits, + cs_data = it->util; + oid_array_filter(&cs_data->new_commits, commit_missing_in_sub, task->repo); /* Are there commits we want, but do not exist? */ - if (commits->nr) { - task->commits = commits; + if (cs_data->new_commits.nr) { + task->commits = &cs_data->new_commits; ALLOC_GROW(spf->oid_fetch_tasks, spf->oid_fetch_tasks_nr + 1, spf->oid_fetch_tasks_alloc); @@ -1684,7 +1700,7 @@ int fetch_populated_submodules(struct repository *r, strvec_clear(&spf.args); out: - free_submodules_oids(&spf.changed_submodule_names); + free_submodules_data(&spf.changed_submodule_names); return spf.result; } From patchwork Tue Feb 15 17:23:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747418 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 65CCFC433FE for ; Tue, 15 Feb 2022 17:24:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242490AbiBORYP (ORCPT ); Tue, 15 Feb 2022 12:24:15 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60470 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242483AbiBORYL (ORCPT ); Tue, 15 Feb 2022 12:24:11 -0500 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 341C3DB4BF for ; Tue, 15 Feb 2022 09:24:01 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id j1-20020a170903028100b0014b1f9e0068so8606827plr.8 for ; Tue, 15 Feb 2022 09:24:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=GP+Mv/TRHvKvjamtcE5/3+xbsFKEIoYI48WaTdBeIyY=; b=DE2fqP8BGIvwEsbOSpKnGPQG/KKhf/ZShrslU1b4P81M/rIlUX7ll1NPhYAB5JDnn5 KS2DV8zJK6wwpQxEXmwxpWe6WeWGnf3jJN+l6PgrHKTy7BcaeIKCKUIaRLqTZLKauBcY K38+xGXaPm1bDZ/X4w9FBgQckpH4zyklVtWQQ4+kVuR/orWQH6ijtrHxTNCo08jKsn4b p8r8K/1Poa790YDNvDKiToHCCAKXuzpivFwRAvPzoCOSVw/gZOCXrq7POE1cWSNihmlr bavcxiOLDP9c90YuiYtznyLvhrj1ZO+IWsIgloPATzn9MdVb9UNjW7xYGnYmWCLxB8se rfkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=GP+Mv/TRHvKvjamtcE5/3+xbsFKEIoYI48WaTdBeIyY=; b=kE6D4okZwpPGM0+Pyjj/KGnV19plH9m4uGXolXEuOpsjZrKApKr96OgLhHecNNE+Wr 4jKkZjQMDr0Nw4Rkk5ajLTpVo/As0z3bVOc8vqXmcVO5EJc0lSo79p4b8mdjN4bXrm+/ 8VXLuHUtkuM6ZM3Bs64yAmSldfB/IPvYhrs2wQxJDxJj9OdaEm9LHCUqtt3YV7brkcss h4WgRZBBDwHVyuBe+LyYiXMfQgx2noUKjdcwrQKL2lWbnzHTc0jtPPA2cEX3Od1pc9AI D8eawfkiEg8fEr/+smfs2pVhcxRbvNgNlWBlJ6u97MQGf9++thTf/7FxCxDk0ZTjYrPV WRkw== X-Gm-Message-State: AOAM530i9+yDtr3GprIIlxl42RLh0yB/JeSytsj80QMSjRaAixUIWF+6 2GAL3qPQzXIBx72a3cUSVLqgYwo0FPUeIz7PipdPHwaLTCKf89dnB8s8pmfvUaGdK/z6yLY7sqB 9zqqr/SbK01d67YGzP060oHybu1fE4b4PTZPXXXqCapdZi17bU2PEeaWy/+/fKOQ= X-Google-Smtp-Source: ABdhPJwvIdctAFyUyDx7cF+A1ww++4E0n83mguQVVYhiJJnTYFVvUhTtXbrAtMlhjrN1bH1c6Bba/o/noPgopg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:841:: with SMTP id q1mr5387472pfk.21.1644945840571; Tue, 15 Feb 2022 09:24:00 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:15 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-7-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 6/9] submodule: extract get_fetch_task() From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org get_next_submodule() configures the parallel submodule fetch by performing two functions: * iterate the index to find submodules * configure the child processes to fetch the submodules found in the previous step Extract the index iterating code into an iterator function, get_fetch_task(), so that get_next_submodule() is agnostic of how to find submodules. This prepares for a subsequent commit will teach the fetch machinery to also iterate through the list of changed submodules (in addition to the index). Signed-off-by: Glen Choo --- Jonathan: I'm really happy with the formatting changes that you suggested because this diff is a lot easier to read, so thanks again! Going forward, I'd appreciate any and all formatting suggestions - if they seem possibly excessive, you can mark them as nits. submodule.c | 62 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/submodule.c b/submodule.c index 5b1aa3fbe8..22d8a1ca12 100644 --- a/submodule.c +++ b/submodule.c @@ -1461,14 +1461,12 @@ static struct repository *get_submodule_repo_for(struct repository *r, return ret; } -static int get_next_submodule(struct child_process *cp, - struct strbuf *err, void *data, void **task_cb) +static struct fetch_task * +get_fetch_task(struct submodule_parallel_fetch *spf, + const char **default_argv, struct strbuf *err) { - struct submodule_parallel_fetch *spf = data; - for (; spf->count < spf->r->index->cache_nr; spf->count++) { const struct cache_entry *ce = spf->r->index->cache[spf->count]; - const char *default_argv; struct fetch_task *task; if (!S_ISGITLINK(ce->ce_mode)) @@ -1488,10 +1486,10 @@ static int get_next_submodule(struct child_process *cp, &spf->changed_submodule_names, task->sub->name)) continue; - default_argv = "on-demand"; + *default_argv = "on-demand"; break; case RECURSE_SUBMODULES_ON: - default_argv = "yes"; + *default_argv = "yes"; break; case RECURSE_SUBMODULES_OFF: continue; @@ -1499,29 +1497,12 @@ static int get_next_submodule(struct child_process *cp, task->repo = get_submodule_repo_for(spf->r, task->sub->path, null_oid()); if (task->repo) { - struct strbuf submodule_prefix = STRBUF_INIT; - child_process_init(cp); - cp->dir = task->repo->gitdir; - prepare_submodule_repo_env_in_gitdir(&cp->env_array); - cp->git_cmd = 1; if (!spf->quiet) strbuf_addf(err, _("Fetching submodule %s%s\n"), spf->prefix, ce->name); - strvec_init(&cp->args); - strvec_pushv(&cp->args, spf->args.v); - strvec_push(&cp->args, default_argv); - strvec_push(&cp->args, "--submodule-prefix"); - - strbuf_addf(&submodule_prefix, "%s%s/", - spf->prefix, - task->sub->path); - strvec_push(&cp->args, submodule_prefix.buf); spf->count++; - *task_cb = task; - - strbuf_release(&submodule_prefix); - return 1; + return task; } else { struct strbuf empty_submodule_path = STRBUF_INIT; @@ -1545,6 +1526,37 @@ static int get_next_submodule(struct child_process *cp, strbuf_release(&empty_submodule_path); } } + return NULL; +} + +static int get_next_submodule(struct child_process *cp, struct strbuf *err, + void *data, void **task_cb) +{ + struct submodule_parallel_fetch *spf = data; + const char *default_argv = NULL; + struct fetch_task *task = get_fetch_task(spf, &default_argv, err); + + if (task) { + struct strbuf submodule_prefix = STRBUF_INIT; + + child_process_init(cp); + cp->dir = task->repo->gitdir; + prepare_submodule_repo_env_in_gitdir(&cp->env_array); + cp->git_cmd = 1; + strvec_init(&cp->args); + strvec_pushv(&cp->args, spf->args.v); + strvec_push(&cp->args, default_argv); + strvec_push(&cp->args, "--submodule-prefix"); + + strbuf_addf(&submodule_prefix, "%s%s/", + spf->prefix, + task->sub->path); + strvec_push(&cp->args, submodule_prefix.buf); + *task_cb = task; + + strbuf_release(&submodule_prefix); + return 1; + } if (spf->oid_fetch_tasks_nr) { struct fetch_task *task = From patchwork Tue Feb 15 17:23:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747419 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 8BAC6C433F5 for ; Tue, 15 Feb 2022 17:24:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242507AbiBORYW (ORCPT ); Tue, 15 Feb 2022 12:24:22 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242485AbiBORYO (ORCPT ); Tue, 15 Feb 2022 12:24:14 -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 2FC66DAAFA for ; Tue, 15 Feb 2022 09:24:03 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id j23-20020a17090a7e9700b001b8626c9170so2923306pjl.1 for ; Tue, 15 Feb 2022 09:24:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=vzrhPCJXUj1fOavLwHq12IVPyJwyXk3DJAKV0QcYVFU=; b=Yce0j/MYSforJEY/FdhK+0Z6XiJnDyDzesijpQL921mhbdxAdzRmzeuJFnNOnSltSS Q3Jg/3c7NPJpfGPhN4dAbZuL3x+OHkgRsXuyI9kWIN5v8SRbUyOeRYjmPAKjZemGO5D2 Vkk9UOliRdDknaHAeg19MowhMkD2H0K6PZgrkqsoGn7h0O4IvqE841d1H2TkFj9ewF/A tCyC7SWS3Anl0xQtc4obYvBGkRLOQD1lXITNUh4vVryD4Z4rLf3dHVDHjq98Qk5CJ6ko /Rw4b0x11IWJ1JpZ2yZtU4Zeolv2x3NT4wyBNeGu6ls9M5RAPId3+sYZWe90202K/6c+ sC3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=vzrhPCJXUj1fOavLwHq12IVPyJwyXk3DJAKV0QcYVFU=; b=X1VxSloTdCx86mmtjti17/1/Kb85kCA8fl29X/xdeXm9PntpHK4R+giZpi0VWsEpXa Yw/KFK/cJS0gkOXf1ZjKBACU54DALbt8xHQ4iKTZWzmtl+2By8M/pW5t3O9LVKKROTqM vIbEBqzHxyxyUkI33GOtY6Z1gdFlbGvjDyQru8+YzkN6MJ5UEc+7f4JgXupVWCsss864 fYLUDxZJ+LP91ftG9eTIUaPGuceSMj8f2G3tcTVCNelWtZ1d4e5gMOlIBP7la/kZb/dl bfXjkFgOyoZg5Vlqr1BO2sOuBBqQ6rrMJoS8zbrIPsVCyKusk9wRkoelqzUqRvsZ9eoU Kq7w== X-Gm-Message-State: AOAM531JxgNCT9nr/vlpT8drAGw+ibAccxC5s9Vd6KfDnZ7oV7vVgAkM uyeCTPvK/hzMM4j7LYtS+BISsw/YIHMN+MSvVJQ36JmEXWZj9RjHt1HZWRGz5vVIQ9MbFyEo1i1 4gNBR1kbmiPVQbU6RmupHbG4HHh7CnPKb5me0YsD9tN2zLmPDs0YnYHhewU+Jz14= X-Google-Smtp-Source: ABdhPJyaehaWAuYwB8SihBXN64/1UlLg9+11CwQvDCfXLxCM1UEVoM/8ZEY6K89WNS+JfxD6MuO5HfZOhc9Qpw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:1ac9:: with SMTP id f9mr5351344pfv.65.1644945842594; Tue, 15 Feb 2022 09:24:02 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:16 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-8-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 7/9] fetch: fetch unpopulated, changed submodules From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org "git fetch --recurse-submodules" only considers populated submodules (i.e. submodules that can be found by iterating the index), which makes "git fetch" behave differently based on which commit is checked out. As a result, even if the user has initialized all submodules correctly, they may not fetch the necessary submodule commits, and commands like "git checkout --recurse-submodules" might fail. Teach "git fetch" to fetch cloned, changed submodules regardless of whether they are populated (this is in addition to the current behavior of fetching populated submodules). Since a submodule may be encountered multiple times (via the list of populated submodules or via the list of changed submodules), maintain a list of seen submodules to avoid fetching a submodule more than once. Signed-off-by: Glen Choo --- As I mentioned in the cover letter, I'm not entirely happy with the name repo_has_absorbed_submodules() - it's not a standardized term AFAIK and it's a little clunky. "absorbed submodule" is just a stand-in for "submodule in .git/modules", so if we have a better term for "submodule in .git/modules", let's use that instead. Documentation/fetch-options.txt | 26 +++-- Documentation/git-fetch.txt | 10 +- builtin/fetch.c | 14 +-- submodule.c | 134 +++++++++++++++++++--- submodule.h | 12 +- t/t5526-fetch-submodules.sh | 195 ++++++++++++++++++++++++++++++++ 6 files changed, 349 insertions(+), 42 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index e967ff1874..38dad13683 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -185,15 +185,23 @@ endif::git-pull[] ifndef::git-pull[] --recurse-submodules[=yes|on-demand|no]:: This option controls if and under what conditions new commits of - populated submodules should be fetched too. It can be used as a - boolean option to completely disable recursion when set to 'no' or to - unconditionally recurse into all populated submodules when set to - 'yes', which is the default when this option is used without any - value. Use 'on-demand' to only recurse into a populated submodule - when the superproject retrieves a commit that updates the submodule's - reference to a commit that isn't already in the local submodule - clone. By default, 'on-demand' is used, unless - `fetch.recurseSubmodules` is set (see linkgit:git-config[1]). + submodules should be fetched too. When recursing through submodules, + `git fetch` always attempts to fetch "changed" submodules, that is, a + submodule that has commits that are referenced by a newly fetched + superproject commit but are missing in the local submodule clone. A + changed submodule can be fetched as long as it is present locally e.g. + in `$GIT_DIR/modules/` (see linkgit:gitsubmodules[7]); if the upstream + adds a new submodule, that submodule cannot be fetched until it is + cloned e.g. by `git submodule update`. ++ +When set to 'on-demand', only changed submodules are fetched. When set +to 'yes', all populated submodules are fetched and submodules that are +both unpopulated and changed are fetched. When set to 'no', submodules +are never fetched. ++ +When unspecified, this uses the value of `fetch.recurseSubmodules` if it +is set (see linkgit:git-config[1]), defaulting to 'on-demand' if unset. +When this option is used without any value, it defaults to 'yes'. endif::git-pull[] -j:: diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index 550c16ca61..e9d364669a 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -287,12 +287,10 @@ include::transfer-data-leaks.txt[] BUGS ---- -Using --recurse-submodules can only fetch new commits in already checked -out submodules right now. When e.g. upstream added a new submodule in the -just fetched commits of the superproject the submodule itself cannot be -fetched, making it impossible to check out that submodule later without -having to do a fetch again. This is expected to be fixed in a future Git -version. +Using --recurse-submodules can only fetch new commits in submodules that are +present locally e.g. in `$GIT_DIR/modules/`. If the upstream adds a new +submodule, that submodule cannot be fetched until it is cloned e.g. by `git +submodule update`. This is expected to be fixed in a future Git version. SEE ALSO -------- diff --git a/builtin/fetch.c b/builtin/fetch.c index f7abbc31ff..faaf89f637 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2122,13 +2122,13 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) max_children = fetch_parallel_config; add_options_to_argv(&options); - result = fetch_populated_submodules(the_repository, - &options, - submodule_prefix, - recurse_submodules, - recurse_submodules_default, - verbosity < 0, - max_children); + result = fetch_submodules(the_repository, + &options, + submodule_prefix, + recurse_submodules, + recurse_submodules_default, + verbosity < 0, + max_children); strvec_clear(&options); } diff --git a/submodule.c b/submodule.c index 22d8a1ca12..3558fddeb7 100644 --- a/submodule.c +++ b/submodule.c @@ -811,6 +811,16 @@ static const char *default_name_or_path(const char *path_or_name) * member of the changed submodule string_list_item. */ struct changed_submodule_data { + /* + * The first superproject commit in the rev walk that points to the + * submodule. + */ + const struct object_id *super_oid; + /* + * Path to the submodule in the superproject commit referenced + * by 'super_oid'. + */ + char *path; /* The submodule commits that have changed in the rev walk. */ struct oid_array new_commits; }; @@ -818,6 +828,7 @@ struct changed_submodule_data { static void changed_submodule_data_clear(struct changed_submodule_data *cs_data) { oid_array_clear(&cs_data->new_commits); + free(cs_data->path); } static void collect_changed_submodules_cb(struct diff_queue_struct *q, @@ -865,6 +876,8 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, if (!item->util) item->util = xcalloc(1, sizeof(struct changed_submodule_data)); cs_data = item->util; + cs_data->super_oid = commit_oid; + cs_data->path = xstrdup(p->two->path); oid_array_append(&cs_data->new_commits, &p->two->oid); } } @@ -1248,14 +1261,28 @@ void check_for_new_submodule_commits(struct object_id *oid) oid_array_append(&ref_tips_after_fetch, oid); } +/* + * Returns 1 if the repo has absorbed submodule gitdirs, and 0 + * otherwise. Like submodule_name_to_gitdir(), this checks + * $GIT_DIR/modules, not $GIT_COMMON_DIR. + */ +static int repo_has_absorbed_submodules(struct repository *r) +{ + struct strbuf buf = STRBUF_INIT; + + strbuf_repo_git_path(&buf, r, "modules/"); + return file_exists(buf.buf) && !is_empty_dir(buf.buf); +} + static void calculate_changed_submodule_paths(struct repository *r, struct string_list *changed_submodule_names) { struct strvec argv = STRVEC_INIT; struct string_list_item *name; - /* No need to check if there are no submodules configured */ - if (!submodule_from_path(r, NULL, NULL)) + /* No need to check if no submodules would be fetched */ + if (!submodule_from_path(r, NULL, NULL) && + !repo_has_absorbed_submodules(r)) return; strvec_push(&argv, "--"); /* argv[0] program name */ @@ -1328,7 +1355,8 @@ int submodule_touches_in_range(struct repository *r, } struct submodule_parallel_fetch { - int count; + int index_count; + int changed_count; struct strvec args; struct repository *r; const char *prefix; @@ -1338,6 +1366,7 @@ struct submodule_parallel_fetch { int result; struct string_list changed_submodule_names; + struct string_list seen_submodule_names; /* Pending fetches by OIDs */ struct fetch_task **oid_fetch_tasks; @@ -1348,6 +1377,7 @@ struct submodule_parallel_fetch { #define SPF_INIT { \ .args = STRVEC_INIT, \ .changed_submodule_names = STRING_LIST_INIT_DUP, \ + .seen_submodule_names = STRING_LIST_INIT_DUP, \ .submodules_with_errors = STRBUF_INIT, \ } @@ -1462,11 +1492,12 @@ static struct repository *get_submodule_repo_for(struct repository *r, } static struct fetch_task * -get_fetch_task(struct submodule_parallel_fetch *spf, - const char **default_argv, struct strbuf *err) +get_fetch_task_from_index(struct submodule_parallel_fetch *spf, + const char **default_argv, struct strbuf *err) { - for (; spf->count < spf->r->index->cache_nr; spf->count++) { - const struct cache_entry *ce = spf->r->index->cache[spf->count]; + for (; spf->index_count < spf->r->index->cache_nr; spf->index_count++) { + const struct cache_entry *ce = + spf->r->index->cache[spf->index_count]; struct fetch_task *task; if (!S_ISGITLINK(ce->ce_mode)) @@ -1476,6 +1507,15 @@ get_fetch_task(struct submodule_parallel_fetch *spf, if (!task) continue; + /* + * We might have already considered this submodule + * because we saw it when iterating the changed + * submodule names. + */ + if (string_list_lookup(&spf->seen_submodule_names, + task->sub->name)) + continue; + switch (get_fetch_recurse_config(task->sub, spf)) { default: @@ -1501,7 +1541,7 @@ get_fetch_task(struct submodule_parallel_fetch *spf, strbuf_addf(err, _("Fetching submodule %s%s\n"), spf->prefix, ce->name); - spf->count++; + spf->index_count++; return task; } else { struct strbuf empty_submodule_path = STRBUF_INIT; @@ -1529,12 +1569,77 @@ get_fetch_task(struct submodule_parallel_fetch *spf, return NULL; } +static struct fetch_task * +get_fetch_task_from_changed(struct submodule_parallel_fetch *spf, + const char **default_argv, struct strbuf *err) +{ + for (; spf->changed_count < spf->changed_submodule_names.nr; + spf->changed_count++) { + struct string_list_item item = + spf->changed_submodule_names.items[spf->changed_count]; + struct changed_submodule_data *cs_data = item.util; + struct fetch_task *task; + + /* + * We might have already considered this submodule + * because we saw it in the index. + */ + if (string_list_lookup(&spf->seen_submodule_names, item.string)) + continue; + + task = fetch_task_create(spf->r, cs_data->path, + cs_data->super_oid); + if (!task) + continue; + + switch (get_fetch_recurse_config(task->sub, spf)) { + default: + case RECURSE_SUBMODULES_DEFAULT: + case RECURSE_SUBMODULES_ON_DEMAND: + *default_argv = "on-demand"; + break; + case RECURSE_SUBMODULES_ON: + *default_argv = "yes"; + break; + case RECURSE_SUBMODULES_OFF: + continue; + } + + task->repo = get_submodule_repo_for(spf->r, task->sub->path, + cs_data->super_oid); + if (!task->repo) { + fetch_task_release(task); + free(task); + + strbuf_addf(err, _("Could not access submodule '%s'\n"), + cs_data->path); + continue; + } + if (!is_tree_submodule_active(spf->r, cs_data->super_oid, + task->sub->path)) + continue; + + if (!spf->quiet) + strbuf_addf(err, + _("Fetching submodule %s%s at commit %s\n"), + spf->prefix, task->sub->path, + find_unique_abbrev(cs_data->super_oid, + DEFAULT_ABBREV)); + spf->changed_count++; + return task; + } + return NULL; +} + static int get_next_submodule(struct child_process *cp, struct strbuf *err, void *data, void **task_cb) { struct submodule_parallel_fetch *spf = data; const char *default_argv = NULL; - struct fetch_task *task = get_fetch_task(spf, &default_argv, err); + struct fetch_task *task = + get_fetch_task_from_index(spf, &default_argv, err); + if (!task) + task = get_fetch_task_from_changed(spf, &default_argv, err); if (task) { struct strbuf submodule_prefix = STRBUF_INIT; @@ -1555,6 +1660,7 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err, *task_cb = task; strbuf_release(&submodule_prefix); + string_list_insert(&spf->seen_submodule_names, task->sub->name); return 1; } @@ -1669,11 +1775,11 @@ static int fetch_finish(int retvalue, struct strbuf *err, return 0; } -int fetch_populated_submodules(struct repository *r, - const struct strvec *options, - const char *prefix, int command_line_option, - int default_option, - int quiet, int max_parallel_jobs) +int fetch_submodules(struct repository *r, + const struct strvec *options, + const char *prefix, int command_line_option, + int default_option, + int quiet, int max_parallel_jobs) { int i; struct submodule_parallel_fetch spf = SPF_INIT; diff --git a/submodule.h b/submodule.h index 784ceffc0e..61bebde319 100644 --- a/submodule.h +++ b/submodule.h @@ -88,12 +88,12 @@ int should_update_submodules(void); */ const struct submodule *submodule_from_ce(const struct cache_entry *ce); void check_for_new_submodule_commits(struct object_id *oid); -int fetch_populated_submodules(struct repository *r, - const struct strvec *options, - const char *prefix, - int command_line_option, - int default_option, - int quiet, int max_parallel_jobs); +int fetch_submodules(struct repository *r, + const struct strvec *options, + const char *prefix, + int command_line_option, + int default_option, + int quiet, 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/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index cb18f0ac21..df44757468 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -399,6 +399,201 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess verify_fetch_result actual.err ' +# Test that we can fetch submodules in other branches by running fetch +# in a commit that has no submodules. +test_expect_success 'setup downstream branch without submodules' ' + ( + cd downstream && + git checkout --recurse-submodules -b no-submodules && + rm .gitmodules && + git rm submodule && + git add .gitmodules && + git commit -m "no submodules" && + git checkout --recurse-submodules super + ) +' + +test_expect_success "'--recurse-submodules=on-demand' should fetch submodule commits if the submodule is changed but the index has no submodules" ' + git -C downstream fetch --recurse-submodules && + # Create new superproject commit with updated submodules + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err + ) && + test_must_be_empty actual.out && + git rev-parse --short HEAD >superhead && + git -C submodule rev-parse --short HEAD >subhead && + git -C deepsubmodule rev-parse --short HEAD >deephead && + verify_fetch_result actual.err && + + # Assert that the fetch happened at the non-HEAD commits + grep "Fetching submodule submodule at commit $superhead" actual.err && + grep "Fetching submodule submodule/subdir/deepsubmodule at commit $subhead" actual.err +' + +test_expect_success "'--recurse-submodules' should fetch submodule commits if the submodule is changed but the index has no submodules" ' + # Fetch any leftover commits from other tests. + git -C downstream fetch --recurse-submodules && + # Create new superproject commit with updated submodules + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git fetch --recurse-submodules >../actual.out 2>../actual.err + ) && + test_must_be_empty actual.out && + git rev-parse --short HEAD >superhead && + git -C submodule rev-parse --short HEAD >subhead && + git -C deepsubmodule rev-parse --short HEAD >deephead && + verify_fetch_result actual.err && + + # Assert that the fetch happened at the non-HEAD commits + grep "Fetching submodule submodule at commit $superhead" actual.err && + grep "Fetching submodule submodule/subdir/deepsubmodule at commit $subhead" actual.err +' + +test_expect_success "'--recurse-submodules' should ignore changed, inactive submodules" ' + # Fetch any leftover commits from other tests. + git -C downstream fetch --recurse-submodules && + # Create new superproject commit with updated submodules + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git -c submodule.submodule.active=false fetch --recurse-submodules >../actual.out 2>../actual.err + ) && + test_must_be_empty actual.out && + git rev-parse --short HEAD >superhead && + # Neither should be fetched because the submodule is inactive + rm subhead && + rm deephead && + verify_fetch_result actual.err +' + +# In downstream, init "submodule2", but do not check it out while +# fetching. This lets us assert that unpopulated submodules can be +# fetched. +test_expect_success 'setup downstream branch with other submodule' ' + mkdir submodule2 && + ( + cd submodule2 && + git init && + echo sub2content >sub2file && + git add sub2file && + git commit -a -m new && + git branch -M sub2 + ) && + git checkout -b super-sub2-only && + git submodule add "$pwd/submodule2" submodule2 && + git commit -m "add sub2" && + git checkout super && + ( + cd downstream && + git fetch --recurse-submodules origin && + git checkout super-sub2-only && + # Explicitly run "git submodule update" because sub2 is new + # and has not been cloned. + git submodule update --init && + git checkout --recurse-submodules super + ) +' + +test_expect_success "'--recurse-submodules' should fetch submodule commits in changed submodules and the index" ' + # Fetch any leftover commits from other tests. + git -C downstream fetch --recurse-submodules && + # Create new commit in origin/super + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Create new commit in origin/super-sub2-only + git checkout super-sub2-only && + ( + cd submodule2 && + test_commit --no-tag foo + ) && + git add submodule2 && + git commit -m "new submodule2" && + + git checkout super && + ( + cd downstream && + git fetch --recurse-submodules >../actual.out 2>../actual.err + ) && + test_must_be_empty actual.out && + + # Assert that the submodules in the super branch are fetched + git rev-parse --short HEAD >superhead && + git -C submodule rev-parse --short HEAD >subhead && + git -C deepsubmodule rev-parse --short HEAD >deephead && + verify_fetch_result actual.err && + # grep for the exact line to check that the submodule is read + # from the index, not from a commit + grep "^Fetching submodule submodule\$" actual.err && + + # Assert that super-sub2-only and submodule2 were fetched even + # though another branch is checked out + super_sub2_only_head=$(git rev-parse --short super-sub2-only) && + grep -E "\.\.${super_sub2_only_head}\s+super-sub2-only\s+-> origin/super-sub2-only" actual.err && + grep "Fetching submodule submodule2 at commit $super_sub2_only_head" actual.err && + sub2head=$(git -C submodule2 rev-parse --short HEAD) && + grep -E "\.\.${sub2head}\s+sub2\s+-> origin/sub2" actual.err +' + test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' add_upstream_commit && echo a >> file && From patchwork Tue Feb 15 17:23:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747420 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 817FBC433EF for ; Tue, 15 Feb 2022 17:24:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242513AbiBORYZ (ORCPT ); Tue, 15 Feb 2022 12:24:25 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60618 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242493AbiBORYW (ORCPT ); Tue, 15 Feb 2022 12:24:22 -0500 Received: from mail-pj1-x1049.google.com (mail-pj1-x1049.google.com [IPv6:2607:f8b0:4864:20::1049]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2560EDB847 for ; Tue, 15 Feb 2022 09:24:05 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id bj11-20020a17090b088b00b001b8ba3e7ce7so2919503pjb.2 for ; Tue, 15 Feb 2022 09:24:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=YtWsRIqNBsOpYmuM8tTo+UFh4Ku4XbbMHXY/iecDSKQ=; b=NdJYDdX2SpV4L4nEN4uu70GexvPYF7QnFirdwF4yVcjotb/4jNMQw/y2urCcbm87bU ITdoOrqgAgGXxeA6eL28ZiTnMLLK0LKUSoZpaR9ti/l6YvhQ9wjWxhhtjq/G7fU3ouCA YEm9N5Yb+3bcOGtAZoypVt4l3roQhuhiPVY2en6NCU6Hrxni5RwBNA9KEglgVm4hUbMp KqKRG7IQODH1c5RgIQBViHXPjWpWIYkFZxb3ET3CkoTmwQRfAzvxK+w/a6hofZwou0AU Hbigv2ralsl+r6B+3q55+vgfDCRpFwC8ZaMKc2Gi5GOCI0qYplta+STah9Xm9dNz/KSf JhnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=YtWsRIqNBsOpYmuM8tTo+UFh4Ku4XbbMHXY/iecDSKQ=; b=cZT0lI4D9MyK9gu7hg8aPMTt4Pt9uKYz/LKTvp1QbwF75iuUwAyA6WH9KDX+XcyV+Z 4j9dny9rqL+oikLWs9b+UuhggkgrZmSkFHR+y3rlCXZkfc8Hy8TqCphI41we0w0BDVcN fpt4Wr/RQPRV/pzrxv58GC5vfBCFR48FOZtJY+M/HxAJ2qcwrN/+fyCW6E83boSlM7ZG DvfweB0PTMUhHV1Ssr6ASLmAaxWvGF7szuFtKXagHzL7ErBoIlIBvsS5H4jmleYyg/6X 7aYOrZJAkGvzAS5o3Wzq5h0QFDPe3P+CyGlRaXD9nTD4wYlFpObwRtqTgbQ4buACuDdx Dx1Q== X-Gm-Message-State: AOAM530zde5dAACKQoDrOcBwqOoHQ2EvFp0TwQWCTOIQrgvEzCrFIBsc xBNBrQDL+21Qz+Ip4ifSr25CbnLd3lTM6h0miQ17WAf0ovjqeD3cXWxjhOy2WI55KOKSaZ1Xq1A 4SC+U9K40ty//FuiWzHRYgKpI2q+vBf5EgRxrTq4pN/0XXKTJhJxZsMa6+z/Qc9o= X-Google-Smtp-Source: ABdhPJzHhgHpDoVYKqTWzMLN+bo7+/7jjI5hHCLWOLblCjYjGQxaYJE7DZDE0Gx/lXBBSwyqAWcsxYftnzAlFg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:902:b189:: with SMTP id s9mr5249777plr.112.1644945844563; Tue, 15 Feb 2022 09:24:04 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:17 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-9-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 8/9] submodule: read shallows when finding changed submodules From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In a repository with submodules, "git fetch --update-shallow" can fail. This happens because "git fetch" does not read shallows when rev walking the newly fetched commits to find changed submodules, thus the rev walk may try to read the parent of a shallow and fail. This can occur when --recurse-submodules is not passed, because the default behavior is to fetch changed submodules i.e. --recurse-submodules=on-demand. Fix this by reading shallows before the rev walk, and test for it. Signed-off-by: Glen Choo --- submodule.c | 4 ++++ t/t5526-fetch-submodules.sh | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/submodule.c b/submodule.c index 3558fddeb7..e62619bee0 100644 --- a/submodule.c +++ b/submodule.c @@ -22,6 +22,7 @@ #include "parse-options.h" #include "object-store.h" #include "commit-reach.h" +#include "shallow.h" static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; static int initialized_fetch_ref_tips; @@ -901,6 +902,9 @@ static void collect_changed_submodules(struct repository *r, save_warning = warn_on_object_refname_ambiguity; warn_on_object_refname_ambiguity = 0; + /* make sure shallows are read */ + is_repository_shallow(the_repository); + repo_init_revisions(r, &rev, NULL); setup_revisions(argv->nr, argv->v, &rev, &s_r_opt); warn_on_object_refname_ambiguity = save_warning; diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index df44757468..ea70c3646f 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -1031,4 +1031,14 @@ test_expect_success 'recursive fetch after deinit a submodule' ' test_cmp expect actual ' +test_expect_success 'recursive fetch does not fail with --update-shallow' ' + git clone --no-local --depth=2 --recurse-submodules . shallow && + git init notshallow && + ( + cd notshallow && + git submodule add ../submodule sub && + git fetch --update-shallow ../shallow/.git refs/heads/*:refs/remotes/shallow/* --recurse-submodules + ) +' + test_done From patchwork Tue Feb 15 17:23:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12747421 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 D32B3C433EF for ; Tue, 15 Feb 2022 17:24:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239611AbiBORY2 (ORCPT ); Tue, 15 Feb 2022 12:24:28 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60636 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242506AbiBORYW (ORCPT ); Tue, 15 Feb 2022 12:24:22 -0500 Received: from mail-pj1-x1049.google.com (mail-pj1-x1049.google.com [IPv6:2607:f8b0:4864:20::1049]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1DB47DB858 for ; Tue, 15 Feb 2022 09:24:07 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id b9-20020a17090aa58900b001b8b14b4aabso2181475pjq.9 for ; Tue, 15 Feb 2022 09:24:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=dhNROOWvhpq6Se+XakL43bU4Q/fyYkU921uUB/pgwFk=; b=hUY9rw4UxRWNvIIkW0/qlE+JMvZ3ikVPoabNTkoxVwXgtZJQkeD7xna9usNCFGP+UY Sao+YtnNhr/CJdubwsOtY5bO7zeb0ZnU4bUTxXoSieTVfG1ohrycC+mdXtaapuXKLOvB Sc/eSRJenQxmccntWyzjpQOY+1GenOfihJZCBv79t1Oe2yaWjeE/qDr28oU+YR5aR/kq 23kSM0skCty8Eblsh/8cblI6opAc3PB5MtFE7Wp+V2iFAkWnGx4xnA5e2r7xwy/+drrk TrCVuth354kO7Bvrtc2Cv6u8nqwx/MldCCHhlTmDT9ipNPqjtHw5z6/aOak2CqR5od24 z8DQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=dhNROOWvhpq6Se+XakL43bU4Q/fyYkU921uUB/pgwFk=; b=uoJT5na2LlBZiK5oo+0qTVYMz5Nac4Z0OdhrA/IJmCdC0M5GnM3tskTVYuc6aUtwTN 031BbyLs4x0IblMQRwrHIExZljhxZXrco6gHlzU7ohHdS8bgUxA/UnYjsPUXplhoSR+i e4JVhnd7YkCYtoCkSuZKcZ+LREhHrp+tqWgRRzqcmHiiqOBdIVputL7Hd2d+Xi9+Xea3 NFZ4YkvxeVGVR18Y00EOgOdQ/YJ33HDGxM9djLY1m7/5sBnQ8QjvCAR2qMbw28upXNmc l1ZysAWF7lh/H+g+z3/T+wReWBng3lEzTTBOh3k2JjnYk0nMNRgkOJRvp4GzvT1dee0J QD5w== X-Gm-Message-State: AOAM531nFj1kJAjZV6Dmua4x1o5x2QfK4HNrZv/68AEXhsAIkf2jd9ha ekxMhG0gRQlOCis0QWHnFpX0Ds3W001VdtLaz11qCHptI+eb/fB6mxy3HOflI36aUHBknllcTOK cbw7E1NXJRA/kYy3FM/Ov7/47Am+BY5HJYBxdQRsgI4brwF9yw8e0yOaUKvgGW1g= X-Google-Smtp-Source: ABdhPJwbIsfqYwjF/qEs2JOPF2qPlg6952XqZMho1Z/jBP433mlU3pDvAn1z6sE07KuchWiTLuVcNQFZ7Er3eg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:aa7:95a5:: with SMTP id a5mr793734pfk.12.1644945846478; Tue, 15 Feb 2022 09:24:06 -0800 (PST) Date: Wed, 16 Feb 2022 01:23:18 +0800 In-Reply-To: <20220215172318.73533-1-chooglen@google.com> Message-Id: <20220215172318.73533-10-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> <20220215172318.73533-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.265.g69c8d7142f-goog Subject: [PATCH v2 9/9] submodule: fix latent check_has_commit() bug From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When check_has_commit() is called on a missing submodule, initialization of the struct repository fails, but it attempts to clear the struct anyway (which is a fatal error). This bug is masked by its only caller, submodule_has_commits(), first calling add_submodule_odb() - the latter fails if the submodule does not exist, making submodule_has_commits() exit early and not invoke check_has_commit(). Fix this bug, and because calling add_submodule_odb() is no longer necessary as of 13a2f620b2 (submodule: pass repo to check_has_commit(), 2021-10-08), remove that call too. This is the last caller of add_submodule_odb(), so remove that function. (Submodule ODBs are still added as alternates via add_submodule_odb_by_path().) Signed-off-by: Glen Choo --- submodule.c | 35 ++--------------------------------- submodule.h | 9 ++++----- 2 files changed, 6 insertions(+), 38 deletions(-) diff --git a/submodule.c b/submodule.c index e62619bee0..2b17440777 100644 --- a/submodule.c +++ b/submodule.c @@ -168,26 +168,6 @@ void stage_updated_gitmodules(struct index_state *istate) static struct string_list added_submodule_odb_paths = STRING_LIST_INIT_NODUP; -/* TODO: remove this function, use repo_submodule_init instead. */ -int add_submodule_odb(const char *path) -{ - struct strbuf objects_directory = STRBUF_INIT; - int ret = 0; - - ret = strbuf_git_path_submodule(&objects_directory, path, "objects/"); - if (ret) - goto done; - if (!is_directory(objects_directory.buf)) { - ret = -1; - goto done; - } - string_list_insert(&added_submodule_odb_paths, - strbuf_detach(&objects_directory, NULL)); -done: - strbuf_release(&objects_directory); - return ret; -} - void add_submodule_odb_by_path(const char *path) { string_list_insert(&added_submodule_odb_paths, xstrdup(path)); @@ -966,7 +946,8 @@ static int check_has_commit(const struct object_id *oid, void *data) if (repo_submodule_init(&subrepo, cb->repo, cb->path, cb->super_oid)) { cb->result = 0; - goto cleanup; + /* subrepo failed to init, so don't clean it up. */ + return 0; } type = oid_object_info(&subrepo, oid, NULL); @@ -997,18 +978,6 @@ static int submodule_has_commits(struct repository *r, { struct has_commit_data has_commit = { r, 1, path, super_oid }; - /* - * Perform a cheap, but incorrect check for the existence of 'commits'. - * This is done by adding the submodule's object store to the in-core - * object store, and then querying for each commit's existence. If we - * do not have the commit object anywhere, there is no chance we have - * it in the object store of the correct submodule and have it - * reachable from a ref, so we can fail early without spawning rev-list - * which is expensive. - */ - if (add_submodule_odb(path)) - return 0; - oid_array_for_each_unique(commits, check_has_commit, &has_commit); if (has_commit.result) { diff --git a/submodule.h b/submodule.h index 61bebde319..40c1445237 100644 --- a/submodule.h +++ b/submodule.h @@ -103,12 +103,11 @@ int submodule_uses_gitfile(const char *path); int bad_to_remove_submodule(const char *path, unsigned flags); /* - * Call add_submodule_odb() to add the submodule at the given path to a list. - * When register_all_submodule_odb_as_alternates() is called, the object stores - * of all submodules in that list will be added as alternates in - * the_repository. + * Call add_submodule_odb_by_path() to add the submodule at the given + * path to a list. When register_all_submodule_odb_as_alternates() is + * called, the object stores of all submodules in that list will be + * added as alternates in the_repository. */ -int add_submodule_odb(const char *path); void add_submodule_odb_by_path(const char *path); int register_all_submodule_odb_as_alternates(void);