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

GLSLANG_EXPORT for C APIs. #2369

Merged
merged 1 commit into from
Aug 17, 2020
Merged

Conversation

ezdiy
Copy link
Contributor

@ezdiy ezdiy commented Aug 10, 2020

Fixes FFI breakage introduced in #2283

Fixes FFI breakage introduced in KhronosGroup#2283
@CLAassistant
Copy link

CLAassistant commented Aug 10, 2020

CLA assistant check
All committers have signed the CLA.

@johnkslang
Copy link
Member

We normally have 11 or twelve checks, but this only saw 2. This PR though is a good candidate to see running on more bots, somehow, before merging.

@ben-clayton is taking a break right now, but would also be nice to have his input on the change itself.

@ben-clayton
Copy link
Contributor

@johnkslang - you need to apply the "kokoro: run" label for the other bots to run.
The change LGTM.

@ezdiy
Copy link
Contributor Author

ezdiy commented Aug 11, 2020

One issue currently not taken into account is https://github.com/KhronosGroup/glslang/blob/master/StandAlone/resource_limits_c.h

This compiles into libglslang-default-resource-limits.so currently not under -fvisibility=hidden regime in the makefiles, hence exports still working (at least on linux, not so sure about win32). Making it export properly would ideally involve collapsing all that into glslang_c_interface.c / glslang_c_interface.h, as the separation to additional library looks to me like some temporary workaround to deal with circular dependency yet to be refactored. Alternatively turn whole C api into separate library that has its own linking mode - ie even if c++ is static, c-export may be dynamic, and perhaps make that a cmake default.

The reasoning here is that FFI from non-C languages (think LWJGL...) demand shared c-export library, yet don't care about nature of t he c++ side whatsoever.

@johnkslang
Copy link
Member

Can you say more about the "kokoro: run" label?

(This project doesn't have such a label, and normally all tests run automatically.)

@johnkslang johnkslang merged commit 8f0c6bd into KhronosGroup:master Aug 17, 2020
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

Successfully merging this pull request may close these issues.

4 participants