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

[shaderc port] automatically adds 'SPIRV-Tools.lib' but it has wrong path at all #28144

Closed
wh1t3lord opened this issue Dec 3, 2022 · 5 comments · Fixed by #31912
Closed

[shaderc port] automatically adds 'SPIRV-Tools.lib' but it has wrong path at all #28144

wh1t3lord opened this issue Dec 3, 2022 · 5 comments · Fixed by #31912
Assignees
Labels
category:port-bug The issue is with a library, which is something the port should already support

Comments

@wh1t3lord
Copy link

Hello!

So for reducing reading time. (IDK, if it is a duplicate and known issue you can close it)

Two projects, A is exe and B is static library that uses shaderc and linking to A project.

(EXE) A <- B

(STATIC LIB) B <- shaderc

I just installed shaderc through vcpkg and followed to heuristic generated commands for cmake.

BUT adding

target_link_libraries(${PROJECT_NAME} PUBLIC unofficial::shaderc::shaderc) automatically adds 'SPIRV-Tools.lib' (yeah, it is only the name 'SPIRV-Tools.lib' without any path to it) in input section of linking and it is not correct, the solution can't be built because it wants to find the 'SPIRV-Tools.lib' but the location is unknown.

Okay, I tried to search SPIRV-Tools like a separated package through find_package (because when you install shaderc it also installs SPIRV-Tools too, it is right) and it doesn't remove SPIRV-Tools.lib it adds a 'new' path to lib's pathes for linking and I don't know how to fix, I tried many variants but it is some kind of a real bug for me.

For example, I added find_package(SPIRV-Tools-opt CONFIG REQUIRED) with target_link_libraries(${PROJECT_NAME} PUBLIC SPIRV-Tools-opt) and it replaced 'SPIRV-Tools-opt.lib' to right path. (yeah, shaderc::shaderc (or maybe it is not shaderc, IDK, but I explain things what I got) adds 'SPIRV-Tools.lib' and 'SPIRV-Tools-opt.lib')

As a proof I just put this screenshot of generated project's settings.
image

So the commands of target_link_libraries in cmake project are:

   ...
    find_package(glslang CONFIG REQUIRED)
    find_package(unofficial-shaderc_util CONFIG REQUIRED)
    find_package(SPIRV-Tools-opt CONFIG REQUIRED)
    find_package(SPIRV-Tools CONFIG REQUIRED)
    find_package(unofficial-shaderc CONFIG REQUIRED)
    find_package(unofficial-spirv-reflect CONFIG REQUIRED)
    ... 

    target_link_libraries(${PROJECT_NAME} PUBLIC glslang::glslang) 
    target_link_libraries(${PROJECT_NAME} PUBLIC SPIRV-Tools)
    target_link_libraries(${PROJECT_NAME} PUBLIC unofficial::shaderc::shaderc)
    target_link_libraries(${PROJECT_NAME} PUBLIC unofficial::shaderc_util::shaderc_util)
    target_link_libraries(${PROJECT_NAME} PUBLIC unofficial::spirv-reflect::spirv-reflect)
    target_link_libraries(${PROJECT_NAME} PUBLIC SPIRV-Tools-opt)

Hope you can repeat the steps and get the same situation what I reached, but it is really strange and I don't understand how it works. I investigated shaderc's project in vcpkg and got this interesting line in vcpkg/packages/shaderc_x64-windows/share/unofficial-shadercTargets.cmake

set_target_properties(unofficial::shaderc::shaderc PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/"
  INTERFACE_LINK_LIBRARIES "\$<LINK_ONLY:glslang::glslang>;\$<LINK_ONLY:glslang::OSDependent>;\$<LINK_ONLY:glslang::OGLCompiler>;\$<LINK_ONLY:glslang::glslang>;\$<LINK_ONLY:unofficial::shaderc_util::shaderc_util>;\$<LINK_ONLY:glslang::SPIRV>;\$<LINK_ONLY:SPIRV-Tools>"
)

Especially, I don't understand why adding SPIRV-Tools-opt as a additional package removes the invalid 'SPIRV-Tools-opt.lib' but doing the same for 'SPIRV-Tools' it doesn't affect at all.

Wasted some hours for fixing it and no results.

P.S. Platform is Windows 11 and it was generated for VS 2022, cmake is 3.25.0.
P.S.S. About SPIRV-Tools, heuristic suggests a wrong thing at all, you can't use -shared and -static at once.

See this pic:
image

P.S.S.S. Writing target_link_libraries(${PROJECT_NAME} PUBLIC SPIRV-Tools) instead of 'suggested' target_link_libraries(${PROJECT_NAME} PUBLIC SPIRV-Tools-static) still adds corrected path to SPIRV-Tools.lib (static version). It is not an error or something, just an observation.

@dg0yt
Copy link
Contributor

dg0yt commented Dec 4, 2022

P.S.S. About SPIRV-Tools, heuristic suggests a wrong thing at all, you can't use -shared and -static at once.

Welcome to #20190.

@MonicaLiu0311 MonicaLiu0311 changed the title shaderc port, automatically adds 'SPIRV-Tools.lib' but it has wrong path at all [shaderc port] automatically adds 'SPIRV-Tools.lib' but it has wrong path at all Dec 5, 2022
@MonicaLiu0311 MonicaLiu0311 added the category:port-bug The issue is with a library, which is something the port should already support label Dec 5, 2022
@MonicaLiu0311
Copy link
Contributor

Fixed by microsoft/vcpkg-tool#849.

@KarimIO
Copy link

KarimIO commented Jun 7, 2023

The shaderc port still seems to be broken and includes the wrong path to SPIRV-Tools.lib even though I have the actual SPIRV-Tools package. Is there any way to overcome that issue?

@KarimIO
Copy link

KarimIO commented Jun 7, 2023

I ended up doing this workaround, which solved my troubles:

find_package(SPIRV-Tools-opt CONFIG REQUIRED)
find_package(SPIRV-Tools CONFIG REQUIRED)
add_library(SPIRV-Tools ALIAS SPIRV-Tools-static)
find_package(unofficial-shaderc_util CONFIG REQUIRED)
find_package(unofficial-shaderc CONFIG REQUIRED)

The alias allows SPIRV-Tools-static to satisfy SPIRV-Tools' requirement in unofficial-shaderc .

I do not think this issue has been resolved. I understand the heuristic-based instructions have been modified, but they're still not helpful for this library in particular. Is there any reason not to remove SPIRV-Tools' inclusion?

@dg0yt
Copy link
Contributor

dg0yt commented Jun 9, 2023

Especially, I don't understand why adding SPIRV-Tools-opt as a additional package removes the invalid 'SPIRV-Tools-opt.lib' but doing the same for 'SPIRV-Tools' it doesn't affect at all.

In CMake link libraries specifications, SPIRV-Tools or SPIRV-Tools-opt initially don't have a special meaning, so they is passed as unqualifed lib name to the linker. This changes when CMake has a target with that name: Linking details are taken from target properties, including the actual location of the link library. And a call to find_package is a way to created "imported targets" for external packages.

Best practice is to never export targets (or created imported targets) without namespace: A prefix with the :: separator signals to CMake at configuration time that a name is a referring to a target. It will report early that probably a find_package call or alias is missing in the project. The linker cannot accidentally pick a release lib when a debug lib was meant.

Here, the names are defined upstream (in the SPIRV-Tools project), so we must use them.
But the problem was that the vcpkg port didn't properly care about setting up link libraries and doing in find_package in the exported CMake config packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants