From patchwork Thu Feb 24 10:08:33 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758287 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 DA3EDC433EF for ; Thu, 24 Feb 2022 10:08:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233215AbiBXKJZ (ORCPT ); Thu, 24 Feb 2022 05:09:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45862 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233218AbiBXKJY (ORCPT ); Thu, 24 Feb 2022 05:09:24 -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 2F52228A108 for ; Thu, 24 Feb 2022 02:08:54 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id p5-20020a170902bd0500b00148cb2d29ecso845750pls.4 for ; Thu, 24 Feb 2022 02:08:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=zPUaivU0kV4nEZLuAVuNnWZk2Ux6yKs7LrUu78EdsYk=; b=NCwMc+8qGG25lWiuB5/umxSj8nouFgd/6Mdr1ag8GMiHJh68fmDCcnQOBG40M1W2TU yKTZEvARHJFNdjecv9e08qDcpM7f7IeNQH/88IP/dpMVqmrg87l4ERmgh6oyfaQJWr9E jMcFb3vkoM+d8ZJITywxE1/QZ9gWmW2zm8bbGnn8927YW+ryoI0idgkuJ2vnQ73UzGqt TDgyPlmitZ8BvEJfXFqHaQBeaQm89xX5EzQNF5K9kUd21L2HrE1aejdtfk2Y706KC/IT 6VNcWutPlzzJJWjveheCoUMtfCP4MVXTHg+7GW3aZTGC7vtXJG6hm3YF8iDCztl2pGmZ 6txw== 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=zPUaivU0kV4nEZLuAVuNnWZk2Ux6yKs7LrUu78EdsYk=; b=KhRjdXzOZj3g21zWaHmj42Y06LtIH1P/rl8Og3R/ALADK6tp7EYlXks4J0lQwGmCwq BLrnSx0c5yFuImrkDHSeKjvGDUXF2Q/kN/5mMRzt8RH6UijXE0B1OLI5US37sIuqXGEl hT0XZxGtUW9aYIhjLqmb4q16n3uwOBlilwY3GqJQuoI6pdEzZJj3wma5kPs97HhXq9fy ktOqorStp1hezzkJtV5nATlL24ZmfCEVto418yhWv6kjlx/z1MERyBQ6BuAOTQpb4iTf o7LPaSJoRvgHFRK4hs+1zcgHgAUgZeFIBoE7USBtGfDKkv0+LgEC5qeLSTcax13LbJEZ xuGg== X-Gm-Message-State: AOAM532bBlF7AsinHnp5/+26EqkcKtqSnzOU9gne42YrRk8g2oG3y+K2 HQUbpJOXO8f0FkcuimZ4dRvBRLJJjiKYns4MROWat5mfChgM9rmvGaVWBECZcdjTEmgTtWWRgy7 pZXrgBsWDz+SHBwBkjI6coMQNYKeoFaJNaREL99esrotdor0iJH4oeyLyTXlPyDs= X-Google-Smtp-Source: ABdhPJx+jQmiwYTw37caEpRuW2NMXly2wvDKA3F2BVi8XAh5bTC8lm+8RTshTc3zl5N9EmiDRIPnU9ZE/gW6Vg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:1143:b0:4f0:e9bf:6333 with SMTP id b3-20020a056a00114300b004f0e9bf6333mr2225881pfm.29.1645697333558; Thu, 24 Feb 2022 02:08:53 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:33 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-2-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 | 136 +++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 55 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 2dc75b80db..0e93df1665 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -13,6 +13,10 @@ export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB pwd=$(pwd) +# For each submodule in the test setup, this creates a commit and writes +# a file that contains the expected err if that new commit were fetched. +# These output files get concatenated in the right order by +# verify_fetch_result(). add_upstream_commit() { ( cd submodule && @@ -22,9 +26,9 @@ add_upstream_commit() { git add subfile && git commit -m new subfile && head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule" > ../expect.err && - echo "From $pwd/submodule" >> ../expect.err && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err + echo "Fetching submodule submodule" > ../expect.err.sub && + echo "From $pwd/submodule" >> ../expect.err.sub && + echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub ) && ( cd deepsubmodule && @@ -34,12 +38,33 @@ add_upstream_commit() { git add deepsubfile && git commit -m new deepsubfile && head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule/subdir/deepsubmodule" >> ../expect.err - echo "From $pwd/deepsubmodule" >> ../expect.err && - echo " $head1..$head2 deep -> origin/deep" >> ../expect.err + echo "Fetching submodule submodule/subdir/deepsubmodule" > ../expect.err.deep + echo "From $pwd/deepsubmodule" >> ../expect.err.deep && + echo " $head1..$head2 deep -> origin/deep" >> ../expect.err.deep ) } +# Verifies that the expected repositories were fetched. This is done by +# concatenating the files expect.err.[super|sub|deep] in the correct +# order and comparing it to the actual stderr. +# +# If a repo should not be fetched in the test, its corresponding +# expect.err file should be rm-ed. +verify_fetch_result() { + ACTUAL_ERR=$1 && + rm -f expect.err.combined && + if [ -f expect.err.super ]; then + cat expect.err.super >>expect.err.combined + fi && + if [ -f expect.err.sub ]; then + cat expect.err.sub >>expect.err.combined + fi && + if [ -f expect.err.deep ]; then + cat expect.err.deep >>expect.err.combined + fi && + test_cmp expect.err.combined $ACTUAL_ERR +} + test_expect_success setup ' mkdir deepsubmodule && ( @@ -77,7 +102,7 @@ test_expect_success "fetch --recurse-submodules recurses into submodules" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "submodule.recurse option triggers recursive fetch" ' @@ -87,7 +112,7 @@ test_expect_success "submodule.recurse option triggers recursive fetch" ' git -c submodule.recurse fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "fetch --recurse-submodules -j2 has the same output behaviour" ' @@ -97,7 +122,7 @@ test_expect_success "fetch --recurse-submodules -j2 has the same output behaviou GIT_TRACE="$TRASH_DIRECTORY/trace.out" git fetch --recurse-submodules -j2 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err && + verify_fetch_result actual.err && grep "2 tasks" trace.out ' @@ -127,7 +152,7 @@ test_expect_success "using fetchRecurseSubmodules=true in .gitmodules recurses i git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--no-recurse-submodules overrides .gitmodules config" ' @@ -158,7 +183,7 @@ test_expect_success "--recurse-submodules overrides fetchRecurseSubmodules setti git config --unset submodule.submodule.fetchRecurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--quiet propagates to submodules" ' @@ -186,7 +211,7 @@ test_expect_success "--dry-run propagates to submodules" ' git fetch --recurse-submodules --dry-run >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "Without --dry-run propagates to submodules" ' @@ -195,7 +220,7 @@ test_expect_success "Without --dry-run propagates to submodules" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "recurseSubmodules=true propagates into submodules" ' @@ -206,7 +231,7 @@ test_expect_success "recurseSubmodules=true propagates into submodules" ' git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--recurse-submodules overrides config in submodule" ' @@ -220,7 +245,7 @@ test_expect_success "--recurse-submodules overrides config in submodule" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--no-recurse-submodules overrides config setting" ' @@ -253,14 +278,14 @@ test_expect_success "Recursion stops when no new submodule commits are fetched" git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.sub && - echo " $head1..$head2 super -> origin/super" >>expect.err.sub && - head -3 expect.err >> expect.err.sub && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && - test_cmp expect.err.sub actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -271,14 +296,16 @@ test_expect_success "Recursion doesn't happen when new superproject commits don' git add file && git commit -m "new file" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.file && - echo " $head1..$head2 super -> origin/super" >> expect.err.file && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + rm expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err.file actual.err + verify_fetch_result actual.err ' test_expect_success "Recursion picks up config in submodule" ' @@ -295,9 +322,8 @@ test_expect_success "Recursion picks up config in submodule" ' git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.sub && - echo " $head1..$head2 super -> origin/super" >> expect.err.sub && - cat expect.err >> expect.err.sub && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && ( cd downstream && git fetch >../actual.out 2>../actual.err && @@ -306,7 +332,7 @@ test_expect_success "Recursion picks up config in submodule" ' git config --unset fetch.recurseSubmodules ) ) && - test_cmp expect.err.sub actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -331,15 +357,13 @@ test_expect_success "Recursion picks up all submodules when necessary" ' git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >> expect.err.2 && - cat expect.err.sub >> expect.err.2 && - tail -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && - test_cmp expect.err.2 actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -375,11 +399,8 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - tail -3 expect.err > expect.err.deepsub && - echo "From $pwd/." > expect.err && - echo " $head1..$head2 super -> origin/super" >>expect.err && - cat expect.err.sub >> expect.err && - cat expect.err.deepsub >> expect.err && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && ( cd downstream && git config fetch.recurseSubmodules false && @@ -395,7 +416,7 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess ) ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' @@ -405,14 +426,16 @@ test_expect_success "'--recurse-submodules=on-demand' stops when no new submodul git add file && git commit -m "new file" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.file && - echo " $head1..$head2 super -> origin/super" >> expect.err.file && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + rm expect.err.deep && ( cd downstream && git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err.file actual.err + verify_fetch_result actual.err ' test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config" ' @@ -426,9 +449,9 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git config fetch.recurseSubmodules on-demand && @@ -440,7 +463,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config git config --unset fetch.recurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err + verify_fetch_result actual.err ' test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' overrides fetch.recurseSubmodules" ' @@ -454,9 +477,9 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git config submodule.submodule.fetchRecurseSubmodules on-demand && @@ -468,7 +491,7 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override git config --unset submodule.submodule.fetchRecurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err + verify_fetch_result actual.err ' test_expect_success "don't fetch submodule when newly recorded commits are already present" ' @@ -480,14 +503,17 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea git add submodule && git commit -m "submodule rewound" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err && - echo " $head1..$head2 super -> origin/super" >> expect.err && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + # This file does not exist, but rm -f for readability + rm -f expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err && + verify_fetch_result actual.err && ( cd submodule && git checkout -q sub @@ -505,9 +531,9 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git rm .gitmodules && git commit -m "new submodule without .gitmodules" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." >expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >>expect.err.2 && + echo "From $pwd/." >expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && rm .gitmodules && @@ -523,7 +549,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git reset --hard ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err && + verify_fetch_result actual.err && git checkout HEAD^ -- .gitmodules && git add .gitmodules && git commit -m "new submodule restored .gitmodules" From patchwork Thu Feb 24 10:08:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758289 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 8CB5AC433EF for ; Thu, 24 Feb 2022 10:09:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233255AbiBXKJh (ORCPT ); Thu, 24 Feb 2022 05:09:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45902 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233220AbiBXKJ0 (ORCPT ); Thu, 24 Feb 2022 05:09:26 -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 3994A28A104 for ; Thu, 24 Feb 2022 02:08:56 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id b9-20020a17090aa58900b001b8b14b4aabso1155699pjq.9 for ; Thu, 24 Feb 2022 02:08:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc:content-transfer-encoding; bh=dki7nJfYFix75j88KqOmfOlRoDEdUO8th/JF5kfALoo=; b=EcOr8eOEDhd9DtiBSHKamx281ikFB6dEG6kZNMg831kkItWV1/sEfTtFufNWjD8shQ zTcWaIqhOqFp+pCHjioG8Homzys0GYliisBnyeKxM3Cd4QWscmZD7LL4x7q4Ke39Cjbc KwSFK0X2yAzwXb5NSV13/uq3cn/mmX7Nrvihmn57nMTL8lZJxc25i9Is6Bvo+XVML0A+ KoHUS3lowDus+larxQAR1sAmu/Xtx+1foAmDsrHp60YX2bWfI36J33TWiFY+QB+zYNiY 3Qs6z283Iq2edhMgLUuVqGeCpecJ3QeJ3Wlk7L3eUef8aDF+EgHqmX8qMuxbEQAqcFVS bPuw== 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:content-transfer-encoding; bh=dki7nJfYFix75j88KqOmfOlRoDEdUO8th/JF5kfALoo=; b=upOXpunxRzmMntnXElxX9LPpgHkgYt5NzowGyZlxLbgs6UGiySHZ5G91q4nNcIbDHt 7utWX1HKkNP1bU4URtoXl8WtIxBJ57mcAIaKG3+SiMd6Oq0w/GBTPDGjeX7KuUJWokF7 CYivBrfR8Tg7OwQuuskFgedo4EFR8PZnsOgkubdg2onThlZeSC/f711zt/UA9AqpIEmJ tldKK07dx4ycJYzEasN0NLA51s0TmF0ItNgpalwD3kgrXifVdnZCf8X4jw2ovslKLKQe cVVEqOTN4cq9wwKoqOGZ3HEEAygjRquWYHbgxzzBjIdmJWSlIqN6hn9b5QnlXlxIFph5 mNYA== X-Gm-Message-State: AOAM5330S72YNb7TMGENgfxHYEDtRERiAc2tG9xZ7YhJfl8XLqO5NwjD CLAPjlx7PbzacUKlOemFM7kjdlEBQx4Qvg489zBnZTOCvs6GMXmwudEqXnKZbiFaky3qvzE28+2 hB7D4WkdC/ReBN8xxL7jLxEARp/5Oy9ApB0gTo90KAB6pyViJXgQIixiQYsdzVQY= X-Google-Smtp-Source: ABdhPJzBN5VFXp2X9tollTRq79mzYP+Eq22RRDI8e8n4d830zPIlKBfYmL2ZPhOIle1dyT0nf4gTy0JKpySFyA== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:1d8b:b0:4f1:bd8:811 with SMTP id z11-20020a056a001d8b00b004f10bd80811mr2079841pfw.25.1645697335566; Thu, 24 Feb 2022 02:08:55 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:34 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-3-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 --- Per Ævar's suggestion [1], I reverted the test suite changes that replaced "test_cmp" with "grep", and opted to munge out irrelevant details with "sed". Opinion on "grep" vs "test_cmp" seems a bit split, and there are good arguments for either, but for these tests, I think test_cmp works better for a few reasons: - The motivation for removing test_cmp in the old patch 1 [2], i.e. we _want_ to ignore changes in the test output, turned out to be absolutely incorrect. We actually care a lot about the changes in test output because it tells us where we are reading the submodule info from. In the v1-2 test scheme, verify_fetch_result() would ignore those changes, so I added extra extra grep-s specifically to check where the submodule is read from. ([3] comments on one of these). I could have generalized verify_fetch_result() to handle those extra grep-s, but since the original motivation for using grep is gone, test_cmp seemed like a viable alternative for an intuitive test scheme. - verify_fetch_result() is easier to reason about because we now assert on the output almost verbatim (besides munging), instead of mixing greps and negative greps to get a similar result. This should be helpful for someone updating the tests later. - When a test can't use verify_fetch_result() (e.g. it involves other submodules, patch 9 adds some of these), I found it a lot easier to write the test using test_cmp instead of grep. - test_cmp tests are sensitive to unmeaningful changes, but this behavior helps us catch unwanted regressions and (in these tests at least) it is relatively easy to change test_cmp tests to do the right thing. [1] https://lore.kernel.org/git/220216.86y22bt8gp.gmgdl@evledraar.gmail.com [2] https://lore.kernel.org/git/20220215172318.73533-2-chooglen@google.com [3] https://lore.kernel.org/git/20220215220229.1633486-1-jonathantanmy@google.com t/t5526-fetch-submodules.sh | 117 +++++++++++++++++------------------- 1 file changed, 56 insertions(+), 61 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 0e93df1665..a3890e2f6c 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -13,6 +13,32 @@ export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB pwd=$(pwd) +check_sub() { + NEW_HEAD=$1 && + cat <<-EOF >$pwd/expect.err.sub + Fetching submodule submodule + From $pwd/submodule + OLD_HEAD..$NEW_HEAD sub -> origin/sub + EOF +} + +check_deep() { + NEW_HEAD=$1 && + cat <<-EOF >$pwd/expect.err.deep + Fetching submodule submodule/subdir/deepsubmodule + From $pwd/deepsubmodule + OLD_HEAD..$NEW_HEAD deep -> origin/deep + EOF +} + +check_super() { + NEW_HEAD=$1 && + cat <<-EOF >$pwd/expect.err.super + 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 @@ -20,27 +46,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 ) } @@ -62,7 +82,8 @@ verify_fetch_result() { if [ -f expect.err.deep ]; then cat expect.err.deep >>expect.err.combined fi && - test_cmp expect.err.combined $ACTUAL_ERR + sed -E 's/[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 Thu Feb 24 10:08:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758291 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 D76C2C433EF for ; Thu, 24 Feb 2022 10:09:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233225AbiBXKJj (ORCPT ); Thu, 24 Feb 2022 05:09:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46134 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233230AbiBXKJg (ORCPT ); Thu, 24 Feb 2022 05:09:36 -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 3579C28A120 for ; Thu, 24 Feb 2022 02:08:58 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id b9-20020a17090aa58900b001b8b14b4aabso1155739pjq.9 for ; Thu, 24 Feb 2022 02:08:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=bav4/cQIrIBkAQqQ2XU5cOLEdpDsTHrsRwskRe9HtGw=; b=ORbhE7h5PM/UxWVskDAizjwYoN7HvQFYJiVFSuN5TdBtQPcLPUyTW7B5wKKf83azy2 2ZaSek5/XbPGRiizKL45a1WEOrhk0ER8yPkGRM/6h3SWfRSB5XSyApCRh7jEEp9nPsXr CCuTHAMbjF1zW4Hd65xsQfYO9Xwge7QcW1M/0OT713HaFJCaqkPvt9Ap//Il7n9oH3rd Wri8HTbcSpFqmYvCZTCmNzm1xuUCy+oKNihFct5kjgjTZkGUR8E05TbR+NMOMSGkHqjS +eYlzl/s2F+ej6oF55rKJh9t2J9HzVCrW45/zTzXhHIAaBRW5GLtHlCWvKQU+mpPJvWU e8aA== 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=bav4/cQIrIBkAQqQ2XU5cOLEdpDsTHrsRwskRe9HtGw=; b=qrKXgAB7yfYPU2YtmzqYOYVSqxK/PEYNU0C0izYLAlqsOzUCEeFUUYBWHu20DV0z1J s+ATJAYo+AA+6Ml6ybZBMJFnKUwX2638t6T/j8LE+Vj5/buFl/2A5dBojHAW2P3UwsgD Nj4O1oANx4jwvEIN/A0hB8tzaC1MqoLV+cDOdL+5RBUlNXnvabfCS4MfImibpRnDlBgq NINNVs4hSyL6hRd0QhoMVoSB5cMXbyEEeE2/6NN1idOpe/Mpvp90iqk49XEz1qfvDKdP tuLrc27a/9+Lk+YCq54maHqdXYn8jB3uegd458rBm4/WPQHi0mGRz2u0lFeXFs+eL8h6 FnRw== X-Gm-Message-State: AOAM533k5lt40caJl7F8j7frqzMyep+kMpKQA2y8VURNntmnwcP8xRIn MLN2JrmP2QyIPNbyK+iiVqoVXXLlD/Co+HXdLDvft3utDyOghJHEYhP0E7xDZNHI28c3S/TjfIl rwLsRrrAKUX3UV9klS2lndXRHM+BczmC1C9vCF1s0REm9PwxRORr+lP1HPla+Yuc= X-Google-Smtp-Source: ABdhPJyge+k9qIoDdJwLQwP7hYXdWBBuaLdEZuKS0kiPcGoUvmBC5/bhAEvMAZ9hBSppAGoEdNSW4TOMVXIbCQ== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:3d0f:b0:1bb:80e9:3b45 with SMTP id h15-20020a17090a3d0f00b001bb80e93b45mr2020087pjc.31.1645697337633; Thu, 24 Feb 2022 02:08:57 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:35 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-4-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 a3890e2f6c..ee4dd5a4a9 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -43,7 +43,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 && @@ -64,6 +64,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 Thu Feb 24 10:08:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758290 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 2A6BCC433F5 for ; Thu, 24 Feb 2022 10:09:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232276AbiBXKJi (ORCPT ); Thu, 24 Feb 2022 05:09:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46142 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233232AbiBXKJg (ORCPT ); Thu, 24 Feb 2022 05:09:36 -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 9CE6128A111 for ; Thu, 24 Feb 2022 02:09:00 -0800 (PST) Received: by mail-pf1-x44a.google.com with SMTP id d5-20020a623605000000b004e01ccd08abso1081518pfa.10 for ; Thu, 24 Feb 2022 02:09:00 -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=n1ZL6MCee0yoExu89Dzs+M82uezz4UW/J718DRkEeK4tTvQg2wdMJ2C/UBPME6GzeL XcjNHNYY2uNb7URGwQjf5/byWxdTCKq9x2ejLPRSFUWVVzUDrBeVPU3ymmQj0WniHWHU QfZbJxkqBLY1wBk4f1vF4GEkuIArnWUCIouXpFfbqVFMY9pRU4XPV/7gQ/02ollqIwES gNnvDOrloo6NpGQKIqQJU+IznRE3DnvuJKeEs1/c9M0GScCSrnHUMkOswoMnWFnj0zdr i5kW+LOemp46biwkbLwCzMEatbGKHrtpJte7JBRIUnnbYkc8MK80FdP6rcFDnFopomZE lQNw== 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=YVDZ8liiRQ9OyOuhbWjjgwXyvAIh0cj7PpZ5Z7AWWvTD46oCeQ3hqp2OoblT1cfl6G 6MUAPiUORi7smVSEUAc8DiMHpaZ7sfdlPf/Gha3e+SSiigpbfN2jYu2O4jgqYxfs315y LfObTN1UtVvzFP5vpy3UrQYfYquJG0U7vfLKKNs/Z78p1WhEtZSje4qExwJ72tUViN9Y tp8fqk9hBv+uErXjr5Os/ufW/Q3HlzMYsVtNOQwiLY1H5uHWHRznjaqrMipMy+7rOgz3 WdElKLhQ0GS15nnBPJO48INfd16juYBkosHKfdJy2qdiwfPWxsCoJ5OPFlT2f48vevGq 7guw== X-Gm-Message-State: AOAM533tracxvvJ9hwy/ttzAXuec/3r/aGiHAvMvNywMbpSRpiSK1fKl FP/9st0hWn9xwp7J/VdZ38Ls1RMLwNNrGcQUJC/rXGH8+0arbKCy0xVzAmBbJ/zBBlIx6OaQQ3F vcz8GJjKtNC1S4V8GwO32IvXnlaokR7Qc5/gziJysOZUtjcweNIZ7d4qdhGxr1hA= X-Google-Smtp-Source: ABdhPJxgkKi5sxuvSFhI7kIP9okH9A8vR/8GTsqqS7WeckM0UIeIq+jP3tJ0MzzeCtMIsyj76pFZlQ16OzjP9w== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:ab08:b0:1b9:c59:82c3 with SMTP id m8-20020a17090aab0800b001b90c5982c3mr13622946pjq.95.1645697340075; Thu, 24 Feb 2022 02:09:00 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:36 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-5-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 Thu Feb 24 10:08:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758292 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 405BCC433EF for ; Thu, 24 Feb 2022 10:09:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233239AbiBXKJm (ORCPT ); Thu, 24 Feb 2022 05:09:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46164 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233240AbiBXKJg (ORCPT ); Thu, 24 Feb 2022 05:09:36 -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 C855228A13B for ; Thu, 24 Feb 2022 02:09:02 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id b5-20020a170902e94500b0014f6d0a417bso845342pll.6 for ; Thu, 24 Feb 2022 02:09:02 -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=Cfp07IxMlWXApZOHJ8chcZo/7veCBBbAslqyaQYHJ9x4tPh7fP2fwp8aBMdJPb7hfo w7Etko0YSAM8berrTQuk1Z+oUWW0TkQ62bdO4+H1xyyXdS5JESTaFCkz7qCQeNjJ3o/o aoN37vu2zb0sAAabjLKIg2XeOuAwB9irg3Stzfz2TRxMgR3q+vcho23D+xkWH3vOTMqx aF8TEqoMtpZ/FzYgpMb1D6L9KryImI0T9fWiH1aRV88DEHSRxdD7WV2gXq3Qa27U9pR5 D92cyC+uhPDeGqiUbXLnVHRZrUuBd1bU5OGNRoFhqofnJc09Cxx6kzgO9VVmn5hhDH6l oUYQ== 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=jPH00hGkMEicvI/XfEyqZiJ8JtoBGQoQFLv4CdsNGtGlhOmBGvIKzSoYXf9QqhB221 usMBCbtjx26eo7cr0jSD4O0MUegLa/9O6+4A2P0Vp5U7w/r/QqyM9Ohg96N/OMRPyV8K 14cdX0SrO3Ryr5yIdIeMCeZePdjFv5lbZqrGDx/Vc8HmDRxiOCJqT7MibjnOSIsivrm1 OarIyUY+MvIl22f1IJaUUYRtX373vOGEs6ToP8m0gldCBnLcyiCrIFVFqLAqhEsY5Tyi WxyPH/KgHKb3Vz506raOYlwtnnDQSmXXgXmvvQj1FFJB3d3uz15g0/riH931T3dy2PA5 Njlg== X-Gm-Message-State: AOAM533D5iEjbSBJP9sBaXI9n8FNURUC2joSJl4vklgHjEfZcrKEe6TA N2QgLNzto9GxFzAz4rv9OeVEoXw+E50r3bX02agkxvz4q96hbElD43P3gkHNnhi4XvJMfJ+mn50 QPfa4DkttxIhqMLlep/1zzxY3PCyD1nxD0K0PupsLHzc2Vc2OYmj+LkUmvPWUuJY= X-Google-Smtp-Source: ABdhPJzU8+e5IvqjaLgXPA2uoR2esfMu/upVx6wi5mCKWGXA7p2hEfzLTHP7KQ6Jh8KndFC45mooP4OpiaCuBw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:1d8b:b0:4f1:bd8:811 with SMTP id z11-20020a056a001d8b00b004f10bd80811mr2080270pfw.25.1645697342198; Thu, 24 Feb 2022 02:09:02 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:37 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-6-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 Thu Feb 24 10:08:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758293 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 343E4C433FE for ; Thu, 24 Feb 2022 10:09:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233258AbiBXKJn (ORCPT ); Thu, 24 Feb 2022 05:09:43 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46134 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230152AbiBXKJh (ORCPT ); Thu, 24 Feb 2022 05:09:37 -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 F1FF828AD8A for ; Thu, 24 Feb 2022 02:09:04 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id h14-20020aa79f4e000000b004f3aa388c1fso1091015pfr.6 for ; Thu, 24 Feb 2022 02:09:04 -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=pmW6ZZvFrHTzJ2Ozh7HteoO76Md/WF+DaWBPeJtbgMkUU4by0SQUakbw30IU/J0+5X MIM5rCzu6ucgvbXc5aKARXQTI2sxy4nZehSYmciqoVzgwScd0xW0A/KWdHNsjG8H6mCA gqO3c8Cob5VjsolezZuoejJ79f3WjOWOPBkVyQ8fFj6MnBwt4ra1sQpwAC2+301YdtGI 2YRNJ2V736W7f1clEk3mgY9CZEgvw7WX+J74R1FG45MNivHNoH1agxUhqL6KyXit9cYA a4CtRrQD07Uh19UBgNNWiPyDbP/lKDhYwsVFRIdm4y+23n0pCjh0yVzAMjSbWiWpTynn 6B+Q== 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=ZZytrxQ2scdUZSEQ7eSJLY0klaZF4hp+F2SpLS4mxxH2VvE0O7GILCo6qAMcmGwzpb 2ZKA0IjPyU3dFKdqQWOGD5pFr1AVACAOEJ4153k+oAuai3szwrk6mz5rlfcafbNxkcsZ Pd7betRGnQY3npaJZDCcBnqSoOIh6HqNOxP4xcpdsZehRsdV2Z83l0q82G0MmIojJWvk wN5jVnmvQXrVHDzc/lmBq2kT+ErxbM260Y+mIKxXmHV1OEcztJGP5RhgOxD9ovkh6Z6d 435r12Nlrmt+qTjWgmAzVQTkdaCbIqubvgH+zMWSGfIuFwMY5cwgd03zSQ+T3caWyPVZ wWcg== X-Gm-Message-State: AOAM532/hwxPabv05mW9NDfjvMir6zoHL2T9gjxF6d2AMBhrA2oKgzxQ YFfFkx1Rs+p1iNRK+BA+4qvFKyGzws6PYB+SQvEV5R5j5vTttUkz89mXVB5rmVBCRxcTIBjKQDj tlznj6xL7o3FeYhVj4ucU6MPF57RMx7qTVb17GFVK9+okB+dKeL79ALKkTTJm5eI= X-Google-Smtp-Source: ABdhPJxJWy5b0+43hrwodIoIcUfEsMQ/RHs8pw8Vps8j10V88CFLJhNjNfF/4axBfsGKjRx8QF2fj+wLRVeMFw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:71c1:b0:1bc:cb52:ee06 with SMTP id m1-20020a17090a71c100b001bccb52ee06mr1464099pjs.232.1645697344352; Thu, 24 Feb 2022 02:09:04 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:38 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-7-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 Thu Feb 24 10:08:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758294 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 13EABC433EF for ; Thu, 24 Feb 2022 10:09:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233281AbiBXKJw (ORCPT ); Thu, 24 Feb 2022 05:09:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46178 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233254AbiBXKJh (ORCPT ); Thu, 24 Feb 2022 05:09:37 -0500 Received: from mail-pg1-x549.google.com (mail-pg1-x549.google.com [IPv6:2607:f8b0:4864:20::549]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2C5CE28ADA1 for ; Thu, 24 Feb 2022 02:09:07 -0800 (PST) Received: by mail-pg1-x549.google.com with SMTP id r8-20020a638f48000000b0036c6a881088so939192pgn.2 for ; Thu, 24 Feb 2022 02:09:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=XJ+XxV4dA7bXW/e75TlW5gYEF4MVzcGC4nrAKF0nmD0=; b=rUiYDGEiWvP6I+1k2+K40P/mCPhCEYXIuTbTFVQMZRp73FbJqHaCKSZk6gtKBuxAyE 535lq9FHeBllwVUiwWcycPEcd9d+YSbf5++Qsv9eyybxtQxesrBotodXecmRv2mR+wxB rVxJ7PX/x1StgKEuM/SlwZd6cAX48TCelBfqqv735jQBYxKQLxuUKkkvDM2P3eReF0eX bbU3gIQ555C3FW05j2lYukql8weJRxFkvA8JkYu12MvHpbxFAMcFelFQi83EcPtunmD5 zoDAyiLyRKIcHwpyVET3erswkA101aTtZU/hZK/Km/qDvvHZ5iomG2u/QmKh5GLh5MN8 ljUg== 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=XJ+XxV4dA7bXW/e75TlW5gYEF4MVzcGC4nrAKF0nmD0=; b=J8NumYWUwlLWRf1izKjtySbFrff+NdfVI1UoQS1YsqUv38jQZvvW2+HIk3HEpBm8kn gzjyZMBnEL1MB+ob6aSpU++AHNG0pWG2gmpbj5REVBMuSqBTdaGqM0qG9LYEORDQgoHg mJdloe6uUb+XlQNy7ugYqOVKI2v/EiKIfu3YNIWqW7I17gKR/n7yf17i9VSu5PjTFsvX /oc/clwE+PrK7waXwDPVf/ZES6NBxphxVUbMLH2eABFwHkKloHsX8YwnMtLu4vbU7iRX 1r2tuZZplG0mORy1wri+7qHTsqyMjDLFUkDl+4zrUpe7i2FQql3IlQLiHdHm2zGNI/gq Vwrg== X-Gm-Message-State: AOAM530/YWf5F2+5q5yX5gf3BnX4UVjuNFyFnj+OwnxwjQCmnIMYlN1j KlmCW2iaU7NfUD+X2Fq/hJZ6tpDig10AIIfP1NTINQ1N4k2Di4NTQi6xuDwJjPXqgDE/+z75vfP zr7qF2uO46uf9l8qTqc81Qth6j5I3dcJ8SuYnNiRUt7Ivo/oRWZh1LFRKeEV9OuQ= X-Google-Smtp-Source: ABdhPJxLrk6GEIWXBIqFP6Z8N3KugkI8gmhmaweigjGOx3FeOx9aE1B8P3Rqcxga7s9sFDiNM7G/1XZ7r7vUbQ== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:9517:b0:1bc:81ef:79cd with SMTP id t23-20020a17090a951700b001bc81ef79cdmr2017752pjo.164.1645697346560; Thu, 24 Feb 2022 02:09:06 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:39 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-8-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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..988757002a 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; 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 Thu Feb 24 10:08:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758295 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 8684CC433EF for ; Thu, 24 Feb 2022 10:09:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233284AbiBXKJx (ORCPT ); Thu, 24 Feb 2022 05:09:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233222AbiBXKJj (ORCPT ); Thu, 24 Feb 2022 05:09:39 -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 3B14828A10F for ; Thu, 24 Feb 2022 02:09:09 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id c14-20020a17090a8d0e00b001bc72e5c3ecso1074990pjo.3 for ; Thu, 24 Feb 2022 02:09: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=hbJoHdJ2QZDbI2dDWLG+p2K6A0OlB+xDeYUBgmECRtc=; b=hig8n+l0c1PFHk3S+W9iMj3rEYM5SAlRFhtQzbr/n+MckSoZoiOI5m7f59zFDe6KbV 0oGIri9NIsODDN1TFJJB7PtHUpi1znmI3O4B7HfGY6IGqoIQxak6TzpjdhV5EVPW54A9 NTWD1sL/zPaOwV+Rs6aTwiAcwSgDG9EScDnHONaOcueu+c7hZMoEyCgvSO4CvItlhrYP 92gjzxWATCeiwFH45v28iZ/39GCm48IC5uWZs27u1CEY02OnjerO6hmvCfZP1hPJpEZc qOvuvRRQAu0LaalZByCLf4Ct5eBZA+Sdu9YRLswcN8rCWOa9h4r/wCOWdeGH9waFNYm8 hWXg== 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=hbJoHdJ2QZDbI2dDWLG+p2K6A0OlB+xDeYUBgmECRtc=; b=oANMMdNVWJ0efl54Nl2jvLduDCJFVbEHydRActQdsFdZlEBtr2i/tRjChwq5LqM623 4Za00O4ln5iZNDhfAV2yMwZiOXmtxAomujGZWRh5SNkHOdJ8OiA9zcHNdEw/kg01AEUH iiogCUaU8gYPPRUXVtCes2zfOvUGDAleHumCAJdNSxyFkiXRS7cBavJz0jo8xD5JQg3G GxIBZ1AvWHP8w7hPwaLNlKfKb2JroI6NTeJezFisCo9U1yQq4gtkmC73+1LoGeSiupWf 9ljBedPbMXkJAiJKeI33lhXgxBqO5+wXWb9BIviICPPzZ4sCPhNrGr+4om0bOADOqeJy jufQ== X-Gm-Message-State: AOAM531VWkSbGseehIZyQ13IZZejh9A60XaW7DffpbwDR3ycW5QKpc+F cVivAAlAy+EKsWCF+7CR8BSl7jmxnXqpKpTjPGFAWG0qqHf+CkD10XKiS2Pw+33I8oXGCQeH8NG 9tOeaOURSu0/igcGjXB5O0hUyx4lHwuR/iRD1cmo0WIQ9FW21lPmng2KoWHcThG4= X-Google-Smtp-Source: ABdhPJxk0AsCNZm1u5RRN5mEPalZme/S7Ph32vrQnhNwsxWn5BqpbspL36SmebCgkiqf1WAtI9vXPl4xXCgrng== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:1310:b0:4ca:cc46:20c7 with SMTP id j16-20020a056a00131000b004cacc4620c7mr1926795pfu.44.1645697348547; Thu, 24 Feb 2022 02:09:08 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:40 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-9-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 --- I think this patch could be squashed into the previous one, let me know if this is a good idea. submodule.c | 99 ++++++++++++++++++++++++++++------------------------- 1 file changed, 52 insertions(+), 47 deletions(-) diff --git a/submodule.c b/submodule.c index 988757002a..03af223aba 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 Thu Feb 24 10:08:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758297 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 D2A13C433FE for ; Thu, 24 Feb 2022 10:09:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233228AbiBXKJ7 (ORCPT ); Thu, 24 Feb 2022 05:09:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46274 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233238AbiBXKJm (ORCPT ); Thu, 24 Feb 2022 05:09:42 -0500 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8F72D28A11F for ; Thu, 24 Feb 2022 02:09:11 -0800 (PST) Received: by mail-pg1-x54a.google.com with SMTP id m15-20020a63580f000000b00370dc6cafe9so933563pgb.5 for ; Thu, 24 Feb 2022 02:09:11 -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=4p4Tyls6g4Qu2sOGu6JLVBxQfSl2NsUML3/gJqhP/t4=; b=JmpTgJYZwz7kDv8Ok5pSO1QTQCvQVlP28D48UxZ6xRnxHqVyh1l3vxnZdVctMgBm/q jrKPcZ6Gct5UY3pAez+/+Q7WRGg1WCwvx4L9ZW6EqChK7m8o12Lt3f/0cz46npEgSAUj GBgW4PBgsTsUDmLUdIwDrgRnGI+Ryl3ray71Z7ZBiqbzIfZV4WAVC6q8klhKTVAQNYn5 NHy/faQ5o1VfvJFJpTPlqkhjHPupUeASNxfJQ2y26o6+q5O4A4h88HTzLJ+f+7fDoSTv Z/qFDF17uSPvOPpne87XPBaBXIzQzdNZ+MZMpPrFAiItRraoN21PuxO+GCPjPz8HtxEb /DYg== 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=4p4Tyls6g4Qu2sOGu6JLVBxQfSl2NsUML3/gJqhP/t4=; b=yYSghVZ2PyMzAYF9tnHTy3KSrQvIiEs9YVxRfGT2CcbkeGqvWr2N7xmC1snWIGjsOw S1ghDbnHWWhMNolrBl3iuZQfbQFoETjYSYDGv41ZR6vtHdL3XqTrEV9CnbB3TETB4b73 5FZs7T3PruGYPWAVRzpzrhDmHbdkf0fqHdawafTYNFyB/NjtlIVWr6r01llPJgwYz5Dy gy824/ypoLYWNrvrGJtH+kQQSJqkdpio4XjHrXhNB1FZjDdWYtuuUKwsa4tsF/of6W5h aOM1y1CAhXJcG+kYedsxdG+D3E8BrQsIk9ktgRtG92ATXwb4oX5yu+2L7lQOAW597PpF cIbA== X-Gm-Message-State: AOAM533ZIl5SHbdf3cmZFn/nyXyK+99FjAgdRGRiiozIdR+sAxPxS94i z9CpDQRFao5PWgL7tW/pVnmrovdWs9j8dHlLtLLAmNe7qOkWo3BAS7Ns+OuaIzVSZcP9yb104g/ FRmNk/4/ioEU+wat2CXuNYttpHt268o3sZ7An0D0OEJarnmpnqSIOJGFMsXXrIuI= X-Google-Smtp-Source: ABdhPJylDakc8zhaAUGtFSK3g9hwD7N1OSdcO/2z+pGSv5EvtVJxUeGvaprF0XGWCul5rzeH863hTiw+eBLhlw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:b398:b0:1bc:3ab0:cea7 with SMTP id e24-20020a17090ab39800b001bc3ab0cea7mr2029888pjr.222.1645697350915; Thu, 24 Feb 2022 02:09:10 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:41 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-10-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 --- In the process of writing the new tests [1], I noticed some failures of the form: # rm the submodule's working tree directory. $ git rm submodule [...] # Do a fetch that requires running a child process from the submodule. $ git fetch --recurse-submodules same-name-1 [...] # Fatal error tells us that we cannot chdir to the deleted working tree. fatal: cannot chdir to '../../../submodule': No such file or directory This happens because submodules set/unset a value for core.worktree when they are checked out/"un-checked out" (see submodule_move_head() and connect_work_tree_and_git_dir()), but "git rm" doesn't know that core.worktree should be updated. I've worked around this by passing "--work-tree=." to the child process [2], but this feels like a hack, especially because this bug should affect all child processes in a "git rm"-ed submodule (this probably includes the "git branch" processes in gc/branch-recurse-submodules, but I haven't confirmed it yet). Some more comprehensive solutions that could be future work are: - Teach "git [add|rm]" to unset core.worktree (the reverse operation, "git restore", should already do the correct thing). This won't detect submodules removed with "rm -r" though. - Teach submodule child processes to ignore stale core.worktree values. - Do more things in-core instead of using child processes (avoiding the failing chdir() call). I'm not sure what future work we should pursue, or even if the "--work-tree=." workaround is even good, so I'd appreciate feedback here. [1] There is a similar, preexisting test that also removes the submodules. However, that test isn't affected because it invokes "git checkout" after doing "git rm". [2] Since the submodule has a git dir but no working tree, I also tried working around the bug by passing "--bare". However, this doesn't work because work tree settings override "bare-ness" settings, as described by t/t1510-repo-setup.sh. Documentation/fetch-options.txt | 26 ++-- Documentation/git-fetch.txt | 10 +- builtin/fetch.c | 14 +- submodule.c | 125 +++++++++++++-- submodule.h | 12 +- t/t5526-fetch-submodules.sh | 260 +++++++++++++++++++++++++++++++- 6 files changed, 404 insertions(+), 43 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index e967ff1874..38dad13683 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -185,15 +185,23 @@ endif::git-pull[] ifndef::git-pull[] --recurse-submodules[=yes|on-demand|no]:: This option controls if and under what conditions new commits of - populated submodules should be fetched too. It can be used as a - boolean option to completely disable recursion when set to 'no' or to - unconditionally recurse into all populated submodules when set to - 'yes', which is the default when this option is used without any - value. Use 'on-demand' to only recurse into a populated submodule - when the superproject retrieves a commit that updates the submodule's - reference to a commit that isn't already in the local submodule - clone. By default, 'on-demand' is used, unless - `fetch.recurseSubmodules` is set (see linkgit:git-config[1]). + submodules should be fetched too. When recursing through submodules, + `git fetch` always attempts to fetch "changed" submodules, that is, a + submodule that has commits that are referenced by a newly fetched + superproject commit but are missing in the local submodule clone. A + changed submodule can be fetched as long as it is present locally e.g. + in `$GIT_DIR/modules/` (see linkgit:gitsubmodules[7]); if the upstream + adds a new submodule, that submodule cannot be fetched until it is + cloned e.g. by `git submodule update`. ++ +When set to 'on-demand', only changed submodules are fetched. When set +to 'yes', all populated submodules are fetched and submodules that are +both unpopulated and changed are fetched. When set to 'no', submodules +are never fetched. ++ +When unspecified, this uses the value of `fetch.recurseSubmodules` if it +is set (see linkgit:git-config[1]), defaulting to 'on-demand' if unset. +When this option is used without any value, it defaults to 'yes'. endif::git-pull[] -j:: diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index 550c16ca61..e9d364669a 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -287,12 +287,10 @@ include::transfer-data-leaks.txt[] BUGS ---- -Using --recurse-submodules can only fetch new commits in already checked -out submodules right now. When e.g. upstream added a new submodule in the -just fetched commits of the superproject the submodule itself cannot be -fetched, making it impossible to check out that submodule later without -having to do a fetch again. This is expected to be fixed in a future Git -version. +Using --recurse-submodules can only fetch new commits in submodules that are +present locally e.g. in `$GIT_DIR/modules/`. If the upstream adds a new +submodule, that submodule cannot be fetched until it is cloned e.g. by `git +submodule update`. This is expected to be fixed in a future Git version. SEE ALSO -------- diff --git a/builtin/fetch.c b/builtin/fetch.c index f7abbc31ff..faaf89f637 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2122,13 +2122,13 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) max_children = fetch_parallel_config; add_options_to_argv(&options); - result = fetch_populated_submodules(the_repository, - &options, - submodule_prefix, - recurse_submodules, - recurse_submodules_default, - verbosity < 0, - max_children); + result = fetch_submodules(the_repository, + &options, + submodule_prefix, + recurse_submodules, + recurse_submodules_default, + verbosity < 0, + max_children); strvec_clear(&options); } diff --git a/submodule.c b/submodule.c index 03af223aba..d60f877b1f 100644 --- a/submodule.c +++ b/submodule.c @@ -811,6 +811,16 @@ static const char *default_name_or_path(const char *path_or_name) * member of the changed submodule string_list_item. */ struct changed_submodule_data { + /* + * The first superproject commit in the rev walk that points to the + * submodule. + */ + const struct object_id *super_oid; + /* + * Path to the submodule in the superproject commit referenced + * by 'super_oid'. + */ + char *path; /* The submodule commits that have changed in the rev walk. */ struct oid_array new_commits; }; @@ -818,6 +828,7 @@ struct changed_submodule_data { static void changed_submodule_data_clear(struct changed_submodule_data *cs_data) { oid_array_clear(&cs_data->new_commits); + free(cs_data->path); } static void collect_changed_submodules_cb(struct diff_queue_struct *q, @@ -865,6 +876,8 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, if (!item->util) item->util = xcalloc(1, sizeof(struct changed_submodule_data)); cs_data = item->util; + cs_data->super_oid = commit_oid; + cs_data->path = xstrdup(p->two->path); oid_array_append(&cs_data->new_commits, &p->two->oid); } } @@ -1253,14 +1266,33 @@ 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) +{ + struct strbuf buf = STRBUF_INIT; + + strbuf_repo_git_path(&buf, r, "modules/"); + return file_exists(buf.buf) && !is_empty_dir(buf.buf); +} + static void calculate_changed_submodule_paths(struct repository *r, struct string_list *changed_submodule_names) { struct strvec argv = STRVEC_INIT; struct string_list_item *name; - /* No need to check if there are no submodules configured */ - if (!submodule_from_path(r, NULL, NULL)) + /* No need to check if no submodules would be fetched */ + if (!submodule_from_path(r, NULL, NULL) && + !repo_has_absorbed_submodules(r)) return; strvec_push(&argv, "--"); /* argv[0] program name */ @@ -1333,7 +1365,8 @@ int submodule_touches_in_range(struct repository *r, } struct submodule_parallel_fetch { - int count; + int index_count; + int changed_count; struct strvec args; struct repository *r; const char *prefix; @@ -1343,6 +1376,7 @@ struct submodule_parallel_fetch { int result; struct string_list changed_submodule_names; + struct string_list seen_submodule_names; /* Pending fetches by OIDs */ struct fetch_task **oid_fetch_tasks; @@ -1353,6 +1387,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 +1425,7 @@ struct fetch_task { const struct submodule *sub; unsigned free_sub : 1; /* Do we need to free the submodule? */ const char *default_argv; + struct strvec git_args; struct oid_array *commits; /* Ensure these commits are fetched */ }; @@ -1425,6 +1461,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 +1501,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 +1534,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 +1554,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 +1582,64 @@ 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: A submodule unpopulated by "git rm" will + * have core.worktree set, but the actual core.worktree + * directory won't exist, causing the child process to + * fail. Forcibly set --work-tree until we get smarter + * handling for core.worktree in unpopulated submodules. + */ + 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 +1649,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 +1662,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 +1777,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 ee4dd5a4a9..639290d30d 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -15,8 +15,9 @@ pwd=$(pwd) check_sub() { NEW_HEAD=$1 && + SUPER_HEAD=$2 && cat <<-EOF >$pwd/expect.err.sub - Fetching submodule submodule + Fetching submodule submodule${SUPER_HEAD:+ at commit $SUPER_HEAD} From $pwd/submodule OLD_HEAD..$NEW_HEAD sub -> origin/sub EOF @@ -24,8 +25,9 @@ check_sub() { check_deep() { NEW_HEAD=$1 && + SUB_HEAD=$2 && cat <<-EOF >$pwd/expect.err.deep - 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 @@ -418,6 +420,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 <<-EOF > expect.err.combined && + 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]+\.\./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 +1011,109 @@ 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 && + 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 && + 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 && + 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 master" && + ( + 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/master && + 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 master && + 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. + git --work-tree=. cat-file -e $head1 && + test_must_fail git --work-tree=. cat-file -e $head2 + ) +' + test_done From patchwork Thu Feb 24 10:08:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12758296 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 62B45C43217 for ; Thu, 24 Feb 2022 10:09:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233219AbiBXKJ4 (ORCPT ); Thu, 24 Feb 2022 05:09:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46300 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233249AbiBXKJn (ORCPT ); Thu, 24 Feb 2022 05:09:43 -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 805BC28A12D for ; Thu, 24 Feb 2022 02:09:13 -0800 (PST) Received: by mail-pf1-x44a.google.com with SMTP id x194-20020a627ccb000000b004e103c5f726so1087430pfc.8 for ; Thu, 24 Feb 2022 02:09:13 -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=Txi60ETKS8WiURrpOogef4iTcsrXpKm1FqsfDJ/toW8=; b=Xq+0/ZqyqvERqtf9nZgvrldLNr5RkqNYwZou3wq2BKOxkr23/MdvaQc3gMLu6eJKvq fBY0z6ThY1LUZbjgjueJpFeG6y0h+bl0YykQjq8CfBJjMBtyaTt5stwKdN6Rd+m2DpsO YWBqro1WEXAi+l7XdnnP7KlFvcUp2QRp2CVxAycFNhSrxaaTo8sE7Gtimg9DATtyl5Oj Wnr4+PJrDXPNq75T609QHna/sNwqaFPH8v3Y7D9OifLGxj+a6WWaPVUnYT2cWmBdCXlR OercDc7I+B1aDhEiLsIYnL1cqumjDxCQXx+tMeuef5cnYQ35yDcH/xoIZ+Nn6dL4+8DU l+kw== 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=Txi60ETKS8WiURrpOogef4iTcsrXpKm1FqsfDJ/toW8=; b=KYohHw5ws5DW+ygoenYokZKy5SqVPDO+2QcKJIMiAYRHpDiTlWdgJi3yDccJK8zyL/ OneQTzB1Eyd1c8XvYIxj6dJtC7GI4RBXK5Lya3atQ5cYTphGNWhasNJpOOepd7fwlUN7 u9FELjBlD3pd9+jv2F85c387fDBK6/WLK/OrRP74yElpURw6Ro2pYh2sE+pblsgjULG6 U5rVSslrtoh+fi0z6E7LSMHMLEiFtaCU1KUEFKJmKNmiC/uoQroNQclmO//P91bp/cCG fg6WOvMXxcSgFY8XUhhA6nCPjhkdnuPQYXSEhW9vJlkREe4smnAbGUgA29QS4qmV1xRa /Ckg== X-Gm-Message-State: AOAM532Y1WFGVMEWrQYua0AnrVAKMT74GQOIGQyd8ht0ggVOjcZybsZO s9je8Ix1KQBikTQPhPZFSzpmubkqlFn3PAt5ZNC5a70QeI6k8b77OAHli6wL2+g6eXqx5wMKBTx Ub44KEZbrBZh5+nqxO8gzeNRpcqX501cdCT8hNnHDGl8dZ4etNy0NrEsKzkL2t2M= X-Google-Smtp-Source: ABdhPJwM1mYfmhNg4K0l2fwvPAHV+cu/8uSgyX/Z8kKOijrA6d9HJLhgpIGnpCndVcOKRMjfUauu7MKslO1mWQ== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:aa7:88d2:0:b0:4f3:5d00:58d8 with SMTP id k18-20020aa788d2000000b004f35d0058d8mr2204134pff.5.1645697352868; Thu, 24 Feb 2022 02:09:12 -0800 (PST) Date: Thu, 24 Feb 2022 18:08:42 +0800 In-Reply-To: <20220224100842.95827-1-chooglen@google.com> Message-Id: <20220224100842.95827-11-chooglen@google.com> Mime-Version: 1.0 References: <20220215172318.73533-1-chooglen@google.com> <20220224100842.95827-1-chooglen@google.com> X-Mailer: git-send-email 2.35.1.473.g83b2b277ed-goog Subject: [PATCH v3 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 d60f877b1f..71495e67f5 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)); @@ -962,7 +942,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); @@ -998,18 +979,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);