Internal discussion at Nordic indicates that the semantics for 'west
manifest --resolve' should probably be something along the lines of
"resolve the manifest but ignore the value of manifest.project-filter,
and print the resolved result". This is consistent with the way that
these commands are not affected by the value of manifest.group-filter.
However, we don't have a way to support this right now: west.manifest
has no API for loading itself but with manifest.project-filter
ignored. Such an API would be straightforward to add, but we don't
have time for that right now, as we're under time pressure to add
support for this option to resolve an issue during the zephyr v3.4
stabilization period.
For now, work around the issue by just erroring out if the option is
set at all, telling the user that they can get in touch with us
if they need this. We'll let the level of noise that results be
our guide in prioritizing this enhancement.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add support for this option. The option's value is a comma-separated
list of regular expressions, each prefixed with '+' or '-', like this:
+re1,-re2,-re3
Project names are matched against each regular expression (re1, re2,
re3, ...) in the list, in order. If the entire project name matches
the regular expression, that element of the list either deactivates or
activates the project. The project is deactivated if the element
begins with '-'. The project is activated if the element begins with
'+'. (Project names cannot contain ',' if this option is used, so the
regular expressions do not need to contain a literal ',' character.)
If a project's name matches multiple regular expressions in the list,
the result from the last regular expression is used. For example,
if manifest.project-filter is:
-hal_.*,+hal_foo
Then a project named 'hal_bar' is inactive, but a project named
'hal_foo' is active.
If a project is made inactive or active by a list element, the project
is active or not regardless of whether any or all of its groups are
disabled. (This is also now the only way to make a project that has no
groups inactive.)
Otherwise, i.e. if a project does not match any regular expressions in
the list, it is active or inactive according to the usual rules
related to its groups.
Within an element of a manifest.project-filter list, leading and
trailing whitespace are ignored. That means these example values
are equivalent:
+foo,-bar
+foo , -bar
Any empty elements are ignored. That means these example values are
equivalent:
+foo,,-bar
+foo,-bar
The lists in the manifest.project-filter options in the system,
global, and local configuration files are concatenated together.
For example, on Linux, with:
/etc/westconfig:
[manifest]
project-filter = +foo
~/.westconfig:
[manifest]
project-filter = -bar_.*
<workspace>/.west/config:
[manifest]
project-filter = +bar_local
Then the overall project filter when within the workspace <workspace>
is:
+foo,-bar_.*,+bar_local
This lets you set defaults in the system or global files that can be
overridden within an individual workspace as needed, without having to
maintain the defaults across multiple workspaces.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This adds initial support for loading a 'manifest.project-filter'
configuration option.
This option is special in that its values in the system, global,
and local configuration files are all considered at the same time,
rather than just whichever one is defined at highest precedence.
This is because the eventual purpose of this configuration option
is to allow people to deactivate or activate projects using the
configuration file system, and it seems nicer to allow people to
progressively refine a filter instead of having to synchronize
global settings that they always want applied into each workspace
they have or may create.
This patch doesn't actually do anything with the option values besides
defining the internal representation, validating the options on the
file system, and saving the results in the import context state that
we carry around while importing projects. Applying the filter itself
will come in future patches. See source code comments for details on
behavior (these will eventually make their way into the
documentation).
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
In order to support the manifest.project-filter configuration option,
we need to forbid certain characters from occurring in project names.
Making that happen is a backwards incompatible change, however, so
phase it in by emitting a warning.
We will make it a hard error if manifest.project-filter is set later
on. That way, users will only see an error if they are opting into the
latest west for now, and other users will get a warning and have time
to migrate their projects.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This will still fail or succeed in the same situations,
but it is convenient to do it this way as prep work for
future enhancements.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Commit 92c18ac55a
("main: move verbose manifest logging to project.py")
did not achieve its purpose.
We are loading the manifest long before we call the setup_logging()
methods in each of the project.py classes, so any messages from the
manifest class have long been discarded by the time that we enable
them.
Now that we have EarlyArgs in main.py, we can use that to centralize
logging and enforce the following:
- warnings and above for west APIs are emitted by default
- info and above emitted with west -v
- debug and above with west -vv or higher
Fixes: #665
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Right now if you run a zephyr extension like 'west build' outside of a
workspace, argparse says:
[...] invalid choice: ‘build’ [...]
This is because argparse's subcommand parser doesn't seem to have any
API to add a catch-all value for when the user provides an unknown
command, so it expects that exactly the subcommands we told it about
are available.
This is confusing to users, and now that we have our own EarlyArgs
parsed, we can do better by printing some west-specific help if we
aren't in a workspace:
usage: west [-h] [-z ZEPHYR_BASE] [-v] [-V] <command> ...
west: unknown command "build"; do you need to run this inside a
workspace?
as well as if you are:
usage: west [-h] [-z ZEPHYR_BASE] [-v] [-V] <command> ...
west: unknown command "foo"; workspace /home/mbolivar/zp does
not define this extension command -- try "west help"
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
We've been avoiding this for a long time, but we can't any longer
and it's time to bite the bullet.
There are some chicken-and-egg problems in our argument parsing that
can only be handled if we do some manual command line argument parsing
to determine what the common options (like --verbose) are set to,
and to determine what the command name (if any) is.
For example, we would ideally like to know the verbosity level of the
west command *before* we set up logging for the west.manifest module,
so that we can set the log level for that module appropriately.
Right now, we can't do that, because we load the manifest to figure
out what the extension commands are, and from there delegate to
argparse to count the '--verbose' options. By the time argparse runs,
then, it's too late: we already loaded the manifest, and any log
messages from west.manifest are lost.
Enable solving this and other problems by writing our own, very
limited, command line parser that just figures out the global
options (west -hvVz) and the command name. We won't need all of this
right away, but we might as well try to be complete from the start.
This is prep work only; we'll put it to work in later patches.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Moving all the "work" of main() into WestApp.run() will
help us in future patches where we need to set up logging
in a way that requires internal state from that object.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The error handling in the _load_imap() function is not being tested.
This was discovered by manually inspecting the HTML coverage data.
Add statement coverage for the error handling.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is a more convenient way to construct manifests, so we might as
well have it around and use it to save some typing.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Fixing the test case logic causes it to fail, so mark it xfail.
This is bug #663.
The test case test_group_filter_self_import() is incorrect, which
conveniently enough provides steps to reproduce.
The test case should read as follows (patch applies to f6f5cf6):
diff --git a/tests/test_manifest.py b/tests/test_manifest.py
index 14f2941..e934b71 100644
--- a/tests/test_manifest.py
+++ b/tests/test_manifest.py
@@ -2828,7 +2828,7 @@ def test_group_filter_imports(manifest_repo):
sha2 = setup_project('project2', '[+gy,+gy,-gz]')
v0_9_expected = ['+ga', '-gc']
- v0_10_expected = ['-ga', '-gb', '-gc', '-gw', '-gy', '-gz']
+ v0_10_expected = ['-ga', '-gb', '-gc', '-gw', '-gz']
#
# Basic tests of the above setup.
In other words, west incorrectly concludes that group 'gy' is disabled
in this scenario, when it should be enabled.
The test creates the following layout, where ./mp/west.yml is the main
manifest:
───────────────────────────────────────
File: ./mp/west.yml
───────────────────────────────────────
manifest:
version: "0.10"
group-filter: [+ga,-gc]
projects:
- name: project1
revision: ...
import: true
- name: project2
revision: ...
import: true
self:
import: self-import.yml
..
───────────────────────────────────────
───────────────────────────────────────
File: ./project1/west.yml
───────────────────────────────────────
manifest:
group-filter: [-gw,-gw,+gx,-gy]
───────────────────────────────────────
───────────────────────────────────────
File: ./project2/west.yml
───────────────────────────────────────
manifest:
group-filter: [+gy,+gy,-gz]
───────────────────────────────────────
───────────────────────────────────────
File: ./mp/self-import.yml
───────────────────────────────────────
manifest:
group-filter: [-ga,-gb]
───────────────────────────────────────
The west docs say:
In other words, let:
- the submanifest resolved from self-import have group filter self-filter
- the top-level manifest file have group filter top-filter
- the submanifests resolved from import-1 through import-N have
group filters filter-1 through filter-N respectively
The final resolved group-filter value is then filter1 + filter-2 +
... + filter-N + top-filter + self-filter, where + here refers to
list concatenation.
- https://docs.zephyrproject.org/latest/develop/west/manifest.html
Applying these rules, the final filter should be concatenated from
./project1/west.yml, ./project2/west.yml, ./mp/west.yml,
./mp/self-import.yml, in that order. Since neither ./mp/west.yml nor
./mp/self-import.yml have a group filter which affects gy, the result
should be that it is enabled, since ./project2/west.yml enables it
explicitly.
Fix the test so it matches the expected behavior. We'll fix the
implementation in a separate commit.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The filename attribute no longer exists. Instead, we have a better
str() result for a ManifestImportFailed. Use it instead.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
There's a field whose initialization procedure differs enough
from the comment describing it that it's worth clarifying.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
When a project is not on the `manifest-rev` it will often be on a branch
or tag. Use the `%d` format to make `west compare` show that
information.
As discussed in #643 and before, git status cannot be fully trusted to
display branch information.
Sample new output:
```
west compare
=== zephyr (zephyr):
--- manifest-rev: e40859f78712 (some_tag) Revert "dma: dw: Do ...
HEAD: e803b77463b4 (zephyrproject/main) samples: net: ...
--- status:
HEAD detached at zephyrproject/main
nothing to commit, working tree clean
```
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Isolate test_compare() from the environment's west config. This makes
the test pass again when the user sets `west config --global
ignore-branches True`. It was failing like this:
```
# By default, a checked-out branch should print output, even if
# the tree is otherwise clean...
check_output(['git', 'checkout', '-b', 'mybranch'], cwd=kconfiglib)
actual = cmd('compare')
> assert actual.startswith('=== Kconfiglib (subdir/Kconfiglib):')
E AssertionError: assert False
```
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
It will be convenient to manipulate configuration files in a safe
way from outside of test_config.py. To do this, make the config_tmpdir
global to all test modules by migrating it to conftest.py.
We still want to keep that fixture as an autouse in test_config.py,
so add a shim to keep that as-is.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Also rename internal parameter `search_for_local` to
`find_local`. `search_for_local` is easier to misinterpret as optional
(not finding a local file is fatal when True).
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
The default west manifest file in zephyrproject-rtos/zephyr
recently added a project with an 'import'.
That means that 'west build' will fail with the confusing
'invalid choice: "build"' error if you pull zephyr without
running 'west update'. This is a worse experience than what happens
when you run e.g. 'west list': running a built-in command that needs
the manifest to do its work will happily tell you you need to run
'west update' to resolve the failed import.
This is confusing people who pull and run 'west build', and we do have
enough information to do better from WestArgumentParser.error(), which
is the official argparse callback API invoked when
parse_known_arguments() fails in this situation.
Extract the error message formatting for this situation into a helper
function and use it from error() to print the same message when we
get ManifestImportError in these situations.
Make the error message take up fewer lines while we are here.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
A few concerns have been voiced about the west compare output:
1. its signal:noise ratio is too low (too many lines per
project without enough useful information per line)
2. the end of the status output from one project is hard to
separate from the status output for the next
3. it isn't obvious enough whether HEAD == manifest-rev or not
To try to resolve 2. without making 1. any worse, explicitly print
HEAD and manifest-rev if they differ, without adding any extra lines,
by consolidating some of the output into the relevant banner line.
To solve 3., indent the status output to the same level as the
'status:' text in the banner.
Fixes: #650
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This command prints information about manifest-rev and "git status"
output for a project if (and only if) at least one of the following is
true:
1. its checked out commit is different than the commit checked
out by the most recent successful "west update"
2. its working tree is not clean (i.e. there are local uncommitted
changes)
3. it has a local checked out branch (unless overridden via
the compare.ignore-branches config option or the --ignore-branches
command line option)
It does something similar for the manifest repository.
The purpose of this is to allow people who are developing on one or
more projects to quickly see if they have anything interesting going
on, or if their projects match the manifest-rev versions.
Fixes: #642
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
West requires python 3.8 or later now. No need to test for 3.6 or
earlier.
I've never received any reports of the ValueError happening for other
versions of Python, so it seems safe to remove the exception handling
as well. If people haven't been reporting, we'll get bug reports soon.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The rationale is to avoid requiring users to use 'version: 0.13' in
their manifest files now that west 1.0 has been released. We should
have done this in time for west 1.0.0, but that ship has sailed. We
can, and will, backport this to the release that becomes v1.0.1,
though.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
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>
Martí Bolívar