zephyrproject-rtos / west

West, Zephyr's meta-tool
https://docs.zephyrproject.org/latest/guides/west/index.html
Apache License 2.0
215 stars 117 forks source link

`manifest.project-filter` #664

Closed mbolivar-nordic closed 1 year ago

mbolivar-nordic commented 1 year ago

Take three at trying to provide an "import filtering" (#543) solution that will work to allow making the bsim project in zephyr/west.yml inactive, so you can west build etc without it cloned.

To test:

# set up a workspace without a bsim project any way you want. Example:
west init zephyrproject
cd zephyrproject

# exercise the new config option in a local workspce:
west list # error because bsim project has 'import' and is not cloned
west config manifest.project-filter -- -bsim
west list # works, and bsim project is not in the output (it's inactive)
west list bsim # works, prints the usual info for bsim

alternatively:

west config --global manifest.project-filter -- -bsim
west init zephyrproject
cd zephyrproject
west list # works

Add support for a new manifest.project-filter configuration option.

The option's value is a comma-separated list of regular expressions, each prefixed with '+' or '-', like this:

    +re1,-re2,-re3

Project names are matched against each regular expression (re1, re2, re3, ...) in the list, in order. If the entire project name matches the regular expression, that element of the list either deactivates or activates the project. The project is deactivated if the element begins with '-'. The project is activated if the element begins with '+'. (Project names cannot contain ',' if this option is used, so the regular expressions do not need to contain a literal ',' character.)

If a project's name matches multiple regular expressions in the list, the result from the last regular expression is used. For example, if manifest.project-filter is:

    -hal_.*,+hal_foo

Then a project named 'hal_bar' is inactive, but a project named 'hal_foo' is active.

If a project is made inactive or active by a list element, the project is active or not regardless of whether any or all of its groups are disabled. (This is also now the only way to make a project that has no groups inactive.)

Otherwise, i.e. if a project does not match any regular expressions in the list, it is active or inactive according to the usual rules related to its groups.

Within an element of a manifest.project-filter list, leading and trailing whitespace are ignored. That means these example values are equivalent:

    +foo,-bar
    +foo , -bar

The lists in the manifest.project-filter options in the system, global, and local configuration files are concatenated together. For example, on Linux, with:

    /etc/westconfig (system file):
    [manifest]
    project-filter = +foo

    ~/.westconfig (global file):
    [manifest]
    project-filter = -bar_.*

    <workspace>/.west/config (local file):
    [manifest]
    project-filter = +bar_local

Then the overall project filter when within the workspace is:

    +foo,-bar_.*,+bar_local

This lets you set defaults in the system or global files that can be overridden within an individual workspace as needed, without having to maintain the defaults across multiple workspaces.


Fixes #653

aescolar commented 1 year ago

Thanks @mbolivar-nordic, it is looking very nice so far. A minor request: It would be nice if the manifest repo was not affected by the filter. Background: I expect users will very often want to do a -.*, or +.* (for example to ignore global config, or have a clean slate), but there is no purpose of having west ignoring the manifest repo, and on the other hand west ignoring would be unexpected (as west does not really manage it)

aescolar commented 1 year ago

Regarding the "default-inactive" feature. Would it make sense to add it right away? (I guess the logic would be something like adding just before https://github.com/zephyrproject-rtos/west/pull/664/files#diff-6ef1afe17ca985687d1222e9ece942bbf4fdca50c2baa784b1dfb3504b078acbR1730 something like

​if  (ret == PFR.NONE) and (project.default_inactive)
   ret = PFR.INACTIVE

(apart from the extra changes to support that extra project key)

mbolivar-nordic commented 1 year ago

Would it make sense to add it right away?

I don't think so. There is still a fair bit of work to do to get the current feature set in a merge-able state and I think such an addition is not a blocker.

mbolivar-nordic commented 1 year ago

A minor request: It would be nice if the manifest repo was not affected by the filter

That's a bug, thanks

mbolivar-nordic commented 1 year ago

To be confirmed: west manifest --resolve desired behavior after this PR

carlescufi commented 1 year ago

For west commands that require the whole manifest to be resolved (e.g. resolve, freeze, etc), the proposed approach is to let west error out if the project-filter configuration option is set at all when invoking them. This is to avoid the local project filtering interfering with full resolution of the manifest.

mbolivar-nordic commented 1 year ago

For west commands that require the whole manifest to be resolved (e.g. resolve, freeze, etc), the proposed approach is to let west error out if the project-filter configuration option is set at all when invoking them.

Note that this is just a temporary solution. We can add full support for these commands at a later date when the enhancement is required for a concrete use case. This differs from #660, which would have prevented us from implementing these commands at all.

tejlmand commented 1 year ago

The lists in the manifest.project-filter options in the system, global, and local configuration files are concatenated together.

i'm not so sure on this. west also supports manifest.group-filter, and one could with good reasons assume that both kind of filters would behave similar.

For example if I can have:

    /etc/westconfig (system file):
    [manifest]
    project-filter = +foo

    ~/.westconfig (global file):
    [manifest]
    project-filter = -bar

    <workspace>/.west/config (local file):
    [manifest]
    project-filter = +bar_local

why can I then not have similar for groups ?

    /etc/westconfig (system file):
    [manifest]
    group-filter = +foo

    ~/.westconfig (global file):
    [manifest]
    group-filter = -bar

    <workspace>/.west/config (local file):
    [manifest]
    group-filter = +bar_local

I know project-filter supports regex'es, but that's the only syntax difference between those filters, and not enough to justify such a behavioral difference. I prefer consistency between those settings in this case. (Feel free to propose group-filter to be concatenated in same fashion as project-filter)

carlescufi commented 1 year ago

@aescolar and myself discussed this comment with @tejlmand. The inconsistency in handling the aggregation of different config levels between group-filter and project-filter is noted, @mbolivar-nordic to decide if he wants to align group-filter to the current behavior of project-filter or the other way around.

mbolivar-nordic commented 1 year ago

OK @tejlmand I will reverse course on the way that manifest.project-filter works and only use the value from the highest precedence configuration file as you suggest. Will send a follow up PR.

@aescolar I will address your nits.