Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
treewide: overhaul manifest path handling
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 zephyrproject-rtos#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>
- Loading branch information