zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.85k stars 6.61k forks source link

Spurious Error: "zephyr-no-west/samples/hello_world" is not in a west installation #14177

Closed marc-hb closed 5 years ago

marc-hb commented 5 years ago

To Reproduce Steps to reproduce the behavior:

  1. Have (at least) two zephyr clones: one created with the west tool + the other without west
  2. Define ZEPHYR_BASE=/path/to/zephyrnowest and try to build samples/hello_word (or anything else) in zephyrnowest while west is in PATH

Expected behavior Everything builds without any error

Impact (hopefully? )minor: confusing error message

Screenshots or console output

Error: "/home/user/zephyrnowest/samples/hello_world" is not in a west installation.
Things to try:
 - Set ZEPHYR_BASE to a zephyr repository path in a west installation.
 - Run "west init" to set up an installation here.
 - Run "west init -h" for additional information.

Then everything seems to build as usual (can someone confirm?)

Environment (please complete the following information): Linux Fedora Reproduced with either zephyr-0.9.5 or "host" v1.14.0-rc1-849-g35b7fde8a60b

Additional context workaround: pip3 uninstall west

mbolivar commented 5 years ago

workaround: pip3 uninstall west

Another workaround if you want to use Zephyr with and without west on the same computer is to install west in a virtualenv. I always do Zephyr development in a Python virtualenv anyway, because it wants such exact versions of all the dependencies that I don't want to have it interfering with my --user site packages.

For managing virtualenvs I recommend:

https://virtualenvwrapper.readthedocs.io/en/latest/ https://pypi.org/project/virtualenvwrapper-win/

mbolivar commented 5 years ago

I'm a bit torn about whether to mark this an 'enhancement' or a low priority bug. @carlescufi, @tejlmand, what do you think?

marc-hb commented 5 years ago

I always liked the subtitle of the "bug" label: "when things don't work as expected". It's nice because it's orthogonal to other things like severity and priority.

So I think your question can be rephrased like this: is this "dual" use case meant to be already supported or not?

ulfalizer commented 5 years ago

This one bit me too, after I installed west to try out a fix with some Nordic stuff (in a separate directory) and then went back to main Zephyr.

Definitely feels like a bug to me. I think it would be best to not check for a west installation within the main CMake files (if that's what's going on), and keep it as a separate layer instead. Too tangled up.

mbolivar commented 5 years ago

always liked the subtitle of the "bug" label: "when things don't work as expected". It's nice because it's orthogonal to other things like severity and priority.

OK, then I'm marking this low priority, since

  1. it will only affect users who have west installed and thus presumably won't be too surprised if a monorepo zephyr installation behaves a bit strangely
  2. a workaround (install west in a virtualenv) is available that avoids the error message

@ulfalizer @marc-hb @carlescufi @tejlmand please speak up if you disagree with this.

I think it would be best to not check for a west installation within the main CMake files (if that's what's going on), and keep it as a separate layer instead. Too tangled up.

The CMake files absolutely must check for a west installation in order to decide whether or not to use west list to get a list of candidate projects to use as Zephyr modules, so this suggestion makes no sense.

andrewboie commented 5 years ago

what is the final, intended scope of west? It used to be an abstraction layer for flashing, then it migrated to a tool like google's repo for checking out code, and now it's being integrated into how we build the kernel itself.

marc-hb commented 5 years ago

I'm marking this low priority bug

Looks good as long as this message is really just a spurious message than can be safely ignored and not the sign of some non obvious side-effect(s) that I haven't discovered yet. Can you confirm @mbolivar ?

now it's being integrated into how we build the kernel itself.

Not sure what you mean @andrewboie

andrewboie commented 5 years ago

now it's being integrated into how we build the kernel itself.

Not sure what you mean @andrewboie

cd samples/hello_world
west build --board=qemu_x86
carlescufi commented 5 years ago

@mbolivar I agree with this being a low-prio bug, feels exactly like that.

ulfalizer commented 5 years ago

This wasted quite a lot of time for me, so for me it's more than a low-prio. Apparently it hit other people too.

In general, I think we should strive towards integrating the flashing helpers and the like into the main repo. West shouldn't be the thing that takes over because it's tangled up in everything and can't be cleanly separated. That's a bad way to do it.

Small and independent easy-to-understand helpers are superior to blobby framework stuff.

carlescufi commented 5 years ago

Hi @andrewboie

what is the final, intended scope of west? It used to be an abstraction layer for flashing, then it migrated to a tool like google's repo for checking out code, and now it's being integrated into how we build the kernel itself.

The scope was the set at the beginning of its development and it has never changed since. West was always going to be a meta-tool that would handle:

Please see this issue created a year ago: https://github.com/zephyrproject-rtos/zephyr/issues/6205 It was used for flashing since (almost) the beginning but that was just because the rest of the features were not ready.

carlescufi commented 5 years ago

@mbolivar my suggested fix would be to query to the bootstrapper whether we find ourselves in an installation or not (that is in fact already possible with west --version) during the detection phase. If we are not, simply set the CMake WEST variable as not found

nashif commented 5 years ago

Error: "/home/user/zephyrnowest/samples/hello_world" is not in a west installation.

west installation is confusing to start with, what is a west installation? We need to make sure we are talking about Zephyr here and not West.

a workaround (install west in a virtualenv) is available that avoids the error message

IMO this just will dig us deeper. All of the sudden we have west in virtualenv, installed on system level, boot-strappers, a clone of west, .west, ... This is starting to get very confusing, and I have been following the development of west since the beginning.

marc-hb commented 5 years ago

I'm sure virtualenv is great for west developers. For me uninstalling and re-installing west takes a couple seconds max. Better having two workarounds to choose from than zero I guess :-)

I always do Zephyr development in a Python virtualenv anyway, because it wants such exact versions of all the dependencies that I don't want to have it interfering with my --user site packages.

Fair enough, I didn't notice: https://github.com/zephyrproject-rtos/zephyr/blob/588d7b068c91/scripts/requirements.txt

PS: could @mbolivar, @carlescufi or some other west code expert confirm this message is not the sign of any side effect and can be safely ignored? In other words a proof that this bug is actually low severity. Ignoring the message: the most convenient "workaround" ever!

carlescufi commented 5 years ago

@nashif

west installation is confusing to start with, what is a west installation? We need to make sure we are talking about Zephyr here and not West.

https://docs.zephyrproject.org/latest/guides/west/repo-tool.html#introduction I quote: "A west installation is the result of running the west init command to either create a new installation or convert a standalone zephyr repository into a multi-repo installation."

IMO this just will dig us deeper. All of the sudden we have west in virtualenv, installed on system level, boot-strappers, a clone of west, .west, ... This is starting to get very confusing, and I have been following the development of west since the beginning.

I agree, I don't think virtualenv is a good solution. Instead I proposed one here: https://github.com/zephyrproject-rtos/zephyr/issues/14177#issuecomment-471222141

@marc-hb

could @mbolivar, @carlescufi or some other west code expert confirm this message is not the sign of any side effect and can be safely ignored? In other words a proof that this bug is actually low severity

I think it's a bug because we promised you'd always be able to build (though not flash) without west, and we are not pulling code from external repos that would require west to list them yet. See here: https://docs.zephyrproject.org/latest/guides/west/without-west.html

mbolivar commented 5 years ago

@carlescufi

The scope was the set at the beginning of its development and it has never changed since. West was always going to be a meta-tool that would handle:

I agree with your pointing out that this scope was written down a long time ago, but actually disagree with this being the "final" scope of west. I think of it as being the convenient front-end tool for doing zephyr development. From my perspective we can add features to it whenever it makes sense as we go.

I agree, I don't think virtualenv is a good solution.

Workaround, not solution :). But as stated, I believe virtualenvs are a good idea for independent reasons.

my suggested fix would be to query to the bootstrapper whether we find ourselves in an installation or not (that is in fact already possible with west --version) during the detection phase. If we are not, simply set the CMake WEST variable as not found

Good idea -- that will work even in a monorepo zephyr tree with ZEPHYR_BASE set.

@nashif

IMO this just will dig us deeper. All of the sudden we have west in virtualenv, installed on system level, boot-strappers, a clone of west, .west, ... This is starting to get very confusing, and I have been following the development of west since the beginning.

You don't understand what I'm saying.

Again, this was just a suggested workaround for not getting the (harmless) error message for people who:

  1. have west installed
  2. have a standalone zephyr tree which they don't want to use west with

And like I said, I would recommend using a virtualenv to anybody doing Zephyr development regardless of if they are using west.

Beyond keeping our extremely exacting version requirements confined to their own sandbox, blowing away a virtualenv and setting it up again is a lot easier than trying to fix things when Zephyr-related pip commands break a ~/.local that's also used for unrelated Python work.

mbolivar commented 5 years ago

@andrewboie

now it's being integrated into how we build the kernel itself.

You seem like you might be frustrated about west, which I can understand since workflow changes are always annoying, but this statement is missing an important (but subtle) point.

You'll always be able to build Zephyr's kernel without west. But Zephyr is much more than a kernel; it's a batteries-included RTOS which integrates with a lot of other code, parts of which live in ext/ today. And as has been discussed ad nauseam elsewhere (including the issue @carlescufi linked to) all the code in ext/ is going to get split into its own repositories soon/eventually.

Given that, the build system for Zephyr applications which want to use that code needs a way to find out where those repositories are somehow, since ZEPHYR_BASE on its own will no longer be enough. Since those repositories' locations in the west installation are fully specified in the west manifest, calling west list --format "{abspath}" from the application's build system to get a list of those directories -- which is the only non-flashing/debugging reason to call west from cmake at all -- makes perfect sense.

In fact, I would argue that doing anything else would be duplicative and redundant work.

marc-hb commented 5 years ago

I cherry-picked the #14251 fix and confirm the error message is gone, thx!

ulfalizer commented 5 years ago

FWIW, I've never felt the need for or used virtualenv, and I don't think it should be considered part of some basic toolbox, especially not in a project with a lot of people who don't code Python.

marc-hb commented 5 years ago

virtualenv aside, getting into Zephyr with zero knowledge or Python and no intention to learn any is probably not a good idea. Getting into any project while trying to completely ignore how it's made has never been a recipe for success.