[v2,0/8] Improvements for ref storage formats with submodules

Message

Patrick Steinhardt Aug. 8, 2024, 7:35 a.m. UTC

Hi,

this is the second version of my patch series that aims to improve
interaction of ref storage formats with submodules.

Changes compared to v1:

  - I noticed that `git submodule add` also used to fail when used with
    a preexisting repository that uses a different ref format.

  - Add a preparatory refactoring that wraps overly long command lines
    in "git-submodule.sh".

  - Wire up support for `--ref-format=` in `git submodule add`.

Notably not included is a change to make `git submodule add/clone` honor
the ref storage format of the parent repository. I think it is somewhat
orthogonal to the changes I do in this series, and it is not entirely
obvious whether it is a good idea or not. We can still do this in a
future patch series, if we ever hear good arguments for it.

Thanks!

Patrick Steinhardt (8):
  git-submodule.sh: break overly long command lines
  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
  builtin/submodule: allow "add" to use different ref storage format
  submodule: fix leaking fetch tasks
  submodule: fix leaking seen submodule names
  object: fix leaking packfiles when closing object store

 Documentation/git-submodule.txt        |  10 +-
 builtin/clone.c                        |  10 +-
 builtin/submodule--helper.c            |  46 +++++++-
 git-submodule.sh                       |  82 ++++++++++++--
 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 | 144 +++++++++++++++++++++++++
 10 files changed, 298 insertions(+), 25 deletions(-)
 create mode 100755 t/t7424-submodule-mixed-ref-formats.sh

Range-diff against v1:
-:  ---------- > 1:  6513c6b17d git-submodule.sh: break overly long command lines
1:  a450759bd1 = 2:  e6cda43878 builtin/submodule: allow cloning with different ref storage format
2:  e5923c0b33 = 3:  ed314f5333 builtin/clone: propagate ref storage format to submodules
3:  aaff9134ed ! 4:  f13356581e refs: fix ref storage format for submodule ref stores
    @@ Commit message
             $ git pull --recursive
             fatal: Unable to find current revision in submodule path 'path/to/sub'
     
    -    Fix the bug by using the submodule repository's ref storage format
    -    instead.
    +    The same issue occurs when adding a repository contained in the working
    +    tree with a different ref storage format via `git submodule add`.
     
    -    Note that only the second added test fails without this fix. The other
    -    one is included regardless as it exercises other parts where we might
    -    end up accessing submodule ref stores.
    +    Fix the bug by using the submodule repository's ref storage format
    +    instead and add some tests. Note that the test for `git submodule
    +    status` was included as a precaution, only. The command worked alright
    +    even without the bugfix.
     
         Reported-by: Jeppe Øland <[e-mail]>
         Signed-off-by: Patrick Steinhardt <[e-mail]>
    @@ t/t7424-submodule-mixed-ref-formats.sh: do
     +	# Some tests migrate the ref storage format, which does not work with
     +	# reflogs at the time of writing these tests.
     +	git config set --global core.logAllRefUpdates false
    ++'
    ++
    ++test_expect_success 'add existing repository with different ref storage format' '
    ++	test_when_finished "rm -rf parent" &&
    ++
    ++	git init parent &&
    ++	(
    ++		cd parent &&
    ++		test_commit parent &&
    ++		git init --ref-format=$OTHER_FORMAT submodule &&
    ++		test_commit -C submodule submodule &&
    ++		git submodule add ./submodule
    ++	)
      '
      
      test_expect_success 'recursive clone propagates ref storage format' '
-:  ---------- > 5:  4ce17e44a1 builtin/submodule: allow "add" to use different ref storage format
4:  8f8371c18a = 6:  d92770290f submodule: fix leaking fetch tasks
5:  732142aaa6 = 7:  e9421189ca submodule: fix leaking seen submodule names
6:  8dc7cc76d5 = 8:  d05737c75f object: fix leaking packfiles when closing object store

Comments

Junio C Hamano Aug. 8, 2024, 4:24 p.m. UTC | #1

Patrick Steinhardt <[e-mail]> writes:

>   - Add a preparatory refactoring that wraps overly long command lines
>     in "git-submodule.sh".

This is very much appreciated.
>
>   - Wire up support for `--ref-format=` in `git submodule add`.

Good find.  Thanks.