apis: west.manifest.Manifest.projects should not include the manifest itself

[mbolivar]

Trying to treat the manifest repository as an ordinary Project was doomed to failure; they're simply too different. We should instead move the valid state that's in ManifestProject today to Manifest attributes, removing MANIFEST_PROJECT_INDEX and the fact that projects[MANIFEST_PROJECT_INDEX] contains the manifest.

This will require a bit of thought into how west list should behave, though. Maybe we need to resurrect west list --all to include manifest project data like {path} and {west_commands}? And perhaps that should abort if you west list --all -f {url} or try to access some other attribute that's not defined for the manifest repository.

[tejlmand]

Trying to treat the manifest repository as an ordinary Project was doomed to failure

what kind of issues are observed with the current behavior. Could you try to list the issues you experience when the manifest repo is treated as an ordinary project.

Cause I remember there were long discussion back and forth on how to treat the manifest project.

Note, I'm not advocating against changing it, just would like to see a list of issues experienced so that everyone looking at this issue know why to change API.

[mbolivar]

Welcome back :).

The manifest repository is a repository for sure, and its Python representation should have some Project attributes (like west_commands, etc.), but it's not truly a Project:

• it doesn't have a revision
• or a url
• or, in some cases (when parsing a manifest from data without self: path: foo), a path

So by "not truly a Project", I mean this in a Liskov substitution principle sense.

Right now we hack it by setting these fields to None, but I've noticed again and again we've messed up updating the docstrings or code in ManifestProject following changes to Project, leading to errors, which is also a bad sign.

From personal experience, I also find myself writing downstream code (nrf/scripts/ncs_west_commands/ncs_commands.py) that wants to iterate over projects and ending up special-casing the manifest repository as a result of crashes on code that naively looped over them. The manifest repository doesn't really have a 'name' in the project sense, and trying to treat it as if it were a project has led to weird edge cases.

I think it was a worthy attempt and what we did made sense at the time, but experience has shown it was not a good idea in retrospect.

[mbolivar]

leading to errors, which is also a bad sign.

For example, west forall -h still says that the projects list defaults to "all projects in the manifest", which is wrong: it's actually all projects in the manifest.... plus the manifest repository, because that's not filtered out.

[tejlmand]

Welcome back :).

Thanks, hope I can still keep a little up to date on west ;-)

The manifest repository is a repository for sure, and its Python representation should have some Project attributes (like west_commands, etc.), but it's not truly a Project:

I agree that it doesn't contain all the project attributes. But thinking further ahead in time, the question is whether we should treat manifest project separately, or if it should be considered that a project may not have all those attributes.

Take a look at: https://github.com/zephyrproject-rtos/west/issues/243

Will those kinds of projects be "true projects" in current sense of true project. Those projects may not have all attributes set, just as the manifest project.

We should avoid ending in a situation where changing this API results in treating other projects types individually, such as those proposed in #243 .

that wants to iterate over projects and ending up special-casing the manifest repository as a result of crashes on code that naively looped over them

Should we consider to be able to return an iterator for projects meeting a special criteria.

• revision != none
• project type == git | svn | tarball other (in future)
• other

[tejlmand]

leading to errors, which is also a bad sign.

For example, west forall -h still says that the projects list defaults to "all projects in the manifest", which is wrong: it's actually all projects in the manifest.... plus the manifest repository, because that's not filtered out.

and such issues can happen regardless of what we do - and should have been fixed a long time ago :(

[mbolivar]

Thanks, hope I can still keep a little up to date on west ;-)

I'm very glad!

Take a look at: #243

Good point. In this case I think as long as the non-git projects show up in manifest: projects: in the manifest file we should be able to keep the model consistent.

In fact, making the change here (#327) may lead us to add another section besides projects: for fetching some non-git "things", e.g. large files, if they aren't cleanly "repositories of some kind with a name, URL, revision, path, (maybe) clone depth, and optional extension commands".

That definition for what a "project" is was how I modeled a project in the multimanifest work I am doing and I think it's nice and general for hg, svn, p5, etc.

Clone depth we copied from google repo, and various issues (e.g. https://github.com/zephyrproject-rtos/west/issues/307, https://github.com/zephyrproject-rtos/west/issues/319) show it's not thought through properly.

and such issues can happen regardless of what we do - and should have been fixed a long time ago :(

Right, but by requiring explicit inclusion of the manifest repository by having to say manifest.foo instead of getting it out of manifest.projects automatically we make them less likely in the future.

So I believe making this change means the API is easier to use correctly if manifest.projects corresponds exactly to, well, manifest: projects: in west.yml, instead of mixing in manifest: self: content.

[tejlmand]

:warning: In case you start doing some work on this, remember that Zephyr modules uses west list to obtain a list of ALL projects, as those are candidates for being a Zephyr module: https://github.com/zephyrproject-rtos/zephyr/blob/51fc680ba225c2df5f3fca9064839b825a9c0511/scripts/zephyr_module.py#L177

and in some cases the manifest project is ALSO a Zephyr module, and it would be bad to have west break backward compatibility to projects based on older Zephyr revisions. I would prefer a new west to also work if i checkout an old version of NCS, as example.

[mbolivar]

For sure, west list should continue to contain the manifest repository. It's just the internal API I want to change.