All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: Glen Choo <chooglen@google.com>, git@vger.kernel.org
Subject: Re: [PATCH 8/8] submodule: fix bug and remove add_submodule_odb()
Date: Fri, 11 Feb 2022 09:19:16 -0800	[thread overview]
Message-ID: <xmqqy22hcodn.fsf@gitster.g> (raw)
In-Reply-To: <20220210230422.690368-1-jonathantanmy@google.com> (Jonathan Tan's message of "Thu, 10 Feb 2022 15:04:22 -0800")

Jonathan Tan <jonathantanmy@google.com> writes:

> I would write the commit message like this:
>
>   submodule: fix latent check_has_commit() bug
>
>   check_has_commit() will attempt to clear a non-initialized struct
>   repository if initialization fails. This bug is masked by its only
>   caller, submodule_has_commits(), first calling add_submodule_odb().
>   The latter fails if the repo does not exist, making
>   submodule_has_commits() exit early and not invoke check_has_commit()
>   in a situation in which initialization would fail.
>
>   Fix this bug, and because calling add_submodule_odb() is no longer
>   necessary as of 13a2f620b2 (submodule: pass repo to
>   check_has_commit(), 2021-10-08), remove that call too.
>
>   This is the last caller of add_submodule_odb(), so remove that
>   function. (Adding submodule ODBs as alternates is still present in the
>   form of add_submodule_odb_by_path().)

Looks more clearly explained.  

We still end up calling add_to_alternate_memory(), so I take the
"let's have this early" back.

  parent reply	other threads:[~2022-02-11 17:19 UTC|newest]

Thread overview: 149+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-10  4:41 [PATCH 0/8] fetch --recurse-submodules: fetch unpopulated submodules Glen Choo
2022-02-10  4:41 ` [PATCH 1/8] submodule: inline submodule_commits() into caller Glen Choo
2022-02-10  4:41 ` [PATCH 2/8] submodule: store new submodule commits oid_array in a struct Glen Choo
2022-02-10 19:00   ` Jonathan Tan
2022-02-10 22:05     ` Junio C Hamano
2022-02-10  4:41 ` [PATCH 3/8] submodule: make static functions read submodules from commits Glen Choo
2022-02-10 19:15   ` Jonathan Tan
2022-02-11 10:07     ` Glen Choo
2022-02-11 10:09     ` Glen Choo
2022-02-10  4:41 ` [PATCH 4/8] t5526: introduce test helper to assert on fetches Glen Choo
2022-02-10  4:41 ` [PATCH 5/8] t5526: use grep " Glen Choo
2022-02-10 19:17   ` Jonathan Tan
2022-02-10  4:41 ` [PATCH 6/8] submodule: extract get_fetch_task() Glen Choo
2022-02-10 19:33   ` Jonathan Tan
2022-02-10  4:41 ` [PATCH 7/8] fetch: fetch unpopulated, changed submodules Glen Choo
2022-02-10 22:49   ` Junio C Hamano
2022-02-11  7:15     ` Glen Choo
2022-02-11 17:07       ` Junio C Hamano
2022-02-10 22:51   ` Jonathan Tan
2022-02-14  4:24     ` Glen Choo
2022-02-14 18:04     ` Glen Choo
2022-02-14 10:17   ` Glen Choo
2022-02-10  4:41 ` [PATCH 8/8] submodule: fix bug and remove add_submodule_odb() Glen Choo
2022-02-10 22:54   ` Junio C Hamano
2022-02-11  3:13     ` Glen Choo
2022-02-10 23:04   ` Jonathan Tan
2022-02-11  3:18     ` Glen Choo
2022-02-11 17:19     ` Junio C Hamano [this message]
2022-02-14  2:52       ` Glen Choo
2022-02-10  7:07 ` [PATCH 0/8] fetch --recurse-submodules: fetch unpopulated submodules Junio C Hamano
2022-02-10  8:51   ` Glen Choo
2022-02-10 17:40     ` Junio C Hamano
2022-02-11  2:39       ` Glen Choo
2022-02-15 17:23 ` [PATCH v2 0/9] " Glen Choo
2022-02-15 17:23   ` [PATCH v2 1/9] t5526: introduce test helper to assert on fetches Glen Choo
2022-02-15 21:37     ` Ævar Arnfjörð Bjarmason
2022-02-15 17:23   ` [PATCH v2 2/9] t5526: use grep " Glen Choo
2022-02-15 21:53     ` Ævar Arnfjörð Bjarmason
2022-02-16  3:09       ` Glen Choo
2022-02-16 10:02         ` Ævar Arnfjörð Bjarmason
2022-02-17  4:04           ` Glen Choo
2022-02-17  9:25             ` Ævar Arnfjörð Bjarmason
2022-02-17 16:16               ` Glen Choo
2022-02-15 17:23   ` [PATCH v2 3/9] submodule: make static functions read submodules from commits Glen Choo
2022-02-15 21:18     ` Jonathan Tan
2022-02-16  6:59       ` Glen Choo
2022-02-15 22:00     ` Ævar Arnfjörð Bjarmason
2022-02-16  7:08       ` Glen Choo
2022-02-15 17:23   ` [PATCH v2 4/9] submodule: inline submodule_commits() into caller Glen Choo
2022-02-15 22:02     ` Ævar Arnfjörð Bjarmason
2022-02-15 17:23   ` [PATCH v2 5/9] submodule: store new submodule commits oid_array in a struct Glen Choo
2022-02-15 21:33     ` Ævar Arnfjörð Bjarmason
2022-02-15 17:23   ` [PATCH v2 6/9] submodule: extract get_fetch_task() Glen Choo
2022-02-15 17:23   ` [PATCH v2 7/9] fetch: fetch unpopulated, changed submodules Glen Choo
2022-02-15 22:02     ` Jonathan Tan
2022-02-16  5:46       ` Glen Choo
2022-02-16  9:11         ` Glen Choo
2022-02-16  9:39           ` Ævar Arnfjörð Bjarmason
2022-02-16 17:33             ` Glen Choo
2022-02-15 22:06     ` Ævar Arnfjörð Bjarmason
2022-02-15 17:23   ` [PATCH v2 8/9] submodule: read shallows when finding " Glen Choo
2022-02-15 22:03     ` Jonathan Tan
2022-02-15 17:23   ` [PATCH v2 9/9] submodule: fix latent check_has_commit() bug Glen Choo
2022-02-15 22:04     ` Jonathan Tan
2022-02-24 10:08   ` [PATCH v3 00/10] fetch --recurse-submodules: fetch unpopulated submodules Glen Choo
2022-02-24 10:08     ` [PATCH v3 01/10] t5526: introduce test helper to assert on fetches Glen Choo
2022-02-25  0:34       ` Junio C Hamano
2022-02-24 10:08     ` [PATCH v3 02/10] t5526: stop asserting on stderr literally Glen Choo
2022-02-24 11:52       ` Ævar Arnfjörð Bjarmason
2022-02-24 16:15         ` Glen Choo
2022-02-24 18:13           ` Eric Sunshine
2022-02-24 23:05       ` Jonathan Tan
2022-02-25  2:26         ` Glen Choo
2022-02-24 10:08     ` [PATCH v3 03/10] t5526: create superproject commits with test helper Glen Choo
2022-02-24 23:14       ` Jonathan Tan
2022-02-25  2:52         ` Glen Choo
2022-02-25 11:42           ` Ævar Arnfjörð Bjarmason
2022-02-28 18:11             ` Glen Choo
2022-02-24 10:08     ` [PATCH v3 04/10] submodule: make static functions read submodules from commits Glen Choo
2022-02-24 10:08     ` [PATCH v3 05/10] submodule: inline submodule_commits() into caller Glen Choo
2022-02-24 10:08     ` [PATCH v3 06/10] submodule: store new submodule commits oid_array in a struct Glen Choo
2022-02-24 10:08     ` [PATCH v3 07/10] submodule: extract get_fetch_task() Glen Choo
2022-02-24 23:26       ` Jonathan Tan
2022-02-24 10:08     ` [PATCH v3 08/10] submodule: move logic into fetch_task_create() Glen Choo
2022-02-24 23:36       ` Jonathan Tan
2022-02-24 10:08     ` [PATCH v3 09/10] fetch: fetch unpopulated, changed submodules Glen Choo
2022-02-24 21:30       ` Junio C Hamano
2022-02-25  3:04         ` Glen Choo
2022-02-25  0:33       ` Junio C Hamano
2022-02-25  3:07         ` Glen Choo
2022-02-25  0:39       ` Jonathan Tan
2022-02-25  3:46         ` Glen Choo
2022-03-04 23:46           ` Jonathan Tan
2022-03-05  0:22             ` Glen Choo
2022-03-04 23:53           ` Jonathan Tan
2022-02-26 18:53       ` Junio C Hamano
2022-03-01 20:24         ` Johannes Schindelin
2022-03-01 20:33           ` Junio C Hamano
2022-03-02 23:25             ` Glen Choo
2022-03-01 20:32         ` Junio C Hamano
2022-02-24 10:08     ` [PATCH v3 10/10] submodule: fix latent check_has_commit() bug Glen Choo
2022-03-04  0:57     ` [PATCH v4 00/10] fetch --recurse-submodules: fetch unpopulated submodules Glen Choo
2022-03-04  0:57       ` [PATCH v4 01/10] t5526: introduce test helper to assert on fetches Glen Choo
2022-03-04  2:06         ` Junio C Hamano
2022-03-04 22:11           ` Glen Choo
2022-03-04  0:57       ` [PATCH v4 02/10] t5526: stop asserting on stderr literally Glen Choo
2022-03-04  2:12         ` Junio C Hamano
2022-03-04 22:41         ` Jonathan Tan
2022-03-04 23:48           ` Junio C Hamano
2022-03-05  0:25             ` Glen Choo
2022-03-04  0:57       ` [PATCH v4 03/10] t5526: create superproject commits with test helper Glen Choo
2022-03-04 22:59         ` Jonathan Tan
2022-03-04  0:57       ` [PATCH v4 04/10] submodule: make static functions read submodules from commits Glen Choo
2022-03-04  0:57       ` [PATCH v4 05/10] submodule: inline submodule_commits() into caller Glen Choo
2022-03-04  0:57       ` [PATCH v4 06/10] submodule: store new submodule commits oid_array in a struct Glen Choo
2022-03-04  0:57       ` [PATCH v4 07/10] submodule: extract get_fetch_task() Glen Choo
2022-03-04  0:57       ` [PATCH v4 08/10] submodule: move logic into fetch_task_create() Glen Choo
2022-03-04  0:57       ` [PATCH v4 09/10] fetch: fetch unpopulated, changed submodules Glen Choo
2022-03-04  2:37         ` Junio C Hamano
2022-03-04 22:59           ` Glen Choo
2022-03-05  0:13             ` Junio C Hamano
2022-03-05  0:37               ` Glen Choo
2022-03-08  0:11                 ` Junio C Hamano
2022-03-04 23:56         ` Jonathan Tan
2022-03-04  0:57       ` [PATCH v4 10/10] submodule: fix latent check_has_commit() bug Glen Choo
2022-03-04  2:17         ` Junio C Hamano
2022-03-04  2:22       ` [PATCH v4 00/10] fetch --recurse-submodules: fetch unpopulated submodules Junio C Hamano
2022-03-08  0:14       ` [PATCH v5 " Glen Choo
2022-03-08  0:14         ` [PATCH v5 01/10] t5526: introduce test helper to assert on fetches Glen Choo
2022-03-08  0:14         ` [PATCH v5 02/10] t5526: stop asserting on stderr literally Glen Choo
2022-03-08  0:14         ` [PATCH v5 03/10] t5526: create superproject commits with test helper Glen Choo
2022-03-08  0:14         ` [PATCH v5 04/10] submodule: make static functions read submodules from commits Glen Choo
2022-03-08  0:14         ` [PATCH v5 05/10] submodule: inline submodule_commits() into caller Glen Choo
2022-03-08  0:14         ` [PATCH v5 06/10] submodule: store new submodule commits oid_array in a struct Glen Choo
2022-03-08  0:14         ` [PATCH v5 07/10] submodule: extract get_fetch_task() Glen Choo
2022-03-08  0:14         ` [PATCH v5 08/10] submodule: move logic into fetch_task_create() Glen Choo
2022-03-08  0:14         ` [PATCH v5 09/10] fetch: fetch unpopulated, changed submodules Glen Choo
2022-03-08  0:14         ` [PATCH v5 10/10] submodule: fix latent check_has_commit() bug Glen Choo
2022-03-08  0:50         ` [PATCH v5 00/10] fetch --recurse-submodules: fetch unpopulated submodules Junio C Hamano
2022-03-08 18:24           ` Glen Choo
2022-03-09 19:13             ` Junio C Hamano
2022-03-09 19:49               ` Glen Choo
2022-03-09 20:22                 ` Junio C Hamano
2022-03-09 22:11                   ` Glen Choo
2022-03-16 21:58                     ` Glen Choo
2022-03-16 22:06                       ` Junio C Hamano
2022-03-16 22:37                         ` Glen Choo
2022-03-16 23:08                           ` Junio C Hamano
2022-03-08 21:42         ` Jonathan Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy22hcodn.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.