fix zstd on windows #16293
Closed
fix zstd on windows #16293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sgowroji
added
team-OSS
meteorcloudy
requested changes
Sep 19, 2022
third_party/zstd-jni/zstd-jni.BUILD
Outdated
| genrule( | ||
| name = "copy_link_jni_md_header", | ||
| srcs = select({ | ||
| "@io_bazel//src/conditions:darwin": ["@bazel_tools//tools/jdk:jni_md_header-darwin"], |
There was a problem hiding this comment.
@bazel_tools//src/conditions:* is better.
|
We should try to enable the tests on Windows as well: https://cs.opensource.google/bazel/bazel/+/master:.bazelci/presubmit.yml;l=244-245 |
coeuvre
requested changes
Sep 19, 2022
third_party/zstd-jni/zstd-jni.BUILD
Outdated
| "src/windows/include/jni_md.h", | ||
| "jni/jni.h", | ||
| ]) + [ | ||
| "jni_md.h", |
Sandboxing on Windows is more limited than on other platforms, so two jni_md.h headers were ending up in the include path. This resulted in the wrong jni_md.h header being used on Windows, which meant that the JNIEXPORT macro was not being set to actually export symbols appropriately on Windows. This caused any attempts to use zstd on windows, whether for http_archive, gRPC cache compression, etc. to result in an UnsatisfiedLinkError being thrown. Change to use a similar method as the main bazel jni stuff, which is to copy the jni.h and jni_md.h headers over. This prevents accidentally picking up a header from the wrong architecture. Relates to bazelbuild#16041
|
Can you also update https://cs.opensource.google/bazel/bazel/+/master:.bazelci/postsubmit.yml;l=252 to the same as the presubmit.yml file? |
done! |
meteorcloudy
approved these changes
Sep 21, 2022
meteorcloudy
added
awaiting-PR-merge
sgowroji
removed
the
awaiting-PR-merge
aiuto
pushed a commit
to aiuto/bazel
that referenced
this pull request
Oct 12, 2022
Relates to bazelbuild#16041 Partial commit for third_party/*, see bazelbuild#16293. Signed-off-by: Sunil Gowroji <sgowroji@google.com>
aiuto
pushed a commit
to aiuto/bazel
that referenced
this pull request
Oct 12, 2022
Sandboxing on Windows is more limited than on other platforms, so two jni_md.h headers were ending up in the include path. This resulted in the wrong jni_md.h header being used on Windows, which meant that the JNIEXPORT macro was not being set to actually export symbols appropriately on Windows. This caused any attempts to use zstd on windows, whether for http_archive, gRPC cache compression, etc. to result in an UnsatisfiedLinkError being thrown. Change to use a similar method as the main bazel jni stuff, which is to copy the jni.h and jni_md.h headers over. This prevents accidentally picking up a header from the wrong architecture. Relates to bazelbuild#16041 Closes bazelbuild#16293. PiperOrigin-RevId: 475839895 Change-Id: Ia5569c50c8764699abe9858207a256b921980b92
aiuto
pushed a commit
to aiuto/bazel
that referenced
this pull request
Oct 12, 2022
Relates to bazelbuild#16041 Partial commit for third_party/*, see bazelbuild#16293. Signed-off-by: Sunil Gowroji <sgowroji@google.com>
aiuto
pushed a commit
to aiuto/bazel
that referenced
this pull request
Nov 14, 2022
Relates to bazelbuild#16041 Partial commit for third_party/*, see bazelbuild#16293. Signed-off-by: Sunil Gowroji <sgowroji@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sandboxing on Windows is more limited than on other platforms, so two jni_md.h headers were ending up in the include path. This resulted in the wrong jni_md.h header being used on Windows, which meant that the JNIEXPORT macro was not being set to actually export symbols appropriately on Windows. This caused any attempts to use zstd on windows, whether for http_archive, gRPC cache compression, etc. to result in an UnsatisfiedLinkError being thrown.
Change to use a similar method as the main bazel jni stuff, which is to copy the jni.h and jni_md.h headers over. This prevents accidentally picking up a header from the wrong architecture.
Relates to #16041