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
Support using mingw-std-threads in gtest_zlib #1573
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## develop #1573 +/- ##
========================================
Coverage 83.88% 83.89%
========================================
Files 132 132
Lines 10843 10843
Branches 2801 2801
========================================
+ Hits 9096 9097 +1
Misses 1048 1048
+ Partials 699 698 -1 Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
endif() | ||
|
||
target_sources(gtest_zlib PRIVATE test_deflate_concurrency.cc) | ||
target_compile_definitions(gtest_zlib PRIVATE USE_MINGW_STDTHREAD _WIN32_WINNT=0x501) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change USE_MINGW_STDTHREAD
to HAVE_MINGW_STDTHREAD
?
endif() | ||
|
||
target_sources(gtest_zlib PRIVATE test_deflate_concurrency.cc) | ||
target_compile_definitions(gtest_zlib PRIVATE USE_MINGW_STDTHREAD _WIN32_WINNT=0x501) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might make better sense to add -D_WIN32_WINNT=0x501
in cmake/toolchain-mingw-i686.cmake
and cmake/toolchain-mingw-x86_64.cmake
instead?
@@ -245,13 +245,39 @@ if(WITH_GTEST) | |||
target_link_libraries(gtest_zlib GTest::GTest) | |||
|
|||
find_package(Threads) | |||
if(Threads_FOUND AND NOT BASEARCH_WASM32_FOUND) | |||
# TODO: Should regular C++11 threads be used when MinGW-w64 is built with pthreads? | |||
if(Threads_FOUND AND NOT BASEARCH_WASM32_FOUND AND NOT MINGW) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmake sets CMAKE_USE_PTHREADS_INIT
if pthreads are available and developer can set CMAKE_THREAD_PREFER_PTHREAD
before calling find_package(Threads)
to prefer pthreads over any other thread implementation.
It's also possible to explicitly check if libpthread.a
or libpthread.dll.a
exists to see if MinGW is built with pthreads.
Split out from PR #1499.
Ideally this should detect if C++11 threads are already supported in the STL and use that instead, but for now mingw-std-threads is used in all MinGW builds.