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
Issues using via CMake's FetchContent or add_subdirectory #3509
Comments
Looks like #3508 will resolve the installation-related problems here. |
Including
|
This makes things consistent between when glslang is installed and imported versus when it's included as nested CMake project. You can now use `glslang::glslang` in both cases instead of needing the non-namespaced version sometimes and the namespaced one other times. Resolves one of the problems discussed in KhronosGroup#3509
Not all platforms provide a new enough version of glslang that the proper CMake config files are present to just import the system installation. That means that some projects will still need to pull in the source and build it to support some platforms.
There are some problems with this, though.
Warningsglslang has code that triggers many commonly-used warnings, and by default will inherit the parent project's warnings. This could be resolved in a a couple of ways:-Wshadow
.~~Explicitly disable warnings on the glslang targets. This could be a hassle as just adding-Wno-shadow
-Wno-implicit-fallthrough
and-Wno-missing-field-initializers
(plus equivalents for other compilers) depends on the order of arguments to work, so it would also need various CMake variables and properties to be scanned and have flags enabling these warnings replaced.This is really a problem with how the downstream project's setting warning flags. You're not supposed to set warning flags globally with
add_compile_options
as that makes your warning flags affect third-party code that wasn't written with these warnings in mind.Alias
When imported through
find_package
, glslang is available asglslang::glslang
, and ancillary libraries are also namespaced likeglslang::SPIRV
. When included viaFetchContent
oradd_subdirectory
, the namespaced versions don't exist, justglslang
andSPIRV
.This can easily be resolved by adding a library alias with
which would make it always available in the namespaced form.
Installing
Since #3439, glslang can only be installed if it's the top-level project.
This is okay for:
BUILD_SHARED_LIBS
off - the static library will be linked so nothing from glslang will need to be installed.BUILD_SHARED_LIBS
on - (presumably - I haven't checked this, so could be wrong) one of the CMake features that installs runtime dependencies for a target would install the shared libraries.This isn't okay for:
BUILD_SHARED_LIBS
off - nothing's been linked, so the glslang static library needs to be installed, and CMake hasn't been told how.BUILD_SHARED_LIBS
on - nothing's been linked, so the glslang shared library (import library with MSVC) needs to be installed, and CMake hasn't been told how, although the runtime part could presumably be installed.In cases where the headers are needed because a library uses glslang headers in its public headers, which could be the case for shared or static libraries, they simply aren't installed.
Right now, the only options for downstream libraries are to duplicate a lot of glslang's CMake, or to use a version before #3439 was merged.
It might be enough to change the condition so that it's also controlled by an option, so the default behaviour is the same as now (install is enabled if glslang is the top-level project and disabled otherwise), but can be overridden if the consuming project needs something different.
Something else that would solve the particular problem I'm hitting would be if glslang could be compiled as an object library. That way, the archiver would put glslang's object files into any static libraries depending on glslang, and a linker would link them into any shared libraries or applications. The
LIB_TYPE
CMake variable could be turned into an option to enable this, but some of the checks that currently useBUILD_SHARED_LIBS
would need to be changed to checkLIB_TYPE
instead. This isn't a particularly standard way to solve this kind of problem, and only solves the specific problem I'm personally hitting rather than the related ones, so it's probably not the sensible approach to take.The text was updated successfully, but these errors were encountered: