-
Notifications
You must be signed in to change notification settings - Fork 863
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
cmake: Install glslang.pc #3898
Conversation
e2b9ea5
to
61771d0
Compare
# Additional sanity check the glslang.pc is correct. | ||
find_package(PkgConfig REQUIRED) | ||
pkg_check_modules(glslang REQUIRED IMPORTED_TARGET glslang) | ||
|
||
add_library(stub_pc STATIC stub.cpp) | ||
target_link_libraries(example PRIVATE | ||
PkgConfig::glslang | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensures a valid glslang.pc file is being shipped. This actually caught the CMAKE_DEBUG_POSTFIX issue.
Installs glslang.pc on all platforms except for Windows. See Test/find_package/CMakeLists.txt for full explanation. closes KhronosGroup#1715
I got this working. But I have my reservations about it. 1.) this subtly breaks installation via --prefix. There is no way around it. Trust me 2.) pkg-config doesn't have a way of supporting multi-configuration builds nicely. This is a particular issue here when the binary names are NOT the same. Only sane solution IMO was to just not install glslang.pc on Windows. 2.5.) Not supporting Windows will just spawn more issues 3.) the logic is hard to maintain. Without integration testing this is VERY error prone. 4.) this only covers the glslang library. pkg-config requires a lot of work for just 1 component to be supported properly. 5.) CMake support for pkg-config is not likely to improve. Especially because of the efforts of trying to standardize on CPS (Common Package Specification). I'm closing this PR due to my reservations (And lack of free time). |
# NOTE: Windows cannot ship a single reliable glslang.pc | ||
# | ||
# pkg-config simply doesn't have a way to handle binaries having | ||
# different names based on the configuration. While CMake configuration files | ||
# do support multi-config. | ||
# https://github.com/microsoft/vcpkg/discussions/37570 | ||
# | ||
# IE: glslangd.lib vs glslang.lib | ||
# | ||
# The debug binaries have a different name on Windows. | ||
# | ||
# CMAKE_DEBUG_POSTFIX has been part of the codebase since 2016 changing it WILL break someone. | ||
# https://github.com/KhronosGroup/glslang/issues/310 | ||
if (WIN32) | ||
if (EXISTS ${glslang_PC}) | ||
message(FATAL_ERROR "glslang.pc invalid on Windows") | ||
endif() | ||
|
||
return() | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highlighting for those curious about the Windows issue I was having.
Installs glslang.pc on all platforms except for Windows.
See Test/find_package/CMakeLists.txt for full explanation.
closes #1715