Skip to content

ninja flash when running without west #12812

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

Closed
tejlmand opened this issue Jan 29, 2019 · 22 comments
Closed

ninja flash when running without west #12812

tejlmand opened this issue Jan 29, 2019 · 22 comments
Assignees
Labels
area: Flashing area: West West utility bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@tejlmand
Copy link
Collaborator

tejlmand commented Jan 29, 2019

Issue
With west as repo tool it is possible to do ninja flash
However, west should not be mandatory for users as repo tool.
ninja flash should still work, even if west was not used for repo installation.

To Reproduce
Follow the guide:
https://github.com/zephyrproject-rtos/zephyr/blob/c2bea667641a6ccb75e1e7511ba8e8f42ef7eef9/doc/tools/west/index.rst#using-zephyr-without-west
edit: https://docs.zephyrproject.org/latest/tools/west/without-west.html

in order to setup zephyr without west
Go into any sample, build, and flash.
Using ninja flash

and you'll see:

WARNING: ZEPHYR_BASE=<path>/zephyr_multi/zephyr in the calling environment, but has been set to <path>/zephyr_multi/zephyr  instead by the command line
Using runner: nrfjprog
FATAL ERROR: command flash cannot be run in this context: Runner nrfjprog received unknown arguments ['--zephyr-base=<path>/zephyr_multi/zephyr ', 'flash']
FAILED: cd <path>/zephyr_multi/zephyr/samples/bluetooth/peripheral_hr/build && /opt/cmake-3.13.1-Linux-x86_64/bin/cmake -E env PYTHONPATH=<path>/zephyr_multi/.west/west//src /usr/bin/python3 <path>/zephyr_multi/.west/west//src/west/main.py --zephyr-base=<path>/zephyr_multi/zephyr\  flash --skip-rebuild
ninja: build stopped: subcommand failed

Expected behavior
Ability to flash using ninja flash

Environment (please complete the following information):

@tejlmand tejlmand added the bug The issue is a bug, or the PR is fixing a bug label Jan 29, 2019
@SebastianBoe
Copy link
Collaborator

NB: I have been able to do "ninja flash" from a Zephyr repo that was not cloned by west.

