What if we keep the old code and also do git clean -ffdx? I agree that git deinit seems cleaner (and easier to understand) but maybe it's not as efficient cache-wise?

I don't think deinit affects the caching:

after deinitializing the Git directory is kept around
(https://git-scm.com/docs/gitsubmodules)

I don't think deinit affects the caching:

after deinitializing the Git directory is kept around
(https://git-scm.com/docs/gitsubmodules)

I missed that, thanks.
And many more thanks for figuring out how to fix this!

Thanks for grasping this one! Does it still fix the original problem which is where the submodule URL changes?

It’s the scenario described in #320 (comment) - I think from my experiments, git submodule deinit followed by a re-init/update still caused the wrong URL in the submodule’s git repo

I think from my experiments, git submodule deinit followed by a re-init/update still caused the wrong URL in the submodule’s git repo

Ah, you're right! If you deinit a submodule and then reinit it, it reuses the address from before the deinit instead of reinitialising it. How annoying.

I've added a second commit with a test for moving a submodule's repository and made some fixes for it. We now also fetch submodules during the fetch step, not when checking out (which might help with caching too).

I also reverted "Checkout to the correct branch in with_checkout" from #333, since it prevents building PRs and therefore makes testing this difficult.

I'm not sure that this Dockerfile is exactly what's happening now, but it demonstrates what I think is still the problem:

FROM ocaml/opam AS base
RUN git clone https://github.com/ocurrent/ocluster.git --recursive
WORKDIR ocluster

FROM base
RUN sed -i -e s/ocurrent/dra27/ .gitmodules
RUN git submodule sync
RUN grep 'url = .*/obuilder\.git' * .git* -rI

FROM base
RUN rm -rf *
RUN git reset --hard HEAD
RUN sed -i -e s/ocurrent/dra27/ .gitmodules
RUN git submodule sync
RUN git submodule deinit --all --force
RUN git submodule update --init --recursive
RUN grep 'url = .*/obuilder\.git' * .git* -rI

In the first run, with the checkout as normal, we see:

.git/config:    url = https://github.com/dra27/obuilder.git
.git/modules/obuilder/config:   url = https://github.com/dra27/obuilder.git
.gitmodules:    url = https://github.com/dra27/obuilder.git

the second run attempts to simulate the default state of an OCurrent .git cache - which should be just a .git directory (because that's what we'd copied). At which point we have:

.git/config:    url = https://github.com/dra27/obuilder.git
.git/modules/obuilder/config:   url = https://github.com/ocurrent/obuilder.git
.gitmodules:    url = https://github.com/dra27/obuilder.git

git submodule update before the reset fixes it (which was the temporary fix). Another - possibly better - fix is to copy the .git directory _and any other .git files in the tree (so in the example, if obuilder/.git is restored after the rm -rf then the three URLs are also correctly updated).

The URLs shouldn't matter in the checkout (after copying .git) because we're not doing any fetching at that point now (enforced with ~fetch:false):

Cmd.git_submodule_update ~init:true ~cancellable:true ~fetch:false ~job ~repo:tmpdir

Oh, I get it, now, thanks!