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>

src/west/app/main.py

@@ -116,7 +116,8 @@ def load_manifest(self):
             return
 
         try:
-            self.manifest = Manifest.from_file(topdir=self.topdir)
+            self.manifest = Manifest.from_topdir(topdir=self.topdir,
+                                                 config=self.config)
         except (ManifestVersionError, MalformedManifest, MalformedConfig,
                 FileNotFoundError, ManifestImportFailed) as e:
             # Defer exception handling to WestCommand.run(), which uses

src/west/app/project.py

@@ -23,7 +23,7 @@
 from west import log
 from west import util
 from west.commands import WestCommand, CommandError
-from west.manifest import ImportFlag, Manifest, MANIFEST_PROJECT_INDEX, \
+from west.manifest import ImportFlag, Manifest, \
     ManifestProject, _manifest_content_at, ManifestImportFailed, \
     _ManifestImportDepth, ManifestVersionError, MalformedManifest
 from west.manifest import is_group as is_project_group
@@ -281,15 +281,13 @@ def bootstrap(self, args) -> Path:
                      if args.manifest_rev else '') +
                     f'  You may need to remove {west_dir} before retrying.')
 
-        # Parse the manifest to get the manifest path, if it declares one.
+        # Parse the manifest to get "self: path:", if it declares one.
         # Otherwise, use the URL. Ignore imports -- all we really
         # want to know is if there's a "self: path:" or not.
-        projects = Manifest.from_file(temp_manifest,
-                                      import_flags=ImportFlag.IGNORE,
-                                      topdir=topdir).projects
-        manifest_project = projects[MANIFEST_PROJECT_INDEX]
-        if manifest_project.path:
-            manifest_path = manifest_project.path
+        manifest = Manifest.from_data(temp_manifest.read_text(),
+                                      import_flags=ImportFlag.IGNORE)
+        if manifest.yaml_path:
+            manifest_path = manifest.yaml_path
         else:
             # We use PurePath() here in case manifest_url is a
             # windows-style path. That does the right thing in that
@@ -421,14 +419,19 @@ def delay(func, project):
             # as SHAs, unless they are specifically requested, and then
             # ensures they are only computed once.
             try:
-                if (isinstance(project, ManifestProject) and not
-                        args.manifest_path_from_yaml):
+                if isinstance(project, ManifestProject):
                     # Special-case the manifest repository while it's
                     # still showing up in the 'projects' list. Yet
                     # more evidence we should tackle #327.
-                    path = self.config.get('manifest.path')
-                    apath = abspath(os.path.join(self.topdir, path))
-                    ppath = Path(apath).as_posix()
+                    if args.manifest_path_from_yaml:
+                        path = self.manifest.yaml_path
+                        apath = (abspath(os.path.join(self.topdir, path))
+                                 if path else None)
+                        ppath = Path(apath).as_posix() if apath else None
+                    else:
+                        path = self.config.get('manifest.path')
+                        apath = self.manifest.repo_abspath
+                        ppath = self.manifest.repo_posixpath
                 else:
                     path = project.path
                     apath = project.abspath
@@ -528,7 +531,7 @@ def do_run(self, args, user_args):
         # errors and printing useful messages. We re-do error checking
         # for manifest-related errors that it won't handle.
         try:
-            manifest = Manifest.from_file(topdir=self.topdir)
+            manifest = Manifest.from_topdir(topdir=self.topdir)
         except _ManifestImportDepth:
             log.die("cannot resolve manifest -- is there a loop?")
         except ManifestImportFailed as mif: