Page 1 of 1

How to handle Git submodules in forks?

Posted: Sun Jun 30, 2019 11:01 pm
by niflizn
Commit #008ad9a on esp-idf replaced the https***github.com/espressif/esp-idf/... URLs in .gitmodules with relative links.

The switch to relative URLs for submodules has broken our fork, because they reference our repo, which does not contain mirrors of the upstream source.

I suppose I could make my own forks of all the required submodules manually, but that is a rather tedious task.
Further, how will we make sure that we have the intended versions.

What is the recommended workflow?

Re: How to handle Git submodules in forks?

Posted: Mon Jul 01, 2019 1:29 am
by WiFive
Yes this is annoying because you can't click on a submodule to browse the source based on that commit on GitHub now either.

Did you try

Code: Select all

git config submodule.SubModuleName.url github_url
?

I don't think git has a way to specify a default url in case the relative url is not valid, unfortunately.

Re: How to handle Git submodules in forks?

Posted: Mon Jul 01, 2019 3:50 am
by ESP_Angus
Hi Nifomorph,

Thanks for bringing this up.

You can use "git config" to rewrite the URLs as WiFive suggests, then re-update submodules. Or if you're starting from scratch then you can clone the upstream repository and then switch over, as follows:

Code: Select all

git clone --recursive https://github.com/espressif/esp-idf.gt
git remote set-url origin <idf fork>
git fetch origin <branch>
git checkout <branch>
Another option would be to add a commit on the fork that rewrites the .gitmodules file. Although this will need updating when new submodules are added upstream.

We're going to add a script to the repo that rewrites all of the URLs to github ones. Once that's available it will be the recommended approach after cloning the base repo.
you can't click on a submodule to browse the source based on that commit on GitHub now either.
Ah, that is annoying. Maybe it's possible to bring it up as a request with GitHub somehow.

Unfortunately, although this approach is annoying in some ways it has also solved some long-standing problems we've had with our own development and continuous integration processes. GitHub does not always work reliably in China, so we keep internal forks of everything ourselves. Gitlab CI (which we use for our CI) has some restrictions around using any custom scripts as part of "git clone", it causes awkwardness with establishing a "clean" working directory. So we were regularly finding strange things happening in our "clean" CI clones.

That said: We don't want to tidy up our own process while breaking our users' processes, so we'll add support for setting the correct submodule paths in forks ASAP.

Re: How to handle Git submodules in forks?

Posted: Mon Jul 01, 2019 12:52 pm
by niflizn
Thanks for the quick reply.

We will try to work around the issue by rewriting the .gitmodules file for now.

It might be nice to have a $SUBMODULE_BASE_URL variable or similar mechanism to point to a common repo for modules referred using relative paths, but I don't know what would be involved in this.

Alternatively, have a GitHub feature to make forks of submodules when forking a repo.

Regarding the issue with clickable submodule links, a request for that has already been opened on GitHub about a month ago:
https://github.community/t5/How-to-use- ... 4650#M7066

You might want to give that request an upvote ;)

Re: How to handle Git submodules in forks?

Posted: Mon Jul 01, 2019 4:39 pm
by niflizn
I ended up rewriting .gitmodules with relative paths, but replaced URLs pointing to an Espressif repo in the following style:

From:

Code: Select all

[submodule "components/bt/controller/lib"]
	path = components/bt/controller/lib
	url = ../esp32-bt-lib.git
To:

Code: Select all

[submodule "components/bt/controller/lib"]
	path = components/bt/controller/lib
	url = ../../espressif/esp32-bt-lib.git
Please consider if this could work for you and maybe modify the 'master' branch accordingly.

I added a PR for this here:
https://github.com/espressif/esp-idf/pull/3721