west update doesn't sync submodule location

[Damian-Nordic]

west update only runs git submodule update ... command for each submodule defined in the west manifest, but it's not enough in case a submodule's URL changes - apparently, git submodule update won't update the submodule's remote. git submodule sync should be called as well.

[Damian-Nordic]

@mbolivar fyi

[marc-hb]

git submodule update won't update the submodule's remote.

What is the error message when the remote changes?

[Damian-Nordic]

In case of Project CHIP, where we spotted the issue, there is no error because the new repository is a copy of the old repository and they both have the same commit ids, but in case some repository switches to a new location and stops pushing changes to the old one you may one day get an error that a commit id of the submodule doesn't exist (I haven't tested it though).

Our primary issue at the moment is that in Project CHIP the pigweed submodule was moved to another location to workaround some firewall/content blocking issues and this won't help to existing users without running git submodule sync.

[marc-hb]

you may one day get an error that a commit id of the submodule doesn't exist

Good, then it would be easy to root cause the change.

but in case some repository switches to a new location and stops pushing changes to the old one

I hope this is rare. I don't think west's complexity should increase to support rare situations that are easy to root cause and address manually.

I'm a bit puzzled by git submodule sync. It's documentation makes it look like a super simple no-brainer. But if it is indeed so simple, then how come there is not even a git submodule update --sync option?! Is there a catch hidden somewhere? Are there situations where sync it's not desirable? Could these situations affect west too?

[mbolivar-nordic]

@Damian-Nordic my feeling so far is that we should add a --sync-submodules option to west update that users can use to sync this up, along with a configuration option you can set to enable this by default.

Would that work for you?

My concern is avoiding git submodule sync overhead if it is not necessary.

[Damian-Nordic]

Unfortunately, I have no answer to the question if there's any hidden catch. I would need to spend some time on it, write tests etc. to understand better if there's any risk, but so far I haven't encountered any issues with running the sync.

Suppose that we don't find any problems during the risk analysis. Then I would argue that git submodule sync should be run automatically with west update because:

1. Its overhead is minimal. It doesn't seem to fetch any changes from the new location, it just updates the remotes' URLs. I may look into the git code to confirm that.
2. It would be more aligned with handling other repositories managed by west. My impression is that west update does its best to make the update succeed and it can even discard local commits, so if it can handle a submodule URL change, it should do it.

I know it may be a rare case, but not impossible, so IMHO it's worth trying to address it.

[mbolivar-nordic]

I've posted #499. @Damian-Nordic please review.

[tejlmand]

@Damian-Nordic my feeling so far is that we should add a --sync-submodules option to west update that users can use to sync this up, along with a configuration option you can set to enable this by default.

seems a generalized --git-opt argument would indeed be handy as mentioned here: #498 (comment)

• adding a --clone-opt option that users can use to add options to git clone

Should we generalize this command:
--git-opt and then pass this raw to git ?
Then we could add this --git-opt to west diff, west update, and west status and it will be the callers responsibility to provide valid argument (or git itself will complain).

[mbolivar-nordic]

seems a generalized --git-opt argument would indeed be handy as mentioned here: #498 (comment)

I am currently -1 on this for reasons explained there.