west fails after init until update has been invoked when using import
tejlmand opened this issue · 7 comments
After running west init -l . then west commands such as status and list fails with a stack trace when any project in the manifest contains import.
Also extension commands in the manifest repo cannot be executed (more precisely, they are not loaded).
Can be reproduced using: https://github.com/nrfconnect/sdk-nrf/
$ cd /<path-to/ncs/nrf
$ west init -l .
$ west status
Traceback (most recent call last):
File "/home/tora/.local/bin/west", line 8, in <module>
sys.exit(main())
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 786, in main
app.run(argv or sys.argv[1:])
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 109, in run
self.run_command(argv)
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 337, in run_command
self.handle_builtin_manifest_load_err(args)
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 195, in handle_builtin_manifest_load_err
p, f = self.mle.project, self.mle.filename
AttributeError: 'ManifestImportFailed' object has no attribute 'filename'
$ west list
Traceback (most recent call last):
File "/home/tora/.local/bin/west", line 8, in <module>
sys.exit(main())
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 786, in main
app.run(argv or sys.argv[1:])
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 109, in run
self.run_command(argv)
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 337, in run_command
self.handle_builtin_manifest_load_err(args)
File "/home/tora/.local/lib/python3.8/site-packages/west/app/main.py", line 195, in handle_builtin_manifest_load_err
p, f = self.mle.project, self.mle.filename
AttributeError: 'ManifestImportFailed' object has no attribute 'filename'
$ west help
usage: west [-h] [-z ZEPHYR_BASE] [-v] [-V] <command> ...
The Zephyr RTOS meta-tool.
optional arguments:
-h, --help get help for west or a command
-z ZEPHYR_BASE, --zephyr-base ZEPHYR_BASE
Override the Zephyr base directory. The default is
the manifest project with path "zephyr".
-v, --verbose Display verbose output. May be given multiple times
to increase verbosity.
-V, --version print the program version and exit
built-in commands for managing git repositories:
init: create a west workspace
update: update projects described in west manifest
list: print information about projects
manifest: manage the west manifest
diff: "git diff" for one or more projects
status: "git status" for one or more projects
forall: run a command in one or more local projects
other built-in commands:
help: get help for west or a command
config: get or set config file values
topdir: print the top level directory of the workspace
Run "west help <command>" for help on each <command>.
if commenting out the import lines here: https://github.com/nrfconnect/sdk-nrf/blob/a4d77a87edca7bc0201e346e41389e2757cdcaf7/west.yml#L54-L95
then the above commands works, as well as west extension commands from the manifest repo is presented:
$ west status
=== status of manifest (nrf):
HEAD detached at ncs/main
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: west.yml
no changes added to commit (use "git add" and/or "git commit -a")
$ west list
manifest nrf HEAD N/A
zephyr zephyr 85a0034809612b332fbdf478357e8a4109c493fd https://github.com/nrfconnect/sdk-zephyr
mcuboot bootloader/mcuboot v1.9.99-ncs1 https://github.com/nrfconnect/sdk-mcuboot
mbedtls-nrf mbedtls 0add9081ba3ab67123ffe2590a1acc6a846a2646 https://github.com/nrfconnect/sdk-mbedtls
nrfxlib nrfxlib 2dee988030c1048cafea2c06f5fa504322c3afd3 https://github.com/nrfconnect/sdk-nrfxlib
trusted-firmware-m modules/tee/tf-m/trusted-firmware-m 37ffd937eb33a648fb2f058387bdbe972696346a https://github.com/nrfconnect/sdk-trusted-firmware-m
tfm-mcuboot modules/tee/tfm-mcuboot v1.7.2-ncs1 https://github.com/nrfconnect/sdk-mcuboot
matter modules/lib/matter 0b457ece3f0c37f5c92b21fe56cacd319874364f https://github.com/nrfconnect/sdk-connectedhomeip
cjson modules/lib/cjson c6af068b7f05207b28d68880740e4b9ec1e4b50a https://github.com/nrfconnect/sdk-cjson
cmock test/cmock 9d092898ef26ece140d9225e037274b64d4f851e https://github.com/ThrowTheSwitch/cmock
unity test/cmock/vendor/unity 74cde089e65c3435ce9aa87d5c69f4f16b7f6ade https://github.com/ThrowTheSwitch/unity
zcbor modules/lib/zcbor 3d36ae19cb05d28cea23b4c870ae965c44215e9c https://github.com/NordicSemiconductor/zcbor
memfault-firmware-sdk modules/lib/memfault-firmware-sdk 0.29.1 https://github.com/memfault/memfault-firmware-sdk
$ west help
...
other built-in commands:
help: get help for west or a command
config: get or set config file values
topdir: print the top level directory of the workspace
extension commands from project manifest (path: nrf):
ncs-loot: list out of tree unreverted NCS patches
ncs-compare: compare upstream manifest with NCS
ncs-sbom: generate Software Bill Of Materials including
license information
Run "west help <command>" for help on each <command>.
Just like running west init without a west update for Zephyr allows me to show status, list projects and run extension commands, then I would expect the same to be possible for other manifest projects.
Expectations:
Minimum
- no stack trace
Preferable
- Ability to list projects and run status
- Ability to execute available extension commands from manifest repository, without cloning other projects.
naturally imported extension commands will not be available until west update has been invoked, but I see no reason why an import statement in a manifest file should prevent users from cloning a manifest repo and run an extension commands if that is a need, without running west update.
For example in CI / github actions it will be a huge performance gain if you can avoid cloning all projects, especially Zephyr, if it is not needed for the github action.
Note: labelled bug due to the stacktrace. If stacktrace is avoided, this can be considered an enhancement.
If stacktrace is avoided, this can be considered an enhancement.
While the root cause seems different, the stack trace is interestingly the same as #586 and other currently open bugs. So maybe some changes there could help with several issues at once?
For example in CI / github actions it will be a huge performance gain if you can avoid cloning all projects, especially Zephyr, if it is not needed for the github action.
Being discussed in
For example in CI / github actions it will be a huge performance gain if you can avoid cloning all projects, especially Zephyr, if it is not needed for the github action.
Being discussed in
- https://github.com/zephyrproject-rtos/zephyr/issues/46801
Thanks for the link, although the issue I raise has a slight twist to it.
If you notice in the description, then as soon as one project has an import statement, then you must have this project cloned for anything to wortk, even extension commands from the manifest repo.
I see no reason why an import statement in a manifest file should prevent users from cloning a manifest repo and run an extension commands if that is a need, without running west update.
The reason why is that this would lead to inconsistent behavior depending on which projects were cloned.
Per the import documentation:
All extension commands defined using west-commands keys discovered while handling imports are available in the resolved manifest.
If an imported manifest file has a west-commands: definition in its self: section, the extension commands defined there are added to the set of available extensions at the time the manifest is imported. They will thus take precedence over any extension commands with the same names added later on.
In the general case, if we have unresolved imports, we cannot know the "true" source of a particular extension command. I try to be consistent about rejecting proposals that lead to inconsistent behavior depending on what you happen to have cloned; it's a recipe for disaster when debugging someone's issue remotely etc.
Expectations:
Minimumno stack trace
Agreed
Preferable
Ability to list projects and run status
Neither west list nor west status has the information it needs to run successfully without the entire manifest cloned into the local workspace, so this proposal feels a bit like "garbage in, garbage out" to me. I think it's cleaner for west to just fail here. But I agree that the stack traces are not nice.
I guess we could consider some additional flags, like west status --ignore-imports or so? Again, I really want to avoid garbage-in-garbage-out here, and that sounds like a separate enhancement.
Ability to execute available extension commands from manifest repository, without cloning other projects.
I think this is not a good idea for reasons I explained above.
Ability to execute available extension commands from manifest repository, without cloning other projects.
I think this is not a good idea for reasons I explained above.
@mbolivar-nordic Let me point you back to your own doc reference:
If an imported manifest file has a west-commands: definition in its self: section, the extension commands defined there are added to the set of available extensions at the time the manifest is imported. They will thus take precedence over any extension commands with the same names added later on.
Extension commands from the manifest repo are always loaded first, hence those commands will always take precedence over any imported extension commands.
So how can loading of those commands lead to inconsistent behavior ?
Note: i'm not asking extension commands to be loaded from imported manifests, only commands from the manifest repo itself as commands in this repo always takes precedence, as I wrote:
Ability to execute available extension commands from manifest repository, without cloning other projects.
Neither west list nor west status has the information it needs to run successfully without the entire manifest cloned into the local workspace, so this proposal feels a bit like "garbage in, garbage out" to me. I think it's cleaner for west to just fail here.
Why not tell the user what we know, and give them a warning, for example:
$ west list
manifest nrf HEAD N/A
zephyr zephyr 35357799b464e03a9fe78e9dbba9fcd539597f3c https://github.com/nrfconnect/sdk-zephyr
mcuboot bootloader/mcuboot 6097de22bd1f1b499e599bfef618a235de7073b2 https://github.com/nrfconnect/sdk-mcuboot
nrfxlib nrfxlib 766cc270caca3975fa6d924ada43177089040fcc https://github.com/nrfconnect/sdk-nrfxlib
.....
WARNING: project 'zephyr' has unresolved imports. project list is not complete.
The output here is not garbage, cause all projects listed are correct from the manifest.
Only there are missing projects because of imported projects from zephyr are not resolved.
Projects in manifest file from manifest repo takes precedence over any imported projects of same name.
In my opinion it's better to return what we do know is correct, and then issue a warning to the user (or fail if you really prefer that here), than to just fail.
@tejlmand you are missing my point. I was careful to write (emphasis added):
In the general case, if we have unresolved imports, we cannot know the "true" source of a particular extension command.
I don't think there is a good reason to give extension commands in the manifest repository special privileges over other types of extension commands. IMO most users don't know or care where extension commands come from, and certainly the manifest parsing code doesn't. Adding special cases like this just to make a particular use case for Nordic CI easier is IMO the wrong approach.
However, I think solving the Nordic CI problems is important and I already encouraged you to open a separate enhancement to try to solve them. I therefore do not think your comments are relevant to this bug. If you'd like to continue this discussion please open a new issue.