From patchwork Thu Feb 10 04:41:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741253 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 0AC9BC433F5 for ; Thu, 10 Feb 2022 04:42:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233482AbiBJEmC (ORCPT ); Wed, 9 Feb 2022 23:42:02 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60614 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229489AbiBJEl5 (ORCPT ); Wed, 9 Feb 2022 23:41:57 -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 1A9681A6 for ; Wed, 9 Feb 2022 20:41:58 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id n4-20020a17090ade8400b001b8bb511c3bso3070428pjv.7 for ; Wed, 09 Feb 2022 20:41: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=f+vtkyIZlv9i3ilY9wRatiFyrKMDOpZkUHyk9HR8YRc=; b=Is+EyJ6kcDZeF/TOo2M4UP2P50VRYUfdpxoZukwRJsWxpdrqgkz2om2ZHgwTsYu1Wi FprzC/Rcp3aQMgWc23CHRRaFsRw8PuGIlEFGI/iCRjh1d2a8fL3c1bA6t73VYNoxb/sa 4g5pXE3V+w3a762WrFv9TzVkx3MLQPTqGfZQ/DeE1aOfnefONl4dlGizbf1l1v3V8ZFS ahqQrobu3G4drM9zNu+jh3CQ3zy2fSWVRwhWEOnvL/5uWoCKLRU1pwTjA139fGF6F4q1 hsA3E8ky0lwW1SQ4Jr8Aw7AAuef7F/573vSvvdZ2i4O8iRb0sLJU2XzB5Xq30wsCrEXu jeQQ== 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=f+vtkyIZlv9i3ilY9wRatiFyrKMDOpZkUHyk9HR8YRc=; b=Ci5RvjarBpyrIYg5ICjYY5voP4ifUERYbsCppcAhPEaoGXRPlf1AQ3ADVG4tckhqjo TtsEz+1jGo9+8fjsVyWcIKEfpkSX75FiqBlNZh/Xb+uvCB0iFOAdnqrK5rGSGXM+nqDK h8GUDLNileVe45Wzvw2jR6JBThk02O/acx2yrcB9PnAN7gVbQz+O5bfAqpOCxmrOqTae Z23Hj6NjzxQGHOUhvullQk+QhVlGqrHtVAP0h2psWgqzkkdDXySlEW2sEQ2RFdr2N6bp w64CknncYgPxNL7A+2OzRc982XdiEvlwN1IwwV6zfSEPujQB5gOp/A/p4towR6Zqnsxz LmWg== X-Gm-Message-State: AOAM532CgOoDAHDc/eS1SDuFAzPdkrDmGQHs5krReVW5xK0di5KXmsN0 HRnpizk1Cuc6cagHXex5q6omwOWzOJx4nXmrHA3IB+L+/ILPEvUiZtQ8Z2nPBRGQzdPpwSZDvGE tAXHVTlmU1cMzHF0sLJdqoLm4xSKdSC5IefOcMsz5fbz6Dej6dYHGDwQoCg9kI5k= X-Google-Smtp-Source: ABdhPJyCfPnfAXmD8jy1YG7+o0SJokluR8gQzO/vWO1MU0shjWb3pnt3fyToyHtt/AFwOjFxqM0t27xVenBNJg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:902:f606:: with SMTP id n6mr5629397plg.32.1644468117523; Wed, 09 Feb 2022 20:41:57 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:45 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-2-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 1/8] submodule: inline submodule_commits() into caller From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan 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. This simplifies the code and makes it easier for the caller to add information to the string_list_item.util. Signed-off-by: Glen Choo --- submodule.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/submodule.c b/submodule.c index 5ace18a7d9..49f9dc5d23 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 10 04:41:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741254 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 20D8CC433EF for ; Thu, 10 Feb 2022 04:42:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233487AbiBJEmF (ORCPT ); Wed, 9 Feb 2022 23:42:05 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60626 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232677AbiBJEl6 (ORCPT ); Wed, 9 Feb 2022 23:41:58 -0500 Received: from mail-pj1-x104a.google.com (mail-pj1-x104a.google.com [IPv6:2607:f8b0:4864:20::104a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4983113E for ; Wed, 9 Feb 2022 20:42:00 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id a15-20020a17090ad80f00b001b8a1e1da50so3329115pjv.6 for ; Wed, 09 Feb 2022 20:42: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=HKgOTLFGE50cSv+qQN0vXG7pGzl1KoId7DOuDb7YA7Y=; b=Tvgq8OnGwZIXYEmhgW6IlA+wK6ONa/Mkplc30pPem549+6vbCNBlEq+88L0pUcz6nW JQfFp/brMnnuRHRAsriyzNS3txE272K4iQQxIglVl6E8eoiYt9qAYEIXrkQ3qrt8eEQJ ng8j4f+E0x1Lco1WkG2L1jbvbWQjZ0zv0wDitVBqnHamfJ1HP8le/q1GMFB7puCK7tus IEmH1y5+j6YEFpMTRZCJvM/uA9hHFiqQliZK9bmbWWVfxMA8PrQW7c6Ngws0CygNd/nA SMeLzMenUfqOTT7ZUJOM6kzNmkjY8inMRmzsct3XKJ+QBP1KtpsbjHB36wtENqnVSTpc QAiw== 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=HKgOTLFGE50cSv+qQN0vXG7pGzl1KoId7DOuDb7YA7Y=; b=JuDH2U+ngOE31api5Hf+v+a9ZtS/d+7BVbekURwccOfiuU1Q+wpNqulkuMGPyZDe2e xyY2sG3DXtQylW4ueXaEl9IlLosLM28I6JTP7UrVjTgYzhePPYZGgUlcMlzu78Hhandk MLmSa9IFy727LZSsiGCIHWxQpDWYhYKOMcsj+NAFWc28341zksIv59xd7LJHSxPCpU5L O5VTixOHMKGcyKDLUEUSZvgUIq27dpK0YdNUVIrjU8ObRXC/QcQRx0UwzQFI/hnXt6uV qRaU8olafzF3Xq8ENJ3wNN8VzXRc0FqZE/VqTn0VV7dVF9a0qUoCPWOJidb+JB1JjHCt XWmQ== X-Gm-Message-State: AOAM531DAdnemQDlModR6W7reTCM3XEtYtug/sm/ovUCcBuikeTzaj6u milo9b6hR3ndN7ZBawQQVyTvJIseK7rhXgZU6iqfSzazyMDU38EUb7gjmRRugo8l20YRjMYNrvd upRayUWphf+tfftdUXlHv1G7MRg80BVJwONJmP+Q7iHOEkwQ91iO28/IYCrGKfGE= X-Google-Smtp-Source: ABdhPJzpTI6dCB21u4uS9o1KqwJPLidKEDc0e2gKmrc/3MBTJp8FR06Yjg8YIMO54tSG4RR2dvZp8HV1UoMRTw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a65:6d89:: with SMTP id bc9mr4731136pgb.260.1644468119687; Wed, 09 Feb 2022 20:41:59 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:46 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-3-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 2/8] submodule: store new submodule commits oid_array in a struct From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan 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 | 60 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/submodule.c b/submodule.c index 49f9dc5d23..e2405c9f15 100644 --- a/submodule.c +++ b/submodule.c @@ -806,6 +806,21 @@ 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); + free(cs_data->new_commits); +} + static void collect_changed_submodules_cb(struct diff_queue_struct *q, struct diff_options *options, void *data) @@ -820,6 +835,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; @@ -847,10 +863,15 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, continue; item = string_list_insert(changed, name); - if (!item->util) + if (item->util) + cs_data = item->util; + else { + cs_data = xcalloc(1, sizeof(struct changed_submodule_data)); /* NEEDSWORK: should we have oid_array_init()? */ - item->util = xcalloc(1, sizeof(struct oid_array)); - oid_array_append(item->util, &p->two->oid); + cs_data->new_commits = xcalloc(1, sizeof(struct oid_array)); + item->util = cs_data; + } + oid_array_append(cs_data->new_commits, &p->two->oid); } } @@ -897,11 +918,12 @@ static void collect_changed_submodules(struct repository *r, reset_revision_walk(); } -static void free_submodules_oids(struct string_list *submodules) +static void free_submodules_data(struct string_list *submodules) { struct string_list_item *item; - for_each_string_list_item(item, submodules) - oid_array_clear((struct oid_array *) item->util); + for_each_string_list_item(item, submodules) { + changed_submodule_data_clear(item->util); + } string_list_clear(submodules, 1); } @@ -1067,7 +1089,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; @@ -1080,11 +1102,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; @@ -1254,7 +1276,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; @@ -1267,8 +1289,8 @@ static void calculate_changed_submodule_paths(struct repository *r, if (!path) continue; - if (submodule_has_commits(r, path, commits)) { - oid_array_clear(commits); + if (submodule_has_commits(r, path, cs_data->new_commits)) { + oid_array_clear(cs_data->new_commits); *name->string = '\0'; } } @@ -1305,7 +1327,7 @@ int submodule_touches_in_range(struct repository *r, strvec_clear(&args); - free_submodules_oids(&subs); + free_submodules_data(&subs); return ret; } @@ -1587,7 +1609,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"); @@ -1615,14 +1637,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); @@ -1680,7 +1702,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 10 04:41:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741256 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 F12EBC433EF for ; Thu, 10 Feb 2022 04:42:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232677AbiBJEmR (ORCPT ); Wed, 9 Feb 2022 23:42:17 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60666 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233481AbiBJEmC (ORCPT ); Wed, 9 Feb 2022 23:42:02 -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 BAF9A1A8 for ; Wed, 9 Feb 2022 20:42:02 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id hi22-20020a17090b30d600b001b8b33cf0efso3359517pjb.1 for ; Wed, 09 Feb 2022 20:42: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=szfxtG21KRnDWftifsRjdD8enTYjbmRO82Wb0Ej+5s8=; b=Cpd5P4tC0qZkHlYbw4h4fQFg59FIFMyEeXrijPCi5PW8Rk2RgKNStOzrYjNjeTez/2 llaICbxvA1/rSfVp+c1xJj94ZbxMkk+4oWbf0IC9m1yU+tXmzAnmiACSgs9WAI8aPR3C 28dc5hX/1F+Zsvvrhc082sK7Vj2CSDoTjUYAp76rgNSKmlj0Bvp1KsX3FaHH88IONb77 IWbJbzsOQIEOwEEzeLt4MzmzZAvc7YVxuYh0WAepGD8MHTzI0NoKeMu5kexUgywDPJcT ZCuD7IFjKGBdbPMidRGSk7x99qMMAKbx6Xzan6X47FC7wMGidOx53OEMczaRu/VoNaV0 TFHg== 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=szfxtG21KRnDWftifsRjdD8enTYjbmRO82Wb0Ej+5s8=; b=MgNPy1YarlalqXZavvZlOtFX+gEGlxTd4rEJn9HnM8fs0FmGav91UITLBhfqqFbILF zp0ebsS1Q6WikLdziTO6g7AtoDP4i9+ZYXmPf77l0VsH05jQT+Z+nVBrPcIRxcdDRUVQ 3O/koZyq9g/lPYsLHD1Uv4A6ZLA9ZqPUchbEH1vljSYAHrfaFGP3xdY+KGAS2hxD3CEO fk69FimdDGWUocUjHlWQV3+JfHBN5iCHGtOwOykOqXM8pLodjqvaiun+l1zTMQnXpd7+ l+LLLAzTgvJuIwY5nILnyqpUPeyZL0xTw7N2K2z5iSaNValoXl4fzM/qpEXSMnDogctH XExA== X-Gm-Message-State: AOAM533UtP6CdNXXT6qFyan9FdJUBD23QA7zMeFSOcBVdhWbdEnWAYUl A6Hbxl6vISD5eruxKN+1vkWZXM3BvzDzDwfOFlpn0y1B7+jcjz+mk9+rOZmXhnVa+j19OPkBsHR pFDbwxwIuGgKPAQVN9Muyk+XiWkumXcIR8cXE16hvm0nmKovv3aGuUx/TqHSDZRY= X-Google-Smtp-Source: ABdhPJy/T1nRIa25IjKQk7g8gyLOcpeoYRNK8OwxLQUOv7cxz19MsktuMnRM5tAG6owqJ+qwcYPc4c7doTk4Xg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90b:4c8e:: with SMTP id my14mr214594pjb.0.1644468121936; Wed, 09 Feb 2022 20:42:01 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:47 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-4-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 3/8] submodule: make static functions read submodules from commits From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A future commit will teach "fetch --recurse-submodules" to fetch unpopulated submodules. Prepare for this by teaching the necessary static functions to read submodules from superproject commits instead of the index and filesystem. Then, store the necessary fields (path and super_oid), and use them in "fetch --recurse-submodules" where possible. As a result, "git fetch" now reads changed submodules using the `.gitmodules` and path from super_oid's tree (which is where "git fetch" actually noticed the changed submodule) instead of the filesystem. Signed-off-by: Glen Choo --- submodule.c | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/submodule.c b/submodule.c index e2405c9f15..d4227ac22d 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; }; @@ -819,6 +829,7 @@ static void changed_submodule_data_clear(struct changed_submodule_data *cs_data) { oid_array_clear(cs_data->new_commits); free(cs_data->new_commits); + free(cs_data->path); } static void collect_changed_submodules_cb(struct diff_queue_struct *q, @@ -869,6 +880,8 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, cs_data = xcalloc(1, sizeof(struct changed_submodule_data)); /* NEEDSWORK: should we have oid_array_init()? */ cs_data->new_commits = xcalloc(1, sizeof(struct oid_array)); + cs_data->super_oid = commit_oid; + cs_data->path = xstrdup(p->two->path); item->util = cs_data; } oid_array_append(cs_data->new_commits, &p->two->oid); @@ -944,6 +957,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) @@ -952,7 +966,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; } @@ -980,9 +994,10 @@ static int check_has_commit(const struct object_id *oid, void *data) static int submodule_has_commits(struct repository *r, const char *path, + const struct object_id *super_oid, struct oid_array *commits) { - struct has_commit_data has_commit = { r, 1, path }; + struct has_commit_data has_commit = { r, 1, path, super_oid }; /* * Perform a cheap, but incorrect check for the existence of 'commits'. @@ -1029,7 +1044,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 @@ -1280,7 +1295,7 @@ static void calculate_changed_submodule_paths(struct repository *r, const struct submodule *submodule; const char *path = NULL; - submodule = submodule_from_name(r, null_oid(), name->string); + submodule = submodule_from_name(r, cs_data->super_oid, name->string); if (submodule) path = submodule->path; else @@ -1289,7 +1304,7 @@ static void calculate_changed_submodule_paths(struct repository *r, if (!path) continue; - if (submodule_has_commits(r, path, cs_data->new_commits)) { + if (submodule_has_commits(r, path, cs_data->super_oid, cs_data->new_commits)) { oid_array_clear(cs_data->new_commits); *name->string = '\0'; } @@ -1414,12 +1429,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, @@ -1451,11 +1467,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; } @@ -1476,7 +1493,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; @@ -1499,7 +1516,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 10 04:41:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741255 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 60F90C433EF for ; Thu, 10 Feb 2022 04:42:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233517AbiBJEmQ (ORCPT ); Wed, 9 Feb 2022 23:42:16 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60704 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232115AbiBJEmE (ORCPT ); Wed, 9 Feb 2022 23:42:04 -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 17C5D1AB for ; Wed, 9 Feb 2022 20:42:05 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id n21-20020a628f15000000b004e05a90ebfdso2168214pfd.8 for ; Wed, 09 Feb 2022 20:42: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=66KfgZMamYnH+VV69MTVJzWXHjEOzcYsC2QaLw8JH38=; b=l5XB7QCuSSqXoEPsznR052RltZWYVI6owNo5KVUMF99i4396uOek6QgBuDHbmrA//P f9lt6V+IdNqtExM5f68rJkC4y8js9WNVFzBlgp1rf16qAX6dwUtOTL0QU71NQy5jtvJs QNCar94kCu7UnmrG4vzXZG2ApFBwdsYYy62XHcqeiPhmMe+k+zSvA1J+JX4Ofjj4HDGR 37I0vcbT9dV8eqPew8nqgEHFIk9HEtIrJg49do3ryigUs/9EH4m9lxSk22g10z5Z0b50 zLKs435RPdV/QLDDfWMhSBlQ1W1QdB+luXTkJ3gtLWldtpdp1Yox/Af75K0KgorO51He lf2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=66KfgZMamYnH+VV69MTVJzWXHjEOzcYsC2QaLw8JH38=; b=5eTudysO75ZDZh2zvrVcUXQL7tbHCRiOQWBG4vSA3wcwT674fUA1zcp2kdaNGLsOvZ 3o6TS2CQluSaZUX/EwqKrrKOfd9nRmO0oh6CF9EcPPJNIYMuh1MjvpsYg8B7yrwl228E Ub0EMTQzizXuJ3oK4evkiTFgNbclCJA48msu+ZsHEuzLi5oGSWyxk+o1bByGLoxTsnsJ JnNB5WeZwNQLb1g0vXjntupoiXVNHN72r0Bgg78Oj8vt6mQ+oY4kuCeKr5OeGpOnh81+ Sp1Aay2znXMhs8Eqf3as1uxX+JO8pdIn78iW067xKOB24QHd2OMnxD67/Ei0evy2xayA 2Rmg== X-Gm-Message-State: AOAM5302J1iOUUvAVsOim3E0eHloQhAiFGrQz82D4xIcX3hWUllEX0r1 cwIh21RVGxUQzrj/v5UJq5Wo42rdEN6EGpX9Or7l7TuwRL5I0wKPxPIeLGZ8gfL+VvpY4p+KbiX RgGKQ8wBExkeOKZc6llb6aCVjtJ1STAt4dWFrhmpUDoKmyTlnveNoRwRehy5Ak8c= X-Google-Smtp-Source: ABdhPJzaaItFKOWYKK+vy8e6MztAMVBXOlXLhSvxPAtaJq2IKFWplfKFmC0UF0AC0Iz1nIPFOW0zPjC7A0Dj0Q== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90b:4c8e:: with SMTP id my14mr214617pjb.0.1644468124170; Wed, 09 Feb 2022 20:42:04 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:48 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-5-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 4/8] t5526: introduce test helper to assert on fetches From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org A future commit will change the stderr of "git fetch --recurse-submodules" and add new tests to t/t5526-fetch-submodules.sh. This poses two challenges: * The tests use test_cmp to assert on the stderr, which will fail on the future test because the stderr changes slightly, even though it still contains the information we expect. * The expect.err file is constructed by the add_upstream_commit() helper as input into test_cmp, but most tests fetch a different combination of repos from expect.err. This results in noisy tests that modify parts of that expect.err to generate the expected output. To address both of these issues, introduce a verify_fetch_result() helper to t/t5526-fetch-submodules.sh that asserts on the output of "git fetch --recurse-submodules" and handles the ordering of expect.err. As a result, the tests no longer construct expect.err manually. test_cmp is still invoked by verify_fetch_result(), but that will be replaced in a later commit. Signed-off-by: Glen Choo --- t/t5526-fetch-submodules.sh | 136 +++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 55 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 2dc75b80db..0e93df1665 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -13,6 +13,10 @@ export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB pwd=$(pwd) +# For each submodule in the test setup, this creates a commit and writes +# a file that contains the expected err if that new commit were fetched. +# These output files get concatenated in the right order by +# verify_fetch_result(). add_upstream_commit() { ( cd submodule && @@ -22,9 +26,9 @@ add_upstream_commit() { git add subfile && git commit -m new subfile && head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule" > ../expect.err && - echo "From $pwd/submodule" >> ../expect.err && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err + echo "Fetching submodule submodule" > ../expect.err.sub && + echo "From $pwd/submodule" >> ../expect.err.sub && + echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub ) && ( cd deepsubmodule && @@ -34,12 +38,33 @@ add_upstream_commit() { git add deepsubfile && git commit -m new deepsubfile && head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule/subdir/deepsubmodule" >> ../expect.err - echo "From $pwd/deepsubmodule" >> ../expect.err && - echo " $head1..$head2 deep -> origin/deep" >> ../expect.err + echo "Fetching submodule submodule/subdir/deepsubmodule" > ../expect.err.deep + echo "From $pwd/deepsubmodule" >> ../expect.err.deep && + echo " $head1..$head2 deep -> origin/deep" >> ../expect.err.deep ) } +# Verifies that the expected repositories were fetched. This is done by +# concatenating the files expect.err.[super|sub|deep] in the correct +# order and comparing it to the actual stderr. +# +# If a repo should not be fetched in the test, its corresponding +# expect.err file should be rm-ed. +verify_fetch_result() { + ACTUAL_ERR=$1 && + rm -f expect.err.combined && + if [ -f expect.err.super ]; then + cat expect.err.super >>expect.err.combined + fi && + if [ -f expect.err.sub ]; then + cat expect.err.sub >>expect.err.combined + fi && + if [ -f expect.err.deep ]; then + cat expect.err.deep >>expect.err.combined + fi && + test_cmp expect.err.combined $ACTUAL_ERR +} + test_expect_success setup ' mkdir deepsubmodule && ( @@ -77,7 +102,7 @@ test_expect_success "fetch --recurse-submodules recurses into submodules" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "submodule.recurse option triggers recursive fetch" ' @@ -87,7 +112,7 @@ test_expect_success "submodule.recurse option triggers recursive fetch" ' git -c submodule.recurse fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "fetch --recurse-submodules -j2 has the same output behaviour" ' @@ -97,7 +122,7 @@ test_expect_success "fetch --recurse-submodules -j2 has the same output behaviou GIT_TRACE="$TRASH_DIRECTORY/trace.out" git fetch --recurse-submodules -j2 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err && + verify_fetch_result actual.err && grep "2 tasks" trace.out ' @@ -127,7 +152,7 @@ test_expect_success "using fetchRecurseSubmodules=true in .gitmodules recurses i git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--no-recurse-submodules overrides .gitmodules config" ' @@ -158,7 +183,7 @@ test_expect_success "--recurse-submodules overrides fetchRecurseSubmodules setti git config --unset submodule.submodule.fetchRecurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--quiet propagates to submodules" ' @@ -186,7 +211,7 @@ test_expect_success "--dry-run propagates to submodules" ' git fetch --recurse-submodules --dry-run >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "Without --dry-run propagates to submodules" ' @@ -195,7 +220,7 @@ test_expect_success "Without --dry-run propagates to submodules" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "recurseSubmodules=true propagates into submodules" ' @@ -206,7 +231,7 @@ test_expect_success "recurseSubmodules=true propagates into submodules" ' git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--recurse-submodules overrides config in submodule" ' @@ -220,7 +245,7 @@ test_expect_success "--recurse-submodules overrides config in submodule" ' git fetch --recurse-submodules >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "--no-recurse-submodules overrides config setting" ' @@ -253,14 +278,14 @@ test_expect_success "Recursion stops when no new submodule commits are fetched" git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.sub && - echo " $head1..$head2 super -> origin/super" >>expect.err.sub && - head -3 expect.err >> expect.err.sub && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && - test_cmp expect.err.sub actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -271,14 +296,16 @@ test_expect_success "Recursion doesn't happen when new superproject commits don' git add file && git commit -m "new file" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.file && - echo " $head1..$head2 super -> origin/super" >> expect.err.file && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + rm expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err.file actual.err + verify_fetch_result actual.err ' test_expect_success "Recursion picks up config in submodule" ' @@ -295,9 +322,8 @@ test_expect_success "Recursion picks up config in submodule" ' git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.sub && - echo " $head1..$head2 super -> origin/super" >> expect.err.sub && - cat expect.err >> expect.err.sub && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && ( cd downstream && git fetch >../actual.out 2>../actual.err && @@ -306,7 +332,7 @@ test_expect_success "Recursion picks up config in submodule" ' git config --unset fetch.recurseSubmodules ) ) && - test_cmp expect.err.sub actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -331,15 +357,13 @@ test_expect_success "Recursion picks up all submodules when necessary" ' git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >> expect.err.2 && - cat expect.err.sub >> expect.err.2 && - tail -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && - test_cmp expect.err.2 actual.err && + verify_fetch_result actual.err && test_must_be_empty actual.out ' @@ -375,11 +399,8 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - tail -3 expect.err > expect.err.deepsub && - echo "From $pwd/." > expect.err && - echo " $head1..$head2 super -> origin/super" >>expect.err && - cat expect.err.sub >> expect.err && - cat expect.err.deepsub >> expect.err && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && ( cd downstream && git config fetch.recurseSubmodules false && @@ -395,7 +416,7 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess ) ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err + verify_fetch_result actual.err ' test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' @@ -405,14 +426,16 @@ test_expect_success "'--recurse-submodules=on-demand' stops when no new submodul git add file && git commit -m "new file" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.file && - echo " $head1..$head2 super -> origin/super" >> expect.err.file && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + rm expect.err.deep && ( cd downstream && git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err.file actual.err + verify_fetch_result actual.err ' test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config" ' @@ -426,9 +449,9 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git config fetch.recurseSubmodules on-demand && @@ -440,7 +463,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config git config --unset fetch.recurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err + verify_fetch_result actual.err ' test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' overrides fetch.recurseSubmodules" ' @@ -454,9 +477,9 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override git add submodule && git commit -m "new submodule" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >> expect.err.2 && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && git config submodule.submodule.fetchRecurseSubmodules on-demand && @@ -468,7 +491,7 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override git config --unset submodule.submodule.fetchRecurseSubmodules ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err + verify_fetch_result actual.err ' test_expect_success "don't fetch submodule when newly recorded commits are already present" ' @@ -480,14 +503,17 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea git add submodule && git commit -m "submodule rewound" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err && - echo " $head1..$head2 super -> origin/super" >> expect.err && + echo "From $pwd/." > expect.err.super && + echo " $head1..$head2 super -> origin/super" >> expect.err.super && + rm expect.err.sub && + # This file does not exist, but rm -f for readability + rm -f expect.err.deep && ( cd downstream && git fetch >../actual.out 2>../actual.err ) && test_must_be_empty actual.out && - test_cmp expect.err actual.err && + verify_fetch_result actual.err && ( cd submodule && git checkout -q sub @@ -505,9 +531,9 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git rm .gitmodules && git commit -m "new submodule without .gitmodules" && head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." >expect.err.2 && - echo " $head1..$head2 super -> origin/super" >>expect.err.2 && - head -3 expect.err >>expect.err.2 && + echo "From $pwd/." >expect.err.super && + echo " $head1..$head2 super -> origin/super" >>expect.err.super && + rm expect.err.deep && ( cd downstream && rm .gitmodules && @@ -523,7 +549,7 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git reset --hard ) && test_must_be_empty actual.out && - test_cmp expect.err.2 actual.err && + verify_fetch_result actual.err && git checkout HEAD^ -- .gitmodules && git add .gitmodules && git commit -m "new submodule restored .gitmodules" From patchwork Thu Feb 10 04:41:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741257 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 8570DC433FE for ; Thu, 10 Feb 2022 04:42:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233562AbiBJEmV (ORCPT ); Wed, 9 Feb 2022 23:42:21 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60866 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233530AbiBJEmO (ORCPT ); Wed, 9 Feb 2022 23:42:14 -0500 Received: from mail-pj1-x104a.google.com (mail-pj1-x104a.google.com [IPv6:2607:f8b0:4864:20::104a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 76EEE1C2 for ; Wed, 9 Feb 2022 20:42:07 -0800 (PST) Received: by mail-pj1-x104a.google.com with SMTP id y10-20020a17090a134a00b001b8b7e5983bso3073243pjf.6 for ; Wed, 09 Feb 2022 20:42: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=aYnNqElXZZv/TrOWzr/fEOBK7uQ0ce/iC+53j9ReqQw=; b=WcF3/w3dARwvnA7RepmVZ0VBb4wGiaMHA3VWLM5Ol2znACd8pKe67y3mMs+4OKz6yF 1ZjM3qx1VcMoLzYP3jDqrQV0X4CvxXsSGDXvA8ZBY7VYMTA2lLXETTfGNXqoLxLx865e FA9aMsXVVONgOAGntSJpjwa0xnJsiWWbkletHA4OXieKkkxGunqz8G1ARJ6RzPQCGlVk GPUHleXh3pvGTkV8C6EGvv/kAOvhhIem4iGD5prUSyoIYdnE+4kjBohF89GZOsPrgqdF UJS8cls+v4qoe3mMv42rLLD53edW7p/kavPWejjgKeIvNzPGLH+TgNHK6hq1cFaxYupT Pn8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=aYnNqElXZZv/TrOWzr/fEOBK7uQ0ce/iC+53j9ReqQw=; b=iFrlhGo0+iNZ86z8BkBHBeKHQl3PXuI5u/3GVCfwtN+SlykvGazpGbn0Tm6XurLA68 JXbj+RentuMb0/KbrvVnhTUbBHyyz2gQ2pLXw7T2ykM+LebnyRMyil8KZ8nb78GTFHH9 qUe+V0yA1OVKrI0Ojtb447wiGm1AYGezj6geCveCBYdWmplgkced5kXM2bh8emW5qgWY KaUh4YKpMrClxmzFPcXGbFT7PQTd3SGTZMWEyIWYYR1NaaHn19gDuekYx/OEaanMlbwr WqFfBO9+cs+iiKbuYjmblFsmbDVpqJxTEGhoNL9k+CX1hqLQeOm7oNwT/7M366h30xRa G9Ww== X-Gm-Message-State: AOAM530bD5hh0RTFO72IMFACoW3pPWZPpCeHtEFYNxrzccgsOXow4tm6 0c7X8reXzft0OnpvzdPyUh0sbD+2ifrOq9cUNwJDafDpuXSz6RpI9FoR/KGFsDp5hNkANlTb8bA c5R3WkJPO3exckbQGTfNmxid/5CLgBmtJK/VPYjtNeU9g/Hmsv3SieV/58HfTyPI= X-Google-Smtp-Source: ABdhPJzS2PjaEyWvLZ8CKyNnv+KPYY9xogg3zgeyC2StFKw1su4AvuWeAJ/TVfouBRfXJyu0HOzoGv2x3nD0Kg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:90a:ae03:: with SMTP id t3mr849412pjq.55.1644468126447; Wed, 09 Feb 2022 20:42:06 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:49 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-6-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 5/8] t5526: use grep to assert on fetches From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In a previous commit, we replaced test_cmp invocations with verify_fetch_result(). Finish the process of removing test_cmp by using grep in verify_fetch_result() instead. This makes the tests less sensitive to changes because, instead of checking the whole stderr, we only grep for the lines of the form * "..\s+branch\s+-> origin/branch" * "Fetching submodule " (if fetching a submodule) when we expect the repo to have fetched. If we expect the repo to not have fetched, grep to make sure the lines are absent. Also, simplify the assertions by using grep patterns that match only the relevant pieces of information, e.g. is irrelevant because we only want to know if the fetch was performed, so we don't need to know where the branch was before the fetch. Signed-off-by: Glen Choo --- t/t5526-fetch-submodules.sh | 131 +++++++++++++----------------------- 1 file changed, 48 insertions(+), 83 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 0e93df1665..cb18f0ac21 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -20,49 +20,52 @@ pwd=$(pwd) add_upstream_commit() { ( cd submodule && - head1=$(git rev-parse --short HEAD) && echo new >> subfile && test_tick && git add subfile && git commit -m new subfile && - head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule" > ../expect.err.sub && - echo "From $pwd/submodule" >> ../expect.err.sub && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub + git rev-parse --short HEAD >../subhead ) && ( cd deepsubmodule && - head1=$(git rev-parse --short HEAD) && echo new >> deepsubfile && test_tick && git add deepsubfile && git commit -m new deepsubfile && - head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule/subdir/deepsubmodule" > ../expect.err.deep - echo "From $pwd/deepsubmodule" >> ../expect.err.deep && - echo " $head1..$head2 deep -> origin/deep" >> ../expect.err.deep + git rev-parse --short HEAD >../deephead ) } # Verifies that the expected repositories were fetched. This is done by -# concatenating the files expect.err.[super|sub|deep] in the correct -# order and comparing it to the actual stderr. +# checking that the branches of [super|sub|deep] were updated to +# [super|sub|deep]head if the corresponding file exists. # -# If a repo should not be fetched in the test, its corresponding -# expect.err file should be rm-ed. +# If the [super|sub|deep] head file does not exist, this verifies that +# the corresponding repo was not fetched. Thus, if a repo should not be +# fetched in the test, its corresponding head file should be +# rm-ed. verify_fetch_result() { ACTUAL_ERR=$1 && - rm -f expect.err.combined && - if [ -f expect.err.super ]; then - cat expect.err.super >>expect.err.combined + # Each grep pattern is guaranteed to match the correct repo + # because each repo uses a different name for their branch i.e. + # "super", "sub" and "deep". + if [ -f superhead ]; then + grep -E "\.\.$(cat superhead)\s+super\s+-> origin/super" $ACTUAL_ERR + else + ! grep "super" $ACTUAL_ERR fi && - if [ -f expect.err.sub ]; then - cat expect.err.sub >>expect.err.combined + if [ -f subhead ]; then + grep "Fetching submodule submodule" $ACTUAL_ERR && + grep -E "\.\.$(cat subhead)\s+sub\s+-> origin/sub" $ACTUAL_ERR + else + ! grep "Fetching submodule submodule" $ACTUAL_ERR fi && - if [ -f expect.err.deep ]; then - cat expect.err.deep >>expect.err.combined - fi && - test_cmp expect.err.combined $ACTUAL_ERR + if [ -f deephead ]; then + grep "Fetching submodule submodule/subdir/deepsubmodule" $ACTUAL_ERR && + grep -E "\.\.$(cat deephead)\s+deep\s+-> origin/deep" $ACTUAL_ERR + else + ! grep "Fetching submodule submodule/subdir/deepsubmodule" $ACTUAL_ERR + fi } test_expect_success setup ' @@ -274,13 +277,10 @@ test_expect_success "Recursion doesn't happen when no new commits are fetched in ' test_expect_success "Recursion stops when no new submodule commits are fetched" ' - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -291,15 +291,12 @@ test_expect_success "Recursion stops when no new submodule commits are fetched" test_expect_success "Recursion doesn't happen when new superproject commits don't change any submodules" ' add_upstream_commit && - head1=$(git rev-parse --short HEAD) && echo a > file && git add file && git commit -m "new file" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && - rm expect.err.sub && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm subhead && + rm deephead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -318,12 +315,9 @@ test_expect_success "Recursion picks up config in submodule" ' ) ) && add_upstream_commit && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && + git rev-parse --short HEAD >superhead && ( cd downstream && git fetch >../actual.out 2>../actual.err && @@ -345,20 +339,13 @@ test_expect_success "Recursion picks up all submodules when necessary" ' git fetch && git checkout -q FETCH_HEAD ) && - head1=$(git rev-parse --short HEAD^) && git add subdir/deepsubmodule && git commit -m "new deepsubmodule" && - head2=$(git rev-parse --short HEAD) && - echo "Fetching submodule submodule" > ../expect.err.sub && - echo "From $pwd/submodule" >> ../expect.err.sub && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub + git rev-parse --short HEAD >../subhead ) && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && + git rev-parse --short HEAD >superhead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -376,13 +363,9 @@ test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no ne git fetch && git checkout -q FETCH_HEAD ) && - head1=$(git rev-parse --short HEAD^) && git add subdir/deepsubmodule && git commit -m "new deepsubmodule" && - head2=$(git rev-parse --short HEAD) && - echo Fetching submodule submodule > ../expect.err.sub && - echo "From $pwd/submodule" >> ../expect.err.sub && - echo " $head1..$head2 sub -> origin/sub" >> ../expect.err.sub + git rev-parse --short HEAD >../subhead ) && ( cd downstream && @@ -395,12 +378,9 @@ test_expect_success "'--recurse-submodules=on-demand' doesn't recurse when no ne ' test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necessary (and ignores config)" ' - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && + git rev-parse --short HEAD >superhead && ( cd downstream && git config fetch.recurseSubmodules false && @@ -421,15 +401,12 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' add_upstream_commit && - head1=$(git rev-parse --short HEAD) && echo a >> file && git add file && git commit -m "new file" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && - rm expect.err.sub && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm subhead && + rm deephead && ( cd downstream && git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err @@ -445,13 +422,10 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' overrides global config ) && add_upstream_commit && git config --global fetch.recurseSubmodules false && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && git config fetch.recurseSubmodules on-demand && @@ -473,13 +447,10 @@ test_expect_success "'submodule..fetchRecurseSubmodules=on-demand' override ) && add_upstream_commit && git config fetch.recurseSubmodules false && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "new submodule" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && git config submodule.submodule.fetchRecurseSubmodules on-demand && @@ -499,15 +470,12 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea cd submodule && git checkout -q HEAD^^ ) && - head1=$(git rev-parse --short HEAD) && git add submodule && git commit -m "submodule rewound" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." > expect.err.super && - echo " $head1..$head2 super -> origin/super" >> expect.err.super && - rm expect.err.sub && + git rev-parse --short HEAD >superhead && + rm subhead && # This file does not exist, but rm -f for readability - rm -f expect.err.deep && + rm -f deephead && ( cd downstream && git fetch >../actual.out 2>../actual.err @@ -526,14 +494,11 @@ test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .git git fetch --recurse-submodules ) && add_upstream_commit && - head1=$(git rev-parse --short HEAD) && git add submodule && git rm .gitmodules && git commit -m "new submodule without .gitmodules" && - head2=$(git rev-parse --short HEAD) && - echo "From $pwd/." >expect.err.super && - echo " $head1..$head2 super -> origin/super" >>expect.err.super && - rm expect.err.deep && + git rev-parse --short HEAD >superhead && + rm deephead && ( cd downstream && rm .gitmodules && From patchwork Thu Feb 10 04:41:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741258 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 9343DC433F5 for ; Thu, 10 Feb 2022 04:42:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233573AbiBJEmW (ORCPT ); Wed, 9 Feb 2022 23:42:22 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60870 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233544AbiBJEmO (ORCPT ); Wed, 9 Feb 2022 23:42:14 -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 3AFB91D3 for ; Wed, 9 Feb 2022 20:42:08 -0800 (PST) Received: by mail-pf1-x44a.google.com with SMTP id 2-20020aa79202000000b004cef2fc59f0so3433012pfo.12 for ; Wed, 09 Feb 2022 20:42:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=dOajazY+Gj0/m5pzttBLZDN1NUigsAJN3WdzJ3SzqTk=; b=sig+6pg8n/GdoTxJMdpMcoFQFKG6nIHSbwXy6WmVB+iJ9GSRnbHIny5Ju5fe8jxPXz qppHhSyf8/VMbfhE2TFHoCTzHOOQSoHgkJih44uvduSGvUXurxyfcXfbvl42Lb/gk0DW tnizT/oyhQpIUyNX8BYVOAba3sIs94yYBtosVLmg5h7rcyD8miBaXiOEQEHeyUTi5/CU 86pqFK4qScg1r3LO+utHcKbQuXIF1CHkucFqsyxlFF9IAkziTD1ebwVZM3uwrnygw8dO AsX0dtPwiVfQ5wzSaPG55TMSYE3ebRtPB9sr9mSrJGey9BwwRhrYZTQO2wg6i6nVSIJw 8qGQ== 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=dOajazY+Gj0/m5pzttBLZDN1NUigsAJN3WdzJ3SzqTk=; b=LWW5aostl5uoAChTx89A5QvdLUQz1T2Y9fOzzuQ1xvTb7zEpINR+Xg2DX6J8K6ig7d Zm4BxYBkDAYHIkYGI9HPGtLLZ4Y++KKuPl09IFaJqyKvm6AQxYdQs7DRVveZt2f0IkE8 KOKGDKLqtQ+IOP+LNGUAhSCDh9fLiVG59T+kLWbzqtnfNFeWTBt6K/C2JHVudzNhKvnw PhC93gmyr+78o3hkhij9KzhykBvuVEzGQVMoyMLSXxOY7YHOk+N/1JAAPdsmx/Ydns5S AMPv5IQOe2zU3G1a/xfRcdDOURlNxybF3tBMjG4QkjBBP+zBZ/CRJfPsgs0WPUkw28f/ sToA== X-Gm-Message-State: AOAM533Ib4HVxOLISdeCouDPjthnxBTr5UO3Tj/eOmKWD2FOrUZx4YkH FQ0tanVvmOpDYOvfMaIUv9DN3xnHhrW1+i2jtvSeraSF15FQ5yuP6E0vtBacKSh5T18k2tqFvRO ZSfRfW8L13PDgUxnCsq1UkghNu+Xb72TdEYPXZYbUOqprJrEc101q+wcVkkMNYbk= X-Google-Smtp-Source: ABdhPJwPA57IBXgjpLYQ/jEUUThrhmUs2pUuvZUYFR4EYGdZRJL/66mpt0P1YFOc9dUGtqCX8ywS7qngAwNzGg== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a65:681a:: with SMTP id l26mr4669876pgt.365.1644468128361; Wed, 09 Feb 2022 20:42:08 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:50 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-7-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 6/8] submodule: extract get_fetch_task() From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan 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 | 75 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/submodule.c b/submodule.c index d4227ac22d..d695dcadf4 100644 --- a/submodule.c +++ b/submodule.c @@ -1480,14 +1480,12 @@ static struct repository *get_submodule_repo_for(struct repository *r, return ret; } -static int get_next_submodule(struct child_process *cp, - struct strbuf *err, void *data, void **task_cb) +static struct fetch_task * +get_fetch_task(struct submodule_parallel_fetch *spf, + const char **default_argv, struct strbuf *err) { - struct submodule_parallel_fetch *spf = data; - for (; spf->count < spf->r->index->cache_nr; spf->count++) { const struct cache_entry *ce = spf->r->index->cache[spf->count]; - const char *default_argv; struct fetch_task *task; if (!S_ISGITLINK(ce->ce_mode)) @@ -1507,41 +1505,17 @@ static int get_next_submodule(struct child_process *cp, &spf->changed_submodule_names, task->sub->name)) continue; - default_argv = "on-demand"; + *default_argv = "on-demand"; break; case RECURSE_SUBMODULES_ON: - default_argv = "yes"; + *default_argv = "yes"; break; case RECURSE_SUBMODULES_OFF: continue; } 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; - } else { + if (!task->repo) { struct strbuf empty_submodule_path = STRBUF_INIT; fetch_task_release(task); @@ -1562,7 +1536,44 @@ static int get_next_submodule(struct child_process *cp, ce->name); } strbuf_release(&empty_submodule_path); + continue; } + if (!spf->quiet) + strbuf_addf(err, _("Fetching submodule %s%s\n"), + spf->prefix, ce->name); + + spf->count++; + return task; + } + return NULL; +} + +static int get_next_submodule(struct child_process *cp, struct strbuf *err, + void *data, void **task_cb) +{ + struct submodule_parallel_fetch *spf = data; + const char *default_argv = NULL; + struct fetch_task *task = get_fetch_task(spf, &default_argv, err); + + if (task) { + struct strbuf submodule_prefix = STRBUF_INIT; + + child_process_init(cp); + cp->dir = task->repo->gitdir; + prepare_submodule_repo_env_in_gitdir(&cp->env_array); + cp->git_cmd = 1; + strvec_init(&cp->args); + strvec_pushv(&cp->args, spf->args.v); + strvec_push(&cp->args, default_argv); + strvec_push(&cp->args, "--submodule-prefix"); + + strbuf_addf(&submodule_prefix, "%s%s/", spf->prefix, + task->sub->path); + strvec_push(&cp->args, submodule_prefix.buf); + *task_cb = task; + + strbuf_release(&submodule_prefix); + return 1; } if (spf->oid_fetch_tasks_nr) { From patchwork Thu Feb 10 04:41:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741259 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 F0A46C433FE for ; Thu, 10 Feb 2022 04:42:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233473AbiBJEmX (ORCPT ); Wed, 9 Feb 2022 23:42:23 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60858 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233570AbiBJEmO (ORCPT ); Wed, 9 Feb 2022 23:42:14 -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 36CBB1D9 for ; Wed, 9 Feb 2022 20:42:10 -0800 (PST) Received: by mail-pf1-x44a.google.com with SMTP id s207-20020a6277d8000000b004e0300e14f7so3402575pfc.23 for ; Wed, 09 Feb 2022 20:42:10 -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=e28xifxc0TecP0d2abvh2V44aTg63b/t34PT8APBmdc=; b=RYnIt+jfE2OoWXJsAJ8aiBYa5sy91wiPHW2FuOS1fbSDQuXEWY19YcclM5OdlrAKYh mfvvesvHGzR4YLAaZ7gtr40KYgNISejSYwoNX5b3LAkBJexupqRQ02KRRScMrlqT4TM/ BEB8t+cvslGg/JaVHTFOnp4rSVdNSoQdl2ezGBWVJ8pf0Gf9zpgMXPbcc/HKRUcxB/X0 DM2eQXMbKESlupoGk5+J9VO/FOjzrOJ89oZP/ufi5DbAOuvk2gl07wWvVDqUPzFYbXv7 55baNNLbkNkfY0+DesjoQcIsczKdzkqca0TaKJOChR4MZe1y4Y1IjwVNdGGno3CUJWhj eeNA== 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=e28xifxc0TecP0d2abvh2V44aTg63b/t34PT8APBmdc=; b=yMecIjEaehnTmApCd/2Gbtj9lYN3yUlgL25G6UBb5W44dO3nBjnYiPjwhRUHiSdbx3 WE7kmk5VdfbYqMd6pANnQ8bNMLZfchU7ckyoQqsDRZdbFrNy71pzkJkDKL4ffGlTFia9 dD/IeoU8y+HquqSVhboBXVeM1QDjeiWzx6sjHFRT/UMtEae8AhEvyAEaA48Z0Rt54YYG xRCNthrjVKH2oTTfJo1y5P2YJ2F+8mCPp/ThqW0G5U48gVdF1p2RMObUZ5UpwpstplEV mc32JfzfDexE7vBFKJqUxUjpLrEWcWtqDUCcYnzBPAferKSwS6so14Uu2gIiJpgJgpyq GLTA== X-Gm-Message-State: AOAM531fliRAV8x2dd/+rAzBrtYFFB7KJgxrZAmmA3WSMsfdtcWPEIUW FNsHrXjEQWyPZAP/AozGDZiRP9x++KMhKFkXajHHSTY0m7OtE5E2ri3nxrWpjeiAE0irya7qpSY VYCy8/xWK/+BC1f7PYEIne9HqNyPp4nfMrZSMmZnwf9zEku93jGdWUBWZLAZpE7Y= X-Google-Smtp-Source: ABdhPJzlbGq7Hs2aachtSUMOhNt4N+UKrWi6aeUXZrp+g7gSZUmyTIMlX1wKavVePrXF2kqewLuzGd1x7/XaHw== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a17:902:d34b:: with SMTP id l11mr5646685plk.137.1644468130362; Wed, 09 Feb 2022 20:42:10 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:51 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-8-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 7/8] fetch: fetch unpopulated, changed submodules From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org "git fetch --recurse-submodules" only considers populated submodules (i.e. submodules that can be found by iterating the index), which makes "git fetch" behave differently based on which commit is checked out. As a result, even if the user has initialized all submodules correctly, they may not fetch the necessary submodule commits, and commands like "git checkout --recurse-submodules" might fail. Teach "git fetch" to fetch cloned, changed submodules regardless of whether they are populated (this is in addition to the current behavior of fetching populated submodules). Since a submodule may be encountered multiple times (via the list of populated submodules or via the list of changed submodules), maintain a list of seen submodules to avoid fetching a submodule more than once. Signed-off-by: Glen Choo --- submodule.c has a seemingly-unrelated change that teaches the "find changed submodules" rev walk to call is_repository_shallow(). This fixes what I believe is a legitimate bug - the rev walk would fail on a shallow repo. Our test suite did not catch this prior to this commit because we skip the rev walk if .gitmodules is not found, and thus the test suite did not attempt the rev walk on a shallow clone. After this commit, we always attempt to find changed submodules (regardless of whether there is a .gitmodules file), and the test suite noticed the bug. Documentation/fetch-options.txt | 26 ++-- Documentation/git-fetch.txt | 10 +- submodule.c | 101 +++++++++++++-- t/t5526-fetch-submodules.sh | 217 ++++++++++++++++++++++++++++++++ 4 files changed, 328 insertions(+), 26 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/submodule.c b/submodule.c index d695dcadf4..0c02bbc9c3 100644 --- a/submodule.c +++ b/submodule.c @@ -22,6 +22,7 @@ #include "parse-options.h" #include "object-store.h" #include "commit-reach.h" +#include "shallow.h" static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; static int initialized_fetch_ref_tips; @@ -907,6 +908,9 @@ static void collect_changed_submodules(struct repository *r, save_warning = warn_on_object_refname_ambiguity; warn_on_object_refname_ambiguity = 0; + /* make sure shallows are read */ + is_repository_shallow(the_repository); + repo_init_revisions(r, &rev, NULL); setup_revisions(argv->nr, argv->v, &rev, &s_r_opt); warn_on_object_refname_ambiguity = save_warning; @@ -1273,10 +1277,6 @@ static void calculate_changed_submodule_paths(struct repository *r, 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)) - return; - strvec_push(&argv, "--"); /* argv[0] program name */ oid_array_for_each_unique(&ref_tips_after_fetch, append_oid_to_argv, &argv); @@ -1347,7 +1347,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; @@ -1357,6 +1358,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; @@ -1367,6 +1369,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, \ } @@ -1481,11 +1484,12 @@ static struct repository *get_submodule_repo_for(struct repository *r, } static struct fetch_task * -get_fetch_task(struct submodule_parallel_fetch *spf, - const char **default_argv, struct strbuf *err) +get_fetch_task_from_index(struct submodule_parallel_fetch *spf, + const char **default_argv, struct strbuf *err) { - for (; spf->count < spf->r->index->cache_nr; spf->count++) { - const struct cache_entry *ce = spf->r->index->cache[spf->count]; + for (; spf->index_count < spf->r->index->cache_nr; spf->index_count++) { + const struct cache_entry *ce = + spf->r->index->cache[spf->index_count]; struct fetch_task *task; if (!S_ISGITLINK(ce->ce_mode)) @@ -1495,6 +1499,15 @@ get_fetch_task(struct submodule_parallel_fetch *spf, if (!task) continue; + /* + * We might have already considered this submodule + * because we saw it when iterating the changed + * submodule names. + */ + if (string_list_lookup(&spf->seen_submodule_names, + task->sub->name)) + continue; + switch (get_fetch_recurse_config(task->sub, spf)) { default: @@ -1542,7 +1555,69 @@ get_fetch_task(struct submodule_parallel_fetch *spf, strbuf_addf(err, _("Fetching submodule %s%s\n"), spf->prefix, ce->name); - spf->count++; + spf->index_count++; + return task; + } + return NULL; +} + +static struct fetch_task * +get_fetch_task_from_changed(struct submodule_parallel_fetch *spf, + const char **default_argv, struct strbuf *err) +{ + for (; spf->changed_count < spf->changed_submodule_names.nr; + spf->changed_count++) { + struct string_list_item item = + spf->changed_submodule_names.items[spf->changed_count]; + struct changed_submodule_data *cs_data = item.util; + struct fetch_task *task; + + /* + * We might have already considered this submodule + * because we saw it in the index. + */ + if (string_list_lookup(&spf->seen_submodule_names, item.string)) + continue; + + task = fetch_task_create(spf->r, cs_data->path, + cs_data->super_oid); + if (!task) + continue; + + switch (get_fetch_recurse_config(task->sub, spf)) { + default: + case RECURSE_SUBMODULES_DEFAULT: + case RECURSE_SUBMODULES_ON_DEMAND: + *default_argv = "on-demand"; + break; + case RECURSE_SUBMODULES_ON: + *default_argv = "yes"; + break; + case RECURSE_SUBMODULES_OFF: + continue; + } + + task->repo = get_submodule_repo_for(spf->r, task->sub->path, + cs_data->super_oid); + if (!task->repo) { + fetch_task_release(task); + free(task); + + strbuf_addf(err, _("Could not access submodule '%s'\n"), + cs_data->path); + continue; + } + if (!is_tree_submodule_active(spf->r, cs_data->super_oid, + task->sub->path)) + continue; + + if (!spf->quiet) + strbuf_addf(err, + _("Fetching submodule %s%s at commit %s\n"), + spf->prefix, task->sub->path, + find_unique_abbrev(cs_data->super_oid, + DEFAULT_ABBREV)); + spf->changed_count++; return task; } return NULL; @@ -1553,7 +1628,10 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err, { struct submodule_parallel_fetch *spf = data; const char *default_argv = NULL; - struct fetch_task *task = get_fetch_task(spf, &default_argv, err); + struct fetch_task *task = + get_fetch_task_from_index(spf, &default_argv, err); + if (!task) + task = get_fetch_task_from_changed(spf, &default_argv, err); if (task) { struct strbuf submodule_prefix = STRBUF_INIT; @@ -1573,6 +1651,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; } diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index cb18f0ac21..f37dca4e09 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -399,6 +399,223 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess verify_fetch_result actual.err ' +# Cleans up after tests that checkout branches other than the main ones +# in the tests. +checkout_main_branches() { + git -C downstream checkout --recurse-submodules super && + git -C downstream/submodule checkout --recurse-submodules sub && + git -C downstream/submodule/subdir/deepsubmodule checkout --recurse-submodules deep +} + +# Test that we can fetch submodules in other branches by running fetch +# in a branch that has no submodules. +test_expect_success 'setup downstream branch without submodules' ' + ( + cd downstream && + git checkout --recurse-submodules -b no-submodules && + rm .gitmodules && + git rm submodule && + git add .gitmodules && + git commit -m "no submodules" && + git checkout --recurse-submodules super + ) +' + +test_expect_success "'--recurse-submodules=on-demand' should fetch submodule commits if the submodule is changed but the index has no submodules" ' + test_when_finished "checkout_main_branches" && + git -C downstream fetch --recurse-submodules && + # Create new superproject commit with updated submodules + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err && + git checkout --recurse-submodules origin/super 2>../actual-checkout.err + ) && + test_must_be_empty actual.out && + git rev-parse --short HEAD >superhead && + git -C submodule rev-parse --short HEAD >subhead && + git -C deepsubmodule rev-parse --short HEAD >deephead && + verify_fetch_result actual.err && + + # Assert that the fetch happened at the non-HEAD commits + grep "Fetching submodule submodule at commit $superhead" actual.err && + grep "Fetching submodule submodule/subdir/deepsubmodule at commit $subhead" actual.err && + + # Assert that we can checkout the superproject commit with --recurse-submodules + ! grep -E "error: Submodule .+ could not be updated" actual-checkout.err +' + +test_expect_success "'--recurse-submodules' should fetch submodule commits if the submodule is changed but the index has no submodules" ' + test_when_finished "checkout_main_branches" && + # Fetch any leftover commits from other tests. + git -C downstream fetch --recurse-submodules && + # Create new superproject commit with updated submodules + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git fetch --recurse-submodules >../actual.out 2>../actual.err && + git checkout --recurse-submodules origin/super 2>../actual-checkout.err + ) && + test_must_be_empty actual.out && + git rev-parse --short HEAD >superhead && + git -C submodule rev-parse --short HEAD >subhead && + git -C deepsubmodule rev-parse --short HEAD >deephead && + verify_fetch_result actual.err && + + # Assert that the fetch happened at the non-HEAD commits + grep "Fetching submodule submodule at commit $superhead" actual.err && + grep "Fetching submodule submodule/subdir/deepsubmodule at commit $subhead" actual.err && + + # Assert that we can checkout the superproject commit with --recurse-submodules + ! grep -E "error: Submodule .+ could not be updated" actual-checkout.err +' + +test_expect_success "'--recurse-submodules' should ignore changed, inactive submodules" ' + test_when_finished "checkout_main_branches" && + # Fetch any leftover commits from other tests. + git -C downstream fetch --recurse-submodules && + # Create new superproject commit with updated submodules + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Fetch the new superproject commit + ( + cd downstream && + git switch --recurse-submodules no-submodules && + git -c submodule.submodule.active=false fetch --recurse-submodules >../actual.out 2>../actual.err + ) && + test_must_be_empty actual.out && + git rev-parse --short HEAD >superhead && + # Neither should be fetched because the submodule is inactive + rm subhead && + rm deephead && + verify_fetch_result actual.err +' + +# Test that we properly fetch the submodules in the index as well as +# submodules in other branches. +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" ' + test_when_finished "checkout_main_branches" && + # Fetch any leftover commits from other tests. + git -C downstream fetch --recurse-submodules && + # Create new commit in origin/super + add_upstream_commit && + ( + cd submodule && + ( + cd subdir/deepsubmodule && + git fetch && + git checkout -q FETCH_HEAD + ) && + git add subdir/deepsubmodule && + git commit -m "new deep submodule" + ) && + git add submodule && + git commit -m "new submodule" && + + # Create new commit in origin/super-sub2-only + git checkout super-sub2-only && + ( + cd submodule2 && + test_commit --no-tag foo + ) && + git add submodule2 && + git commit -m "new submodule2" && + + git checkout super && + ( + cd downstream && + git fetch --recurse-submodules >../actual.out 2>../actual.err && + git checkout --recurse-submodules origin/super-sub2-only 2>../actual-checkout.err + ) && + test_must_be_empty actual.out && + + # Assert that the submodules in the super branch are fetched + git rev-parse --short HEAD >superhead && + git -C submodule rev-parse --short HEAD >subhead && + git -C deepsubmodule rev-parse --short HEAD >deephead && + verify_fetch_result actual.err && + # Assert that submodule is read from the index, not from a commit + ! grep "Fetching submodule submodule at commit" actual.err && + + # Assert that super-sub2-only and submodule2 were fetched even + # though another branch is checked out + super_sub2_only_head=$(git rev-parse --short super-sub2-only) && + grep -E "\.\.${super_sub2_only_head}\s+super-sub2-only\s+-> origin/super-sub2-only" actual.err && + grep "Fetching submodule submodule2 at commit $super_sub2_only_head" actual.err && + sub2head=$(git -C submodule2 rev-parse --short HEAD) && + grep -E "\.\.${sub2head}\s+sub2\s+-> origin/sub2" actual.err && + + # Assert that we can checkout the superproject commit with --recurse-submodules + ! grep -E "error: Submodule .+ could not be updated" actual-checkout.err +' + test_expect_success "'--recurse-submodules=on-demand' stops when no new submodule commits are found in the superproject (and ignores config)" ' add_upstream_commit && echo a >> file && From patchwork Thu Feb 10 04:41:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 12741260 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 3A5C6C433F5 for ; Thu, 10 Feb 2022 04:42:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233635AbiBJEmY (ORCPT ); Wed, 9 Feb 2022 23:42:24 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60886 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233584AbiBJEmO (ORCPT ); Wed, 9 Feb 2022 23:42:14 -0500 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0AA7D1E3 for ; Wed, 9 Feb 2022 20:42:13 -0800 (PST) Received: by mail-pl1-x64a.google.com with SMTP id t14-20020a1709027fce00b0014d5ddc9dfbso663783plb.22 for ; Wed, 09 Feb 2022 20:42: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=X2My/v548O0WSWonCNPHRXOJOwdhMaIOacjPpvhpI+k=; b=Rw8mJgofMHUqMGyFoXLjvRiil6OTTIxKgtdEUK6Oam/fuckkW8zuwrvUxHkhumpHmb h6qeninzZo0An8N8frgJ/EK4vTVLQ2UsGWGnyOOPERtAsXw+Zb/B+cNsYHRw7SQqCQlt vc0hs21XI64rMx+gcvdQkE/FTfkKm6PFdi4v5Hhd9EUD/Q+lFGQRGlJBnkwPUW1OHvda kW69EPSb/C6/BAdX9+5xjwpk81FG7mhqTPWGIFx6/N5z+tPvwaM07GwQ/+ioqCzeoQjK n258KlfcJ/NfL44QaT5iWje6iOtuKIn6C/gxBEincUT5unpU7vXoQ/zqVJQrL98IxckK QqKw== 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=X2My/v548O0WSWonCNPHRXOJOwdhMaIOacjPpvhpI+k=; b=aNnpIwztFrb5An4iGSSqTX9yaWB9mW/YBN3BN+6eAN2kU+iVpAcGTPKF2adEUDn1b6 oj7CGQp0T5Armkwj4+I5NYX8TFvXTPbfC3bwc4h9Oo8ctivhogSzU3uTl8nGtrJpCbIo kCvbUqGcQB2YFdE1vThTnUAef6Qduhr4qShtiEa2kpNQMxEzIBpd33AMa+ZKoj+ryXba ckcPnpxN99kUEPFh70lihiUZW8LBTAqmIhbUAm5j4V7SHb9lEPfeFoWPwlNa/8sGhcZF T5aC5Ecv6p/C2t5AUgQ7lXQf7kI6pOcZb6Vq4fWTCq7HErK6Kasj/ZjqRxBHU7PIh4qj COFw== X-Gm-Message-State: AOAM5305KOoSyFtLe42pypjWilTh0/WFksF+5uVaRPlzePkG7dfKE12X CgMYV9Vn5Ew01R9wHKjnWm05V+T4VLzbgZKkuYKI7QYHsAbb6sDjYCxFz4YF9o5g2ntkraaWY5r 3r7Dn585PnKddKv48VhP5h8bPgNHzpPULgM/HMLVGEtZSJHEr3tOketGztfR5qDs= X-Google-Smtp-Source: ABdhPJzxFQiQSzVOP33I+HzCRz7bzSyxbLDdQeIhFtL8FTQlYuyAzjtOzrXHIGhgmOSy+46BX/D8clVm32L+qQ== X-Received: from chooglen.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:26d9]) (user=chooglen job=sendgmr) by 2002:a05:6a00:a8e:: with SMTP id b14mr5799567pfl.32.1644468132414; Wed, 09 Feb 2022 20:42:12 -0800 (PST) Date: Thu, 10 Feb 2022 12:41:52 +0800 In-Reply-To: <20220210044152.78352-1-chooglen@google.com> Message-Id: <20220210044152.78352-9-chooglen@google.com> Mime-Version: 1.0 References: <20220210044152.78352-1-chooglen@google.com> X-Mailer: git-send-email 2.35.0.263.gb82422642f-goog Subject: [PATCH 8/8] submodule: fix bug and remove add_submodule_odb() From: Glen Choo To: git@vger.kernel.org Cc: Glen Choo , Jonathan Tan Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org add_submodule_odb() is a hack - it adds a submodule's odb as an alternate, allowing the submodule's objects to be read via the_repository. Its last caller is submodule_has_commits(), which calls add_submodule_odb() to prepare for check_has_commit(). This used to be necessary because check_has_commit() used the_repository's odb, but this is longer true as of 13a2f620b2 (submodule: pass repo to check_has_commit(), 2021-10-08). Removing add_submodule_odb() reveals a bug in check_has_commit(), where check_has_commit() will segfault if the submodule is missing (e.g. the user has not init-ed the submodule). This happens because the submodule's struct repository cannot be initialized, but check_has_commit() tries to cleanup the uninitialized struct anyway. This was masked by add_submodule_odb(), because add_submodule_odb() fails when the submodule is missing, causing the caller to return early and avoid calling check_has_commit(). Fix the bug and remove the call to add_submodule_odb(). Since add_submodule_odb() has no more callers, remove it too. Note that submodule odbs can still by added as alternates via add_submodule_odb_by_path(). Signed-off-by: Glen Choo --- This bug only exists because we can't call repo_clear() twice on the same struct repository. So instead of just fixing this site, an alternative (and maybe better) fix would be to fix repo_clear(). If others think that's a good idea, I'll do that instead. submodule.c | 35 ++--------------------------------- submodule.h | 9 ++++----- 2 files changed, 6 insertions(+), 38 deletions(-) diff --git a/submodule.c b/submodule.c index 0c02bbc9c3..fdfddd3aac 100644 --- a/submodule.c +++ b/submodule.c @@ -168,26 +168,6 @@ void stage_updated_gitmodules(struct index_state *istate) static struct string_list added_submodule_odb_paths = STRING_LIST_INIT_NODUP; -/* TODO: remove this function, use repo_submodule_init instead. */ -int add_submodule_odb(const char *path) -{ - struct strbuf objects_directory = STRBUF_INIT; - int ret = 0; - - ret = strbuf_git_path_submodule(&objects_directory, path, "objects/"); - if (ret) - goto done; - if (!is_directory(objects_directory.buf)) { - ret = -1; - goto done; - } - string_list_insert(&added_submodule_odb_paths, - strbuf_detach(&objects_directory, NULL)); -done: - strbuf_release(&objects_directory); - return ret; -} - void add_submodule_odb_by_path(const char *path) { string_list_insert(&added_submodule_odb_paths, xstrdup(path)); @@ -972,7 +952,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); @@ -1003,18 +984,6 @@ static int submodule_has_commits(struct repository *r, { struct has_commit_data has_commit = { r, 1, path, super_oid }; - /* - * Perform a cheap, but incorrect check for the existence of 'commits'. - * This is done by adding the submodule's object store to the in-core - * object store, and then querying for each commit's existence. If we - * do not have the commit object anywhere, there is no chance we have - * it in the object store of the correct submodule and have it - * reachable from a ref, so we can fail early without spawning rev-list - * which is expensive. - */ - if (add_submodule_odb(path)) - return 0; - oid_array_for_each_unique(commits, check_has_commit, &has_commit); if (has_commit.result) { diff --git a/submodule.h b/submodule.h index 784ceffc0e..ca1f12b78b 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);