For missing files, the new output looks like this:
FATAL ERROR: manifest file not found: /home/mbolivar/zp/zephyr/nosuchfile
Please check manifest.file and manifest.path in /home/mbolivar/zp/.west/config
The old output looks like this:
FATAL ERROR: file not found: /home/mbolivar/zp/zephyr/nosuchfile
The new output is clearly more helpful.
Make a similar improvement for permission errors.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This was requested by a user in a comment in issue #144.
The purpose is to make it easy to run commands on interesting subsets
of projects in a convenient way.
See help text for details.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
By default, the Python interpreter will dump stack if you interrupt
the process by pressing control-C at the terminal (or otherwise send
SIGINT):
$ python my_script.py
^CTraceback (most recent call last):
<stack trace goes here>
KeyboardInterrupt
This is true on Unix and Windows. For more details on the
interpreter's overrides to the default signal handler, see:
https://docs.python.org/3/library/signal.html#note-on-signal-handlers-and-exceptions
West catches KeyboardInterrupt because we do not want to dump stack in
this case. This is an expected exception, and west only wants to dump
stack on unexpected exceptions.
However, we shouldn't be exiting with status code 0 in this case,
because we did not successfully complete the west command.
Instead, we should use the conventional nonzero exit code that command
line programs respond with when they exit asynchronously due to
SIGINT. This code varies by platform, and following conventions
properly influences the behavior of the caller's environment to make
our exit cause clear. One important case is when west is run as part
of a shell or .bat script that checks "$?" or "%ERRORLEVEL%"
respectively to determine what happened with the west process.
On Unix, exiting with the expected status code is not enough, however.
If that's all we do, the calling shell will not detect that we were
interrupted. This results in badness in situations like this (tested
in bash on Linux):
$ while true; do west foo ... ; done
Setting the "I was interrupted" exit code alone is not enough to
signal to the shell that the west process was interrupted: the while
loop will still continue forever, which is not what the user wants.
Instead, delegating to the platform's default SIGINT handler on Unix
properly informs the shell that the child was interrupted and control
flow should exit the loop.
Use platform-specific exception handling to fix these issues.
See comments for references to more details.
Fixes: #636
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is not a real west release. It is just a marker that we have
forked v1.0-branch off of main, and that main is moving independently
of the release branch.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Commit 2ac3e279ff
("WestCommand: use Configuration objects") introduced a
configuration attribute to WestCommand instances, and used
it to look up configuration options.
However, it stored this attribute at the wrong time in
WestCommand.run(). We need to stash the configuration before doing
anything else, because otherwise calls like self.die() in the same
method will look up self.color_ui, which looks up the configuration,
which isn't present, which calls self.die(), leading to an infinite
recursion.
If we stash the configuration immediately, we instead have a valid
configuration option and we get an error printed instead.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
I'd like to call the next west release 1.0, with accompanying
stability guarantees, so the deprecation timeline for west.log needs
to move to v2.0 to satisfy semantic versioning.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add a regression test for
https://github.com/zephyrproject-rtos/west/issues/545.
Mark it xfail for now since we don't have a fix. We'll remove that
once we have the fix in place.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The west package contains type annotations, but mypy doesn't know
about them because of a missing metadata file. This prevents us from
type-checking calls into the west APIs, which we should be able to do.
Add the necessary metadata to make this work.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The message is only printed if the ``level`` parameter is _lower or
equal to_ (not "at least) the current verbosity level.
We need a "sign errors" joke similar to this one:
https://www.martinfowler.com/bliki/TwoHardThings.html
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Since west v0.11.1, this must be a branch or tag name.
See e283d9986f
("init: clone the manifest repository, don't fetch it")
for more details.
The 'revision' language in the --mr help text is at best misleading,
since a manifest file's 'revision' can be a SHA, while this option
must take a branch or a tag. Fix this by being explicit.
Fixes: e283d9986f
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This prevents python's argparse library from allowing it to
generate its own shortened command line options which can cause
problems when commands are added in future with similar names
to existing arguments.
Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Fix this issue reported by newer versions of mypy (the error message
is self-explanatory):
src/west/commands.py:577: error: Incompatible default for argument
"manifest" (default has type "None", argument has type "Manifest")
[assignment]
src/west/commands.py:577: note: PEP 484 prohibits implicit
Optional. Accordingly, mypy has changed its default to
no_implicit_optional=True
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add test cases for this option, making sure not to use it in every
situation where we run west update with submodules to maintain
coverage for the case where it's not given.
Note that this is actually a work around for the test suite failing
using recent versions of git as well.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This option is necessary in some edge cases (including west's own test
suite) to work around new git behavior discussed in:
https://github.blog/2022-10-18-git-security-vulnerabilities-announced/#cve-2022-39253
Since 'west update' uses 'git submodule update --init --recursive' to
clone submodules, users may run into problems in (likely rare)
situations where they are updating a submodule from a "remote"
repository which is actually a file on the local host with symlinks
under .git. In this case, the 'west update' will fail because the file
protocol is disallowed at the 'git submodule update' step.
We don't want to force users (including our own test suite...) to
allow this protocol globally, since upstream git is telling us that is
a security problem. But we do want to allow that protocol to be
enabled on a case-by-case basis within west when the repository is
known not to be malicious. This option allows us to do exactly that by
running:
west update --submodule-init-config protocol.file.allow=always ...
Fixes: #619
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is a work-around for:
https://github.blog/2022-10-18-git-security-vulnerabilities-announced/#cve-2022-39253
Our test cases are failing since they are using 'git submodule add' on
a URL which is a file. Recent versions of git are now rejecting this
by default due to the above CVE. We can override this default behavior
on a per-git-command basis by overriding the configuration option for
the duration of that process. There is no risk here since the
associated repositories are not maliciously crafted.
This work-around follows a suggestion here:
https://bugs.launchpad.net/ubuntu/+source/git/+bug/1993586/comments/3
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Test that west leaves the local repo alone when HEAD~0 is used.
HEAD~0 is introduced in the docs as the canonical way of doing this.
HEAD~0 is used instead of HEAD because HEAD causes west to instead fetch
the remote HEAD.
Signed-off-by: Øyvind Rønningstad <oyvind.ronningstad@nordicsemi.no>
Fixes#600
De-duplicate the `option.split('.', 1)` "parsing" code into a new,
minimalistic and centralized `_InternalCF.key_parse` function. Add two
lines of error handling in this new function in order to provide a
meaningful error message instead of this cryptic and context free exception
when using a "build_board" key name without a dot:
```
...
File "/usr/lib/python3.10/site-packages/west/configuration.py", in _get
section, key = option.split('.', 1)
ValueError: not enough values to unpack (expected 2, got 1)
```
The API user now gets this:
```
...
File "/usr/lib/python3.10/site-packages/west/configuration.py", in parse_key
raise ValueError(f"Invalid key name: '{dotted_name}'")
ValueError: Invalid key name: 'build_board'
```
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Verify that the user-provided option name contains a '.' before
passing it off to API functions that require that.
Fixes: #600
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Remove most west.log API calls by delegating the output to the
WestCommand instance we are using.
Most cases are handled by saving the output we want to print in an
instance attribute 'queued_io', which gets flushed by the command
itself as a pre-run hook.
Any I/O we need to do in exception handling blocks following the
execution of the command can just use the command directly.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Command instances are now responsible for I/O, but commands can't
actually *do* any I/O until they've stored a WestCommand.config
instance, because they need the value of 'color.ui' to decide whether
or not to use color.
This presents a problem for refactoring main.py to get rid of west.log
APIs, because main.py makes a lot of decisions about warnings and
debug messages that should get printed before we've established which
command we are going to run.
Solve this by adding a deferred I/O mechanism to WestCommand. This is
a general purpose API because "why not", but it's really meant for
main.py to use. I doubt anybody else is actually instantiating
WestCommand instances.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Adjust the default verbosity by the amount implied by the number of
--verbose arguments given to west, for built-in commands.
This intermediate commit will be reworked later when main.py removes
its west.log calls. It's here for bisectability while we're reworking
other modules to use WestCommand I/O methods instead of west.log.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is basically a duplicate of the west.log interface, except with:
- better names
- no use of global state
- support for a 'quiet' mode
The goal is to move over built-in commands to using this interface in
order to eliminate global state. That in turn may help us with our
goals of running more west tests in parallel and is just better design
anyway since the west APIs should be clean when used as a library
outside of any command.
We can move over extension functions in zephyr etc. over time as well,
probably also by using a helper that can detect older versions of west
and work anyway.
That will then allow us to deprecate west.log, removing it in the long
term, and adding support for a global "west --quiet <command> args"
style of invocation for the folks who just hate terminal output. I
want to get this conversion all done by the time Zephyr LTS3 is done,
so it's time to get the ball rolling now. I missed the boat on LTS2
and that makes me sad.
Part of: #149
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The dataclass feature was added in Python v3.7, so we couldn't use it
while west still supported v3.6. West now requires v3.8 or later, so
we can use it.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Upstream Zephyr has moved to python v3.8 as a minimum version, so it's
OK for west to move too. Make that happen.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Extension commands can fail to load early in the process, when we load
the extension command information from the file system. We are not
currently handling this in a nice way, resulting in tracebacks instead
of sensible error messages.
Fix this by catching ExtensionCommandError and using similar error
handling as we are already doing from within run_command().
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Be clearer about what happened when we lack permissions to load the
manifest, but we need to do so in order to proceed. This can happen,
for instance, if it is owned by another user and read access is
denied.
Fixes: #579
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Move a few things around to group the manifest-related exception
handling together a bit better. This will make a future patch cleaner.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Commit 2327610945
("manifest: ManifestImportFailed: relax argument specification")
changed the attributes in ManifestImportFailed instances, but the
corresponding error handling code in main.py was not updated.
This is incorrect and results in an ugly traceback instead of a
properly formatted error message.
Restore the preferred behavior by using the new attribute 'imp'.
Fixes: #588
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Commit e8080e9c3f
("manifest: simplify and fix get_projects()") caused a regression in
the west command line interface, which turns out to be due to a bug in
main.py. The fix itself seems fine.
The bug is in main.py's set_zephyr_base() function, where we search
for any project named or with path 'zephyr' like this:
projects = manifest.get_projects(['zephyr'])
This was always incorrect: when searching for a project by path,
we cannot make any assumptions about the current working directory.
This line of code happens to work if you are running west from the
workspace topdir, which is what we do in testing, but it fails if you
are somewhere else in the file system, e.g. in WEST_TOPDIR/zephyr.
We happened to get bizarrely lucky before this fix to get_projects(),
because prior to that fix, we were comparing
Path(THE_MANIFEST_REPOSITORY_PATH).resolve()
with
Path('zephyr').resolve()
in the get_projects() call from set_zephyr_base().
As long as THE_MANIFEST_REPOSITORY_PATH is 'zephyr' (which it is by
default when using the upstream zephyr repository as the manifest
repository), this comparison will always result in get_projects()
returning the ManifestProject no matter where you are on the file
system, since we're comparing the same things to each other.
But *with* that fix in manifest.py, we need to also fix main.py to
pass a fully resolved WEST_TOPDIR/zephyr path to get_projects() when
searching by path, since what we are trying to do is find a project
with either name 'zephyr' or path 'zephyr' inside the current
workspace.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Commit c757d6650f
("configuration: add Configuration class") was part of a general
re-work of how configuration file handling went.
As part of this change, an internal _whence() method was added, which
returns a list of configuration file paths on the file system for a
given ConfigFile enumerator. _whence() is currently erroring out with
a RuntimeError() when a local configuration file is requested, but not
found.
This breaks error handling in some cases.
For example, consider a flow like this:
# This command fails for some reason, leaving a .west
# directory but no .west/config
$ west init
# This command bombs out with RuntimeError: a topdir is
# found, so main.py tries to create a Manifest, which asks
# for the 'manifest.path' configuration option from the
# local file, which ... boom
$ west list
It would be better to raise MalformedConfig from here instead. This
lets higher layers handle this error better. In the above case,
we now get this instead:
$ west list
FATAL ERROR: can't load west manifest
local configuration file not found
This is clearly better. There are probably still some other error
handling edge cases that aren't being handled properly as a result of
this change, but I'd be curious to know how many of them this fixes.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
It makes more sense to define the MalformedConfig exception from the
west.configuration module. Move it there. We'll use it in a subsequent
patch from the same west.configuration module. (This is currently not
possible without a circular dependency because west.manifest imports
west.configuration.)
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Without a file argument, we should still use fall_back=True to search
for the workspace in the environment. This behavior was removed in the
recent rework of path handling, introducing a regression where the
manifest repository cannot be found when outside the workspace. Fix
it.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
(cherry picked from commit d275142374)
Martí Bolívar