Skip to content

Conversation

@kwryankrattiger
Copy link
Contributor

Extracted Qt6 updates from #192

Comment on lines 229 to 238
# OpenGL headers are required at build time
# GL, provided by the Qt application is required at runtime.
# Do not include GL in the "link" rpaths as GL may be loaded at runtime
# and differ from the GL that provided the headers.
depends_on("gl", when="+opengl", type=("build", "link"))
depends_on("glu", when="+opengl", type=("build", "link"))
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this.

Do not include GL in the "link" rpaths as GL may be loaded at runtime

but:

depends_on("gl", when="+opengl", type=("build", "link"))

which is exactly the same as the previous:

depends_on("gl", when="+opengl")

with the dependencies made explicit. It's also of link type, which seems to contradict the comment. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like some of the components do in fact link GL...These changes have been going on since before May, so it is possible the comment went stale.

Copy link
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

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

There are legitimate failures in pipelines:

@kwryankrattiger
Copy link
Contributor Author

There are legitimate failures in pipelines:

This PR needs #2057 which fixes issues with FindOpenGL

wdconinc
wdconinc previously approved these changes Nov 2, 2025
Copy link
Contributor

@wdconinc wdconinc left a comment

Choose a reason for hiding this comment

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

Looks good to me with the changes suggested by @alalazo .

Copy link
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

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

Still one question

Comment on lines 233 to 234
depends_on("gl", when="+opengl", type="build")
depends_on("glu", when="+opengl", type="build")
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this? I thought the fix was to also link? Are we trying to enable LD_LIBRARY_PATH with Spack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I think you are right, I don't remember why the link got dropped anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants