From patchwork Thu Sep 9 18:47:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 12483647 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0FDEEC433FE for ; Thu, 9 Sep 2021 18:47:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E8F7061100 for ; Thu, 9 Sep 2021 18:47:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244321AbhIISsz (ORCPT ); Thu, 9 Sep 2021 14:48:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48262 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242122AbhIISsp (ORCPT ); Thu, 9 Sep 2021 14:48:45 -0400 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 8A4EEC061574 for ; Thu, 9 Sep 2021 11:47:35 -0700 (PDT) Received: by mail-pg1-x54a.google.com with SMTP id 1-20020a630e41000000b002528846c9f2so1733479pgo.12 for ; Thu, 09 Sep 2021 11:47:35 -0700 (PDT) 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=gQ4CAY6JSfqC4SP7Uc1+cPdBqMt/HnlNWH5cF3NjiuU=; b=j4xLrZF2/DmumC36DRbiNMJRwsCp7GKSUFfApn2RKq7Ammm34oMJYKScsGK5dOZv8m Wi72EfKvoXfzo3grsNssipD4zxGsrIkUtz2xhw7OhV9PevfIAekBhDHkcpuGaK8ZChCT Iw1E+KB1B8W1Q1GNT72kV+poT7uUG59Pa1xgfwWqWDgJFtFM0PskHVXHaXuROEMlrYJ+ YdpwWCaTuUYHgE77MUYcKI5sRySnHnC7tK8RaVbxWtKtmR8LCgh/9L8CtPPsqG5r7fjF 7geocbmIzM4JYD04FIozJkmyaL92SEDg8syCyrovNO8ZHa5/HLcMTA0IW1xFZnR4gAQm 45rQ== 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=gQ4CAY6JSfqC4SP7Uc1+cPdBqMt/HnlNWH5cF3NjiuU=; b=z++BI12c12Y5QWi1sVPT7lQBzS8ObD2jtAnTP7LWDs3z00dHDUSBLOcUyDMJymllC1 DJXs4zUxjpRGS+gzA6kjcUhWCGBRJhdsCXK8wtHUlAARBQjdooBaCHoBxXwYZQC9Hbtg s7nBiqH/zV292DXQO5gWW0E9RrFiPhPY/2IKFt3fl/ftcHg43YEU8/o5MJJUfKUshUrn 1M8U+VhB/xKq1oWlOi+xeDikUf1uBxd8KkE6+RVlVe2H+9YBzkc04ukb9RrJgXQeuR3n y4sPZbd2C6VJl318oht1vqXWFA/6CvR8DhPAQTB8pBrE9ZI9XRfwmt8JggWQLUPoQHZa 4MzA== X-Gm-Message-State: AOAM530nc2O3DZymFXvf+cH5nEQH/CeJnQM2ukXrx8x3fE83w2edWuBT 6bNzNFr2jzSq6hNgcwBN2TJ7rXC4WKF5yqVAVrfDZTdPBnH/NbMgc/DmCQx8uFsuQ/aE33jgY8R OJccanchstPYS+RYI9WBo85Ovlr+euMzEPQPEYNBnhfh/WRj1QsHmA3iFmVJGGC0qe7kpiJoXcB Dr X-Google-Smtp-Source: ABdhPJzKlnw82yDkq5J0w5L7VLhY6/8CIjtIXdqaDb7feHvj8QUmrTbI+he1VXJACOqjVnNz+XNbrJbFN7WnQTvI08Ho X-Received: from twelve4.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:437a]) (user=jonathantanmy job=sendgmr) by 2002:a17:902:e749:b0:13a:148d:2d32 with SMTP id p9-20020a170902e74900b0013a148d2d32mr3901206plf.51.1631213254835; Thu, 09 Sep 2021 11:47:34 -0700 (PDT) Date: Thu, 9 Sep 2021 11:47:27 -0700 In-Reply-To: Message-Id: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.33.0.309.g3052b89438-goog Subject: [PATCH v2 1/3] submodule: remove unnecessary unabsorbed fallback From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , gitster@pobox.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In get_submodule_repo_for(), there is a fallback code path for the case in which a submodule has an unabsorbed gitdir. (See the documentation for "git submodule absorbgitdirs" for more information about absorbed and unabsorbed gitdirs.) However, this code path is unnecessary, because such submodules are already handled: when the fetch_task is created in fetch_task_create(), it will create its own struct submodule with a path and name, and repo_submodule_init() can handle such a struct. This fallback was introduced in 26f80ccfc1 ("submodule: migrate get_next_submodule to use repository structs", 2018-12-05). It was unnecessary even then, but perhaps it escaped notice because its parent commit d5498e0871 ("repository: repo_submodule_init to take a submodule struct", 2018-12-05) was the one that taught repo_submodule_init() to handle such created structs. Before, it took a path and always checked .gitmodules, so it truly would have failed if there were no entry in .gitmodules. (Note to reviewers: in 26f80ccfc1, the "own struct submodule" I mentioned is in get_next_submodule(), not fetch_task_create().) Signed-off-by: Jonathan Tan --- submodule.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/submodule.c b/submodule.c index 8de1aecaeb..3af3da5b5e 100644 --- a/submodule.c +++ b/submodule.c @@ -1409,19 +1409,8 @@ static struct repository *get_submodule_repo_for(struct repository *r, struct repository *ret = xmalloc(sizeof(*ret)); if (repo_submodule_init(ret, r, 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. - */ - struct strbuf gitdir = STRBUF_INIT; - strbuf_repo_worktree_path(&gitdir, r, "%s/.git", sub->path); - if (repo_init(ret, gitdir.buf, NULL)) { - strbuf_release(&gitdir); - free(ret); - return NULL; - } - strbuf_release(&gitdir); + free(ret); + return NULL; } return ret; From patchwork Thu Sep 9 18:47:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 12483649 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C554C433EF for ; Thu, 9 Sep 2021 18:47:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 53FE961100 for ; Thu, 9 Sep 2021 18:47:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242122AbhIISs5 (ORCPT ); Thu, 9 Sep 2021 14:48:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48254 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241321AbhIISsq (ORCPT ); Thu, 9 Sep 2021 14:48:46 -0400 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 416FEC061756 for ; Thu, 9 Sep 2021 11:47:37 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id 202-20020a6219d3000000b0040b60510fd8so1842833pfz.5 for ; Thu, 09 Sep 2021 11:47:37 -0700 (PDT) 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=Fzlc6amka7eNrMuGFqF2NLCb1375EEG8spEKcE1vdcc=; b=m3bQl6gJULIcnXWPrbm3P2okCku5J5UbrjGfj5sOJ+Tg/flGYA5jrUAGGSRrKWF2Nj MhUgeTzFAHD/UNxKfkpDPjHf5thf+XgubAd9O41EnNH/k7Qq1VrCDQqbX67++tLsgwqJ THf5DXy0gCaQegCgx5yXEFw8XsjUTmwbKN7An0O1GsKF8rTz85IDi8G812KWyM2ufLyG UzWW+31jSlmiXJskNkWJn8maoTfbpZWCZZzVx8NbNRS4HkXxp/M6e47AiiqIqWTF11zH vKlCgGSjqUQlMhKbZexCbAR7JCAKD46Immi2Xw1GFRSbLKVSfon6mzEVEMuZs3Q1lhWW 0gGw== 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=Fzlc6amka7eNrMuGFqF2NLCb1375EEG8spEKcE1vdcc=; b=MG6Wm6TGTTsHUdllyzXI6LYKa6nTD/0+37BLKq03xlc2ZezdrARkRTXusnFU96jThM J+90tPZr+DoAM3sIxJIHOmzdVlTTB6Zb0l1b2idhkDKeKDz3oIO1rg1zLkF35JprYmjW gxVlkafOvDyBcl0SfZWYqb9NR8oQjnGybxnSmBNbWkoJTIGQXUk/X7jbcGPJze3KZ7Q9 nF1Zok8d+/jsXFFYsyg6m7zSZEKMg4VCDf7T6w6g8ZRkQVSwPebLr9B/vEUptWuj+2R/ Goau7SO5jhEZp/dld4Yh7WTGFWbppgfY9fv/VyG68LGFqLvOxCjzd4Lu19A+zGAee81M 3cBg== X-Gm-Message-State: AOAM530PdwA88Cy11MiWo+/ugR+e8heiBqhN/+1wyVmMYdXHWEvJ+I/F zR7lBXqyZX7t4KbbrXJNzJMvIG+v6YXaU7X0BU944a6M/+evgUIzxfWOwf9WSiDUFBk06lXXXIH 3u52s9ZffqYLL9fBZSaIxTevV+FamaOqDO2quADRJq/JYJ2Lkb90S2D8gUVDp1COFQGch7PAZ1l aY X-Google-Smtp-Source: ABdhPJyLTd21rDd+ntUY0OslorkIJKb66Wj1X4qjjCLxb2P1q4o7487+uH1ZJyjlWNtct4xrgTr972gSZMgJq9AROIhq X-Received: from twelve4.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:437a]) (user=jonathantanmy job=sendgmr) by 2002:a17:902:e282:b0:13a:45b7:d2cd with SMTP id o2-20020a170902e28200b0013a45b7d2cdmr3978828plc.86.1631213256725; Thu, 09 Sep 2021 11:47:36 -0700 (PDT) Date: Thu, 9 Sep 2021 11:47:28 -0700 In-Reply-To: Message-Id: <39aca057cc88d1255d5b166cbf0630f8e9a3a5b0.1631212893.git.jonathantanmy@google.com> Mime-Version: 1.0 References: X-Mailer: git-send-email 2.33.0.309.g3052b89438-goog Subject: [PATCH v2 2/3] repository: support unabsorbed in repo_submodule_init From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , gitster@pobox.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In preparation for a subsequent commit that migrates code using add_submodule_odb() to repo_submodule_init(), teach repo_submodule_init() to support submodules with unabsorbed gitdirs. (See the documentation for "git submodule absorbgitdirs" for more information about absorbed and unabsorbed gitdirs.) Signed-off-by: Jonathan Tan --- builtin/grep.c | 5 +---- builtin/ls-files.c | 4 +--- builtin/submodule--helper.c | 7 +------ repository.c | 21 +++++++++++--------- repository.h | 15 ++++++++------ submodule.c | 9 +++------ t/helper/test-submodule-nested-repo-config.c | 4 +--- 7 files changed, 28 insertions(+), 37 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 51278b01fa..8af5249a7b 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -433,17 +433,14 @@ static int grep_submodule(struct grep_opt *opt, { struct repository *subrepo; struct repository *superproject = opt->repo; - const struct submodule *sub; struct grep_opt subopt; int hit = 0; - sub = submodule_from_path(superproject, null_oid(), path); - if (!is_submodule_active(superproject, path)) return 0; subrepo = xmalloc(sizeof(*subrepo)); - if (repo_submodule_init(subrepo, superproject, sub)) { + if (repo_submodule_init(subrepo, superproject, path, null_oid())) { free(subrepo); return 0; } diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 29a26ad8ae..ec19bf54b2 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -209,10 +209,8 @@ static void show_submodule(struct repository *superproject, struct dir_struct *dir, const char *path) { struct repository subrepo; - const struct submodule *sub = submodule_from_path(superproject, - null_oid(), path); - if (repo_submodule_init(&subrepo, superproject, sub)) + if (repo_submodule_init(&subrepo, superproject, path, null_oid())) return; if (repo_read_index(&subrepo) < 0) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index ef2776a9e4..6718f202db 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2540,7 +2540,6 @@ static int push_check(int argc, const char **argv, const char *prefix) static int ensure_core_worktree(int argc, const char **argv, const char *prefix) { - const struct submodule *sub; const char *path; const char *cw; struct repository subrepo; @@ -2550,11 +2549,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix) path = argv[1]; - sub = submodule_from_path(the_repository, null_oid(), path); - if (!sub) - BUG("We could get the submodule handle before?"); - - if (repo_submodule_init(&subrepo, the_repository, sub)) + if (repo_submodule_init(&subrepo, the_repository, path, null_oid())) die(_("could not get a repository handle for submodule '%s'"), path); if (!repo_config_get_string_tmp(&subrepo, "core.worktree", &cw)) { diff --git a/repository.c b/repository.c index b2bf44c6fa..e4a1afb0ac 100644 --- a/repository.c +++ b/repository.c @@ -190,19 +190,15 @@ int repo_init(struct repository *repo, int repo_submodule_init(struct repository *subrepo, struct repository *superproject, - const struct submodule *sub) + const char *path, + const struct object_id *treeish_name) { struct strbuf gitdir = STRBUF_INIT; struct strbuf worktree = STRBUF_INIT; int ret = 0; - if (!sub) { - ret = -1; - goto out; - } - - strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", sub->path); - strbuf_repo_worktree_path(&worktree, superproject, "%s", sub->path); + strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", path); + strbuf_repo_worktree_path(&worktree, superproject, "%s", path); if (repo_init(subrepo, gitdir.buf, worktree.buf)) { /* @@ -212,6 +208,13 @@ int repo_submodule_init(struct repository *subrepo, * in the superproject's 'modules' directory. In this case the * submodule would not have a worktree. */ + const struct submodule *sub = + submodule_from_path(superproject, treeish_name, path); + if (!sub) { + ret = -1; + goto out; + } + strbuf_reset(&gitdir); strbuf_repo_git_path(&gitdir, superproject, "modules/%s", sub->name); @@ -225,7 +228,7 @@ int repo_submodule_init(struct repository *subrepo, subrepo->submodule_prefix = xstrfmt("%s%s/", superproject->submodule_prefix ? superproject->submodule_prefix : - "", sub->path); + "", path); out: strbuf_release(&gitdir); diff --git a/repository.h b/repository.h index 3740c93bc0..c24e177c7e 100644 --- a/repository.h +++ b/repository.h @@ -172,15 +172,18 @@ void initialize_the_repository(void); int repo_init(struct repository *r, const char *gitdir, const char *worktree); /* - * Initialize the repository 'subrepo' as the submodule given by the - * struct submodule 'sub' in parent repository 'superproject'. - * Return 0 upon success and a non-zero value upon failure, which may happen - * if the submodule is not found, or 'sub' is NULL. + * Initialize the repository 'subrepo' as the submodule at the given path. If + * the submodule's gitdir cannot be found at /.git, this function calls + * submodule_from_path() to try to find it. treeish_name is only used if + * submodule_from_path() needs to be called; see its documentation for more + * information. + * Return 0 upon success and a non-zero value upon failure. */ -struct submodule; +struct object_id; int repo_submodule_init(struct repository *subrepo, struct repository *superproject, - const struct submodule *sub); + const char *path, + const struct object_id *treeish_name); void repo_clear(struct repository *repo); /* diff --git a/submodule.c b/submodule.c index 3af3da5b5e..ecda0229af 100644 --- a/submodule.c +++ b/submodule.c @@ -520,9 +520,6 @@ static void prepare_submodule_repo_env_in_gitdir(struct strvec *out) /* * Initialize a repository struct for a submodule based on the provided 'path'. * - * Unlike repo_submodule_init, this tolerates submodules not present - * in .gitmodules. This function exists only to preserve historical behavior, - * * Returns the repository struct on success, * NULL when the submodule is not present. */ @@ -1404,11 +1401,11 @@ static void fetch_task_release(struct fetch_task *p) } static struct repository *get_submodule_repo_for(struct repository *r, - const struct submodule *sub) + const char *path) { struct repository *ret = xmalloc(sizeof(*ret)); - if (repo_submodule_init(ret, r, sub)) { + if (repo_submodule_init(ret, r, path, null_oid())) { free(ret); return NULL; } @@ -1452,7 +1449,7 @@ static int get_next_submodule(struct child_process *cp, continue; } - task->repo = get_submodule_repo_for(spf->r, task->sub); + task->repo = get_submodule_repo_for(spf->r, task->sub->path); if (task->repo) { struct strbuf submodule_prefix = STRBUF_INIT; child_process_init(cp); diff --git a/t/helper/test-submodule-nested-repo-config.c b/t/helper/test-submodule-nested-repo-config.c index e3f11ff5a7..dc1c14bde3 100644 --- a/t/helper/test-submodule-nested-repo-config.c +++ b/t/helper/test-submodule-nested-repo-config.c @@ -11,15 +11,13 @@ static void die_usage(const char **argv, const char *msg) int cmd__submodule_nested_repo_config(int argc, const char **argv) { struct repository subrepo; - const struct submodule *sub; if (argc < 3) die_usage(argv, "Wrong number of arguments."); setup_git_directory(); - sub = submodule_from_path(the_repository, null_oid(), argv[1]); - if (repo_submodule_init(&subrepo, the_repository, sub)) { + if (repo_submodule_init(&subrepo, the_repository, argv[1], null_oid())) { die_usage(argv, "Submodule not found."); } From patchwork Thu Sep 9 18:47:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Tan X-Patchwork-Id: 12483651 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5488C433F5 for ; Thu, 9 Sep 2021 18:47:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id ADF1661100 for ; Thu, 9 Sep 2021 18:47:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244169AbhIISs6 (ORCPT ); Thu, 9 Sep 2021 14:48:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48260 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242867AbhIISst (ORCPT ); Thu, 9 Sep 2021 14:48:49 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0D6DEC061574 for ; Thu, 9 Sep 2021 11:47:39 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id y134-20020a25dc8c000000b0059f0301df0fso3522694ybe.21 for ; Thu, 09 Sep 2021 11:47:39 -0700 (PDT) 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=wnC8ZH7elaZ7Pnc3nqYiInbIDIgUNm86EluSqxmBEt0=; b=pnuiYs7r1p5pvsmrBCWqHh9K5NoQ0tLvrn3S+DKNMHDdmb+FgPvohUTSRJohUTvusA FgcckkJRmaAxCe+MXNLrTwm1RfPxXhUbtKYZ2NS4RmU3cR82t1XwWSK1RL1oNAdXIHQV tGyEBT3+gdhv4J56EdiGqtAPmyJ3tjyqec2TbvlGpd3sh9oKd3WFZN+3bssFhHlHfA5B Mji8gWGHA9LNVga7fMyzZCxWrObBWHdr66ETY7E3njNCKA8QKuHpSGthNTXkOyTWBJPh kEotsnzNsfpqQE+xl1him3JX97Z1VeTogrNdVys5y6lOY5x+uBivy717D6x1fq0j/bLk Xx8g== 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=wnC8ZH7elaZ7Pnc3nqYiInbIDIgUNm86EluSqxmBEt0=; b=LSTwuT+IAJmhUnCpvQ6i0XmnOwn9PNJTHSf35cB2Mh2ec6+fqLxObIfQjxAl1vPZ57 FBXMsQ3f6A5LUFm3enoFnqzNcb/c39Vos/LdCeFqLJ67xxWTM8QamyfUxkGII44kTjD8 lJcmacmE/S9FD2g4DlA1cAJIJG9HtUtoJgvyfoKzBoAYS7wGJWlDEzCwKxhEegXiJbSC Mwe8Ied1X36hyIw8ALYOLxBcSqNGq5/ywcHMguI9S3/sJ2zibJsloij7+QnxVO6vRALp /ptECddBfgswmWZVAtz2Y6bmc3h+O0iHKvGUuYfUAiKb4xMa5NSRQPWnFOloTlPzIKzs Zvqg== X-Gm-Message-State: AOAM533UAwcZvuxmtMNbJ6bw0NX2mzK0ZuZ/j6Al07Nq2Ho6QKBJktAQ Gu+BsbME89dw0VpnvbavunUqpk/goMum1fauNqusP8HV8S9ktw/DNGe1EELEwFrjpQleByY3oHI j593fa9ezkFp/U43JYjRHMFM3pYuugqhlwvNnzdZ0JX0+fPvDcAvztUVODoylHT9DKPg3IOI9CJ Gg X-Google-Smtp-Source: ABdhPJxgDwVTo6/VHFUXpPS8/niNQVB1WCzTXPfS4bnWVO4jJ+mxHAd983xwSm9AeFoFZZf27igZwX96mptpM5trNwaY X-Received: from twelve4.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:437a]) (user=jonathantanmy job=sendgmr) by 2002:a05:6902:1148:: with SMTP id p8mr5696539ybu.319.1631213258257; Thu, 09 Sep 2021 11:47:38 -0700 (PDT) Date: Thu, 9 Sep 2021 11:47:29 -0700 In-Reply-To: Message-Id: <978174fdc25cdb7ee766a9e6a056145e127f52a4.1631212893.git.jonathantanmy@google.com> Mime-Version: 1.0 References: X-Mailer: git-send-email 2.33.0.309.g3052b89438-goog Subject: [PATCH v2 3/3] revision: remove "submodule" from opt struct From: Jonathan Tan To: git@vger.kernel.org Cc: Jonathan Tan , gitster@pobox.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Clean up a TODO in revision.h by removing the "submodule" field from struct setup_revision_opt. This field is only used to specify the ref store to use, so use rev_info->repo to determine the ref store instead. The only users of this field are merge-ort.c and merge-recursive.c. However, both these files specify the superproject as rev_info->repo and the submodule as setup_revision_opt->submodule. In order to be able to pass the submodule as rev_info->repo, all commits must be parsed with the submodule explicitly specified; this patch does that as well. (An incremental solution in which only some commits are parsed with explicit submodule will not work, because if the same commit is parsed twice in different repositories, there will be 2 heap-allocated object structs corresponding to that commit, and any flag set by the revision walking mechanism on one of them will not be reflected onto the other.) Signed-off-by: Jonathan Tan --- merge-ort.c | 53 +++++++++++++++++++++++++++++++---------------- merge-recursive.c | 49 ++++++++++++++++++++++++++++--------------- revision.c | 16 +++++--------- revision.h | 1 - 4 files changed, 72 insertions(+), 47 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index 6eb910d6f0..b8efaee8e0 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -32,6 +32,7 @@ #include "promisor-remote.h" #include "revision.h" #include "strmap.h" +#include "submodule-config.h" #include "submodule.h" #include "tree.h" #include "unpack-trees.h" @@ -1499,7 +1500,6 @@ static int find_first_merges(struct repository *repo, xsnprintf(merged_revision, sizeof(merged_revision), "^%s", oid_to_hex(&a->object.oid)); repo_init_revisions(repo, &revs, NULL); - rev_opts.submodule = path; /* FIXME: can't handle linked worktrees in submodules yet */ revs.single_worktree = path != NULL; setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts); @@ -1509,7 +1509,7 @@ static int find_first_merges(struct repository *repo, die("revision walk setup failed"); while ((commit = get_revision(&revs)) != NULL) { struct object *o = &(commit->object); - if (in_merge_bases(b, commit)) + if (repo_in_merge_bases(repo, b, commit)) add_object_array(o, NULL, &merges); } reset_revision_walk(); @@ -1524,7 +1524,7 @@ static int find_first_merges(struct repository *repo, contains_another = 0; for (j = 0; j < merges.nr; j++) { struct commit *m2 = (struct commit *) merges.objects[j].item; - if (i != j && in_merge_bases(m2, m1)) { + if (i != j && repo_in_merge_bases(repo, m2, m1)) { contains_another = 1; break; } @@ -1545,10 +1545,12 @@ static int merge_submodule(struct merge_options *opt, const struct object_id *b, struct object_id *result) { + struct repository subrepo; + struct strbuf sb = STRBUF_INIT; + int ret = 0; struct commit *commit_o, *commit_a, *commit_b; int parent_count; struct object_array merges; - struct strbuf sb = STRBUF_INIT; int i; int search = !opt->priv->call_depth; @@ -1564,6 +1566,10 @@ static int merge_submodule(struct merge_options *opt, if (is_null_oid(b)) return 0; + /* + * NEEDSWORK: Remove this when all submodule object accesses are + * through explicitly specified repositores. + */ if (add_submodule_odb(path)) { path_msg(opt, path, 0, _("Failed to merge submodule %s (not checked out)"), @@ -1571,39 +1577,48 @@ static int merge_submodule(struct merge_options *opt, return 0; } - if (!(commit_o = lookup_commit_reference(opt->repo, o)) || - !(commit_a = lookup_commit_reference(opt->repo, a)) || - !(commit_b = lookup_commit_reference(opt->repo, b))) { + if (repo_submodule_init(&subrepo, opt->repo, path, null_oid())) { + path_msg(opt, path, 0, + _("Failed to merge submodule %s (not checked out)"), + path); + return 0; + } + + if (!(commit_o = lookup_commit_reference(&subrepo, o)) || + !(commit_a = lookup_commit_reference(&subrepo, a)) || + !(commit_b = lookup_commit_reference(&subrepo, b))) { path_msg(opt, path, 0, _("Failed to merge submodule %s (commits not present)"), path); - return 0; + goto cleanup; } /* check whether both changes are forward */ - if (!in_merge_bases(commit_o, commit_a) || - !in_merge_bases(commit_o, commit_b)) { + if (!repo_in_merge_bases(&subrepo, commit_o, commit_a) || + !repo_in_merge_bases(&subrepo, commit_o, commit_b)) { path_msg(opt, path, 0, _("Failed to merge submodule %s " "(commits don't follow merge-base)"), path); - return 0; + goto cleanup; } /* Case #1: a is contained in b or vice versa */ - if (in_merge_bases(commit_a, commit_b)) { + if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) { oidcpy(result, b); path_msg(opt, path, 1, _("Note: Fast-forwarding submodule %s to %s"), path, oid_to_hex(b)); - return 1; + ret = 1; + goto cleanup; } - if (in_merge_bases(commit_b, commit_a)) { + if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) { oidcpy(result, a); path_msg(opt, path, 1, _("Note: Fast-forwarding submodule %s to %s"), path, oid_to_hex(a)); - return 1; + ret = 1; + goto cleanup; } /* @@ -1615,10 +1630,10 @@ static int merge_submodule(struct merge_options *opt, /* Skip the search if makes no sense to the calling context. */ if (!search) - return 0; + goto cleanup; /* find commit which merges them */ - parent_count = find_first_merges(opt->repo, path, commit_a, commit_b, + parent_count = find_first_merges(&subrepo, path, commit_a, commit_b, &merges); switch (parent_count) { case 0: @@ -1652,7 +1667,9 @@ static int merge_submodule(struct merge_options *opt, } object_array_clear(&merges); - return 0; +cleanup: + repo_clear(&subrepo); + return ret; } static void initialize_attr_index(struct merge_options *opt) diff --git a/merge-recursive.c b/merge-recursive.c index 3355d50e8a..fc8ac39d8c 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -24,6 +24,7 @@ #include "repository.h" #include "revision.h" #include "string-list.h" +#include "submodule-config.h" #include "submodule.h" #include "tag.h" #include "tree-walk.h" @@ -1113,7 +1114,6 @@ static int find_first_merges(struct repository *repo, xsnprintf(merged_revision, sizeof(merged_revision), "^%s", oid_to_hex(&a->object.oid)); repo_init_revisions(repo, &revs, NULL); - rev_opts.submodule = path; /* FIXME: can't handle linked worktrees in submodules yet */ revs.single_worktree = path != NULL; setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts); @@ -1123,7 +1123,7 @@ static int find_first_merges(struct repository *repo, die("revision walk setup failed"); while ((commit = get_revision(&revs)) != NULL) { struct object *o = &(commit->object); - if (in_merge_bases(b, commit)) + if (repo_in_merge_bases(repo, b, commit)) add_object_array(o, NULL, &merges); } reset_revision_walk(); @@ -1138,7 +1138,7 @@ static int find_first_merges(struct repository *repo, contains_another = 0; for (j = 0; j < merges.nr; j++) { struct commit *m2 = (struct commit *) merges.objects[j].item; - if (i != j && in_merge_bases(m2, m1)) { + if (i != j && repo_in_merge_bases(repo, m2, m1)) { contains_another = 1; break; } @@ -1174,6 +1174,8 @@ static int merge_submodule(struct merge_options *opt, const struct object_id *base, const struct object_id *a, const struct object_id *b) { + struct repository subrepo; + int ret = 0; struct commit *commit_base, *commit_a, *commit_b; int parent_count; struct object_array merges; @@ -1197,27 +1199,36 @@ static int merge_submodule(struct merge_options *opt, if (is_null_oid(b)) return 0; + /* + * NEEDSWORK: Remove this when all submodule object accesses are + * through explicitly specified repositores. + */ if (add_submodule_odb(path)) { output(opt, 1, _("Failed to merge submodule %s (not checked out)"), path); return 0; } - if (!(commit_base = lookup_commit_reference(opt->repo, base)) || - !(commit_a = lookup_commit_reference(opt->repo, a)) || - !(commit_b = lookup_commit_reference(opt->repo, b))) { - output(opt, 1, _("Failed to merge submodule %s (commits not present)"), path); + if (repo_submodule_init(&subrepo, opt->repo, path, null_oid())) { + output(opt, 1, _("Failed to merge submodule %s (not checked out)"), path); return 0; } + if (!(commit_base = lookup_commit_reference(&subrepo, base)) || + !(commit_a = lookup_commit_reference(&subrepo, a)) || + !(commit_b = lookup_commit_reference(&subrepo, b))) { + output(opt, 1, _("Failed to merge submodule %s (commits not present)"), path); + goto cleanup; + } + /* check whether both changes are forward */ - if (!in_merge_bases(commit_base, commit_a) || - !in_merge_bases(commit_base, commit_b)) { + if (!repo_in_merge_bases(&subrepo, commit_base, commit_a) || + !repo_in_merge_bases(&subrepo, commit_base, commit_b)) { output(opt, 1, _("Failed to merge submodule %s (commits don't follow merge-base)"), path); - return 0; + goto cleanup; } /* Case #1: a is contained in b or vice versa */ - if (in_merge_bases(commit_a, commit_b)) { + if (repo_in_merge_bases(&subrepo, commit_a, commit_b)) { oidcpy(result, b); if (show(opt, 3)) { output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path); @@ -1227,9 +1238,10 @@ static int merge_submodule(struct merge_options *opt, else ; /* no output */ - return 1; + ret = 1; + goto cleanup; } - if (in_merge_bases(commit_b, commit_a)) { + if (repo_in_merge_bases(&subrepo, commit_b, commit_a)) { oidcpy(result, a); if (show(opt, 3)) { output(opt, 3, _("Fast-forwarding submodule %s to the following commit:"), path); @@ -1239,7 +1251,8 @@ static int merge_submodule(struct merge_options *opt, else ; /* no output */ - return 1; + ret = 1; + goto cleanup; } /* @@ -1251,10 +1264,10 @@ static int merge_submodule(struct merge_options *opt, /* Skip the search if makes no sense to the calling context. */ if (!search) - return 0; + goto cleanup; /* find commit which merges them */ - parent_count = find_first_merges(opt->repo, &merges, path, + parent_count = find_first_merges(&subrepo, &merges, path, commit_a, commit_b); switch (parent_count) { case 0: @@ -1281,7 +1294,9 @@ static int merge_submodule(struct merge_options *opt, } object_array_clear(&merges); - return 0; +cleanup: + repo_clear(&subrepo); + return ret; } static int merge_mode_and_contents(struct merge_options *opt, diff --git a/revision.c b/revision.c index cddd0542a6..31fc1884d2 100644 --- a/revision.c +++ b/revision.c @@ -2561,8 +2561,7 @@ static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void return for_each_bisect_ref(refs, fn, cb_data, term_good); } -static int handle_revision_pseudo_opt(const char *submodule, - struct rev_info *revs, +static int handle_revision_pseudo_opt(struct rev_info *revs, const char **argv, int *flags) { const char *arg = argv[0]; @@ -2570,7 +2569,7 @@ static int handle_revision_pseudo_opt(const char *submodule, struct ref_store *refs; int argcount; - if (submodule) { + if (revs->repo != the_repository) { /* * We need some something like get_submodule_worktrees() * before we can go through all worktrees of a submodule, @@ -2579,9 +2578,8 @@ static int handle_revision_pseudo_opt(const char *submodule, */ if (!revs->single_worktree) BUG("--single-worktree cannot be used together with submodule"); - refs = get_submodule_ref_store(submodule); - } else - refs = get_main_ref_store(revs->repo); + } + refs = get_main_ref_store(revs->repo); /* * NOTE! @@ -2699,12 +2697,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s { int i, flags, left, seen_dashdash, revarg_opt; struct strvec prune_data = STRVEC_INIT; - const char *submodule = NULL; int seen_end_of_options = 0; - if (opt) - submodule = opt->submodule; - /* First, search for "--" */ if (opt && opt->assume_dashdash) { seen_dashdash = 1; @@ -2733,7 +2727,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (!seen_end_of_options && *arg == '-') { int opts; - opts = handle_revision_pseudo_opt(submodule, + opts = handle_revision_pseudo_opt( revs, argv + i, &flags); if (opts > 0) { diff --git a/revision.h b/revision.h index fbb068da9f..d8e0e93026 100644 --- a/revision.h +++ b/revision.h @@ -339,7 +339,6 @@ extern volatile show_early_output_fn_t show_early_output; struct setup_revision_opt { const char *def; void (*tweak)(struct rev_info *, struct setup_revision_opt *); - const char *submodule; /* TODO: drop this and use rev_info->repo */ unsigned int assume_dashdash:1, allow_exclude_promisor_objects:1; unsigned revarg_opt;