[cmake] refactor: shared/static handling#6481
Conversation
This significantly redoes the way shared and static libraries are
handled. Now, it's controlled by two options: `SPIRV_TOOLS_BUILD_STATIC`
and `SPIRV_TOOLS_BUILD_SHARED`.
The default configuration (no `BUILD_SHARED_LIBS` set, options left at
default) is to build shared ONLY if this is the master project, or
static ONLY if this is a subproject (e.g. FetchContent, CPM.cmake). Also
I should note that static-only (i.e. no shared) is now a supported
target, this is done because projects including it as a submodule e.g.
on Android or Windows may prefer this.
Now the shared/static handling:
- static ON, shared OFF: Only generates `.a` libraries.
- static ON, shared ON: Generates `.a` libraries, but also
`libSPIRV-Tools.so`
- static OFF, shared ON: Only generates `.so` libraries.
Notable TODOs:
- SPIRV-Tools-shared.pc seems redundant--how should we handle which one
to use in the case of distributions that distribute both types (MSYS2
for instance)?
* *Note: pkgconfig sucks at this and usually just leaves it up to the
user, so the optimal solution may indeed be doing absolutely
nothing.* CMake is unaffected :)
- use namespaces in the CMake config files pleaaaaase
This is going to change things a good bit for package maintainers, but
cest la vie. It's for the greater good, I promise.
Signed-off-by: crueter <crueter@eden-emu.dev>
Signed-off-by: crueter <crueter@eden-emu.dev>
|
|
|
done a bit of studying on PC files... I think the way to go is utilizing Libs.private |
Signed-off-by: crueter <crueter@eden-emu.dev>
|
Poke; Fixed up pkgconfig files |
Signed-off-by: crueter <crueter@eden-emu.dev>
|
Sorry for the delay reviewing this. Your original post doesn't motivate why the change needs to happen, or should happen I agree the current setup is baroque; I don't know how it evolved that way. I presume there are significant downstream uses that depend on it being the way it is. I'd need very good motivation to break those folks. re: CMake namespaces; SPIRV-Tools was using CMake 2.18 for a long time (to give the ecosystem time to catch up), and we never later adopted them. I'm open to improvements that don't break backward compatibility, and tag @dj2 for guidance here. I'm handing off review to others. |
|
I don't think we want to do this without significant reason. Breaking things for downstream packages is a big hurdle to jump over without justification. Can you please file a bug with the reasoning and motivation for this change. What breaks for downstream users, what breaks for other users. Why are we doing this? This feels like there are multiple things shoved into the same CL. Is the Please file a bug for adding a namespace, we can add in |
|
The primary motivations for this:
For users this doesn't really break anything major. The defaults are generally the same as before--subprojects build static only, building it as the master project builds shared only--and BUILD_SHARED_LIBS is properly respected now too. If anyone faces significant challenges from this then there is something seriously wrong with their build system. re homebrew: It's been so long I forget most of this, but I believe I could reproduce that build failure and it was fixed on this PR. I'll have to check again once I have access to a linux/aarch64 environment.
It probably could be moved to a separate PR. I kept it in here since it's in the same domain of what I was already working on; the rest of the PR is also dependent on this in order to not break things.
Having two separate near-identical pkg-config files is generally frowned upon. Using Frankly it was so wrong before that I'm not certain what the best way to go about keeping backwards compatibility is, or if it should even be done.
Looking back, I severely over-exaggerated the downstream consequences. It really doesn't change anything for anyone, unless they manually handle the pkg-config files. |
|
At some point I was going to move everything to use OBJECT libraries to prevent rebuilding the same files twice (which could also allow to have both shared/static versions of link, opt etc.). I can't remember what I did here to help with that, maybe it was the visibility nonsense? |
4 participants
This significantly redoes the way shared and static libraries are
handled. Now, it's controlled by two options:
SPIRV_TOOLS_BUILD_STATICand
SPIRV_TOOLS_BUILD_SHARED.The default configuration (no
BUILD_SHARED_LIBSset, options left atdefault) is to build shared ONLY if this is the master project, or
static ONLY if this is a subproject (e.g. FetchContent, CPM.cmake). Also
I should note that static-only (i.e. no shared) is now a supported
target, this is done because projects including it as a submodule e.g.
on Android or Windows may prefer this.
Now the shared/static handling:
.alibraries..alibraries, but alsolibSPIRV-Tools.so.solibraries.Slightly reworked SPIRV-Tools.pc to use
Libs.privatefor libraries only needed w/ the static library.The primary motivation behind this change--beyond general code cleanup--is to allow for static-only builds, respect BUILD_SHARED_LIBS, and consolidate the pkg-config files (could be moved to a separate PR maybe).
Signed-off-by: crueter crueter@eden-emu.dev