Recursive group filter imports are not working properly

[mbolivar-nordic]

NOTE: the original analysis in the issue description was wrong. See https://github.com/zephyrproject-rtos/west/issues/663#issuecomment-1572755032 for details. Keeping this here for the record.

---

The test case test_group_filter_self_import() is incorrect, which conveniently enough provides steps to reproduce.

The test case should read as follows (patch applies to https://github.com/zephyrproject-rtos/west/commit/f6f5cf686f3b039d4e4a2d5bcc07b6431970d917 or today's main):

diff --git a/tests/test_manifest.py b/tests/test_manifest.py
index 14f2941..e934b71 100644
--- a/tests/test_manifest.py
+++ b/tests/test_manifest.py
@@ -2828,7 +2828,7 @@ def test_group_filter_imports(manifest_repo):
     sha2 = setup_project('project2', '[+gy,+gy,-gz]')

     v0_9_expected = ['+ga', '-gc']
-    v0_10_expected = ['-ga', '-gb', '-gc', '-gw', '-gy', '-gz']
+    v0_10_expected = ['-ga', '-gb', '-gc', '-gw', '-gz']

     #
     # Basic tests of the above setup.

In other words, west incorrectly concludes that group gy is disabled in this scenario, when it should be enabled.

The test creates the following layout, where ./mp/west.yml is the main manifest:

───────┬────────────────────────────────────────────────────────────────────
       │ File: ./mp/west.yml
───────┼────────────────────────────────────────────────────────────────────
   1   │ manifest:
   2   │   version: "0.10"
   3   │ 
   4   │   group-filter: [+ga,-gc]
   5   │ 
   6   │   projects:
   7   │     - name: project1
   8   │       revision: ...
   9   │       import: true
  10   │     - name: project2
  11   │       revision: ...
  12   │       import: true
  13   │ 
  14   │   self:
  15   │     import: self-import.yml
...
───────┴────────────────────────────────────────────────────────────────────
───────┬────────────────────────────────────────────────────────────────────
       │ File: ./project1/west.yml
───────┼────────────────────────────────────────────────────────────────────
   1   │ manifest:
   2   │   group-filter: [-gw,-gw,+gx,-gy]
───────┴────────────────────────────────────────────────────────────────────
───────┬────────────────────────────────────────────────────────────────────
       │ File: ./project2/west.yml
───────┼────────────────────────────────────────────────────────────────────
   1   │ manifest:
   2   │   group-filter: [+gy,+gy,-gz]
───────┴────────────────────────────────────────────────────────────────────
───────┬────────────────────────────────────────────────────────────────────
       │ File: ./mp/self-import.yml
───────┼────────────────────────────────────────────────────────────────────
   1   │ manifest:
   2   │   group-filter: [-ga,-gb]
───────┴────────────────────────────────────────────────────────────────────

From https://docs.zephyrproject.org/latest/develop/west/manifest.html#group-filters, we have the following rules:

In other words, let:

• the submanifest resolved from self-import have group filter self-filter
• the top-level manifest file have group filter top-filter
• the submanifests resolved from import-1 through import-N have group filters filter-1 through filter-N respectively

The final resolved group-filter value is then filter1 + filter-2 + ... + filter-N + top-filter + self-filter, where + here refers to list concatenation.

Applying these rules, the final filter should be concatenated from ./project1/west.yml, ./project2/west.yml, ./mp/west.yml, ./mp/self-import.yml, in that order. Since neither ./mp/west.yml nor ./mp/self-import.yml have a group filter which affects gy, the result should be that it is enabled, since ./project2/west.yml enables it explicitly.

However, west doesn't do this.

[mbolivar-nordic]

Well, oops. This was actually a documentation bug, not a west bug, but I'm going to keep this open so that we can close it automatically when we turn off the xfail in the tests introduced by commit d0e6b9a54b4d176cdf474b7ae7a05e8d547cf17c.

Earlier in the same section, we see:

The resolved manifest has a group-filter value which is the result of concatenating the group-filter values in the top-level manifest and any imported manifests.

Manifest files which appear earlier in the import order have higher precedence and are therefore concatenated later into the final group-filter.

Where the "import order" is defined as:

Importing is done in this order:

1. Manifests from self-import are imported first.
2. The top-level manifest file’s definitions are handled next.
3. Manifests from import-1, …, import-N, are imported in that order.

This means that import-1 happens earlier than import-N, and should therefore have higher precedence when it comes to computing the final group filter.

That means that, instead of saying this:

The final resolved group-filter value is then filter1 + filter-2 + ... + filter-N + top-filter + self-filter, where + here refers to list concatenation.

The documentation should say:

The final resolved group-filter value is then filter-N + ... + filter-1 + top-filter + self-filter, where + here refers to list concatenation.

Notice it currently goes from filter-1 to filter-N. It should go from filter-N to filter-1.