This relates to issue #38, the TL;DR of which is that we've never
gotten around to properly separating west's application internals from
its API in source code, and instead relied on documentation to spell
out exactly what the API was that users could rely on.
Let's start fixing that by moving everything users can't rely on into
a new west.app. This includes everything in west.commands except the
__init__ module, so we can just make that a new src/west/commands.py
file and have it be a module instead of a package. This lets its
pykwalify schema file, west-commands-schema.yml, sit next to it in
src/west, which is flatter than before.
The code changes in this commit (source lines changed, rather than
files just getting moved around) are:
- change the entry point in setup.py to west.app.main:main
- change some imports in src/west/app/main.py to import from
west.app instead of west.commands
- add a new src/west/app/__init__.py, since we're not using
namespace packages
- changes in MANIFEST.in and test_help.py to reflect new paths
- adjust some comments and docstrings
This change makes the API divide clearer. This in turn exposes some
problems with our use of west.log from west.manifest:
1. logging to stdout is a bad practice from a library
2. west.log also uses global state (which relates to #149 also)
which non-command-line users won't have set up properly
(this is an example of why #1 is true)
Subsequent commits will move to only using west.log from within
west.app.* and the existing deprecated west.build and west.cmake APIs,
which users should be migrating away from anyway.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This completes the feature set for west manifest imports. We include a
test of name whitelists from the documentation as a basic sanity
check. Further testing is left to future work.
To close out this feature after this, we should only need more error
handling and testing. In particular, beyond general coverage issues,
we need to try to reject any obvious import cycles, and add tests for
the same.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add additional line coverage for manifest.py using output from pycov.
I'm pretty happy with the resulting level of statement coverage (94%)
and the number of bugs that got shaken out by writing these (5 real
ones along with a couple of other improvements).
The remaining 6% is mostly trivial or part of the unimplemented
functions needed to import maps. The trivial stuff can get addressed
some other day if we're interested in getting to 100%, but I think
it's time to move on to implementing map imports after this, now that
we've got better test coverage for the manifest API in general and its
basic import feature in particular.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Improve test coverage for when west init ought to die with an error
because it's being asked to re-bootstrap something that's already
initialized.
- bootstrap init must not initialize a subdirectory that's already
- subprocess failure is not enough: we need to see 'already
initialized' in the error output, or it could be some other failure
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The term "west installation" is inaccurate since 0.6, since we no longer
clone west into .west/west.
The term "workspace" is more familiar and seems to be more readily
understood, so switch to that instead. Keep backwards compatibility
with the requires_installation kwarg and attribute to WestCommand for
now.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Use f-strings instead of str.format() or string concatenation with +
whenever practical. Tweak a few log messages while we're here.
I may be getting a little f-string-happy, but I think having a single,
consistent style throughout the code base makes things easier to read,
and f-strings look like the natural default choice to me.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add a remote repository to the setup we create in conftest.py whose
revision is a tag. Add testing for it in project.py, generalizing
update_helper() a bit while doing this to keep the code readable.
This adds coverage for basic west operations when manifest projects
have revisions which are tags.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add the API-level features required to implement manifest imports of
simple files, directories, and sequences of these.
Defer implementation of maps as import keys and import loop detection
for now. There's already quite a bit to test here. We'll add the rest
in follow-up commits, once we've fleshed out the built-in commands.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The main() function has gotten really long, and it's getting hard to
read. I want to break it up, but there's a fair bit of interdependent
state to manage.
Rather than using global variables, create a WestApp instance to
contain them all and use it instead. This helps with readability
without making #149 harder to do.
We'll extend this later to handle manifest imports.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Let's just get rid of this entirely to make it super clear that those
sections only have a meaning within the parsing of a single manifest
file, and are not relevant to the abstract representation of a
manifest.
This lets us rewrite the Project() constructor so it matches the tuple
representation in the manifest imports documentation quite closely.
This should hopefully make the API easier to use and understand.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The code being tested is fine; there are just issues in the test cases
which make them fail on Windows.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
We are getting an extra newline at the end of our help, which is
inconsistent with how argparse usually works and is causing tox issues
on Windows.
Avoid having an empty line at the end of help messages.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Functional differences:
- Project and ManifestProject don't use __slots__ anymore
- you can no longer set ManifestProject.revision
- the Manifest remotes, defaults, get_remote() attributes/method are gone
- the Project remote attribute is gone
- frozen dictionaries have inlined URLs now, not remotes/defaults
These breaking changes are going to be needed for manifest importing.
We can't keep exposing defaults and remotes objects anymore, because
the semantics for project overrides mean that they aren't sensible
concepts at the API level.
Also save enough state in Project and ManifestProject to find out when
an attribute's data was explicitly part of the manifest file, or came
from a default. This will be useful when applying project overrides.
Update test_project.py and test_manifest.py to keep up with these
changes.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Upon review, we've decided to version the manifest files using a
separate-but-related namespace to west version numbers.
Summary of changes:
- If the manifest schema changes in west X.Y, that new manifest schema
version is also X.Y. So if west 0.8 adds new manifest features not
available in 0.7, west 0.8 supports manifest schema 0.8.
- If the manifest schema does *not* change in west X.(Y+1), the
manifest schema is the same as it was in X.Y. So if west 0.9 has the
same manifest schema as west 0.8, then west 0.9 still has manifest
schema 0.8.
This change lets us pin a single manifest schema version to each set
of changes, without sacrificing our ability to say "you need at least
west X.Y to support this manifest".
Suggested-by: Carles Cufí <carles.cufi@nordicsemi.no>
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This refactoring will make it easier to handle ManifestVersionError.
Make 'west help' invoke a WestCommand too. API improvements made over
time make this easy to do.
It also has the pleasant side effect of making it really easy for
"west --help init" to be equivalent to "west help init" instead of
"west --help".
It works slightly different in case of error, though
("west --help unknown" produces different output than "west help
unknown"). I can't fix that without breaking the argparse abstraction
barrier, though, so it's OK.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
As we evolve the manifest data, we want to be able to enforce a
minimum version of west required to parse it. Add an optional
'version' key to the data to make this possible.
We have to validate this before checking the manifest against the
schema, as later versions of west will likely extend the schema in
ways that would raise errors if we used our schema to check the data.
We allow the version to be parsed as a YAML float (like 1.0) and
convert it to a string as a convenience for the user.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
It's convenient when using this API to be able to pass a string
containing YAML data instead of requiring the caller to load the YAML
data every time.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add a mandatory space between shell arguments. Fix the test cases so
they actually test this as advertised.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Use the phrase "extension commands" and print the project path along
with its name.
We don't use {name_and_path} here because users probably aren't used
to that format yet.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This just prints the top level directory of the west installation.
To fit it into the 'west help' output, tweak the section headings.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This slipped through the cracks when adding the manifest field to each
command instance in 7694f88 ("commands: save the parsed manifest from
main"). It only shows up for extensions -- built-ins work fine.
Add a regression test.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Adjust the name of the extension command provided in the test
environment, along with some related identifiers, so they're easier to
grep for. Make the help in the yaml file consistent with the python file.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Adjust some comments and remove parentheses (which are unnecessary in
assert statements in Python).
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
The west.manifest module is not in great shape:
- Though it's possible to create a manifest from data, this still
looks for a topdir and a manifest.path configuration option on
disk. This is wrong and contrary to the goal that we can parse
manifest data independently of the file system.
You can see that this doesn't really make sense in the hacks that
the west init implementation needs to make things work, and the fact
that all the tests which operate on data only have to patch
west_topdir(), etc.
- The Project definition repeatedly finds the west topdir. Even
when this is necessary, it should only be done once, and the value
cached and passed around.
- The ManifestProject abstraction makes several bizarre semantic
decisions, some of which never made sense, some of which are
just stale leftovers from when it was SpecialProject.
- The manifest_path() helper mutates the global configuration object
Add some new kwargs to the Manifest factories and constructor, which
allow the user to avoid using west_topdir() or west.configuration when
they do not want to, and propagate the new information down into
Project and ManifestProject.
This allows users to parse manifest files and data without forcing
them to be inside an actual installation (and/or to parse other
manifest files than the one currently pointed to by manifest.path), at
the cost of not having abspath or posixpath attributes in some cases.
Fix up Project by taking a new topdir kwarg and saving it in a slot.
Fix up ManifestProject so that it generally makes more sense. This now
enforces an existing requirement in the documentation that no project
can be named "manifest", as that's reserved for the manifest
repository. Fix some other thinkos as well by deprecating the revision
and url kwargs, which are SpecialProject leftovers, and taking the
path as an argument as-is, even though that means it can be None now.
I've tried to preserve backwards compatibility whenever possible,
especially in situations where the user just wants to parse the
manifest in the current west installation based on file system
searches and configuration options.
The additional kwargs to Project and ManifestProject are breaking
changes for any code which was using this module for something else,
but the lack of sense that those made before probably meant such
code didn't do anything useful, so that's okay.
This also breaks compatibility with "west list zephyr" when run
outside of the top level directory -- you need to say "west list
manifest" or give the complete relative path to zephyr now.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add a topdir argument to each of these functions:
- read_config()
- update_config()
- delete_config()
If set, then topdir/.west/config will be used as the local
configuration file if necessary. This allows users of this module to
decouple themselves from west.util.west_dir() without having to resort
to temporarily setting environment variables.
Keep the tests up to date, adding a couple of test cases to make sure
the new kwarg works as expected.
While we're here, re-work the fixture setup so we don't ever have to
monkey-patch west.configuration._location() by always pointing the
WEST_CONFIG_xxx environment variables inside the tmpdir we're running
each test case from. This lets us avoid some extra work in tox.ini and
makes the resulting test code easier to read, without sacrificing any
safety in terms of touching the user's actual files.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
The control flow when there is an unknown project is wrong: this is
calling is_cloned() on None. Indent it properly and add a regression
test.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
The diff and status commands with extra arguments never worked.
Remove the invalid test cases for them. The next patch is going to
fail this sort of invocation with an error instead of silently
ignoring it.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Users have provided negative feedback about some specifics related to
the new restriction that project names must be unique.
In particular, there is a use case for fetching the same project (the
LittleVGL graphics library) into two separate paths with two separate
revisions (to allow Kconfig to select between two major versions).
Since project names must now be unique, this would require the use of
the `url` attribute instead of `remote` (since the URL is of course
the same for both "projects", "remote.url-base + name" cannot be used
without violating name uniqueness).
To make it possible to keep using remotes, add a new optional
'repo-path' attribute to each project. If given, then 'remote.url-base
+ repo-path' forms the fetch URL.
Like remote itself, 'repo-path' cannot be combined with 'url'.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Manifests which use both default remotes and projects with URL
elements may incorrectly be rejected as invalid. Fix this and add a
regression test.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
- The default west list format string is not particularly readable
most of the time. Tweak it so it's better.
- replace multi-word strings like "(not set)" with single word
equivalents like "N/A", which are easier to deal with in pipelines
to cut, awk, etc.
- add a new {sha} format specifier that always produces a 40-character
SHA, or the string N/A padded to the correct length.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
We currently have no way to delete options once set. This is not good
for users that want to delete things so they go back to their default
values. Add a library API and tests for this; we'll extend the config
command with options that use them next.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add three new environment variables which can be used to override the
configuration file locations: WEST_CONFIG_SYSTEM, WEST_CONFIG_GLOBAL,
and WEST_CONFIG_LOCAL.
Use them in the tests. This also has the beneficial side effect of
making sure that "real" system settings do not interfere with the test
environment.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
We'll use this the next patch. It's useful for isolation, because the
environment is one of the things that does live on between tests.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This prints all existing config options and their values.
It can be combined with the --system, --global, and --local options
to just list the contents of those files.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This makes sense, and matches git config's behavior too. Stay silent
by default, but log.dbg() what happened.
Add a test case, too.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add basic tests for the behavior of the west.configuration APIs for
ConfigFile.SYSTEM, which were missing since we couldn't patch out the
location to somewhere safe.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add a helper to create the configuration file if it doesn't already
exist, and call it from update_config().
Add test cases for the API-level changes, which we can do now that we
can patch out all the config file locations.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
The XDG_CONFIG_HOME variable, if set, does not specify an additional
place to look for config options: it says they should go exactly
there. Fix that. Add support for BSD platforms while we are here.
To improve testability in future patches, move the code which gets the
location for a config file into a function that we can monkeypatch
later. This will make it easier to test system level config settings.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Improve the test coverage and make sure that the command line
interface and module API work together as documented.
Improve readability by adding helpers, decreasing verbosity, and
marking the mandatory setup fixture autouse=True to cut boilerplate.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Instead of bothering to create a proper west init-based installation
when testing the configuration commands, use a fixture that just
creates what is needed, and also validates module-wide assumptions
instead of repeating them in each test case.
This improves the quality of the test by only testing what is needed,
helping to isolate these tests from unrelated errors in the manifest
handling code.
It also drops the test time from ~3.5 seconds to about ~2.5 seconds in
my Linux environment.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
We don't have west in repos_tmpdir. This is yet another old docstring
that I forgot to update when we removed the bootstrapper.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This file is giving false negatives on Windows. To fix:
- test_manifest_freeze: expect (and escape) Windows paths
- test_forall: echo may be available in cmd.exe, but single-quoted
strings sure aren't
- test_extension_command_*: handle newlines portably
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This fixes test_project.py::test_list on Windows, as otherwise quoting
and splitting the relevant paths using shlex with POSIX rules behaves
incorrectly.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Martí Bolívar