Skip to content

[shaderc] Re-fix installation#16781

Closed
JackBoosY wants to merge 4 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/16137
Closed

[shaderc] Re-fix installation#16781
JackBoosY wants to merge 4 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/16137

Conversation

@JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented Mar 19, 2021

Related: #16137.

@JackBoosY JackBoosY added category:port-bug The issue is with a library, which is something the port should already support info:internal labels Mar 19, 2021
@JackBoosY
Copy link
Contributor Author

@jmoguillansky-gpsw Can you please test this PR?

Thanks.

@JackBoosY JackBoosY requested a review from NancyLi1013 March 19, 2021 06:20
@NancyLi1013
Copy link
Contributor

Related issue #16658

Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see #16137 (comment)

@jmoguillansky-gpsw
Copy link

@jmoguillansky-gpsw Can you please test this PR?

Thanks.

Hi @JackBoosY I tested your branch, I was able to build shaderc shared lib successfully.
Thanks, much appreciated!

@JackBoosY
Copy link
Contributor Author

@ras0219-msft glslang and spir-tools use some symbols that are not exported in shaderc, so if we need to build them, we must first build static shaderc.

@ras0219-msft
Copy link
Contributor

glslang and spir-tools use some symbols that are not exported in shaderc

shaderc depends on glslang and spirv-tools; therefore they cannot be depending on symbols exported by shaderc:

# ports/control/shaderc

Build-Depends: glslang, spirv-tools

Therefore, if glslang and spirv-tools can only be built statically, shaderc should also be built statically. Dynamic libraries should (generally) not depend on static libraries.

@jmoguillansky-gpsw
Copy link

jmoguillansky-gpsw commented Mar 29, 2021

glslang and spir-tools use some symbols that are not exported in shaderc

shaderc depends on glslang and spirv-tools; therefore they cannot be depending on symbols exported by shaderc:

# ports/control/shaderc

Build-Depends: glslang, spirv-tools

Therefore, if glslang and spirv-tools can only be built statically, shaderc should also be built statically. Dynamic libraries should (generally) not depend on static libraries.

I don't think shaderc links against glslang or spriv-tools.
I think it includes a binary executable, glslc, which uses glslang and spirv-tools.
See the README: https://github.com/google/shaderc

@DatCaptainHorse
Copy link

Any updates on this? Would love to use shaderc thru vcpkg as it has issues with path size limits in my project.

@JackBoosY
Copy link
Contributor Author

@CaptainHorse Will continue to handle this PR later.

…jack/16137

And re-fix installation, remove install util
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 1257354a3ab0bebd8abe95281ca561537853578c -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/s-/shaderc.json b/versions/s-/shaderc.json
index ce2fa76..2f2112f 100644
--- a/versions/s-/shaderc.json
+++ b/versions/s-/shaderc.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "acddbab84ab19cc4d1e04ecb5277d3969cf9b093",
+      "git-tree": "f6a82ca83afbf20c08d6d001f133cd5fd2672108",
       "version": "2021.1",
       "port-version": 2
     },

@JackBoosY
Copy link
Contributor Author

Requires #19219.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 29, 2021
@JackBoosY
Copy link
Contributor Author

Linux regression related to KhronosGroup/SPIRV-Tools#1569.

@JackBoosY JackBoosY marked this pull request as draft October 12, 2021 07:44
@PhoebeHui PhoebeHui assigned Cheney-W and unassigned NancyLi1013 Nov 22, 2021
@PhoebeHui
Copy link
Contributor

The related upstream issue on linux has been fixed, please reopen and continue this PR if you have time.

@PhoebeHui PhoebeHui closed this Feb 9, 2022
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 depends:different-pr This PR or Issue depends on a PR which has been filed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants