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>
The requires_installation constructor kwarg and property have been
deprecated for some time now, and they aren't in use in either Zephyr
v1.14 LTS or v2.7 LTS. It should be fine to remove them at this point.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Switch configuration file parsing to use a Configuration object.
This prserves existing functionality, but allows us to start more
easily passing around the configuration to other places that might
want it pre-loaded.
Retain existing behavior for backwards compatibility, but move the
initialization of the global west.configuration.config object over to
using the new API.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is a high-level class that lets you load and interact with the
configuration files in an object-oriented way. It's similar to how
west.manifest.Manifest lets you interact with the manifest, but for
configuration files.
The new API uses 'config.get("some.option")' style methods instead of
the configparser style 'config.get("some", "option")' with a separated
section and key. This makes the code match the options as they are
documented, making it easier to read and grep for.
Having an object around will allow us to deprecate the current
implementation, which relies on global state. It will also make it
easier to override default configuration behavior in certain
circumstances that will be useful in later patches.
Part of the road towards resolving #149 and at least one other issue.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add a kwarg that makes _location() skip the workspace search if the
local configuration file path is requested.
No behavioral changes expected. This is prep work for reusing this
helper in a new configuration API.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Tweak some incidental details and add a couple of extra asserts.
Prep work for adding another test later on.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The argparse module changed some implementation details in in a way
that's causing a false failure in testing. Handle it.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
If a project has created a tag or branch with the name HEAD (which
they really shouldn't do) it will cause the command:
'git rev-parse --abbrev-ref HEAD' to fail to provide a ref.
Check if 'git rev-parse --abbrev-ref HEAD' fails to return a value and
if so do a `log.die()` rather than failing at the 'git merge-base'
stage with a less useful error.
Closes#561
The addition of project userdata was a breaking change for older
versions of west. This means we should have bumped the schema version
when it was added, but this was missed. Do it now.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This isn't working properly because the ManifestProject's revision is
"HEAD", so it looks like an ordinary project and we try to get
manifest-rev from it. That is obviously incorrect. Fix it by detecting
the ManifestProject explicitly and doing the right thing there.
Fixes: #550
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
We no longer have access to 3.6 in the system that provides our python
interpreter, so we'll have to do without it in CI.
I think in practice this is going to mean that 3.7 is the lowest
version of Python that west supports, but that's really a Zephyr
decision and I don't feel comfortable make it unilaterally here. So
for now we'll continue to treat failures on 3.6 as bugs, but we won't
test on that platform in CI. That's not ideal and I'll try to do
release testing on 3.6 for as long as I can, as it's the best we've
got.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This reverts commit 56ddb937a0.
Parallel updates are a great feature, but they break color.ui and
attempts to fix that have caused unexpected exceptions to be thrown on
Windows, so we need to back the feature out for now until we can get
it done in a way that works with both color and Windows.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This reverts commit 14b27b0940.
Parallel updates are a great feature, but they break color.ui and
attempts to fix that have caused unexpected exceptions to be thrown on
Windows, so we need to back the feature out for now until we can get
it done in a way that works with both color and Windows.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This reverts commit b1350a668c.
Parallel updates are a great feature, but they break color.ui and
attempts to fix that have caused unexpected exceptions to be thrown on
Windows, so we need to back the feature out for now until we can get
it done in a way that works with both color and Windows.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This reverts commit 040dd57ad0.
Parallel updates are a great feature, but they break color.ui and
attempts to fix that have caused unexpected exceptions to be thrown on
Windows, so we need to back the feature out for now until we can get
it done in a way that works with both color and Windows.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This reverts commit c851664e04.
Parallel updates are a great feature, but they break color.ui and
attempts to fix that have caused unexpected exceptions to be thrown on
Windows, so we need to back the feature out for now until we can get
it done in a way that works with both color and Windows.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
For 'git fetch', neither --tags nor --no-tags is the default value. The
default value is a middle ground that fetches "any tag that points into
the histories being fetched is also fetched" - second paragraph in "git
help fetch". Rather than duplicating this relatively non-intuitive git
help in such a small space, be completely transparent about what the
west code does and lead the reader to the original git documentation.
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Make sure the userdata round-trips properly through 'west manifest
--freeze' by adding it to the project's dict representation.
Add a missing repr() inside the userdata related part of
Project.__repr__.
Fixes: #536
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add a new optional 'userdata' key to projects.
The value will be parsed and stored in the userdata attribute of the
corresponding west.manifest.Project object.
Example manifest file:
manifest:
projects:
- name: foo
- name: bar
userdata: a-string
- name: baz
userdata:
key: value
defaults:
remote: r
remotes:
- name: r
url-base: base
Example Python usage:
manifest = west.manifest.Manifest.from_file()
foo, bar, baz = manifest.get_projects(['foo', 'bar', 'baz'])
print(foo.userdata) # None
print(bar.userdata) # prints 'a-string'
print(baz.userdata) # prints {'key': 'value'}
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
`filter` is not indexable, but the parallel update implementation requires that
it is.
Before this patch, `west update -j2 PROJECT_NAME` failed with:
TypeError: 'filter' object is not subscriptable
Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
`enumerate` on `imap_unordered` does (obviously) not return the index of the
element that finished processing.
That resulted in printing the wrong log file which might not even be finished
and thus swallowing future logs of that job.
Instead, let each job return it's index.
Fixes: #528
Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
Commit 6d42388f20
("tests: adjust create_repo() to keep working on newer git") got tox
working again in Arch on my laptop, and in CI for whatever GitHub
decides to use for ubuntu-latest, but it breaks tox on my Ubuntu 20.04
LTS testing environment that I use for release testing.
Fix it, again, and hopefully for the last time. See comments in the
source code for details on the fix.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Updates for clarity and to match current practice.
Reflect some practical changes that I've been doing in the past couple
of release that cut down on review latency once it's been agreed that
it's release time. In particular, we are cutting out the review cycles
for forking the release branch, changing the versions, and tagging the
main branch after it's moved from the release branch.
Rework the section in to step 1-2-3 style while we're here, to make it
easier to follow along with.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Fixes: #523
The Manifest class has these instance attributes:
- _projects_by_name: maps project names -> Projects, has 'manifest' as
a key and the ManifestProject as a value (this is one reason why
'manifest' is a reserved project name)
- _projects_by_rpath: maps resolve()d Paths -> Projects, has the
manifest repository's path as a key, pointing to the ManifestProject
So get_projects() ought to just rely on these dicts to do its job.
However, it's currently got extra special-case code it doesn't need,
and the code is buggy. Specifically this part:
mpath: Optional[Path] = Path(mp.path).resolve()
This is trying to get the ManifestProject's resolved path, but it only
works if you happen to be in the workspace topdir, since mp.path is
relative.
We got "lucky" and can still find the manifest project by its relative
path because we fall back on _projects_by_rpath.
But this is still a correctness problem. When the current
directory *isn't* the topdir, but has a subdirectory or file inside it
whose name happens to match mp.path, we could return the
ManifestProject as a result, incorrectly.
Just remove all the unnecessary extra code and use _projects_by_name /
_projects_by_rpath to do the work.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
- names are not supposed to be paths
- it may be a mis-use of `repo-path`'s default value
- it would make using the name for log files harder
More info: https://github.com/zephyrproject-rtos/west/pull/515
Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
West init without --mr is leaving the manifest repository in a
detached HEAD state.
Fixes: #522
The convoluted logic we use to "clone" the manifest repository using
init and fetch is only in place to allow using a magic GitHub pull
request ref as --manifest-rev. This small benefit is no longer worth
the downsides.
To fix it, just use 'git clone' instead. This also lets us
drop a bunch of extra logic trying to figure out what the remote wants
us to use by default.
It also avoids getting the now-infamous "Using 'master' as the name
for the initial branch." message on the first west init if the user
doesn't have init.defaultBranch set.
Preserve 'west init --manifest-rev foo' using 'git clone --branch
foo'. This works fine with branches or tags, but you'll no longer be
able to initialize a workspace from a 'pull/1234/head' GitHub pull
request reference or a SHA. Affected users are going to need to do
'west init', then fetch the revision they need before running 'west
update'.
Unlike other branch related options (like 'git init --initial
branch'), we've had 'git clone --branch' since somewhere in the git
v1.x days, so it's safe to use unconditionally.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Now that git isn't necessarily creating that branch,
we should update our comments for correctness.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
On the git v2.32.0 that's currently running on the Arch Linux install
I use for day to day development, I'm getting errors when I run tox.
This is because the test cases assume that newly created repositories
have 'master' branches, which isn't true in general anymore unless you
ask for it explicitly.
To keep things working for now, add a kwarg to the create_repo()
helper that preserves the old way of doing things by forcing a master
branch to exist.
Use 'git init --initial-branch' for this if that's available, but
otherwise just fall back on 'git checkout -b' for older versions of
Git to keep things consistent.
Since git itself is moving away from an assumption of a default branch
named 'master', we're going to need to do the same thing in west when
it comes to the default project revision. But that's a problem for
another day; for now let's just keep things duct taped together with
existing assumptions, since revisiting that will require a manifest
schema change.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This will be useful for creating new repositories in the test cases
with newer versions of git, which starting with v2.28 are moving away
from a default initial branch.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Martí Bolívar