But I was required to mv the zephyr repo inside of a "'west' parent directory" (don't know the terminology).

So this issue is reporting that the zephyr repo should not need to be in a "'west' parent directory", right?

@tejlmand
Copy link
Collaborator Author

NB: I have been able to do "ninja flash" from a Zephyr repo that was not cloned by west.

then you most likely had the bootstrapper installed, i.e. pip install --user west, correct ?

@SebastianBoe
Copy link
Collaborator

correct.

I interpreted that this was about not requiring users to manage their repo's with west, not about not requiring that users install and set up a multirepo direcotry with west.

@tejlmand
Copy link
Collaborator Author

Root cause: arguments are passed on to underlying command (in this case nrfjprog) instead of being handled by the extension command.

Related to this issue:
zephyrproject-rtos/west#168

@tejlmand
Copy link
Collaborator Author

I interpreted that this was about not requiring users to manage their repo's with west, not about not requiring that users install and set up a multirepo direcotry with west.

Agree, this might not be completely obviously.
Having the bootstrapper installed will make some commands work.

The expectation is, that someone not wanting to use west for repo handling, may not use pip to install west at all.

@galak galak added the priority: medium Medium impact/importance bug label Jan 29, 2019
@mbolivar
Copy link
Contributor

mbolivar commented Jan 29, 2019

ninja flash should still work, even if west was not used for repo installation.

TBH, I was not expecting this to be a requirement. But it should work / be working now:

$ mkdir zephyrproject
$ git clone https://github.com/zephyrproject-rtos/zephyr zephyrproject/zephyr
$ git clone https://github.com/zephyrproject-rtos/west zephyrproject/.west/west
$ cat > zephyrproject/.west/config <<EOF
[manifest]
path = zephyr
EOF
$ cd zephyrproject/
$ export WEST_DIR=$PWD/.west/west
$ mkdir build && cd build
$ cmake -GNinja -DBOARD=nrf52840_pca10056 ../zephyr/samples/hello_world/
$ ninja
$ ninja flash

I've tested the above without west installed on my system and it worked for me. @SebastianBoe @tejlmand does that meet your requirements?

@carlescufi
Copy link
Member

@tejlmand I think we can close this one now?

@RandomReaper
Copy link
Contributor

@carlescufi, do you mean to close this bug with ninja flash not working, or only working with the hack from @mbolivar ?

I think zehpyr tools (like ninja flash) should just work as documented, this may require saying west is mandatory in the documentation.

@pabigot
Copy link
Collaborator

pabigot commented Jan 30, 2019

I also believe ninja flash should work. west flash does, but I get this:

lilith[17]$ ninja flash
[0/1] cd /mnt/devel/external/zp/zephyr/samples/boards/nrf52/mesh/onoff-app/pca10056/zephyr/cmake/flash && /usr/local/cmake-3.13.2/bin/... path does not support '/usr/bin/ninja flash', ensure west is installed and not only the bootstrapper. run 'west init' to fetch west."
West version found in path does not support '/usr/bin/ninja flash', ensure west is installed and not only the bootstrapper. run 'west init' to fetch west.
lilith[23]$ export WEST_DIR=/mnt/devel/external/zp/.west/west/
lilith[24]$ ls ${WEST_DIR}
LICENSE      README.rst  setup.py  tests    tox_deps.txt
MANIFEST.in  scripts	 src	   tox.ini
lilith[25]$ ninja flash
[0/1] cd /mnt/devel/external/zp/zephyr/samples/boards/nrf52/mesh/onoff-app/pca10056/zephyr/cmake/flash && /usr/local/cmake-3.13.2/bin/... path does not support '/usr/bin/ninja flash', ensure west is installed and not only the bootstrapper. run 'west init' to fetch west."
West version found in path does not support '/usr/bin/ninja flash', ensure west is installed and not only the bootstrapper. run 'west init' to fetch west.

@pabigot
Copy link
Collaborator

pabigot commented Jan 30, 2019

Amusingly now I get:

lilith[49]$ ninja flash
[0/1] Flashing nrf52840_pca10056
WARNING: ZEPHYR_BASE=/mnt/devel/external/zp/zephyr in the calling environment, but has been set to /mnt/devel/external/zp/zephyr  instead by the command line

and it works. In the same shell window, albeit after moving back and forth between pre-west and post-west branches (but both tests at the same commit 4c3bcba which is a recent head of master).

@mbolivar
Copy link
Contributor

mbolivar commented Jan 30, 2019

I think zehpyr tools (like ninja flash) should just work as documented, this may require saying west is mandatory in the documentation.

ninja flash has relied on west code behind the scenes for many months. For a long time, until west was ready to merge, we actually maintained a copy of west inside zephyr for this purpose. That copy has been removed now that west is merged.

So this is not a "hack": you really do need the west code to be able to run the flashers and debuggers; they have been part of west for a long time.

That doesn't mean you have to install west via pip. There are workaround in place in the build system that let you just clone the west code in the right place and still use ninja flash which is shown in my shell lines above. But if you've been using ninja flash, you've been using west and just might not have known it.

@carlescufi
Copy link
Member

@pabigot pre-west branches will add $ZEPHYR_BASE/scripts to your PATH and expect an executable west there. You will need to remove that from your PATH so that bash uses the bootstrapper

@carlescufi
Copy link
Member

@RandomReaper let's not close this since the doc is actually wrong, but there's a PR to fix it already. After that, as @mbolivar mentioned, west has always been a requirement, it was just copied inside the zephyr repo. Once we switch to having all of west in PyPi the instructions should get simpler too.

@carlescufi
Copy link
Member

carlescufi commented Jan 30, 2019

@RandomReaper alternatively you can go the easy way which is to install west via pip and then run west init -l zephyr/ around your current zephyr folder. As mentioned, neither are hacks, they are just what was expected after moving the west entry point outside of the zephyr repository.

Edit:
https://docs.zephyrproject.org/latest/tools/west/index.html#design-constraints

Optional: it is always possible to drop back to “raw” command-line tools, i.e. use Zephyr without using west (although west itself might need to be installed and accessible to the build system). It may not always be convenient to do so, however

https://docs.zephyrproject.org/latest/getting_started/getting_started.html#get-the-source-code

I think we make it quite clear that while it's possible to use zephyr without west, this requires extra effort.

@pabigot
Copy link
Collaborator

pabigot commented Jan 30, 2019

@pabigot pre-west branches will add $ZEPHYR_BASE/scripts to your PATH and expect an executable west there. You will need to remove that from your PATH so that bash uses the bootstrapper

Current master also adds $ZEPHYR_BASE/scripts to the path, and removing it would break things like sanitycheck. There is no west there, though.

I had an expectation that bisecting across the west switchover would work. I'm not sure it does/will. But west is just weird to me, so I'll keep slogging along.

@carlescufi
Copy link
Member

Current master also adds $ZEPHYR_BASE/scripts to the path, and removing it would break things like sanitycheck. There is no west there, though.

Sorry, as mentioned on Slack this is only a problem if you are juggling two or more zephyr trees, since you can then end up with an executable west in the path that interferes with the pip one.

@RandomReaper
Copy link
Contributor

@carlescufi, thank you for the clarification, I was effectively not aware that west was used behind the scene.

@mbolivar
Copy link
Contributor

mbolivar commented Jan 31, 2019

So I had a thought.

Since the runner code is inside the Zephyr repository, I could create a script, let's call it ninja_run.py, that would take a single argument, which is one of: flash, debug, debugserver, and attach.

It would then use the same infrastructure the west extension commands use to accomplish the "default" flash or debug behavior.

No command line options or other configuration would be possible: that is what west flash et al are for. However, it would restore the old functionality of letting users without the west repository run the simple ninja flash.

Would people find that useful?

@pabigot
Copy link
Collaborator

pabigot commented Jan 31, 2019

I have a recollection that there was an implicit if not explicit promise that after the transition to external west people could still to develop and use Zephyr without west. Not sure what constraints were implied (maybe only that there would be a non-west multi-repo workaround), but allowing the previously working ninja commands to work without an external west would be a good thing.

@mbolivar
Copy link
Contributor

Let me provide some historical context here.

I have a recollection that there was an implicit if not explicit promise that after the transition to external west people could still to develop and use Zephyr without west.

Yes. And that is still possible, just as it always has been, using your target's flashing tools, which west wraps in its flash and debug commands.

Flashing and debugging was actually the genesis of west. In the long distant past (say, a year and half ago), Zephyr was using shell scripts borrowed from the RIOT RTOS in the recipes for the flash, debug, etc. targets provided by the Kbuild build system we used to use.

As part of the push to make Windows a first-class citizen, I rewrote them in Python. And things were okay, because Kbuild uses Makefiles directly and make <target> invocations can take additional arguments, like make flash FLASH_ARG=--some-option-to-affect-flashing or so.

Then we transitioned to CMake, also as part of making Windows a first class citizen, and things were not okay anymore, because targets in a CMake build system cannot take arguments (except via environment variables, which did not meet usability criteria on Windows). So we decided that flashing and debugging would be the first features implemented by a generic meta-tool. We'd keep the old ninja flash etc alive because muscle memory matters, but this CMake limitation meant that we were always going to be moving these features to the then-unnamed meta-tool.

Not sure what constraints were implied (maybe only that there would be a non-west multi-repo workaround), but allowing the previously working ninja commands to work without an external west would be a good thing.

I have always tried to be clear that this was a west feature, we were keeping it on life support using the copy of west code in the Zephyr tree, and that there would be a transition when we felt west was ready for general use.

Honestly if you need to do anything nontrivial with flashing and debugging (e.g. flashing a particular board by ID number if you've got multiple plugged in, maybe as part of a script that flashes all the ones on your system) I find using west is the only way to go due to the above limitation.

That said, muscle memory does matter, so I'll see what I can do.

@mbolivar
Copy link
Contributor

mbolivar commented Feb 1, 2019

That said, muscle memory does matter, so I'll see what I can do.

This turns out to be impossible. The extension commands rely too much on modules inside west, such as west.cmake. Sorry, everyone. I tried.

@carlescufi
Copy link
Member

I am going to close this issue because, while we have always maintained that one should be able to avoid using west for repo management, the fact that west is required in order to flash and debug is not new. Furthermore the issue with the doc that triggered the opening of this issue has been resolved: https://docs.zephyrproject.org/latest/tools/west/without-west.html even for out-of-tree apps now. If there is enough requests for being able to flash and debug using exclusively the zephyr repository then let's open a new issue, in the form of enhancement and we can consider that. That said, we will transition west completely to pypi in the mid-term, which will make "having west" as simple as using pip3 to install it, no git cloning involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Flashing area: West West utility bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

7 participants