-
Notifications
You must be signed in to change notification settings - Fork 216
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
Remove CONFIG mode specification from find_package(glslang) #1272
Conversation
We had to add the CONFIG to make sure that compatible versions of glslang were picked up - glslang has be distributed with varying degrees of broken CMake support, it's been a case of wack a mole trying to keep things working across all platform and build combinations - it's while for a while I had to move the build into the core VSG. I don't know whether it's possible to just drop the CONFIG right now and still have it work for the most users as sometimes old 3rd party packaging can get fixed/replaced and resolve the issues we had to workaround. What platforms/build combinations have to tested against? What platforms did you have issue that necessitated the change? |
I ran into this on Windows. It also seems related to (if not the exact same issue) vsg-dev/vsgFramework#15. There's other options we could explore - such as passing in options to toggle CONFIG. But, I've worked with CMake for awhile, and typically CONFIG is not set so that maximum compatibility with user configuration is attained. |
I've used CMake since it's early days, and it's the glslang project is the first one to be so inconsistently packaged that we've had to fallback to using the CONFIG hint to make sure a modern version of the packaging is used. What error do you see with the current use of CONFIG? What version of glslang do you have installed and how do you install it? |
There's no It sounds like you're conflating module mode (where CMake looks for and uses a The linked vsgFramework issue is because, despite what it says in the readme, vsgFramework doesn't install any projects, it just builds them, and it's the install step that generates the CMake config files. For whatever reason (e.g. the All this taken into account, this PR is a red herring. There might well be a real problem that it makes go away (it's not clear from your posts that doing this has made anything work that didn't before - you've claimed it makes module mode possible, but that can't be right as there's no Having tried |
As another data point, I tried the same test but with |
Thanks for your detailed analysis. Let me be more clear on my local setup, so that you can draw conclusions from that. One thing I want to point out is that as an end user I was unable to get vsgFramework even configuring without many of the steps I'll list below. Here's what I had to do:
Turning off Even if this PR is a "red herring" as you say, I'm glad it's at-least serving as a catalyst for us to discuss these CMake issues. Hopefully through this discussion, we can land on some concrete steps (including addressing glslang upstream), to make vsg easier to build against in a variety of configurations. I want to point out that pulling and building dependencies as in-source is an important and valuable configuration. It allows developers to easily upgrade dependencies and control the build process. It also centralizes development configuration, and typically cuts down on the number of installed packages and local setup steps a user needs - which is important when trying to optimize team efforts. I think we can move this discussion into #1199, or we can create a new discussion to address the wider issue of build configurations (static, shared, in-source, installed dependencies, cross-platform, etc). |
Description
Removes the hard-coded CONFIG mode from
VulkanSceneGraph/CMakeLists.txt
Line 47 in b64dbb2
Why
This allows projects to more easily use vsg as a code dependency (MODULE mode). Projects can still use it as an installed dependency (CONFIG mode) since CMake gracefully falls-back to CONFIG mode in the event MODULE mode fails. (see https://cmake.org/cmake/help/latest/command/find_package.html#id7)
This allows developers to easily use both modes.
Note that by default CMake starts in MODULE mode (which can easily be overridden if users set
CMAKE_FIND_PACKAGE_PREFER_CONFIG ON
).Fixes # #1271
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I have a private project where I tested this as working.
Checklist: