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

Allow import of optional projects from manifest file #696

Open
pdgendt opened this issue Oct 16, 2023 · 7 comments
Open

Allow import of optional projects from manifest file #696

pdgendt opened this issue Oct 16, 2023 · 7 comments
Assignees
Labels
Partial imports Incomplete or changing imports are much more complicated than you think

Comments

@pdgendt
Copy link
Collaborator

pdgendt commented Oct 16, 2023

Is your feature request related to a problem? Please describe.
When using out-of-tree applications, I would like to add optional projects from the application's manifest file.
For example, I use nanopb which is an optional package in zephyr.

Describe the solution you'd like
It would be nice to support project-filter: [+nanopb] directly from the manifest file.

Currently I have to configure a workspace with:

west config manifest.project-filter -- +nanopb

While my west.yml already has it in the name-allowlist:

manifest:
  self:
    path: app

  remotes:
    - name: upstream
      url-base: https://github.com/zephyrproject-rtos

  projects:
    - name: zephyr
      revision: main
      remote: upstream
      import:
        name-allowlist:
          - cmsis
          - hal_atmel
          - nanopb
          - picolibc

Describe alternatives you've considered
I can add group-filter: [+optional] to the manifest file, but this can lead to more projects being updated than necessary if I would use import: true instead of an allow list.

@nordicjm
Copy link
Contributor

I can add group-filter: [+optional] to the manifest file, but this can lead to more projects being updated than necessary.

How? You have an allow list

@pdgendt
Copy link
Collaborator Author

pdgendt commented Oct 16, 2023

How? You have an allow list

In this particular case, yes. But I think it would be nice to have using import: true too.

@mbolivar-ampere mbolivar-ampere transferred this issue from zephyrproject-rtos/zephyr Oct 26, 2023
@mbolivar-ampere
Copy link
Collaborator

@pdgendt Have you given any thought to whether this should be higher or lower precedence than the manifest.project-filter configuration option? I guess the configuration option should have higher precedence, since it's the only convenient way to do things on a per-workspace basis. Agree?

@mbolivar-ampere
Copy link
Collaborator

Another question I would like you to answer is: how should this work across import: statements?

This is subtle and tricky. Having group-filter propagate across imports was a disaster at first and we had to reverse course and tell people not to use group-filter with import in west 0.9, essentially deprecating that release entirely: https://docs.zephyrproject.org/latest/develop/west/release-notes.html#v0-9-0

Be sure to consider what happens if you import two different projects, with conflicting project-filter values.

I seem to recall that complexities of this type are why we did not implement this in the first place.

@marc-hb marc-hb added the Partial imports Incomplete or changing imports are much more complicated than you think label Oct 27, 2023
@marc-hb
Copy link
Collaborator

marc-hb commented Oct 27, 2023

we had to reverse course and tell people not to use group-filter with import in west 0.9, essentially deprecating that release entirely

Longer story in:

This is subtle and tricky.

I tried to gather all relevant discussions in Partial imports Incomplete or changing imports are much more complicated than you think (+ click "closed")

cc:

@pdgendt
Copy link
Collaborator Author

pdgendt commented Oct 27, 2023

@mbolivar-ampere thanks for voicing your concerns, I didn't know the complexity was this deep, so it might not be something we want to bother with adding.

It's because of a recent change that the optional group is left out by default, and the "installation instructions" of our projects changed because of that. It would've been nice to update the west.yml file only (we can, but with group-filter) instead of modifying readme files, CI jobs, ...

It also looks quite odd that a project that is added explicitly to the import whitelist, is still ignored.

@harristomy
Copy link

What I did was add the "optional" to the group filter in my west.yml, and then use the allowlist to only pull in the specific modules I wanted, both from the main manifest and the optional manifest. Am I misunderstanding what you're trying to do @pdgendt ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Partial imports Incomplete or changing imports are much more complicated than you think
Projects
None yet
Development

No branches or pull requests

5 participants