Re: 2.46 submodule breakage

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3370 bytes --]

On Wed, Aug 07, 2024 at 08:40:10AM +0200, Patrick Steinhardt wrote:
> On Tue, Aug 06, 2024 at 02:26:35PM -0400, Eric Sunshine wrote:
> > On Tue, Aug 6, 2024 at 9:21 AM Jeppe Øland <joland@gmail.com> wrote:
> > > I did a bunch of testing to narrow down the cause:
> > >
> > > It is not related to the Windows port - all the testing was done in WSL.
> > > It only happens when the clone containing submodules is done with
> > > --ref-format=reftable.
> > > The commit breaking it is: 129cb1b99d hash: drop (mostly) unused
> > > `is_empty_{blob,tree}_sha1()` functions
> > 
> > Cc:'ing Patrick, the author of that change.
> 
> Thanks for Cc'ing me, and thanks for the report.
> 
> I just wanted to say that I couldn't reproduce, but then spotted that
> you said that it only happens when using `--ref-format=reftable` during
> the git-clone(1) step. And indeed, that causes the following test to
> fail:
> 
>     test_expect_success 'recursive pull with different non-default ref format' '
>         # Set up the initial structure. This is an upstream repository that has
>         # a submodule, as well as a downstream clone of the upstream
>         # repository.
>         git init submodule &&
>         test_commit -C submodule submodule-base &&
>         git init upstream &&
>         test_commit -C upstream upstream-base &&
>         git -C upstream submodule add "file://$(pwd)/submodule" &&
>         git -C upstream commit -m "upstream submodule" &&
>         git clone --ref-format=reftable --recurse-submodules "file://$(pwd)/upstream" downstream &&
> 
>         # Update the submodule.
>         test_commit -C submodule submodule-update &&
>         git -C upstream/submodule pull &&
>         git -C upstream commit -am "update the submodule" &&
> 
>         git -C downstream pull --recurse-submodules
>     '
> 
> The issue here is that the recursive clone causes a mixture of ref
> formats: the parent repository uses the "reftable" backend, whereas the
> child repository uses "files". In theory, I think Git should be able to
> handle a mixture of ref formats like this, but I'm not surprised that it
> actually fails in practice. The question is whether this is sensible, or
> whether submodules should use the same ref format as their parent.
> 
> So it feels like the commit you have bisected this to only unearths this
> issue, but isn't the actual root cause.
> 
> I'll investigate further and will try to come up with a patch later this
> week.

For the record, the fix is as simple as the below diff. We indeed end up
initializing submodule ref stores with the parent ref storage format,
not with the one of the subrepo. I'll spend some more time though to
check whether other commands are impacted, as well, and write some more
tests.

Patrick

diff --git a/refs.c b/refs.c
index 915aeb4d1d..e4b1f4f8b1 100644
--- a/refs.c
+++ b/refs.c
@@ -2011,7 +2011,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
        free(subrepo);
        goto done;
    }
-	refs = ref_store_init(subrepo, the_repository->ref_storage_format,
+	refs = ref_store_init(subrepo, subrepo->ref_storage_format,
                  submodule_sb.buf,
                  REF_STORE_READ | REF_STORE_ODB);
    register_ref_store_map(&repo->submodule_ref_stores, "submodule",

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]