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)
This is an API break. We are still allowed to do that until west 1.0.
The west.manifest.Manifest class's path handling has become
unmaintainable and we need to overhaul it. The from_data() and
from_file() constructors are too clever and try to allow too many use
cases, which results in the guts of the manifest construction code
often not being able to tell whether it's in a real workspace or not.
This makes it difficult to know when we can safely read configuration
files or not. That in turn makes it difficult to accurately
distinguish between the 'manifest.path' value in the local configuration
file and the 'self: path:' value in the manifest data, especially when
we can take keyword arguments that say "hey, pretend this is your
path".
Additionally, there are a few assumptions in the code that pretend
that the manifest file itself always lives in the top level directory
of the manifest repository. That was a valid assumption up to the
point the 'manifest.file' configuration variable was introduced, but
it's no longer valid anymore, since 'manifest.file' can point to a
subdirectory of the manifest repository. This leaves us with some
hacks to find the git repository we live in that shouldn't be
necessary and which we can now remove with this overhaul.
Rework the whole thing so that we can correctly handle the following
situations:
- when running in a workspace, you should now use from_file() or
a newly introduced from_topdir() to make your Manifest
- when running outside of any workspace, use from_data()
From now on, we forbid creating a manifest from data "as if" it were
in a real workspace. If all you have is data, the abspath attributes
will all be None, reflecting reality.
Accordingly, rework the Manifest.__init__() method to either take a
topdir or a bit of data, but not both. Additionally, now that we have
both manifest.path and manifest.file configuration options, it's not
usually the right thing to ask for a Manifest from a *file*: what you
really usually want is a Manifest from a *workspace*, and for that you
just need a topdir. From the topdir, you can create a
west.configuration.Configuration, and from there you can read
manifest.path and manifest.file to get the complete path to the top
level manifest within its repository. For that reason, we remove the
source_file keyword argument from __init__() in favor of topdir and a
new 'config' argument.
For backwards compatibility, it's still allowed to use from_file() to
load another manifest file than the one pointed to by
topdir/manifest.path/manifest.file. However, this handling code is now
just a bit of special case trickery in from_file(), and it also
introduces a restriction that the other file is still in a git
repository in the workspace. This moves potential sources for error or
confusion out of Manifest.__init__() and the functions it calls. (This
has proven to be a useful feature in practice; I am aware of west
extensions that use it to compare the parsed contents of two different
manifest files within the same workspace.)
To migrate away from the now-outdated from_file(), introduce a new
from_topdir() factory method and use it throughout the tree. This is
clearer and more accurate code, which always avoids the hacks left
behind for compatibility in from_file().
Finally, add various new instance attributes to the Manifest class
that will let us tell the YAML path from the actual path, and the path
to the manifest file from the path to the manifest repository in the
workspace. These in turn let us remove some hacky code from the 'west
list' implementation, and allow us to rework our internals so that
ManifestProject is just a legacy remnant at this point, further
helping along with #327.
Keep tests up to date, removing obsolete code as needed and updating
cases to reflect API breakage.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Commit 3f81eb4195
("manifest: project names must be unique") fixed an issue where
projects with duplicate names were previously permitted, but the test
case file that was meant to catch regressions was incorrectly left
empty. Fix it.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Adjust some comments, docstrings, and logging calls, and do some
renaming. No functional changes expected; this will cut down on
clutter in the diffs in later patches.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Continue to accept all arguments previously accepted, but widen the
allowed argument types as follows:
- Allow passing a None argument as the project to signal the manifest
repository is where the import is happening. This will be useful for
allowing us to continue using this exception type without mandating
use of the ManifestProject class, which we'd like to get rid of
eventually.
- Generalize the 'filename' argument to allow specifying arbitrary
import data which could not be imported. This will be useful for
specifying failures that happen when importing a map by giving the
entire map that failed to import, instead of just a file name.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
It will be convenient in a future patch for west.manifest.validate()
to always return the validated dict which is loaded from YAML if all
we have is a str, so extend the behavior of the function by returning
the input dictionary it receives, or returning the dictionary it
parses.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
There's no reason to print the name; it's always "manifest".
And even if we were going to print the name, it should have been
quoted, so anyway this behavior is wrong.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
All built-in west commands are using the new WestCommand.config state
to access the configuration values. Extension commands which are
reading configuration values should migrate to using this interface as
well, as should any other out of tree users of the west.configuration
API.
Deprecation, rather than removal, is necessary since 'west build' and
other important Zephyr extensions rely on the existing API.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add a 'config' property to WestCommand. This is a
west.configuration.Configuration accessible for the duration of
do_run(). For the same reason as the 'WestCommand.manifest' property
exists the way it does, we cannot add a new constructor argument to
capture the configuration. Handle this as a kwarg to run().
Take a Configuration in extension_commands().
Handle the necessary changes in main.py. These changes should be
invisible to all the extension commands I am aware of in the wild.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Martí Bolívar