Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manifest: bsim: Do not import but replicate in main manifest #59023

Merged
merged 3 commits into from
Jun 8, 2023

Conversation

aescolar
Copy link
Member

@aescolar aescolar commented Jun 7, 2023

manifest: bsim: Do not import but replicate in main manifest

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 replicate its content
inside Zephyr's main manifest.

Having a replica of the babblesim manifest content
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

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 replicate its content
inside Zephyr's main manifest.

Having a replica of the babblesim manifest content
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.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
This reverts commit 00130b7.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
@zephyrbot
Copy link
Collaborator

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
babblesim_base None 02838ca04c4562e68dc876196828d8121679e537 N/A
babblesim_ext_2G4_channel_NtNcable None 20a38c997f507b0aa53817aab3d73a462fff7af1 N/A
babblesim_ext_2G4_channel_multiatt None e09bc2d14b1975f969ad19c6ed23eb20e5dc3d09 N/A
babblesim_ext_2G4_device_WLAN_actmod None 9cb6d8e72695f6b785e57443f0629a18069d6ce4 N/A
babblesim_ext_2G4_device_burst_interferer None 5b5339351d6e6a2368c686c734dc8b2fc65698fc N/A
babblesim_ext_2G4_device_playback None 85c645929cf1ce995d8537107d9dcbd12ed64036 N/A
babblesim_ext_2G4_libPhyComv1 None 9018113a362fa6c9e8f4b9cab9e5a8f12cc46b94 N/A
babblesim_ext_2G4_modem_BLE_simple None ce975a3259fd0dd761d371b60435242d54794bad N/A
babblesim_ext_2G4_modem_magic None cb70771794f0bf6f262aa474848611c68ae8f1ed N/A
babblesim_ext_2G4_phy_v1 None cf2d86e736efac4f12fad5093ed2da2c5b406156 N/A
babblesim_ext_libCryptov1 None eed6d7038e839153e340bd333bc43541cb90ba64 N/A

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@aescolar aescolar requested a review from carlescufi June 7, 2023 14:38
As the bsim repo is disabled by default, west list bsim -f {sha}
fails.
Instead get the revision field which works.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
@carlescufi
Copy link
Member

The changes themselves LGTM, but before we merge this we need to decide betweent this approach and #57361.

@aescolar aescolar removed the DNM This PR should not be merged (Do Not Merge) label Jun 8, 2023
@nashif nashif merged commit 9af9a81 into zephyrproject-rtos:main Jun 8, 2023
@aescolar aescolar deleted the bsim_subwest2 branch June 9, 2023 07:37
aescolar added a commit to aescolar/zephyr that referenced this pull request Apr 18, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos#55696
and
zephyrproject-rtos#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
nashif pushed a commit that referenced this pull request Apr 23, 2024
Babblesim has been in the manifest as an optional
set of west projects since
#55696
and
#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this pull request Apr 24, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos/zephyr#55696
and
zephyrproject-rtos/zephyr#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

(cherry picked from commit eddb382)

Original-Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
GitOrigin-RevId: eddb382
Change-Id: Ieee5cae1c22fc7b1ec2303cec4150d6e34cc8f85
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5482306
Tested-by: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com>
Tested-by: Dawid Niedźwiecki <dawidn@google.com>
Reviewed-by: Dawid Niedźwiecki <dawidn@google.com>
Commit-Queue: Dawid Niedźwiecki <dawidn@google.com>
jman168 pushed a commit to Gator-Motorsports/zephyr that referenced this pull request Apr 25, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos#55696
and
zephyrproject-rtos#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
smaerup pushed a commit to smaerup/zephyr that referenced this pull request May 1, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos#55696
and
zephyrproject-rtos#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment