Use Win32 threads when compiling with Clang on MinGW#2381
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2381 +/- ##
==========================================
- Coverage 78.78% 78.77% -0.02%
==========================================
Files 620 620
Lines 108077 108077
Branches 15349 15348 -1
==========================================
- Hits 85153 85140 -13
- Misses 22266 22280 +14
+ Partials 658 657 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
CI failure seems unrelated edit: oops, it should be MinGW specific |
justsmth
left a comment
There was a problem hiding this comment.
Thanks for the PR! ❤️ I'll try to get a CI job setup to test this.
you can try to setup llvm-mingw to get a minimal clang+mingw toolchain. a good example on downloading and using such toolchain is available in mingw-w64 mirror |
|
@ognevny -- I think I've almost got it all together... and it only took me a few dozen iterations. 🤣 The tests are failing and it's not clear why. (?) I plan to cherry-pick this commit onto your PR branch for testing. |
just my guess, but go toolchains are mixed. try to set GOROOT environment before CMake invocation. also you can use a perl from MSYS environment, it's located under /usr instead of /clang64. it's usually installed along with other packages, but to ensure it you can add |
|
locally ran tests and it also failed for me, but with an another error |
2abd797 to
74a11ba
Compare
|
seems like you have to pass |
99068d6 to
042297b
Compare
|
cmake itself uses different Go: |
042297b to
807b7ae
Compare
807b7ae to
316cb30
Compare
|
Test are passing: https://github.com/aws/aws-lc/actions/runs/14776095409/job/41484616153?pr=2381 😊 |
|
great! how long will it take to integrate this change into |
After this is merged, we would need tag a release of AWS-LC then prepare a release for aws-lc-sys that aligns with it. We could get this all together by next week. I'll see if I can speed that up. |
cargo manages to use vendored dependencies when they're listed with path also change style for rustup recipe a bit: '\t' -> ' ' and remove extra '/'
|
is there any issue that should be addressed in this PR? |
|
|
There is a CI test failing, not related to the changes in this PR. I restarted the failed build and will merge the PR when it succeeds. |
thanks, it succeeded now |
To pull in aws/aws-lc#2381 which fixes build on MinGW with Clang Release Notes: - N/A
This is required to fix the build with clang+mingw on Windows, see aws/aws-lc#2381 Updated via "cargo update -p aws-lc-rs --precise 1.13.1" Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/2237>
This pulls some build improvements and fixes, including aws/aws-lc#2381
This pulls some build improvements and fixes, including aws/aws-lc#2381
This pulls some build improvements and fixes, including aws/aws-lc#2381
Issues:
Resolves #2362
Description of changes:
when compiling aws-lc with clang pthreads are currently used. it's common to build Clang itself with Win32 thread model so usually
-lpthreadoption is not passed for*-pc-windows-gnullvmtargets, for example. this PR fixes the issue when aws-lc-rs fails to build on such targets because required pthread symbols are not being linked in resulting binaryCall-outs:
-
Testing:
for now some Rust dependent software in MSYS2 is being built with this patch and there wasn't any runtime issue yet
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.