-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Fix Mingw-w64 build #856
Fix Mingw-w64 build #856
Conversation
if ($env:generator -eq "MinGW Makefiles") { | ||
$env:path = $env:path.replace("C:\Program Files\Git\usr\bin;", "") | ||
if ($env:compiler -eq "gcc-5.3.0-posix") { | ||
$cxx_path = "C:\mingw-w64\i686-5.3.0-posix-dwarf-rt_v4-rev0\mingw32\bin" |
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.
Why are we being so specific here and specifying a path off of C:\ ?
Sorry I don't understand why so many files are being rewritten here.
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.
Why are we being so specific here?
For "MinGW Makefiles" CMake generator we remove Git from PATH environment variable and add mingw-w64 to PATH, so CMake able to build gtest.
... and specifying a path off of C:\
AppVeyor has preinstalled mingw-w64 i686-5.3.0 to this PATH https://www.appveyor.com/docs/installed-software/#mingw-msys-cygwin
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.
bc9c69b
to
b2df26b
Compare
dc19770
to
b7c6562
Compare
b7c6562
to
e9f9147
Compare
Now it should works with all cases |
This patch works for me with MinGW 4.8.2. I'm not exercising the AppVeyor part of of it though. |
@@ -396,7 +396,7 @@ | |||
# include <io.h> | |||
# endif | |||
// In order to avoid having to include <windows.h>, use forward declaration | |||
#if GTEST_OS_WINDOWS_MINGW | |||
#if GTEST_OS_WINDOWS_MINGW && !defined(__MINGW64_VERSION_MAJOR) |
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.
I can confirm that MinGW 4.8.2 g++ does not define this symbol.
The only symbols defined by that compiler with "MINGW" in its name are:
__MINGW32__
for the 32 bit compiler.- both
__MINGW32__
and__MINGW64__
for the 64 bit compiler
This PR fixes google-test building for me. Today's (commit Windows 10 I'm looking forward to this PR merge 👍 |
Hi @BillyDonahue please take another look at this. |
Is this the old mingw/msys or the new msys2/mingw64 from https://github.com/Alexpux/MINGW-packages? |
e9f9147
to
d8fe70f
Compare
@rakhimov I'm using g++-mingw-w64 4.8.2-10ubuntu2+12 from Ubuntu trusty. As far as I know it's the stock mingw for Ubuntu trusty. |
Update Google Test library to a version that contains a fix for building under MinGW. This issue seems to have been resolved in pull request google/googletest#856
Update Google Test library to a version that contains a fix for building under MinGW. This issue seems to have been resolved in pull request google/googletest#856
When compiling with MinGW, we no longer need to use -Dgtest_disable_pthreads=ON. See google/googletest#856 and google/googletest#721
When compiling with MinGW, we no longer need to use -Dgtest_disable_pthreads=ON. See google/googletest#856 and google/googletest#721
When compiling with MinGW, we no longer need to use -Dgtest_disable_pthreads=ON. See google/googletest#856 and google/googletest#721
@BillyDonahue PR #721 break build with Mingw-w64, this PR should fix this