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

BUILD_SHARED_LIBS output libSPIRV-Tools.a instead of libSPIRV-Tools.so #3626

Open
inochisa opened this issue Jul 31, 2020 · 7 comments
Open
Assignees

Comments

@inochisa
Copy link

BUILD_SHARED_LIBS I think this option should lead to libSPIRV-Tools.so. but today I only find libSPIRV-Tools.a after compiling.
After searching log, I think maybe it's related to #3490
This is my cmake options

  cmake \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=/usr \
    -DCMAKE_INSTALL_LIBDIR=lib \
    -DBUILD_SHARED_LIBS=ON \
    -DSPIRV-Headers_SOURCE_DIR="${PWD}/../../spirv-headers" \
    -DSPIRV_WERROR=OFF \
    -SPIRV_TOOLS_EXTRA_DEFINITIONS=-DNDEBUG \
    ..
@akien-mga
Copy link

Confirmed in b27e039 while trying to update my Mageia package.

I see Fedora maintainer @airlied also chose to revert #3490 to get things to work: https://src.fedoraproject.org/rpms/spirv-tools/c/5c006681b867e6264294b4ac341f1bf9fbf2214e?branch=master

@ben-clayton
Copy link
Contributor

Hi @SakuraLife & @akien-mga,

Sorry you're having trouble with this.

I authored #3490 to fix #3482.

#3490 keeps the install artifacts identical for BUILD_SHARED_LIBS=0, but aliases the SPIRV_TOOLS CMake target to point to either the static target or shared target. This emulates the behaviour of BUILD_SHARED_LIBS from the perspective of outer projects, while attempting to avoid significant changes to the project's targets.

While working on this, I observed that the existing build rules create (and install) the following files regardless of the value of BUILD_SHARED_LIBS:

  • <install>/lib/pkgconfig/SPIRV-Tools-shared.pc
  • <install>/lib/libSPIRV-Tools-shared.so

Clearly the install artifacts are different for BUILD_SHARED_LIBS=1. I didn't think this was a problem as we've been building libSPIRV-Tools-shared.so for over 3 years and I had incorrectly assumed that BUILD_SHARED_LIBS=0 was what package maintainers had been using all along (especially as the project failed to build with BUILD_SHARED_LIBS=1 on some platforms).

I have to admit it seems a little odd to have both SPIRV-Tools.pc and SPIRV-Tools-shared.pc be emitted as shared, when using BUILD_SHARED_LIBS=1. That said, I think the oddness really stems from the existence of the -shared target, which probably never should have been added in the first place.

This is all a bit of a mess, and I apologise for making this worse. Maybe reverting #3490 is the best option. However, this change has now been in master for a little over 2 months now, and I fear this might break people now depending on using BUILD_SHARED_LIBS=1 build on Windows, or directly depending on the new ${SPIRV_TOOLS}-static target.

Before making any further changes that might impact yet more people, I'd be interested in hearing any suggestions on how to best fix this.

@ben-clayton
Copy link
Contributor

One usage of ${SPIRV_TOOLS}-static has been spotted here.

@ben-clayton
Copy link
Contributor

And another here.

@ben-clayton
Copy link
Contributor

I've filed #3909 as it's clear there's a lot of long-term work to be done here.

To fix the immediate issue of rebuilding a package, I've started taking a look at adding some new CMake flags that can emulate the old behavior / output. These will default to off, so not to disrupt other people.

ben-clayton added a commit to ben-clayton/SPIRV-Tools that referenced this issue Oct 13, 2020
If enabled the following targets will be created:

* `${SPIRV_TOOLS}-static` - `STATIC` library. Has full public symbol visibility.
* `${SPIRV_TOOLS}-shared` - `SHARED` library. Has default-hidden symbol visibility.
* `${SPIRV_TOOLS}`        - will alias to one of above, based on BUILD_SHARED_LIBS.

If disabled the following targets will be created:

* `${SPIRV_TOOLS}`        - either `STATIC` or `SHARED` based on the new `SPIRV_TOOLS_LIBRARY_TYPE` flag. Has full public symbol visibility.
* `${SPIRV_TOOLS}-shared` - `SHARED` library. Has default-hidden symbol visibility.

Defaults to `ON`, matching existing build behavior.

This flag can be used by package maintainers to ensure that all libraries are build as shared objects.

Issue: KhronosGroup#3626
See also: KhronosGroup#3909
@ben-clayton
Copy link
Contributor

Hi @SakuraLife and @akien-mga

I've created #3910 which adds the SPIRV_TOOLS_BUILD_STATIC flag. Setting this and BUILD_SHARED_LIBS to 1 should produce identical install output to what we had before #3490.

If at all possible, please can you confirm that this fixes your problem?

I'd also be very interested in hearing your thoughts on #3909.

Cheers,
Ben

@inochisa
Copy link
Author

@ben-clayton confirmed, everything looked the same as before.
As for #3909, I think we can provide two different option to output static build and shared build separately, because different linux distribution prefer different library type. Some prefer static build, some prefer shared build, some prefer build all,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants