Re: [PATCH v3 0/7] No more adding submodule ODB as alternate

On 2021.09.29 16:06, Jonathan Tan wrote:

This is on a merge of jk/ref-paranoia and jt/add-submodule-odb-clean-up
(same as v2).

Here's the same patch set except that the repo is plumbed into the ref
stores. (Iterators currently do not have any reference to their ref
stores, so some of them still need repo fields. But because the ref
stores now know their repos, calling code does not need to pass a repo
when these iterators are instantiated.)

As you can see from the shorter patch list, this eliminates the need for
some patches.

Jonathan Tan (7):
  refs: plumb repo into ref stores
  refs: teach arbitrary repo support to iterators
  refs: peeling non-the_repository iterators is BUG
  merge-{ort,recursive}: remove add_submodule_odb()
  object-file: only register submodule ODB if needed
  submodule: pass repo to check_has_commit()
  submodule: trace adding submodule ODB as alternate

This is a summary of discussion from our Review Club meeting today;
feedback comes from Jonathan Nieder, Emily Shaffer, Glen Choo, and
me.

(Side note for the list: we're trying to be more open with our Review
Club meetings. If you're interested in having video-chat discussions of
patch series with us every other week, please get in touch with me.)

Summary: this series looks good to me, with a few nits (replied to the
patches directly for these). So, once those are addressed, this series
has my
Reviewed-by: Josh Steadmon <redacted>

Eliminating submodules as alternates has (at least) a couple of nice
benefits, namely:
* we don't have to scan through the alternates one by one looking
  for missing objects
* having submodule objects show up as alternates makes it tricky to
  support partial clones of submodules, since the partial clone
  machinery doesn't know to talk to the submodule remote to find its
  objects

This series builds on the approach in
jt/grep-wo-submodule-odb-as-alternate, which recently graduated to
master. Specifically, we plumb `struct repository` pointers to various
places that previously relied on `the_repository`, then we use
GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB to show that no code paths in the
test suite still rely on registering submodules as alternates.

Specifically to this series, we're adding struct repository pointers to
`struct ref_store` and various iterators. This seems reasonable as we
don't keep a lot of these structs around, so the additional memory usage
isn't much of a concern.

Thanks for this series, really glad to see the end of submodule
alternates!