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

v14 doesn't install required headers #3476

Closed
dmanlfc opened this issue Jan 15, 2024 · 8 comments
Closed

v14 doesn't install required headers #3476

dmanlfc opened this issue Jan 15, 2024 · 8 comments

Comments

@dmanlfc
Copy link

dmanlfc commented Jan 15, 2024

apps that build against glslang cannot find the headers, turns out they're not installed.

my fix...

diff --git a/glslang/CMakeLists.txt b/glslang/CMakeLists.txt
index 37eecaadd4..0e9037bf7a 100644
--- a/glslang/CMakeLists.txt
+++ b/glslang/CMakeLists.txt
@@ -254,11 +254,8 @@ if(PROJECT_IS_TOP_LEVEL)
 
     set(PUBLIC_HEADERS
         Public/ResourceLimits.h
-        Public/ShaderLang.h
         Public/resource_limits_c.h
-        Include/glslang_c_interface.h
-        Include/glslang_c_shader_types.h
-        Include/ResourceLimits.h
+        ${GLSLANG_HEADERS}
         MachineIndependent/Versions.h)
 
     foreach(file ${PUBLIC_HEADERS})
@FireBurn
Copy link

FireBurn commented Jan 19, 2024

It looks like it was deliberate 1dcb072 #3397

@arcady-lunarg
Copy link
Contributor

Could you list which headers you are using that aren't being installed? We are certainly open to tweaking the set of installed headers, though we still would prefer not to install absolutely everything by default.

@dmanlfc
Copy link
Author

dmanlfc commented Jan 19, 2024

@arcady-lunarg unfortunately the downstream package Cemu required most to be reinstated. Hence using ${GLSLANG_HEADERS

@arcady-lunarg
Copy link
Contributor

From a quick grep through the source at https://github.com/cemu-project/Cemu, I see that it does depend on glslang and its header files but only uses glslang/Public/ShaderLang.h and glslang/SPIRV/GlslangToSpv.h, both of which are in the set that are installed.

@dmanlfc
Copy link
Author

dmanlfc commented Jan 19, 2024

Actually it doesn't. It requests Types, Common, ResourcesLimits etc. I would have had to make the change if everything was provided.

@dmanlfc
Copy link
Author

dmanlfc commented Jan 19, 2024

Note, they have just added a commit to move to using the Public Headers.

@FireBurn
Copy link

That was me

@arcady-lunarg
Copy link
Contributor

Sounds like just using the public headers was fine then, I'm going to close this issue.

@arcady-lunarg arcady-lunarg closed this as not planned Won't fix, can't repro, duplicate, stale Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants