-
Notifications
You must be signed in to change notification settings - Fork 391
fix build in win32 #116
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 build in win32 #116
Conversation
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.
The changes in CMake are necessary for building with Visual Studio. The library 'm' for example does not exist.
Why is this actually needs to rename all google tests? |
This question need address to google because I just update gogletest lib. |
Ah sorry those are the libs ones. That's of course fine. I have just tried to build it but getting:
which wasn't the case before this change. Doesn't this need to set explicitly |
I should say that I build on Linux with cmake 3.16.3. |
Think no, cause all builds are successful, and all tests passed. |
@bukka, should I update googletest to lastest version (1.10)? |
Basically the line that is giving warning is
so I guess that has changed and might require the GOOGLETEST_VERSION variable to be set. It won't probably prevent to run it (that's why warning) but it would be good to check what should be done about that as I would like to keep it without any warning. |
Looks like the 1.10 doesn't have it when looking to https://github.com/google/googletest/blob/release-1.10.0/CMakeLists.txt so it might actually help to update it. Looks like even update to 1.8 should help so anything above should be fine. I would go for the latest that works though so if 1.10 works, then go for it. |
Actually, line with VERSION there https://github.com/google/googletest/blob/release-1.10.0/googletest/CMakeLists.txt#L54 .
So, I try to update to 1.10 |
Tested and now warnings now! Merged thanks! |
tested for MinGW-gcc 8.3 and MSVC2017