Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'import-group-filters' top level manifest key #660

Commits on May 16, 2023

  1. manifest: clarify an internal attribute's initialization

    There's a field whose initialization procedure differs enough
    from the comment describing it that it's worth clarifying.
    
    Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
    mbolivar-nordic committed May 16, 2023
    Configuration menu
    Copy the full SHA
    08d7d00 View commit details
    Browse the repository at this point in the history
  2. commands: manifest: fix error path

    The filename attribute no longer exists. Instead, we have a better
    str() result for a ManifestImportFailed. Use it instead.
    
    Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
    mbolivar-nordic committed May 16, 2023
    Configuration menu
    Copy the full SHA
    33ebab6 View commit details
    Browse the repository at this point in the history
  3. tests: fix test_group_filter_imports()

    Fixing the test case logic causes it to fail, so mark it xfail.
    This is bug zephyrproject-rtos#663.
    
    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 f6f5cf6):
    
    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
        ───────────────────────────────────────
         manifest:
           version: "0.10"
    
           group-filter: [+ga,-gc]
    
           projects:
             - name: project1
               revision: ...
               import: true
             - name: project2
               revision: ...
               import: true
    
           self:
             import: self-import.yml
        ..
        ───────────────────────────────────────
    
        ───────────────────────────────────────
         File: ./project1/west.yml
        ───────────────────────────────────────
         manifest:
           group-filter: [-gw,-gw,+gx,-gy]
        ───────────────────────────────────────
    
        ───────────────────────────────────────
         File: ./project2/west.yml
        ───────────────────────────────────────
         manifest:
           group-filter: [+gy,+gy,-gz]
        ───────────────────────────────────────
    
        ───────────────────────────────────────
         File: ./mp/self-import.yml
        ───────────────────────────────────────
         manifest:
           group-filter: [-ga,-gb]
        ───────────────────────────────────────
    
    The west docs say:
    
      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.
    
      - https://docs.zephyrproject.org/latest/develop/west/manifest.html
    
    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.
    
    Fix the test so it matches the expected behavior. We'll fix the
    implementation in a separate commit.
    
    Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
    mbolivar-nordic committed May 16, 2023
    Configuration menu
    Copy the full SHA
    ec6a736 View commit details
    Browse the repository at this point in the history
  4. tests: add Manifest.from_topdir() helper

    This is a more convenient way to construct manifests, so we might as
    well have it around and use it to save some typing.
    
    Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
    mbolivar-nordic committed May 16, 2023
    Configuration menu
    Copy the full SHA
    72e669e View commit details
    Browse the repository at this point in the history
  5. manifest: add 'import-group-filters:' support

    This involves a rework of the group filter mechanism, which has the
    side effect of fixing its behavior to match its documentation. We can
    therefore drop the xfail on a related test.
    
    This is a top level manifest key that takes a boolean. The default
    value is 'true' and is west's current behavior. The behavior when
    this value is false can be shown by this example:
    
      manifest:
        group-filter: [-foo]
        import-group-filters: false
        projects:
          - name: proj
            import: true
        self:
          import: submanifest.yml
    
    Here, the final Manifest.group_filter attribute for the manifest
    whose data are shown above will always be [-foo], regardless of what
    whe imported manifest data from 'proj' or 'submanifest.yml' are.
    
    The purpose of this extension is to allow the combination of 'groups'
    and 'import':
    
    - In previous versions of west, we could not combine the two, because
      in pathological edge cases, we could end up resolving an import for a
      group which was later disabled by an import that happened later on.
    
    - With this new backwards-compatible enhancement to the manifest file
      format, we can combine 'import:' with 'groups:' in a single project
      whenever 'import-group-filters:' is 'false'. This is because we will
      know the final 'group_filter' attribute for the entire Manifest at
      the time we perform the import, and it cannot be changed later by
      any imports we might perform.
    
    Since this new behavior would break backwards compatibility if made the
    default, we make it explicitly opt-in by requiring
    'import-group-filters: false'.
    
    Fixes: zephyrproject-rtos#663
    Fixes: zephyrproject-rtos#652
    
    Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
    mbolivar-nordic committed May 16, 2023
    Configuration menu
    Copy the full SHA
    c70f4f6 View commit details
    Browse the repository at this point in the history

Commits on May 17, 2023

  1. tests: manifest: improve group filter coverage

    Add initial coverage for "import-group-filters: false" as well
    as some other related improvements.
    
    Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
    mbolivar-nordic committed May 17, 2023
    Configuration menu
    Copy the full SHA
    3463028 View commit details
    Browse the repository at this point in the history

Commits on May 23, 2023

  1. manifest: handle missing top level manifest file

    Improve error handling to avoid dumping stack.
    
    Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
    mbolivar-nordic committed May 23, 2023
    Configuration menu
    Copy the full SHA
    279deb6 View commit details
    Browse the repository at this point in the history