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

Enforce consistency of gl providers #44307

Merged

Conversation

alalazo
Copy link
Member

@alalazo alalazo commented May 21, 2024

fixes #44184

Many recipes in develop use a wrong idiom of declaring a direct dependency on a provider. This might bring into the DAG two potential providers, when the one declared as a direct dependency is not the preferred one.

This PR reworks a few directives to avoid that problem, and simplifies the recipes in the process. Pipelines have been updated accordingly.

alalazo added 12 commits May 21, 2024 19:04
This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.
This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.
This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.
Instead, enforce consistency using the "gl" virtual that allows
only one provider.
@spackbot-app spackbot-app bot added conflicts core PR affects Spack core functionality dependencies gitlab Issues related to gitlab integration update-package labels May 21, 2024
Copy link

spackbot-app bot commented May 21, 2024

Hi @alalazo! I noticed that the following package(s) don't yet have maintainers:

  • mesa-demos
  • mesa-glu
  • of-catalyst
  • trilinos-catalyst-ioss-adapter

Are you interested in adopting any of these package(s)? If so, simply add the following to the package class:

    maintainers("alalazo")

If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with spack blame:

$ spack blame mesa-demos

Thank you for your help! Please don't add maintainers without their consent.

You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer.

@alalazo
Copy link
Member Author

alalazo commented May 21, 2024

@bollig I commented out the two paraview builds in the aws-isc stacks, as I think they are already covered by e4s. Let me know if it is fine with you, or if you'd like to have them back.

@bollig
Copy link
Member

bollig commented May 21, 2024 via email

Copy link
Contributor

@kwryankrattiger kwryankrattiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good. I am going to have to get used to using the [virtuals=gl] ... syntax but I think this is much cleaner in representing the GL stack overall. Thanks!

Copy link
Contributor

@kwryankrattiger kwryankrattiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alalazo !

@kwryankrattiger kwryankrattiger enabled auto-merge (squash) May 23, 2024 15:47
Copy link
Contributor

@biddisco biddisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not review everything thoroughly, but this looks much cleaner and less prone to producing conflicts/etc. Nice job.

Copy link
Contributor

@biddisco biddisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question - if the variant GL has been removed from places, and instead the virtual provider is used as the flag, one can no longer say

spec paraview +egl

but one must instead encourage spack to use egl as the provider. What is the canonical syntax for that =- something like ???

spack spec paraview ^[virtuals=gl] egl

@alalazo
Copy link
Member Author

alalazo commented May 23, 2024

@biddisco From the command line, what you wrote is correct:

$ spack spec paraview ^[virtuals=gl] egl

Otherwise you can configure the provider from YAML files:

packages:
  gl:
    require:
    - egl

@kwryankrattiger kwryankrattiger merged commit 8b7abac into spack:develop May 23, 2024
35 checks passed
@alalazo alalazo deleted the packages/clean-graphics-stack-further branch May 24, 2024 06:48
alalazo added a commit that referenced this pull request Jun 5, 2024
* glew: rework dependency on gl

This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.

* mesa-demos: rework dependency on gl

This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.

* mesa-glu: rework dependency on gl

This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.

* paraview: fix dependency on glew

* mesa: group dependency on when("+glx")

* Add missing dependency on libxml2

* paraview: remove the "osmesa" and "egl" variant

Instead, enforce consistency using the "gl" virtual that allows
only one provider.

* visit: remove osmesa variant

* Disable paraview in the aws-isc stacks

* data-vis-sdk: rework constrains to enforce front-ends

* e4s-power: remove redundant paraview

* Pipelines: update osmesa variants

* trilinos-catalyst-ioss-adapter: make gl a run dependency
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
* glew: rework dependency on gl

This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.

* mesa-demos: rework dependency on gl

This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.

* mesa-glu: rework dependency on gl

This simplifies the package and ensures a single gl implementation is
pulled in. Before we were adding direct dependencies, and those are
not unified through the virtual.

* paraview: fix dependency on glew

* mesa: group dependency on when("+glx")

* Add missing dependency on libxml2

* paraview: remove the "osmesa" and "egl" variant

Instead, enforce consistency using the "gl" virtual that allows
only one provider.

* visit: remove osmesa variant

* Disable paraview in the aws-isc stacks

* data-vis-sdk: rework constrains to enforce front-ends

* e4s-power: remove redundant paraview

* Pipelines: update osmesa variants

* trilinos-catalyst-ioss-adapter: make gl a run dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts core PR affects Spack core functionality dependencies gitlab Issues related to gitlab integration update-package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many packages do not declare correctly a dependency on gl
4 participants