From patchwork Fri Mar 4 00:57:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768354 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 2B9E8C433F5 for ; Fri, 4 Mar 2022 00:58:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237471AbiCDA65 (ORCPT ); Thu, 3 Mar 2022 19:58:57 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43768 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236860AbiCDA6z (ORCPT ); Thu, 3 Mar 2022 19:58:55 -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 1FB04F1EB1 for ; Thu, 3 Mar 2022 16:58:08 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id x6-20020a1709029a4600b0014efe26b04fso3752245plv.21 for ; Thu, 03 Mar 2022 16:58:08 -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=OmedMzQwk2A+2KKPR+3WZqRF/VcgdnACdWhsY0RDGWg=; b=l6FRu+3SN0mfdnT2JMxqdd/mbCbYPGj0O9MSRKRWNnlgIGkTkFSOnUugxXymDd2H1p P403BOsktxZBlAIM2e8Rf9W9/LE8f1sMuJ9peeSdA8khLGRXDkw8BiN8OodPJ0vKAAde bAyxTbcu6MjziHWHEY7DV3+ZIyuGXFBBbzpRO4bRvaKSRq0lTaXxGoYuSeVIlmLUFeRN YqYTI2Zb0z7CuJ7XxTK0fZg7UttHYv7oogwOQL+kx5CilyeoAcDS5xUYe/Snz8tJklHk poej7XXNmH2B+Bn4gIibhVI81d1Hncxct6X6MpmBsx2BITomSu67Ac9ten0TGEpHGMuZ jE0g== 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=OmedMzQwk2A+2KKPR+3WZqRF/VcgdnACdWhsY0RDGWg=; b=hVKFvq7NfSqVloJY6mHUm3ArfRwWMz1Snc+qrU9p8yHZSLTAVskLTAhdmfgwvdqirL ytRsm/d2Nc2ZobuQiQI4hgWIjaA6KWz21l1Icwguq79iHuqtzcn7EMe5ALuPgwI42jS/ na1HPJYTXMfpgizLtcHmUQHyKbo0HTxDrmal2k4EmCFQ8XoYcJqhBZqcKfWQs+Ds92q+ nNfk1PIQ8Zb1pDQO6WjjtEm283uAC4U/PiNvJkwqK+kr/01Ea67oj3iGmTyBR3izaFxF fu/X3IZqi5zzSN3VGfuBiZrORto3KokW+drkBGpnNwdS9MgSt/VLeIXmXzph1l/BEwaS ZxgQ== X-Gm-Message-State: AOAM533FY8pOwrTGq7PDY96eJv085ASg4NtpKfAOUHF3bz89vFDizM9H ph8iw8YqatOn4D/dQPAri/DXb3GgszjCy6EvZLCIGE/zKEA2wanhpjcLZnDzWiFCqtILY0H/VCs a7tLps20suavu4GZXSOGV1v80QDf+0OQbqg/2ZrWswD+0V8Ct3octSvSMeu+jXdU= X-Google-Smtp-Source: ABdhPJwXILvI2IjVee1nSwn8q4IikKn1jigcWxmKR+mc4H0kdUzGIPli371lAbMa7V8fbmx2Rcvuy66p8yFJFA== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:26c1:b0:4e1:3135:97a9 with SMTP id p1-20020a056a0026c100b004e1313597a9mr40985916pfw.13.1646355487231; Thu, 03 Mar 2022 16:58:07 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:48 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-2-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 01/10] t5526: introduce test helper to assert on fetches From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Tests in t/t5526-fetch-submodules.sh are unnecessarily noisy: * The tests have extra logic in order to reproduce the expected stderr literally, but not all of these details (e.g. the head of the remote-tracking branch before the fetch) are relevant to the test. * 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. Tests still consider the old head of the remote-tracking branch ("$head1"), but that will be fixed in a later commit. Signed-off-by: Glen Choo --- t/t5526-fetch-submodules.sh | 139 ++++++++++++++++++++++-------------- 1 file changed, 84 insertions(+), 55 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 840c89cc8b..dff7a4b90b 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -10,6 +10,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 && @@ -19,9 +23,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 && @@ -31,12 +35,36 @@ 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 test -f expect.err.super + then + cat expect.err.super >>expect.err.combined + fi && + if test -f expect.err.sub + then + cat expect.err.sub >>expect.err.combined + fi && + if test -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 && ( @@ -74,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" ' @@ -84,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" ' @@ -94,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 ' @@ -124,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" ' @@ -155,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" ' @@ -183,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" ' @@ -192,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" ' @@ -203,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" ' @@ -217,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" ' @@ -250,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 ' @@ -268,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" ' @@ -292,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 && @@ -303,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 ' @@ -328,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 ' @@ -372,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 && @@ -392,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)" ' @@ -402,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" ' @@ -423,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 && @@ -437,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" ' @@ -451,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 && @@ -465,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" ' @@ -477,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 @@ -502,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 && @@ -520,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 Fri Mar 4 00:57:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768356 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 EB18FC433FE for ; Fri, 4 Mar 2022 00:58:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237477AbiCDA7A (ORCPT ); Thu, 3 Mar 2022 19:59:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43852 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237462AbiCDA65 (ORCPT ); Thu, 3 Mar 2022 19:58:57 -0500 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F0907DEA20 for ; Thu, 3 Mar 2022 16:58:09 -0800 (PST) Received: by mail-pl1-x64a.google.com with SMTP id y3-20020a1709029b8300b0014c8bcb70a1so3794194plp.3 for ; Thu, 03 Mar 2022 16:58:09 -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=UtiFQDHyGZ99aHGiC9oJyVT0l3UY3UzqhzzOESuQXws=; b=RZhXrscyuLHDHKEyNDZ0oA9s9/G8jK4UNZrcM61NF/7UzTgxybDlMrAMDt8z0KRdwh 6zX1c/1hJ8xB1Qie17fcnwUSeMmC6572PciYVCPE3Xr79Vb2P9LwtQ9Vz0HzhCvsP79Q QpILb65ArP/NXxorfg0SZe9jATT7Ubo4VqQF0YgXqn7CLQDXd/06S7iCoczjlaivlRXs sPT75H/Sl1iQN0AeYS3oxWor3fjTjCG5mjTsu4QZBj2torpOAprlVE15zSQFfSmk4L6g qYKlclUF4usWjopW0BBSke9V5LDlGdmCm/EbisLkgbFoV5h+E7Fk3Z5agOAGyxmbpVW2 RRCw== 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=UtiFQDHyGZ99aHGiC9oJyVT0l3UY3UzqhzzOESuQXws=; b=Emyy+p1kaojMKA4JLHNsK+VnCRZflN/5vkwpYWjGJ100LK0CMdO+MvMAp7RoO44y7f l0q9hGbc+5soL3xxzHVNnC9BaLnEyUHQQnJOH8yCbIfwH776aYg5ANcEfKxMukM+4CuE pPl1tTURvr/U+Pc/Mv8uKsY6a3uEnHLD/uDC1gXehD6JtE3oUH1ipslUmbDGDsaDLJyz afRbXZRaKy31LKmqfC93ONteFD15hKhCuR6KB3yj2ah8ibUmRLXGAhQ6xFZWP3bWhmjk ZaFlCaTCqB6c0EdrCdpTxl0iyjN1PBAczvDxo9zQwJ5eZVo6L+11QUsuCp4Q0Oj7Qb08 9ddg== X-Gm-Message-State: AOAM532BacmnJ1JjiW69F49XholmBXdQe74earLKv3T1YesdKciOMMZZ fuLwQGvPE/SdpMwScggr90pcrGrNRzpgvCsmNK3wXByuJgdEpeGr14Y1Pqdw40A0L2AxZi4pQg4 Mor2TgpMEo5YdJ8hOe1J6N+CZOprQdFn5I8wBLhyfWSfjdPzcRAQTi7+vOGzeOys= X-Google-Smtp-Source: ABdhPJxTegZysOoNs3OQ82ov5RifdZzpXEA+3T0aG4v2oUjWihXEtGnXFz7DkmZDKgtFI/qC7aD3V0Cw3hcdcA== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:aa7:820b:0:b0:4f1:2d08:5e90 with SMTP id k11-20020aa7820b000000b004f12d085e90mr34820993pfi.31.1646355489349; Thu, 03 Mar 2022 16:58:09 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:49 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-3-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 02/10] t5526: stop asserting on stderr literally From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In the previous commit message, we noted that not all of the "git fetch" stderr is relevant to the tests. Most of the test setup lines are dedicated to these details of the stderr: 1. which repos (super/sub/deep) are involved in the fetch 2. the head of the remote-tracking branch before the fetch (i.e. $head1) 3. the head of the remote-tracking branch after the fetch (i.e. $head2) 1. and 3. are relevant because they tell us that the expected commit is fetched by the expected repo, but 2. is completely irrelevant. Stop asserting on $head1 by replacing it with a dummy value in the actual and expected output. Do this by introducing test helpers (check_*()) that make it easier to construct the expected output, and use sed to munge the actual output. Signed-off-by: Glen Choo --- t/t5526-fetch-submodules.sh | 119 +++++++++++++++++------------------- 1 file changed, 57 insertions(+), 62 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index dff7a4b90b..6b24d37b2b 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -10,6 +10,32 @@ export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB pwd=$(pwd) +check_sub () { + NEW_HEAD=$1 && + cat >$pwd/expect.err.sub <<-EOF + Fetching submodule submodule + From $pwd/submodule + OLD_HEAD..$NEW_HEAD sub -> origin/sub + EOF +} + +check_deep () { + NEW_HEAD=$1 && + cat >$pwd/expect.err.deep <<-EOF + Fetching submodule submodule/subdir/deepsubmodule + From $pwd/deepsubmodule + OLD_HEAD..$NEW_HEAD deep -> origin/deep + EOF +} + +check_super () { + NEW_HEAD=$1 && + cat >$pwd/expect.err.super <<-EOF + From $pwd/. + OLD_HEAD..$NEW_HEAD super -> origin/super + EOF +} + # 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 @@ -17,27 +43,21 @@ 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 + new_head=$(git rev-parse --short HEAD) && + check_sub $new_head ) && ( 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 + new_head=$(git rev-parse --short HEAD) && + check_deep $new_head ) } @@ -47,7 +67,7 @@ add_upstream_commit() { # # If a repo should not be fetched in the test, its corresponding # expect.err file should be rm-ed. -verify_fetch_result() { +verify_fetch_result () { ACTUAL_ERR=$1 && rm -f expect.err.combined && if test -f expect.err.super @@ -62,7 +82,8 @@ verify_fetch_result() { then cat expect.err.deep >>expect.err.combined fi && - test_cmp expect.err.combined $ACTUAL_ERR + sed -e 's/[0-9a-f][0-9a-f]*\.\./OLD_HEAD\.\./' "$ACTUAL_ERR" >actual.err.cmp && + test_cmp expect.err.combined actual.err.cmp } test_expect_success setup ' @@ -274,12 +295,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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && rm expect.err.deep && ( cd downstream && @@ -291,13 +310,11 @@ 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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && rm expect.err.sub && rm expect.err.deep && ( @@ -318,12 +335,10 @@ 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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && ( cd downstream && git fetch >../actual.out 2>../actual.err && @@ -345,20 +360,15 @@ 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 + new_head=$(git rev-parse --short HEAD) && + check_sub $new_head ) && - 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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -376,13 +386,10 @@ 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 + new_head=$(git rev-parse --short HEAD) && + check_sub $new_head ) && ( cd downstream && @@ -395,12 +402,10 @@ 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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && ( cd downstream && git config fetch.recurseSubmodules false && @@ -421,13 +426,11 @@ 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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && rm expect.err.sub && rm expect.err.deep && ( @@ -445,12 +448,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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && rm expect.err.deep && ( cd downstream && @@ -473,12 +474,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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && rm expect.err.deep && ( cd downstream && @@ -499,12 +498,10 @@ 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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && rm expect.err.sub && # This file does not exist, but rm -f for readability rm -f expect.err.deep && @@ -526,13 +523,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 && + new_head=$(git rev-parse --short HEAD) && + check_super $new_head && rm expect.err.deep && ( cd downstream && From patchwork Fri Mar 4 00:57:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768357 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 31B83C433F5 for ; Fri, 4 Mar 2022 00:58:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237497AbiCDA7L (ORCPT ); Thu, 3 Mar 2022 19:59:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43946 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237474AbiCDA66 (ORCPT ); Thu, 3 Mar 2022 19:58:58 -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 1A709DE2F1 for ; Thu, 3 Mar 2022 16:58:12 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id s12-20020a17090a13cc00b001bee1e1677fso3091687pjf.0 for ; Thu, 03 Mar 2022 16:58:12 -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=sjsCTsfNsW+DgcsxwDdXr8+Pg/0xxaH4ZN6XwoYz89o=; b=Jr316B7dhf3Kz4pCc5OZOBSbeMxNHfXHIMvF9JXW+8cxwtTuB0SpLH6y6kGm1CIJ2S P2FKILMnKRNvzkfnkbmYqi0xa0xlfCjTXyT0eXzhamAc6UkpjN/tZhLJLphZRHO3opVI 1qKfwV4qiaQbT3ai4BAuIquoAHstaR8JQTR7aZ30IjpfhY1nIglFwCUdcEs5PnAqiY5x /5KVaO0OL2Ctcvo+ErBdF8Yxy+OHVLPFFCpd16KWsbbR6okVy+g4grg15DCAUFuMXndl O6lNFUTqqkg0cST4zy/s1vQLm6UYd7JeGu7UYGMLWFuth6Ps45LcFEWPKeP7oCb+oqpq 9WUA== 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=sjsCTsfNsW+DgcsxwDdXr8+Pg/0xxaH4ZN6XwoYz89o=; b=O5zH1WJlTq/kp+FHTt7R1VtobZx8SMjIglVtpfap+jaXVHbCmm8nj81aIeSq7sXn60 GJWq4iITzozoVb5ODFSSeq05aF1+7Hns40ZIwVKHn8hIgkJjC42asCyMT5XmsI66VMn2 MVz25++aug6osQ5GoU0AhnS905GREKYiir9NBSFiYTuaRiwmAtxoCqpRvdw0NPO2JS5p Tjn8lPZUVlN0Nd6ZTdWRqYl0Q4A/4ULt9CXPieb0cbGa9lN0iTS+c28oEHyfWX8K3EPQ zzyAShoP94AM/FDs2F1oao25TJ1v4moxk1lwb1cnttcmnP9Ls/yj4uB1VbhtMOfBcW0c Ojmw== X-Gm-Message-State: AOAM532JCw8rlFJl0h9N1bBKzm5xXNGWfzZkR6LJgkTrBjF4EYGlCkqV 8KfihuAyioAYLEVaEzPTSRAC8v/iieXCv+M0C7QN2FBhO31ySUzzue2MlLvnV8Dhhh/Xu9X7L1V kd7go2AdDGxliJjjtoxqe9ThcExfIjeiJsglrnlZaMy/udfXOKa6UZVcOMgVgFyA= X-Google-Smtp-Source: ABdhPJw1K2iPgjsV71YKeE+lgWAO3P0gPsB29Zf5UbAHxjG3v7DB/9JPRxxfBCXev3et4pBbJ38IiDsw9Obk9Q== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90b:945:b0:1bd:561e:b24d with SMTP id dw5-20020a17090b094500b001bd561eb24dmr8191832pjb.202.1646355491467; Thu, 03 Mar 2022 16:58:11 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:50 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-4-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 03/10] t5526: create superproject commits with test helper From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A few tests in t5526 use this pattern as part of their setup: 1. Create new commits in the upstream submodules (using add_upstream_commit()). 2. In the upstream superprojects, add the new submodule commits from the previous step. A future commit will add more tests with this pattern, so reduce the verbosity of present and future tests by introducing a test helper that creates superproject commits. Since we now have two helpers that add upstream commits, rename add_upstream_commit() to add_submodule_commits(). Signed-off-by: Glen Choo --- t/t5526-fetch-submodules.sh | 94 +++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 50 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 6b24d37b2b..4cae2e4f7c 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -40,7 +40,7 @@ check_super () { # 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() { +add_submodule_commits () { ( cd submodule && echo new >> subfile && @@ -61,6 +61,30 @@ add_upstream_commit() { ) } +# For each superproject in the test setup, update its submodule, add the +# submodule and create a new commit with the submodule change. +# +# This requires add_submodule_commits() to be called first, otherwise +# the submodules will not have changed and cannot be "git add"-ed. +add_superproject_commits () { + ( + 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" && + super_head=$(git rev-parse --short HEAD) && + sub_head=$(git -C submodule rev-parse --short HEAD) && + check_super $super_head && + check_sub $sub_head +} + # 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. @@ -117,7 +141,7 @@ test_expect_success setup ' ' test_expect_success "fetch --recurse-submodules recurses into submodules" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && git fetch --recurse-submodules >../actual.out 2>../actual.err @@ -127,7 +151,7 @@ test_expect_success "fetch --recurse-submodules recurses into submodules" ' ' test_expect_success "submodule.recurse option triggers recursive fetch" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && git -c submodule.recurse fetch >../actual.out 2>../actual.err @@ -137,7 +161,7 @@ test_expect_success "submodule.recurse option triggers recursive fetch" ' ' test_expect_success "fetch --recurse-submodules -j2 has the same output behaviour" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && GIT_TRACE="$TRASH_DIRECTORY/trace.out" git fetch --recurse-submodules -j2 2>../actual.err @@ -148,7 +172,7 @@ test_expect_success "fetch --recurse-submodules -j2 has the same output behaviou ' test_expect_success "fetch alone only fetches superproject" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -177,7 +201,7 @@ test_expect_success "using fetchRecurseSubmodules=true in .gitmodules recurses i ' test_expect_success "--no-recurse-submodules overrides .gitmodules config" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && git fetch --no-recurse-submodules >../actual.out 2>../actual.err @@ -226,7 +250,7 @@ test_expect_success "--quiet propagates to parallel submodules" ' ' test_expect_success "--dry-run propagates to submodules" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && git fetch --recurse-submodules --dry-run >../actual.out 2>../actual.err @@ -245,7 +269,7 @@ test_expect_success "Without --dry-run propagates to submodules" ' ' test_expect_success "recurseSubmodules=true propagates into submodules" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && git config fetch.recurseSubmodules true && @@ -256,7 +280,7 @@ test_expect_success "recurseSubmodules=true propagates into submodules" ' ' test_expect_success "--recurse-submodules overrides config in submodule" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && ( @@ -270,7 +294,7 @@ test_expect_success "--recurse-submodules overrides config in submodule" ' ' test_expect_success "--no-recurse-submodules overrides config setting" ' - add_upstream_commit && + add_submodule_commits && ( cd downstream && git config fetch.recurseSubmodules true && @@ -309,7 +333,7 @@ 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 && + add_submodule_commits && echo a > file && git add file && git commit -m "new file" && @@ -334,7 +358,7 @@ test_expect_success "Recursion picks up config in submodule" ' git config fetch.recurseSubmodules true ) ) && - add_upstream_commit && + add_submodule_commits && git add submodule && git commit -m "new submodule" && new_head=$(git rev-parse --short HEAD) && @@ -352,23 +376,8 @@ test_expect_success "Recursion picks up config in submodule" ' ' test_expect_success "Recursion picks up all submodules when necessary" ' - add_upstream_commit && - ( - cd submodule && - ( - cd subdir/deepsubmodule && - git fetch && - git checkout -q FETCH_HEAD - ) && - git add subdir/deepsubmodule && - git commit -m "new deepsubmodule" && - new_head=$(git rev-parse --short HEAD) && - check_sub $new_head - ) && - git add submodule && - git commit -m "new submodule" && - new_head=$(git rev-parse --short HEAD) && - check_super $new_head && + add_submodule_commits && + add_superproject_commits && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -378,19 +387,7 @@ test_expect_success "Recursion picks up all submodules when necessary" ' ' test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no new commits are fetched in the superproject (and ignores config)" ' - add_upstream_commit && - ( - cd submodule && - ( - cd subdir/deepsubmodule && - git fetch && - git checkout -q FETCH_HEAD - ) && - git add subdir/deepsubmodule && - git commit -m "new deepsubmodule" && - new_head=$(git rev-parse --short HEAD) && - check_sub $new_head - ) && + add_submodule_commits && ( cd downstream && git config fetch.recurseSubmodules true && @@ -402,10 +399,7 @@ 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)" ' - git add submodule && - git commit -m "new submodule" && - new_head=$(git rev-parse --short HEAD) && - check_super $new_head && + add_superproject_commits && ( cd downstream && git config fetch.recurseSubmodules false && @@ -425,7 +419,7 @@ 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 && + add_submodule_commits && echo a >> file && git add file && git commit -m "new file" && @@ -446,7 +440,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config cd downstream && git fetch --recurse-submodules ) && - add_upstream_commit && + add_submodule_commits && git config --global fetch.recurseSubmodules false && git add submodule && git commit -m "new submodule" && @@ -472,7 +466,7 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override cd downstream && git fetch --recurse-submodules ) && - add_upstream_commit && + add_submodule_commits && git config fetch.recurseSubmodules false && git add submodule && git commit -m "new submodule" && @@ -522,7 +516,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git cd downstream && git fetch --recurse-submodules ) && - add_upstream_commit && + add_submodule_commits && git add submodule && git rm .gitmodules && git commit -m "new submodule without .gitmodules" && From patchwork Fri Mar 4 00:57:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768358 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 7228FC433FE for ; Fri, 4 Mar 2022 00:58:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237485AbiCDA7M (ORCPT ); Thu, 3 Mar 2022 19:59:12 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44864 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237480AbiCDA7J (ORCPT ); Thu, 3 Mar 2022 19:59:09 -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 5BDE41201AD for ; Thu, 3 Mar 2022 16:58:14 -0800 (PST) Received: by mail-pf1-x44a.google.com with SMTP id t184-20020a6281c1000000b004e103c5f726so4087256pfd.8 for ; Thu, 03 Mar 2022 16:58:14 -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=jMKFFMj25zrRdrj/VKq4uwdhmkhAO9UMRLTmQPqt2tU=; b=BrF9fDQqyYkef1lgYdFYq5Eom7uGqe/nhAvj6653Oqt+UD1zfTq84Zb15Zy1ETxHD7 ydaOxPUpr4MkhJm1tznxucGCX/PeyaE77VYFsEU+4cORk+3ESpN+7q7/YXI+u5Mrwr3i gMlZAxYjywQr55tKsKhQdEWrRNcTNsf3kmbh1a4c7yARyxYRYNlUwtbyzHLO6ZRXIoBv K2xwlb93DwsGI9mEJT2lMNrwK/kCv7DkRALu+Ljx34KfGrILrFbRXi7S3WOhpA8XizTZ +CauBZnKrgnkPF60pbSuhfaSK4F6e/zwSskDh9T8UhUqbRymH1nbhQhRxd3auPER7u+b aXzw== 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=jMKFFMj25zrRdrj/VKq4uwdhmkhAO9UMRLTmQPqt2tU=; b=xZYBvZuUvR5kUYWoRbfh9VVhG4z+CY9lNHmUfd0eN+NK8vQNVhHf+TI0f72tL15X9I eKILtSjGVWgTVX1lmxq2Zq9CkOTo7O2rybJoTm7PYnxk9oDpoOM/o5ywG87JD4Xcrc/G 0elZy6nXAjgVIrTj1lit2iKcQXBRJNy2EbXMvRpxcLDV0XGAC556dPoayN88vmFpHf9Y PL8uug6K6jNP3GVSvkqWfNZmrjGJ4abgrTu9aDMMPdQQ+RCi4aDd+JixABj0TZT8aUbo ayQghQUS4hA+RQNm9fPsFweiQE7/luCVInCbenFrMaaYznNRc2/KuSW1cL+u/gOpTsp6 c1AQ== X-Gm-Message-State: AOAM530SAWOE97jcMaZ91DKoma0mAEW7bucf5KlXMrOMpWBt56TT8MMy jqzxxdfI3DeYX5J/k8qgX8Rv6j4FnpuzB/2uDy2u2xozfNtg0JrzGQJxOoPZilUfSkvxSD4n+AV HtVkgTXmWcO4a69nzutMzkJlFMXp26V/54W0B6370EgBdxHqXnZQe6dBUjI+pL/I= X-Google-Smtp-Source: ABdhPJwJMYtzcKMQdrJMEj5hmm4yR3i3wS1+8uj3Kwzr8GKT8hu3BPongXUWZvaBS3PLMhpAUKSgfvOxYU49Vw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:1646:b0:4f4:58f:b64 with SMTP id m6-20020a056a00164600b004f4058f0b64mr25132783pfc.47.1646355493648; Thu, 03 Mar 2022 16:58:13 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:51 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-5-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 04/10] submodule: make static functions read submodules from commits From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " 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. Signed-off-by: Glen Choo --- submodule.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/submodule.c b/submodule.c index 5ace18a7d9..4f3300f2cb 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,15 @@ 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 = { + .repo = r, + .result = 1, + .path = path, + .super_oid = super_oid + }; /* * Perform a cheap, but incorrect check for the existence of 'commits'. @@ -1017,7 +1024,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 +1284,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 +1409,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 +1447,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 +1473,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 +1496,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 Fri Mar 4 00:57:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768359 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 50717C4332F for ; Fri, 4 Mar 2022 00:58:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236189AbiCDA7O (ORCPT ); Thu, 3 Mar 2022 19:59:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44938 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237486AbiCDA7K (ORCPT ); Thu, 3 Mar 2022 19:59:10 -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 229E1EFFBD for ; Thu, 3 Mar 2022 16:58:16 -0800 (PST) Received: by mail-pf1-x44a.google.com with SMTP id g72-20020a62524b000000b004f3e21965e8so4100752pfb.5 for ; Thu, 03 Mar 2022 16:58:16 -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=bqdqXeJL9WT1k5g0viCfj7gJvexmtha42dMO/axoffw=; b=qM48TkLoXOqJGcMiV1a3AZheyzWTkf2d/lK5qwGT9KitX0nOzweJImh/SWqrEpje+3 BpKuaQXZ4X6ScOC9lguYX3xoNgysdX7/jFpoUd8PNN4OZUxXW3zoFM/CHY/Lq+IFWwmN vnPZpRMT2H8V9ntZx/9rkoOpJfekdnYWe6AKgZdrlNYGc7T64KQmvGxeUol8GCmFPuT4 uXgdsryp7NOPmi+VAcAS7R3Etyu5+Ie2yeyLNxNGZl4KROqso63+qJBnL7QIGO2KSn7/ Pk9N+AS3cVLCmrF7MjHWiCgOoqyewukj1M8ff2Ff2U6/0b31gZDMiXKjzt6UECTjxrsB 3EPw== 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=bqdqXeJL9WT1k5g0viCfj7gJvexmtha42dMO/axoffw=; b=ijSGaRFKLRWF0Gsz/t3MvvzOZaUQDVUsa92I3rBW24EK4yN4bgk9aUxwuMVKMrpM85 LCEThdqs3Inhas30Ed/rk3saIGlZhKTi1/NjSoR6t9jMUm5PQ2knFeo/YXyn5NX99vqs pYQIO4w0ZBh9ucPuE8aWYnndf+VCeez7x0S9O6zUsTvq7MJOQxTTp6Pqa094JPxjDfqm 2LqnbNEVSe03BOafbhiQ/n8Wa8gpwBbo6upDdNc1HvodhkSCKgkuhA7SGJ3DspB728pj zmcErKigI9TWQHbULOK414P7tIUfBM1nC78vQd4S2xn3rgrnDBprickTolpoXMaoe344 D3Bw== X-Gm-Message-State: AOAM533lL6BYfkdi4ZoQDhXJbDY+v7u5TwIpAnmrUsM5mq7Z5LF2yTGO L9YmsNFTKxj32ACVa+rnbGj5hzwEoEZNOGCTbWuf6AqdQBj/T3+51qgIvV31OBjOSQs9q8IiXxR U9GPF5DinmamH6q38nPXuHL8n1bhyvwcijwwYTSBd8EZA23A6a81L9TqCN5wq2Yc= X-Google-Smtp-Source: ABdhPJxm7BCVQjz5YPCEXkGgmmGI2Jfj9QIxRTMm4uBfau+N3mMiPeAqEhArdZQ0l8h4xR+JUMaBABXQcmtTMA== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:902:e051:b0:151:b485:3453 with SMTP id x17-20020a170902e05100b00151b4853453mr3342794plx.116.1646355495489; Thu, 03 Mar 2022 16:58:15 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:52 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-6-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 05/10] submodule: inline submodule_commits() into caller From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " 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 4f3300f2cb..3bc189cf05 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 Fri Mar 4 00:57:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768360 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 40234C433F5 for ; Fri, 4 Mar 2022 00:58:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237480AbiCDA7O (ORCPT ); Thu, 3 Mar 2022 19:59:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44972 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237493AbiCDA7L (ORCPT ); Thu, 3 Mar 2022 19:59:11 -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 3DD2A13775D for ; Thu, 3 Mar 2022 16:58:18 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id n135-20020a628f8d000000b004e16d5bdcdbso4083258pfd.20 for ; Thu, 03 Mar 2022 16:58:18 -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=s/FLsWLRzpv4s8A/gCDPZWY0SRQ54rF476qzo5TKO5Y=; b=qOLri9Vll53yCv/GS8jw6ThOY6yDK1oeAt8IUe0FL2zpvnoUd6AYAXaXLhx4ny0ooc vdwC+Ej3lRrIBJhza8/UDY39TCG1i/Yu9K9Y/Vi9EV4E/2uI8gnt+76AZpr8eGXJcytA AKXuQl4qt691BqTgq/AXBOPvNQ3ZRdSFVUyb4CdmYlI5BZ6fP7fEi48Slxvurcc3dIGb SiTjpCuZpUev6ETYeYJSVnTRiOpQNnrHzfIL4jp/UpK/++LLB8L9Y/vopCTHtQoKaQtb KNrxNYGu5XRfJ3wPtgu9nlOYyG29JbJfMbO2OkrcVROl+rXL978GHHUMHw7wpwS00qFC /xXA== 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=s/FLsWLRzpv4s8A/gCDPZWY0SRQ54rF476qzo5TKO5Y=; b=DKXiw78ytkCvBFtyB85YnQE0775mMsmluNiDoYqx22yG26eZ6mKCXRH+L3UCMTtz0w eQlDz0JVW4PdgFoRx7ycemc+PON2EdrzoYNevMY0V6JLy3B9YUQ2om47ElKmfPT+9Bsl hvV1cVmkbj5bcqYyQLM63h8YEhjTDZnBK+YQKlAdLSA9Z/nqYl46+vChMl1zjAZb63Lp DudOHc8bn6mFomyUJtlWZk9UtGWLJtVDY6oowCrKUP6QQyhGHK2g9tBxDaadQhQSSf2u UxIYQGBZwcRq+6sHUqN4VhrCTtBVWlnWAGpZBqTEPVX0j91bnBIu6ykTkBiePyu4FU31 1M1Q== X-Gm-Message-State: AOAM531Eu4w3qvx7ON+rh6ahYj1c+N4rhMb8TPD60EUujmo2sV4sqvM3 +eTuPGIQq4BLkS50EDpJCkVCWz/f2jJCxcvA1vZmF2V4TAJ0Xukj4oqV7wkHyJb/5UUyBDcDIJb iCYhCMlSlzUbkHoiaLOHUuVEtu7GtjfDr4YvXkcUPQgTHm4pkaAgS7A+a41s4iN8= X-Google-Smtp-Source: ABdhPJyQSRIxGq5x7rJ+aVm/m43nyKhQvQA182P2VZo0q9CsR43svBEB1kmfF7gcs6BRakEzDbn+fULlzMEwGw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:902:6bc1:b0:14a:c390:a44b with SMTP id m1-20020a1709026bc100b0014ac390a44bmr40228056plt.11.1646355497658; Thu, 03 Mar 2022 16:58:17 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:53 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-7-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 06/10] 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 , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " 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 | 52 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/submodule.c b/submodule.c index 3bc189cf05..0b9c25f9d3 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); + changed_submodule_data_clear(item->util); + string_list_clear(submodules, 1); } @@ -1074,7 +1090,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; @@ -1087,11 +1103,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; @@ -1261,7 +1277,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; @@ -1274,8 +1290,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'; } } @@ -1312,7 +1328,7 @@ int submodule_touches_in_range(struct repository *r, strvec_clear(&args); - free_submodules_oids(&subs); + free_submodules_data(&subs); return ret; } @@ -1596,7 +1612,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"); @@ -1624,14 +1640,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); @@ -1689,7 +1705,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 Fri Mar 4 00:57:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768361 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 A53B4C433F5 for ; Fri, 4 Mar 2022 00:58:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237476AbiCDA7Q (ORCPT ); Thu, 3 Mar 2022 19:59:16 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44862 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237482AbiCDA7L (ORCPT ); Thu, 3 Mar 2022 19:59:11 -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 5936E13AA3B for ; Thu, 3 Mar 2022 16:58:20 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id d10-20020a17090a564a00b001beeadd3da1so3866988pji.3 for ; Thu, 03 Mar 2022 16:58:20 -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=0+DUoUS5ilqFWzyXgb3pLwZXXCHYiCAPtFOWIKm3PTw=; b=R/RRVCgABltLqeOAYLRfg7PXNUpgNoIxO2JrkuX2Cz+Nn4vUjyZZPkE4mHJ0FkURsP /G4kViVHLViWOUjcmNJRhF7qxfsL4cQ3Czc3Ws6QR6a2Bsl2ghg7miYcZzUS65j4ywmP UlaI1AkMSQ09/ghfcBDe8OaziYDhvAWfPk7uTkdsv7ap+0ZyBVEBHvVYQORycyi7gjKl vp8fczEOFAz+NSnjTJ+N4nEnez+zQnDGFSuJ11oels7xPvwsmIJZScHqsESFoMDaqfd9 Xj+dIgm3f79DUe7Wv2s3EKaVfoT1bBmJ6IE4P1FkFLwqA/zkdVat0HqcHTXQSd1yi/22 S+Ng== 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=0+DUoUS5ilqFWzyXgb3pLwZXXCHYiCAPtFOWIKm3PTw=; b=5dTMgBt/q/GPBSWOryXVF0nwjyXfNw/tukFL6Bi6Mi2L9SbdYhJnBCrlkkPpPxNfxZ IAP2ufcmVW+TWBVfeJFOnRzUzQ7032JiVSa1zds9JQ0+whjvIRiN0XrNZLFR/JpF3LLt 14UPLugVXJZtf6g9zpqbZ+jgJIO2rFk3a9kXCuyIIxaQ+ioUyhsAQ5+M9iNCvBZgyFro e+325BfBIri55Ru6jJl5c4Q9BGPct8Ya18g01YphSpWszZEzUCLf2BIkvLFdpLGFEmDW 4zIp+pSZtjPdHnpeHvwF4RocKhKvhkM6Zr2Oya9J+MQ8ny06WwMIalEPc8jlUcfYWu14 pNpw== X-Gm-Message-State: AOAM532oe3tgLp3x7j5jpvstW4N/CQdGLozxX7jFk1hEce7Vx6F3hsaK 5w0csYfokC++TnSU5/ZVpabzCuSzPRTOHA0vb06gH06CFkECN+fLqO+yd5qhkKjn8PTRmovodCc Wuzdy/RTC/Afh/y/CTQbJxD/glED7WGvzf0yK+uMynNl3bmURYzFjTyvhCT/0YZI= X-Google-Smtp-Source: ABdhPJyfeKt0oX9yW9RB7cChasdJ8jm6UAwDC6YHC8s9jNppGRYMwXdZdllejYL4H0I7uxvEXdSWgLWBkbwgoA== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a63:c61:0:b0:370:592b:3ad1 with SMTP id 33-20020a630c61000000b00370592b3ad1mr32233478pgm.240.1646355499653; Thu, 03 Mar 2022 16:58:19 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:54 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-8-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 07/10] submodule: extract get_fetch_task() From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " 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 --- submodule.c | 61 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/submodule.c b/submodule.c index 0b9c25f9d3..7a5316b6f7 100644 --- a/submodule.c +++ b/submodule.c @@ -1389,6 +1389,7 @@ struct fetch_task { struct repository *repo; const struct submodule *sub; unsigned free_sub : 1; /* Do we need to free the submodule? */ + const char *default_argv; /* The default fetch mode. */ struct oid_array *commits; /* Ensure these commits are fetched */ }; @@ -1466,14 +1467,11 @@ 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, 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)) @@ -1493,10 +1491,10 @@ static int get_next_submodule(struct child_process *cp, &spf->changed_submodule_names, task->sub->name)) continue; - default_argv = "on-demand"; + task->default_argv = "on-demand"; break; case RECURSE_SUBMODULES_ON: - default_argv = "yes"; + task->default_argv = "yes"; break; case RECURSE_SUBMODULES_OFF: continue; @@ -1504,29 +1502,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; @@ -1550,6 +1531,36 @@ 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; + struct fetch_task *task = get_fetch_task(spf, 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, task->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 Fri Mar 4 00:57:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768364 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 AF9C8C433EF for ; Fri, 4 Mar 2022 00:58:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237501AbiCDA7W (ORCPT ); Thu, 3 Mar 2022 19:59:22 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45022 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237503AbiCDA7L (ORCPT ); Thu, 3 Mar 2022 19:59:11 -0500 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7E6F91405F4 for ; Thu, 3 Mar 2022 16:58:22 -0800 (PST) Received: by mail-pl1-x64a.google.com with SMTP id w13-20020a1709027b8d00b0014fb4f012d3so3773560pll.12 for ; Thu, 03 Mar 2022 16:58:22 -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=m3OQr6dzy8LMsoOOQyCKMsA/7E5JvXbC+Yn9WukhCU4=; b=eXyY8X/EfB1Z4PLwLOTJgyG8Z3fxuzkXQTSS8VCsWi+HbRBjz50Pd2bqSeC44v01Ca ktWu3jLO50NZpONTyeKoN0oxFQDaMhxQvTW5FJCF8sYLh3KmTuHHPvBeqF1BBZgUVW89 Sg7uR2yiALxkUjNFTMmd8qN+bNUD7BRJs5mjw0UTpna6ZLzwmBCCYHzjBbzrKsxUbLpf Fcjv3qEmQK3KysMOFwoQVDF0pEg5Rm1C4q0aAhxlxk3KqWimy/lM+p5WzR+pDtgvXzAb U3nOxVHwwodnCkSquqMgc3Lfx1iygDkfyousGDlWJGCHRCC1127YTnj9+NEMXpxActqz 77mQ== 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=m3OQr6dzy8LMsoOOQyCKMsA/7E5JvXbC+Yn9WukhCU4=; b=2h8Q+nDEjkSumjTaheygjKq6emdyZtyueBIvIQDxzHcPx/iijo/FJ+MNO/TYSb7yu6 UIBk/OQnaivcBHV8mm/x8V3wsXKD1mLvIcSZmzP2JqAD1uNWV092UedNZnl0lSJ2MW09 2YVheDGEYXJ/Q73KJQjrV1SGLu6vGu8KfCahvKfBvv5FmgILVQsrTYJQ04wBdzWxBHbq aJ6BUOT7bdeamHxJy1nRsT4s74X/yahE56LE9yXMhJsZPNFztOEYumCeFVDntAy5MuBG FZeBMmUOESUv7y3I86ZiYnMemUVmz2po+lAYW7+vfK39bKW3qoNEFC7vhT3ZWR6sv0iF ZIsw== X-Gm-Message-State: AOAM530HAX+np6IdwqpCIaQ2lLFUjyMoKjQpMx04aEAlWzGWkdfNI3Sz LrLx8BqUVebOmdVHtL0A/JFLeefPGM5bXrf+e3BhlxkrkWUnyPfSQLbbACDrUaNr32D9wuGple/ V48qR9uewjgdTi7KQSUWORTU/jTDJtXinzi8vDa20sCbXxONLHu7Rvt0hcShS7JQ= X-Google-Smtp-Source: ABdhPJzDw3Xe2VxFCZhOJlnyHiAwxNv/Lg48UAtuc4Vu2HHqCXafHbBwNgAO5ggQEKZ+ZVrh54Q1cZm/wHN/cg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:ad0:b0:4e1:2d96:2ab0 with SMTP id c16-20020a056a000ad000b004e12d962ab0mr40750802pfl.3.1646355501741; Thu, 03 Mar 2022 16:58:21 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:55 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-9-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 08/10] submodule: move logic into fetch_task_create() From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org get_fetch_task() gets a fetch task by iterating the index; a future commit will introduce a similar function, get_fetch_task_from_changed(), that gets a fetch task from the list of changed submodules. Both functions are similar in that they need to: * create a fetch task * initialize the submodule repo for the fetch task * determine the default recursion mode Move all of this logic into fetch_task_create() so that it is no longer split between fetch_task_create() and get_fetch_task(). This will make it easier to share code with get_fetch_task_from_changed(). Signed-off-by: Glen Choo --- submodule.c | 99 ++++++++++++++++++++++++++++------------------------- 1 file changed, 52 insertions(+), 47 deletions(-) diff --git a/submodule.c b/submodule.c index 7a5316b6f7..b36ef26752 100644 --- a/submodule.c +++ b/submodule.c @@ -1415,32 +1415,6 @@ static const struct submodule *get_non_gitmodules_submodule(const char *path) return (const struct submodule *) ret; } -static struct fetch_task *fetch_task_create(struct repository *r, - 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, treeish_name, path); - if (!task->sub) { - /* - * No entry in .gitmodules? Technically not a submodule, - * but historically we supported repositories that happen to be - * in-place where a gitlink is. Keep supporting them. - */ - task->sub = get_non_gitmodules_submodule(path); - if (!task->sub) { - free(task); - return NULL; - } - - task->free_sub = 1; - } - - return task; -} - static void fetch_task_release(struct fetch_task *p) { if (p->free_sub) @@ -1467,6 +1441,57 @@ static struct repository *get_submodule_repo_for(struct repository *r, return ret; } +static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf, + 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(spf->r, treeish_name, path); + + if (!task->sub) { + /* + * No entry in .gitmodules? Technically not a submodule, + * but historically we supported repositories that happen to be + * in-place where a gitlink is. Keep supporting them. + */ + task->sub = get_non_gitmodules_submodule(path); + if (!task->sub) + goto cleanup; + + task->free_sub = 1; + } + + switch (get_fetch_recurse_config(task->sub, spf)) + { + default: + case RECURSE_SUBMODULES_DEFAULT: + case RECURSE_SUBMODULES_ON_DEMAND: + if (!task->sub || + !string_list_lookup( + &spf->changed_submodule_names, + task->sub->name)) + goto cleanup; + task->default_argv = "on-demand"; + break; + case RECURSE_SUBMODULES_ON: + task->default_argv = "yes"; + break; + case RECURSE_SUBMODULES_OFF: + goto cleanup; + } + + task->repo = get_submodule_repo_for(spf->r, path, treeish_name); + + return task; + + cleanup: + fetch_task_release(task); + free(task); + return NULL; +} + static struct fetch_task * get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err) { @@ -1477,30 +1502,10 @@ get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err) if (!S_ISGITLINK(ce->ce_mode)) continue; - task = fetch_task_create(spf->r, ce->name, null_oid()); + task = fetch_task_create(spf, ce->name, null_oid()); if (!task) continue; - switch (get_fetch_recurse_config(task->sub, spf)) - { - default: - case RECURSE_SUBMODULES_DEFAULT: - case RECURSE_SUBMODULES_ON_DEMAND: - if (!task->sub || - !string_list_lookup( - &spf->changed_submodule_names, - task->sub->name)) - continue; - task->default_argv = "on-demand"; - break; - case RECURSE_SUBMODULES_ON: - task->default_argv = "yes"; - break; - case RECURSE_SUBMODULES_OFF: - continue; - } - - task->repo = get_submodule_repo_for(spf->r, task->sub->path, null_oid()); if (task->repo) { if (!spf->quiet) strbuf_addf(err, _("Fetching submodule %s%s\n"), From patchwork Fri Mar 4 00:57:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768362 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 A7808C433F5 for ; Fri, 4 Mar 2022 00:58:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237495AbiCDA7S (ORCPT ); Thu, 3 Mar 2022 19:59:18 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44944 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237490AbiCDA7M (ORCPT ); Thu, 3 Mar 2022 19:59:12 -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 61D83145AD6 for ; Thu, 3 Mar 2022 16:58:24 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id h3-20020a628303000000b004e12f44a262so4061091pfe.21 for ; Thu, 03 Mar 2022 16:58:24 -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=FdcClAz5hedC4BxhUeS7NUb0EA7jwhOnLCUjNeCfKw8=; b=Hsx5GC2CoN7j4t8IfbJyfII6urC9UqxUFEOYqtW5f5+X8Jj0BRaZZhpQiC1wRC5HD3 lMF4pEiw3hdiJnukqChByB5sdEU5GKCd+NqNQjDKLISlTLcybqztusl9l5jH3+7lMevF i5cC7l6WXT5gWXO6JtPS6IBdi21qZ+y+EbsCIn0Rzi5Z8M52fTf4Pe7TWipRGylfujEz KM2ZXYj6o+/eLse/UxKkx11KPxj42QDFLaXCIf+gk/dH1jU/yM7tX/zzLgfFhgALTTgQ b5xkxDIEaWigavnvKV7xj6JidiXF/8Y1waOd+MaqmA426juvWbPFDPwwm1H8kKrrAzXS SVpQ== 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=FdcClAz5hedC4BxhUeS7NUb0EA7jwhOnLCUjNeCfKw8=; b=AXWCMNmDzhEgh1N6XqWX6HXNkmt0cgqNYmxXuCLuazc8VGRfTz8J2K61qBIKPPWXb4 4vDMhoBddSpXNssuPZEc/sD2wiuAyouv7ejr/VS/Kj7YQJbQMYWXm0wgOFQDzY9GgCUp LSpOrxjvEqhrSemn1+JhOFANgFwzlD25RhT8JOTnwzE0gONv+fi5GK9ruESVM0LKs7vl RL7SRz7BpfdpWeUpTprqE/uXhyyku+m0VRVAhyhXNKW9oLu3jBgt5aG1GoGoOHrgts/Y 91l6nTSR0PmjShc2s9NloQ+XLNhs7ew8uBp5oYR8ZKQUB6hfDz6A4rQmm3u1KBsTLTTE oEEg== X-Gm-Message-State: AOAM531A3PbgiokxI/1Hv9ev0sgj0+75WTNgw99rfHMHSDf/hR1D6Qd6 XWMZdizlFly1bW9mXSA/NQgFoUAaGAf0VC1q7GsFLVfA+kAGJ7zl6KZxuL3qGWej6ps9UGb5bnf h23NMiIHnzDwD+wSF6VOxlByWPcNO1SDvw1awWX35xQD9IhsAwac9ymJjJDDElkQ= X-Google-Smtp-Source: ABdhPJyZsU92L5toY/6I/SjElivMnHBOMRawY906CH6a2wvbZ2n0CgkN/lC5qcOzVsthmhQEQntKhT+co7YY2g== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:cc3:b0:1be:ded9:67d9 with SMTP id 3-20020a17090a0cc300b001beded967d9mr8115433pjt.54.1646355503798; Thu, 03 Mar 2022 16:58:23 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:56 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-10-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 09/10] fetch: fetch unpopulated, changed submodules From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " 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 (which is always attempted regardless of what was fetched in the superproject, or even if nothing was fetched in the superproject). A submodule may be encountered multiple times (via the list of populated submodules or via the list of changed submodules). When this happens, "git fetch" only reads the 'populated copy' and ignores the 'changed copy'. Amend the verify_fetch_result() test helper so that we can assert on which 'copy' is being read. Signed-off-by: Glen Choo --- Documentation/fetch-options.txt | 26 ++-- Documentation/git-fetch.txt | 10 +- builtin/fetch.c | 14 +- submodule.c | 178 +++++++++++++++++++-- submodule.h | 12 +- t/t5526-fetch-submodules.sh | 263 +++++++++++++++++++++++++++++++- 6 files changed, 457 insertions(+), 46 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index f903683189..6cdd9d43c5 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -186,15 +186,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 95832ba1df..97a89763c8 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2178,13 +2178,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 b36ef26752..1f5f39ce18 100644 --- a/submodule.c +++ b/submodule.c @@ -808,9 +808,25 @@ static const char *default_name_or_path(const char *path_or_name) /* * Holds relevant information for a changed submodule. Used as the .util - * member of the changed submodule string_list_item. + * member of the changed submodule name string_list_item. + * + * (super_oid, path) allows the submodule config to be read from _some_ + * .gitmodules file. We store this information the first time we find a + * superproject commit that points to the submodule, but this is + * arbitrary - we can choose any (super_oid, path) that matches the + * submodule's name. */ 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 +834,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, @@ -862,9 +879,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, continue; item = string_list_insert(changed, name); - if (!item->util) + if (item->util) + cs_data = item->util; + else { item->util = xcalloc(1, sizeof(struct changed_submodule_data)); - cs_data = item->util; + 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); } } @@ -1253,14 +1275,36 @@ void check_for_new_submodule_commits(struct object_id *oid) oid_array_append(&ref_tips_after_fetch, oid); } +/* + * Returns 1 if there is at least one submodule gitdir in + * $GIT_DIR/modules and 0 otherwise. This follows + * submodule_name_to_gitdir(), which looks for submodules in + * $GIT_DIR/modules, not $GIT_COMMON_DIR. + * + * A submodule can be moved to $GIT_DIR/modules manually by running "git + * submodule absorbgitdirs", or it may be initialized there by "git + * submodule update". + */ +static int repo_has_absorbed_submodules(struct repository *r) +{ + int ret; + struct strbuf buf = STRBUF_INIT; + + strbuf_repo_git_path(&buf, r, "modules/"); + ret = file_exists(buf.buf) && !is_empty_dir(buf.buf); + strbuf_release(&buf); + return ret; +} + 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 */ @@ -1333,7 +1377,16 @@ int submodule_touches_in_range(struct repository *r, } struct submodule_parallel_fetch { - int count; + /* + * The index of the last index entry processed by + * get_fetch_task_from_index(). + */ + int index_count; + /* + * The index of the last string_list entry processed by + * get_fetch_task_from_changed(). + */ + int changed_count; struct strvec args; struct repository *r; const char *prefix; @@ -1342,7 +1395,16 @@ struct submodule_parallel_fetch { int quiet; int result; + /* + * Names of submodules that have new commits. Generated by + * walking the newly fetched superproject commits. + */ struct string_list changed_submodule_names; + /* + * Names of submodules that have already been processed. Lets us + * avoid fetching the same submodule more than once. + */ + struct string_list seen_submodule_names; /* Pending fetches by OIDs */ struct fetch_task **oid_fetch_tasks; @@ -1353,6 +1415,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, \ } @@ -1390,6 +1453,7 @@ struct fetch_task { const struct submodule *sub; unsigned free_sub : 1; /* Do we need to free the submodule? */ const char *default_argv; /* The default fetch mode. */ + struct strvec git_args; /* Args for the child git process. */ struct oid_array *commits; /* Ensure these commits are fetched */ }; @@ -1425,6 +1489,8 @@ static void fetch_task_release(struct fetch_task *p) if (p->repo) repo_clear(p->repo); FREE_AND_NULL(p->repo); + + strvec_clear(&p->git_args); } static struct repository *get_submodule_repo_for(struct repository *r, @@ -1463,6 +1529,9 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf task->free_sub = 1; } + if (string_list_lookup(&spf->seen_submodule_names, task->sub->name)) + goto cleanup; + switch (get_fetch_recurse_config(task->sub, spf)) { default: @@ -1493,10 +1562,12 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf } static struct fetch_task * -get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err) +get_fetch_task_from_index(struct submodule_parallel_fetch *spf, + 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)) @@ -1511,7 +1582,7 @@ get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err) 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; @@ -1539,11 +1610,83 @@ get_fetch_task(struct submodule_parallel_fetch *spf, struct strbuf *err) return NULL; } +static struct fetch_task * +get_fetch_task_from_changed(struct submodule_parallel_fetch *spf, + 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; + + if (!is_tree_submodule_active(spf->r, cs_data->super_oid,cs_data->path)) + continue; + + task = fetch_task_create(spf, cs_data->path, + cs_data->super_oid); + if (!task) + continue; + + if (!task->repo) { + strbuf_addf(err, _("Could not access submodule '%s' at commit %s\n"), + cs_data->path, + find_unique_abbrev(cs_data->super_oid, DEFAULT_ABBREV)); + + fetch_task_release(task); + free(task); + 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++; + /* + * NEEDSWORK: Submodules set/unset a value for + * core.worktree when they are populated/unpopulated by + * "git checkout" (and similar commands, see + * submodule_move_head() and + * connect_work_tree_and_git_dir()), but if the + * submodule is unpopulated in another way (e.g. "git + * rm", "rm -r"), core.worktree will still be set even + * though the directory doesn't exist, and the child + * process will crash while trying to chdir into the + * nonexistent directory. + * + * In this case, we know that the submodule has no + * working tree, so we can work around this by + * setting "--work-tree=." (--bare does not work because + * worktree settings take precedence over bare-ness). + * However, this is not necessarily true in other cases, + * so a generalized solution is still necessary. + * + * Possible solutions: + * - teach "git [add|rm]" to unset core.worktree and + * discourage users from removing submodules without + * using a Git command. + * - teach submodule child processes to ignore stale + * core.worktree values. + */ + strvec_push(&task->git_args, "--work-tree=."); + 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; - struct fetch_task *task = get_fetch_task(spf, err); + struct fetch_task *task = + get_fetch_task_from_index(spf, err); + if (!task) + task = get_fetch_task_from_changed(spf, err); if (task) { struct strbuf submodule_prefix = STRBUF_INIT; @@ -1553,6 +1696,8 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err, prepare_submodule_repo_env_in_gitdir(&cp->env_array); cp->git_cmd = 1; strvec_init(&cp->args); + if (task->git_args.nr) + strvec_pushv(&cp->args, task->git_args.v); strvec_pushv(&cp->args, spf->args.v); strvec_push(&cp->args, task->default_argv); strvec_push(&cp->args, "--submodule-prefix"); @@ -1564,6 +1709,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; } @@ -1678,11 +1824,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 4cae2e4f7c..e844ae9e42 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -12,8 +12,9 @@ pwd=$(pwd) check_sub () { NEW_HEAD=$1 && + SUPER_HEAD=$2 && cat >$pwd/expect.err.sub <<-EOF - Fetching submodule submodule + Fetching submodule submodule${SUPER_HEAD:+ at commit $SUPER_HEAD} From $pwd/submodule OLD_HEAD..$NEW_HEAD sub -> origin/sub EOF @@ -21,8 +22,9 @@ check_sub () { check_deep () { NEW_HEAD=$1 && + SUB_HEAD=$2 && cat >$pwd/expect.err.deep <<-EOF - Fetching submodule submodule/subdir/deepsubmodule + Fetching submodule submodule/subdir/deepsubmodule${SUB_HEAD:+ at commit $SUB_HEAD} From $pwd/deepsubmodule OLD_HEAD..$NEW_HEAD deep -> origin/deep EOF @@ -399,6 +401,7 @@ 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)" ' + add_submodule_commits && add_superproject_commits && ( cd downstream && @@ -418,6 +421,155 @@ 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 && + git rm .gitmodules && + git rm submodule && + 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" ' + add_submodule_commits && + add_superproject_commits && + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err + ) && + super_head=$(git rev-parse --short HEAD) && + sub_head=$(git -C submodule rev-parse --short HEAD) && + deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) && + + # assert that these are fetched from commits, not the index + check_sub $sub_head $super_head && + check_deep $deep_head $sub_head && + + test_must_be_empty actual.out && + verify_fetch_result actual.err +' + +test_expect_success "'--recurse-submodules' should fetch submodule commits if the submodule is changed but the index has no submodules" ' + add_submodule_commits && + add_superproject_commits && + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git fetch --recurse-submodules >../actual.out 2>../actual.err + ) && + super_head=$(git rev-parse --short HEAD) && + sub_head=$(git -C submodule rev-parse --short HEAD) && + deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) && + + # assert that these are fetched from commits, not the index + check_sub $sub_head $super_head && + check_deep $deep_head $sub_head && + + test_must_be_empty actual.out && + verify_fetch_result actual.err +' + +test_expect_success "'--recurse-submodules' should ignore changed, inactive submodules" ' + add_submodule_commits && + add_superproject_commits && + + # 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 && + super_head=$(git rev-parse --short HEAD) && + check_super $super_head && + # Neither should be fetched because the submodule is inactive + rm expect.err.sub && + rm expect.err.deep && + 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" ' + # Create new commit in origin/super + add_submodule_commits && + add_superproject_commits && + + # 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 && + deep_head=$(git -C submodule/subdir/deepsubmodule rev-parse --short HEAD) && + sub_head=$(git -C submodule rev-parse --short HEAD) && + sub2_head=$(git -C submodule2 rev-parse --short HEAD) && + super_head=$(git rev-parse --short HEAD) && + super_sub2_only_head=$(git rev-parse --short super-sub2-only) && + + # Use test_cmp manually because verify_fetch_result does not + # consider submodule2. All the repos should be fetched, but only + # submodule2 should be read from a commit + cat > expect.err.combined <<-EOF && + From $pwd/. + OLD_HEAD..$super_head super -> origin/super + OLD_HEAD..$super_sub2_only_head super-sub2-only -> origin/super-sub2-only + Fetching submodule submodule + From $pwd/submodule + OLD_HEAD..$sub_head sub -> origin/sub + Fetching submodule submodule/subdir/deepsubmodule + From $pwd/deepsubmodule + OLD_HEAD..$deep_head deep -> origin/deep + Fetching submodule submodule2 at commit $super_sub2_only_head + From $pwd/submodule2 + OLD_HEAD..$sub2_head sub2 -> origin/sub2 + EOF + sed -e "s/[0-9a-f][0-9a-f]*\.\./OLD_HEAD\.\./" actual.err >actual.err.cmp && + test_cmp expect.err.combined actual.err.cmp +' + test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' add_submodule_commits && echo a >> file && @@ -860,4 +1012,111 @@ test_expect_success 'recursive fetch after deinit a submodule' ' test_cmp expect actual ' +test_expect_success 'setup repo with upstreams that share a submodule name' ' + mkdir same-name-1 && + ( + cd same-name-1 && + git init -b main && + test_commit --no-tag a + ) && + git clone same-name-1 same-name-2 && + # same-name-1 and same-name-2 both add a submodule with the + # name "submodule" + ( + cd same-name-1 && + mkdir submodule && + git -C submodule init -b main && + test_commit -C submodule --no-tag a1 && + git submodule add "$pwd/same-name-1/submodule" && + git add submodule && + git commit -m "super-a1" + ) && + ( + cd same-name-2 && + mkdir submodule && + git -C submodule init -b main && + test_commit -C submodule --no-tag a2 && + git submodule add "$pwd/same-name-2/submodule" && + git add submodule && + git commit -m "super-a2" + ) && + git clone same-name-1 -o same-name-1 same-name-downstream && + ( + cd same-name-downstream && + git remote add same-name-2 ../same-name-2 && + git fetch --all && + # init downstream with same-name-1 + git submodule update --init + ) +' + +test_expect_success 'fetch --recurse-submodules updates name-conflicted, populated submodule' ' + test_when_finished "git -C same-name-downstream checkout main" && + ( + cd same-name-1 && + test_commit -C submodule --no-tag b1 && + git add submodule && + git commit -m "super-b1" + ) && + ( + cd same-name-2 && + test_commit -C submodule --no-tag b2 && + git add submodule && + git commit -m "super-b2" + ) && + ( + cd same-name-downstream && + # even though the .gitmodules is correct, we cannot + # fetch from same-name-2 + git checkout same-name-2/main && + git fetch --recurse-submodules same-name-1 && + test_must_fail git fetch --recurse-submodules same-name-2 + ) && + super_head1=$(git -C same-name-1 rev-parse HEAD) && + git -C same-name-downstream cat-file -e $super_head1 && + + super_head2=$(git -C same-name-2 rev-parse HEAD) && + git -C same-name-downstream cat-file -e $super_head2 && + + sub_head1=$(git -C same-name-1/submodule rev-parse HEAD) && + git -C same-name-downstream/submodule cat-file -e $sub_head1 && + + sub_head2=$(git -C same-name-2/submodule rev-parse HEAD) && + test_must_fail git -C same-name-downstream/submodule cat-file -e $sub_head2 +' + +test_expect_success 'fetch --recurse-submodules updates name-conflicted, unpopulated submodule' ' + ( + cd same-name-1 && + test_commit -C submodule --no-tag c1 && + git add submodule && + git commit -m "super-c1" + ) && + ( + cd same-name-2 && + test_commit -C submodule --no-tag c2 && + git add submodule && + git commit -m "super-c2" + ) && + ( + cd same-name-downstream && + git checkout main && + git rm .gitmodules && + git rm submodule && + git commit -m "no submodules" && + git fetch --recurse-submodules same-name-1 + ) && + head1=$(git -C same-name-1/submodule rev-parse HEAD) && + head2=$(git -C same-name-2/submodule rev-parse HEAD) && + ( + cd same-name-downstream/.git/modules/submodule && + # The submodule has core.worktree pointing to the "git + # rm"-ed directory, overwrite the invalid value. See + # comment in get_fetch_task_from_changed() for more + # information. + git --work-tree=. cat-file -e $head1 && + test_must_fail git --work-tree=. cat-file -e $head2 + ) +' + test_done From patchwork Fri Mar 4 00:57:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12768363 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 ED3DDC433EF for ; Fri, 4 Mar 2022 00:58:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237498AbiCDA7U (ORCPT ); Thu, 3 Mar 2022 19:59:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44920 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237487AbiCDA7N (ORCPT ); Thu, 3 Mar 2022 19:59:13 -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 99134EFFBD for ; Thu, 3 Mar 2022 16:58:26 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id j10-20020a17090a7e8a00b001bbef243093so6385004pjl.1 for ; Thu, 03 Mar 2022 16:58:26 -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=vdbgcs2Fpier1UCS12yHjFqUWjg8CLpnX1fa82efzXc=; b=rITIIE9rx1FtRoIslcKG/QabqGlI0T4Ebh7q4jb15+gkLyURlc5olx+nstkTpJKHxj BYqyaWdwV0usmuDLt2bdWvdBFuVIjbys5tjmGmlFNHk3qLs34kZ2N4mqOfwjqHmfVH9D qQMwZkhk1lK6W8r9dUtjFxScwCAJHreokfMsSRHCyPzzbhWZ/pJsgwMZryaQ3m+CKvFj wLuKszfQ8qQT/RsAG+Jm2UX+x+t5NbEWM51MMKMO196bT/hOAxwG0jP14QrOSTHQM52O fh09i/ZcX6acKFHB1XPczspoNwbhOWlQLzJ1T72EIZbHrM3ItlfOPSMDZPEdx5gUBGN8 pZJg== 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=vdbgcs2Fpier1UCS12yHjFqUWjg8CLpnX1fa82efzXc=; b=6RVrS4ERwCtVzGQR0522Z8NBTYMzQ2UkEcfOVXKmCKOIhzOI1mkKyqn/g5fhtMaHvC MnOolpmBlMTurZ6jQnOXsWr856KTHslN7CP6HnVBZaPvz4M4yPwRDvOezVJPe6/sheM5 3bsr7Ia7TDgOwQaLBBO8HOhjtTVXwDfZqQwyN3hgThyxbXGgT1cHGxW9TfpfFjMEka2B DzUDbcyrN0ocRyla/yVrVAT1vP2bDN06bm2tuaqoIaPfUcSuJnXoYRyPrl3gkUmFO3iP dMfbYRlN5EcGF5P6+tKQHXZq27ff+DxSZ3yi1Bf6jeCQxqx0DkIh1TImZ6ueECHIscQQ duqg== X-Gm-Message-State: AOAM530GridOmeRd/ELPWmWdUVlK5kZn3yNWkhdYCSGh9hIbYgmWkhNR RDGkWldvGqMETZWZVZcubvZhOxJLnhvyx8ZYvi/u4EGMnLVJTnTvVSXW66g/5mpSLXvRdr/g743 jz3NH4L7tovzxT2HvQJ0ODBPiQ2n4oRULK3WMfdbr8tRozhFvq2dp54YnmseG7ZY= X-Google-Smtp-Source: ABdhPJzRVHWI3zph0hRG8AwZkd70/PDAO2+OwbeS6N/BlcL7D45ZhY3mIcT5E0OXJP1KMos7nfUh7ApXKGYxtw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a63:88c6:0:b0:37c:4e10:1a1f with SMTP id l189-20020a6388c6000000b0037c4e101a1fmr6231344pgd.162.1646355505910; Thu, 03 Mar 2022 16:58:25 -0800 (PST) Date: Thu, 3 Mar 2022 16:57:57 -0800 In-Reply-To: <20220304005757.70107-1-chooglen@google.com> Message-Id: <20220304005757.70107-11-chooglen@google.com> Mime-Version: 1.0 References: <20220224100842.95827-1-chooglen@google.com> <20220304005757.70107-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.616.g0bdcbb4464-goog Subject: [PATCH v4 10/10] submodule: fix latent check_has_commit() bug From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan , Junio C Hamano , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBC?= =?utf-8?b?amFybWFzb24=?= " 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 1f5f39ce18..6e6b2d04e4 100644 --- a/submodule.c +++ b/submodule.c @@ -167,26 +167,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)); @@ -971,7 +951,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); @@ -1007,18 +988,6 @@ static int submodule_has_commits(struct repository *r, .super_oid = 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);