-
Notifications
You must be signed in to change notification settings - Fork 368
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
Use a proper SONAME for libshaderc.so #381
Comments
Fixed by #493 |
There's just one thing I'd still like to nitpick about. You still appear to call it |
I paged that out of my brain. I think @AWoloszyn might remember. |
I think @antiagainst was the one who added shaderc_shared. If I remember correctly, it was because for some reason we couldn't or didn't want to use the cmake BUILD_SHARED_LIBS flag to get a shared library out. We end up building both libshaderc.a and libshaderc_shared.so, and they are named as such because CMake does not allow two targets to have the same name. We can probably fix this with a simple
Although looking at the documentation I am not sure if we would also have to specify the extension/prefix, in which case that would get a bit more complicated. |
I don't really have preferences. Having the shared library be |
Maybe we can stop building both static and shared library but let the user choose which one to build by exposing a option around CMake |
I think we would be better off with the BUILD_SHARED_LIBS approach. We should be able to detect, and then build one or the other, and then the user gets to choose. |
I think the usual approach is to let the user choose the build type (shared vs static). For most users, I don't see much of a point in building both at the same time. |
I've uploaded #498 for this. |
Instead of arbitrarily calling it
libshaderc_shared.so
it should be calledlibshaderc.so.VER
where VER encodes the ABI version. This would also allow users to compile with-lshaderc
universally instead of needing to change it to-lshaderc_shared
to build a dynamically linked executable.For more information see https://en.wikipedia.org/wiki/Soname
The text was updated successfully, but these errors were encountered: