zephyr: bsim module must be cloned for west to work

Describe the bug When running west a prompt that looks like the following appears. FATAL ERROR: failed manifest import in bsim (tools/bsim) Failed importing "west.yml" from revision "908ffde6298a937c6549dbfa843a62caab26bfc5" Hint: for this to work: - bsim must be cloned - its manifest-rev ref must point to a commit with the import data To fix, run: west update this seems to be a little backwards since it forces pulling in additional modules, outside of the minimum necessary to build.

To Reproduce Steps to reproduce the behavior:

  1. Follow the getting started guide but replace west update with west update only_what_I_need
  2. Run a west status You will get an error message that looks like the one above.
  3. Run west build -b qemu_x86 samples/hello_world/, you get: west: error: argument <command>: invalid choice: 'build'
  4. git revert 01a12f658 makes everything work again instantly (no need to re-init, update or anything)

Expected behavior We should still be able to build without the bsim module. Or at least we should provide a work around to build without it.

Environment (please complete the following information):

  • OS: Seen on MacOS and Linux

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Reactions: 5
  • Comments: 39 (24 by maintainers)

Most upvoted comments

@nordicjm Please note that people is already using this bsim_manifest repo and the bsim project in the main manifest to fetch the simulator. We can’t just remove it, or move things around without careful thought. So, “let’s just revert that commit” is hardly a good solution. That will just cause another minor breakage for whoever is doing adhoc things (instead of a full west update), but adapted to needing bsim (by maybe doing a west update bsim in their scripting); but more importantly it will cause a worse break for those who started getting and using the simulator in this way. This was added because quite many users requested it. I can only imagined some of those are using it already.

I don’t think this matters. This is a development branch so breakages are expected regularly (e.g. #55290, #57127, #57298,…). What matters is how things will work in the future. This commit has been merged only one week ago whereas west update only what I need has been working since the start of the project; so if we cared about the past (we don’t) then this commit should absolutely be reverted.

as this seems to require meta to understand their problem and evaluate a proposed solution.

This sounds quite dismissive. Commit 01a12f658c5b broke west update only what I need in a very confusing way and there’s nothing specific to Meta about that. Meta just happened to hit this first, this commit was merged less than one week ago. Reverting 01a12f658c5b makes everything instantly work again.

west update only what I need has been working with zephyr/west.yml since forever so I think this is major regression that deserves a priority higher than “low”. This type of manifest stuffing of the default zephyr/west.yml is also crossing a major line IMHO.

Given that the bsim module is so small, is there a possibility it can be added to the main repo?

+1, if it’s small and very popular then just add it like all other repos. People who don’t want it can west update only what I need as they always have.

PS @jgl-meta: I took the liberty to add west update only what I need to your description above, please review.

Run a west command such as update or init

@jgl-meta west init will never fail this way. west update will not either. It is the rest of the commands that will indeed fail.

No, users will have it by default, at least for the time being. But you will be able to disable it permanently locally so that it doesn’t affect you at all.

This is not a solution anyone with this problem has agreed to, so doesn’t resolve anything for anyone with the above issues. @cfriedt

Removing the “release blocker”, as this seems to require meta to understand their problem, and evaluate a proposed solution.

So again (I’ve already mentioned this) - please do not single out Meta. We’ve already heard from at least 3 or 4 companies that this is a problem. Those companies include (in chronological order FWIU):

  1. Intel
  2. Meta
  3. STM
  4. Nordic

Anas also mentioned it shortly before going on vacation on Discord.

I’m not against bsim particularly, I am against this whole concept in general. Once one module is allowed, that’s it for allowing all modules to work like this.

Agreed 200%. This is simply not scalable at all.

User-friendliness is critical, especially around version control which too many developers don’t want to hear about. I understand the #55696 desire to make life easier for babblesim users but what about users of other modules? Can they all stuff their default locations in zephyr/west.yml too? This is obviously not scalable.

Ironically, all other users now have to know about the secret west update bsim trick. How user friendly!

Downloading everything by default with a plain west update is slow and not user friendly either - and obviously not scalable. It should always be possible to west update only_what_I_need out of the box.

The official zephyr/west.yml should be kept simple and point only at official zephyrproject-rtos projects, directly or indirectly. It probably does not need any import.

This is the flawed, “manifest stuffing” opt-in logic in a nutshell:

  • Stuff default manifests with many locations by default for user friendliness
  • group-disable them by default because most users don’t want them (group-filter: [-babblesim])
  • Ask users who want opt-ins to “disable the disable”

The usual, scalable opt-in approach is to simply download and drop opt-ins in a well known submanifests/ location. Almost as user friendly, a familiar approach, infinitely scalable and no “double negation”. Compose by adding, not by stuffing+remove+re-add. I already pointed at this flawed opt-in approach in 2021 in the major group filter change https://github.com/zephyrproject-rtos/west/pull/481 (west v0.10) but I was “overruled” 😃

@nordicjm Please note that people is already using this bsim_manifest repo and the bsim project in the main manifest to fetch the simulator. We can’t just remove it, or move things around without careful thought.

I don’t remember being asked for thoughts for having this change added in the first place, hence why I’m bringing it up here. This change has not been put into any releases so the true number of users it will cause problems for is not known, it has actually completely broken a private CI task I have for synchronising zephyr to a different git server because I can no longer run e.g. west list to just get a list of repos. And seemingly at least 2 members of the TSC that voted on it were not aware of the changes it entailed.

@cfriedt I think you accidentally edited my comment, so I’m reverting it and putting your comment here:

Traditionally I could get a selective clone of zephyr ready for development (i.e. west init, edit west.yml with a script to get the modules I specifically need, west update,

@nordicjm - you can also run west update [module names…] to get only the modules you need without editing west.yml.

Same here though - this is a showstopper for our workflow. I haven’t looked into the relevant west code myself, so I can’t really speculate at a solution. There were some possibilities mentioned in a linked issue though.

From my point of view as a non-TSC member, I’ll leave my comments here: this is a complete hindrance to my every day usage of zephyr. Traditionally I could get a selective clone of zephyr ready for development (i.e. west init, edit west.yml with a script to get the modules I specifically need, west update, git checkout west.yml) and develop as needed, switch branches to the many open PRs I have. This is now an impossibility, if the bsim repo is in the manifest and I do not have it pulled down by west, I’m no longer able to build or develop anything, I can’t even build the documentation because I have not got the bsim module, which is completely irrelevant for any and all development I do and I would argue for the vast majority of all development everyone does in zephyr. My vote would be on reverting the change that introduced this 01a12f658c5b8c4bd985a581e3753fd67d1ee10a and only adding it back once west is updated to support optionally loading this module if it exists or ignoring it if it has not been fetched without needing to have a separate manifest with an allow/block list.

Now it might be said that bsim is a small repo with only a minimum number of files, but if that module has been allowed to be added like this, that means any module can be added like this. What if it’s now e.g. 6 different modules with imports that have a size in the gigabyte range which similar to bsim are used by the minority of users - I’m not against bsim particularly, I am against this whole concept in general. Once one module is allowed, that’s it for allowing all modules to work like this. e.g. let’s force all hals to be required to build any project for any platform - want to build hello_world for native_posix, well let’s prevent that unless you have every vendor’s hal fetched and downloaded.

Created an issue in west to track possible improvements https://github.com/zephyrproject-rtos/west/issues/652

I updated the description - able to repro fairly easily with west status.