manifest: bsim: Do not import explicitly but keep copy in submanifests/

[aescolar]

Due to a limitation in the west import feature,
a project cannot both have an import and be part of a group.
Moreover, when a project has an import and is not filtered
out, it is required for that project to be present
for most west commands to work.

As the bsim project is not filtered by default,
it causes trouble for users who never run a
west update but try to use west further.

To work around this issue, let's disable the import
in the bsim project, and instead copy its manifest
into the submanifests/ folder.

Having a replica of the babblesim manifes in the submanifest
folder is likely to cause some confusion in users,
wrt to which version of components they are using.
So whenever west supports both imports and groups, or another
simple and nicer way of working around this, it should be used.

Fixes #57198

[nordicjm]

Works great with this, thanks for fixing.

[cfriedt]

Wow - @aescolar - this is amazing. Actually a very elegant solution. Thanks!

[cfriedt]

Again, not debating bsim at all - it is a great tool for BLE development.

I just wanted to add, that this fixes the issue that we at Meta had with including the external bsim manifest in west.yml.

Easy repro recipe to validate the fix:

west init /tmp/zephyrproject
cd /tmp/zephyrproject/zephyr
curl -L -o /tmp/57361.patch 'https://github.com/zephyrproject-rtos/zephyr/pull/57361.patch'
git apply /tmp/57361.patch
west status &>/dev/null
echo $?
# 0

But in the interest of scalability, virtually everything in Zephyr should be optional (#54276). E.g. in order to run native_posix, qemu_x86_64, or qemu_riscv64 tests, there were previously no other dependencies other than the main zephyr repo. That's a nice "feature" if you will, and we should preserve that.

Whether adding to submanifests/ is OK with with the TSC is a separate story. I don't see any harm in adding submanifests/bsim/west.yml.

Meta would definitely prefer this solution though over needing to internally host one or more manifest repositories. We would prefer to continue being able to mirror Zephyr as-is.

[mbolivar-nordic]

This seems really error-prone since the manifests have to be kept in sync with each other, right?

[aescolar]

This seems really error-prone since the manifests have to be kept in sync with each other, right?

Yes, it is hardly ideal. It is a stop gap that is transparent to everybody except whoever updates the bsim pointed at in the manifest, who needs to remember to update this submanifests/bsim/west.yml copy at the same time.

@carlescufi feedback addressed (no fundamental changes).

[cfriedt]

This works for me.

Again, thanks @aescolar, @carlescufi, @mbolivar-nordic, and everyone who put in effort to come to a mutually agreeable and technically sound solution here 👍

[nashif]

do not think submanifests should be used for this, all modules/projects shall be in west.yaml. This is also differrs from the solution presented to the TSC without any objections.

[cfriedt]

do not think submanifests should be used for this, all modules/projects shall be in west.yaml. This is also differrs from the solution presented to the TSC without any objections.

@nashif - @nordicjm made a good point to me (offline). This solution at least de-risks things for now from a policy perspective - i.e. avoids setting the precedent in a release that it is ok to add things to west.yml that require additional (possibly external) modules / a non-minimal clone.

It's completely valid though for Nordic and Oticon or anyone else to want to do Zephyr things and to do them upstream.

Maybe this just buys a bit more time to sort out a better technical solution? 🤷‍♂️

It's a good topic to discuss at the TSC F2F in any case.

[nashif]

this solution also works, without submanifests, without imports and without any workarounds, and it is how we have been doing things since day 1:

diff --git a/west.yml b/west.yml
index ca33049f81..bd30635cc7 100644
--- a/west.yml
+++ b/west.yml
@@ -21,17 +21,91 @@ manifest:
   remotes:
     - name: upstream
       url-base: https://github.com/zephyrproject-rtos
+    - name: bsim
+      url-base: https://github.com/BabbleSim

   group-filter: [-babblesim]

   #
   # Please add items below based on alphabetical order
   projects:
-    - name: bsim
-      repo-path: babblesim-manifest
-      revision: 908ffde6298a937c6549dbfa843a62caab26bfc5
-      import:
-        path-prefix: tools
+    - name: babblesim_base
+      remote: bsim
+      repo-path: base.git
+      path: bsim/components
+      revision: 02838ca04c4562e68dc876196828d8121679e537
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_libPhyComv1
+      remote: bsim
+      repo-path: ext_2G4_libPhyComv1.git
+      path: bsim/components/ext_2G4_libPhyComv1
+      revision: 9018113a362fa6c9e8f4b9cab9e5a8f12cc46b94
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_phy_v1
+      remote: bsim
+      repo-path: ext_2G4_phy_v1.git
+      path: bsim/components/ext_2G4_phy_v1
+      revision: cf2d86e736efac4f12fad5093ed2da2c5b406156
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_channel_NtNcable
+      remote: bsim
+      repo-path: ext_2G4_channel_NtNcable.git
+      path: bsim/components/ext_2G4_channel_NtNcable
+      revision: 20a38c997f507b0aa53817aab3d73a462fff7af1
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_channel_multiatt
+      remote: bsim
+      repo-path: ext_2G4_channel_multiatt.git
+      path: bsim/components/ext_2G4_channel_multiatt
+      revision: e09bc2d14b1975f969ad19c6ed23eb20e5dc3d09
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_modem_magic
+      remote: bsim
+      repo-path: ext_2G4_modem_magic.git
+      path: bsim/components/ext_2G4_modem_magic
+      revision: cb70771794f0bf6f262aa474848611c68ae8f1ed
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_modem_BLE_simple
+      remote: bsim
+      repo-path: ext_2G4_modem_BLE_simple.git
+      path: bsim/components/ext_2G4_modem_BLE_simple
+      revision: ce975a3259fd0dd761d371b60435242d54794bad
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_device_burst_interferer
+      remote: bsim
+      repo-path: ext_2G4_device_burst_interferer.git
+      path: bsim/components/ext_2G4_device_burst_interferer
+      revision: 5b5339351d6e6a2368c686c734dc8b2fc65698fc
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_device_WLAN_actmod
+      remote: bsim
+      repo-path: ext_2G4_device_WLAN_actmod.git
+      path: bsim/components/ext_2G4_device_WLAN_actmod
+      revision: 9cb6d8e72695f6b785e57443f0629a18069d6ce4
+      groups:
+        - babblesim
+    - name: babblesim_ext_2G4_device_playback
+      remote: bsim
+      repo-path: ext_2G4_device_playback.git
+      path: bsim/components/ext_2G4_device_playback
+      revision: 85c645929cf1ce995d8537107d9dcbd12ed64036
+      groups:
+        - babblesim
+    - name: babblesim_ext_libCryptov1
+      remote: bsim
+      repo-path: ext_libCryptov1.git
+      path: bsim/components/ext_libCryptov1
+      revision: eed6d7038e839153e340bd333bc43541cb90ba64
+      groups:
+        - babblesim
     - name: canopennode
       revision: dec12fa3f0d790cafa8414a4c2930ea71ab72ffd
       path: modules/lib/canopennode

[aescolar]

this solution also works, without submanifests,..

@nashif Would you be so kind as to elaborate what is the concern with submanifests?
If the concern is that it would be in the submanifest folder, that can be solved by placing it anywhere else.
(Note also that from west point of view the submanifests/bsim/ folder is different than submanifests/, as west does not crawl subfolders, it is effectively just an unrelated folder).

( Note that though the yml in the proposal in #57361 (comment) does not work as is, it is possible to dump the submanifests entries into the main manifest with some minor changes and it would work.
Still the benefits of keeping the babblesim projects in a submanifest is that a) we don't clutter the main manifest, and we keep related projects together/separate concerns b) as a submanifest we can keep it is an exact copy of the manifest in the bsim repo as pointed by the bsim repo sha, which eases maintenance, c) we can keep background documentation with it).

Note that this PR is done so as to cause no inconvenience whatsoever to users, but eliminating the issue of bsim not being optional (#57198).
That is, with this PR, developers using bsim and CI can continue doing exactly what they do today (all previous instructions work, everything is in the same place and named the same; whoever added to their CI/workflow a west update bsim can keep doing it), and developers who are not interested in bsim don't get its manifest repo by default.

Meaning the tradeoff here is just:

• Minor maintenance burden to keep the bsim manifest copy, + , the "aesthetic" matter of keeping the copy in Zephyr
  vs
• Fixing #57198

[aescolar]

Superseded by #59023