mbox series

[0/6] Improvements for ref storage formats with submodules

Message ID cover.1723032100.git.ps@pks.im (mailing list archive)
Headers show
Series Improvements for ref storage formats with submodules | expand

Message

Patrick Steinhardt Aug. 7, 2024, 12:43 p.m. UTC
Hi,

this small patch series contains some improvements for ref storage
formats and their interaction with submodules. Notably:

  - Use the correct format for submodules in situations where the parent
    repository uses a different ref storage format than the submodule.

  - Wire up `--ref-format=` for git-submodule(1), such that users can
    explicitly use a different ref format for their submodules.

  - Propagate the `--ref-format=` flag of git-clone(1) into submodules
    when using `--recursive`.

The first three patches implement improvements for the above three
issues and introduce tests. The test did hit some memory leaks, which
get fixed by patches 3 to 6 such that the new test can be marked as leak
free.

Thanks!

Patrick

Patrick Steinhardt (6):
  builtin/submodule: allow cloning with different ref storage format
  builtin/clone: propagate ref storage format to submodules
  refs: fix ref storage format for submodule ref stores
  submodule: fix leaking fetch tasks
  submodule: fix leaking seen submodule names
  object: fix leaking packfiles when closing object store

 Documentation/git-submodule.txt        |   5 +-
 builtin/clone.c                        |  10 ++-
 builtin/submodule--helper.c            |  30 +++++++
 git-submodule.sh                       |   9 ++
 object.c                               |   9 ++
 refs.c                                 |   2 +-
 submodule.c                            |  18 ++--
 t/t5572-pull-submodule.sh              |   1 +
 t/t7418-submodule-sparse-gitmodules.sh |   1 +
 t/t7424-submodule-mixed-ref-formats.sh | 120 +++++++++++++++++++++++++
 10 files changed, 191 insertions(+), 14 deletions(-)
 create mode 100755 t/t7424-submodule-mixed-ref-formats.sh

Comments

Patrick Steinhardt Aug. 7, 2024, 1:18 p.m. UTC | #1
On Wed, Aug 07, 2024 at 02:43:44PM +0200, Patrick Steinhardt wrote:
> Hi,
> 
> this small patch series contains some improvements for ref storage
> formats and their interaction with submodules. Notably:
> 
>   - Use the correct format for submodules in situations where the parent
>     repository uses a different ref storage format than the submodule.
> 
>   - Wire up `--ref-format=` for git-submodule(1), such that users can
>     explicitly use a different ref format for their submodules.
> 
>   - Propagate the `--ref-format=` flag of git-clone(1) into submodules
>     when using `--recursive`.
> 
> The first three patches implement improvements for the above three
> issues and introduce tests. The test did hit some memory leaks, which
> get fixed by patches 3 to 6 such that the new test can be marked as leak
> free.

Just got a shower thought that this isn't quite there yet. Most
importantly, I think we should default to the ref storage format of the
parent submodule both when adding submodules and when cloning a
submodule into a preexisting repository.

I'll wait until tomorrow before addressing this though to ideally get
some feedback until then.

Patrick
Junio C Hamano Aug. 8, 2024, 1:09 a.m. UTC | #2
Patrick Steinhardt <ps@pks.im> writes:

> this small patch series contains some improvements for ref storage
> formats and their interaction with submodules. Notably:
>
>   - Use the correct format for submodules in situations where the parent
>     repository uses a different ref storage format than the submodule.
>
>   - Wire up `--ref-format=` for git-submodule(1), such that users can
>     explicitly use a different ref format for their submodules.
>
>   - Propagate the `--ref-format=` flag of git-clone(1) into submodules
>     when using `--recursive`.
>
> The first three patches implement improvements for the above three
> issues and introduce tests. The test did hit some memory leaks, which
> get fixed by patches 3 to 6 such that the new test can be marked as leak
> free.

Nicely done.  I've read through the series and did not find any
design decisions in the series questionable.

As to propagating the choice of ref backend down from the
superproject to the submodule, I am not sure if it matters all that
much, so I view it as a relative low priority.  If somebody wants to
use a specific ref backend for their repositories, then they want
all their "git init" (or init_db()) to use that ref backend, and
would arrange configuration to make it so.  When "git submodule
init" internally calls "git init" (or init_db()), as long as we make
sure such a choice would propagate to the new repository that
happens to the one used for that submodule, we do not necessarily
need to have a custom logic that says "ah, the user did not say
anything about the ref backend, so let me peek the one used in the
superproject and propagate it down".

Thanks.
Patrick Steinhardt Aug. 8, 2024, 7 a.m. UTC | #3
On Wed, Aug 07, 2024 at 06:09:04PM -0700, Junio C Hamano wrote:
> As to propagating the choice of ref backend down from the
> superproject to the submodule, I am not sure if it matters all that
> much, so I view it as a relative low priority.  If somebody wants to
> use a specific ref backend for their repositories, then they want
> all their "git init" (or init_db()) to use that ref backend, and
> would arrange configuration to make it so.  When "git submodule
> init" internally calls "git init" (or init_db()), as long as we make
> sure such a choice would propagate to the new repository that
> happens to the one used for that submodule, we do not necessarily
> need to have a custom logic that says "ah, the user did not say
> anything about the ref backend, so let me peek the one used in the
> superproject and propagate it down".

You know, I'll just skip this for now. It feels somewhat orthogonal to
the changes in this series, and we're not closing the door on anything.

Patrick