CheckSubmodules: Run git submodule sync

[JohannesLorenz]

This is required in case the URL of a submodule changes. Without this, if you use CheckSubmodules.cmake, submodule URL changes will cause warnings when you update, e.g. "ref ... could not be found on remote".

Note: One possible point of criticism might be that the URL changes without that the user sees it. Some might see this as a security issue, however, the submodule URLs are defined by our team - if you cannot trust our team to put valid URLs into gitmodules, you cannot trust us to write non-malicious code. So I think the fix should be fine.

@tresf Added you since it's your module, and since we discussed it in Discord a while ago.

[tresf]

Untested, but LGTM.

[JohannesLorenz]

From reading the code, it looks like the macro GIT_SUBMODULE is only ever run for submodules which have not been inited yet. However, git submodule sync is only useful to update submodule that already have been inited. This means my change has no effect and I can reject my PR - unless I am misunderstanding the GIT_SUBMODULE macro.

[tresf]

From reading the code, it looks like the macro GIT_SUBMODULE is only ever run for submodules which have not been inited yet. However, git submodule sync is only useful to update submodule that already have been inited. This means my change has no effect and I can reject my PR - unless I am misunderstanding the GIT_SUBMODULE macro.

I think we expect an error code in order to call any of our recovery/fallback logic and in the case of a URL change, this may not happen (it will depend if the old URL and new URL are coincidentally contain that particular commit).

I suppose we could try to detect if it's changed by comparing the existing submodule path (by calling git config --get remote.origin.url from within SUBMODULE_PATH and comparing this value to SUBMODULE_URL). Of course, the disadvantage to this is that it may make it slightly harder to work locally from a submodule's fork.

[JohannesLorenz]

Of course, the disadvantage to this is that it may make it slightly harder to work locally from a submodule's fork.

Yes, I am also against such kind of automatism, especially when they only worked for some cases. I will close the PR. Sorry, thanks again for reviewing.