-
Notifications
You must be signed in to change notification settings - Fork 273
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
Cross-compiling: Version readout fixed #189
Conversation
Test.cmake: Path handling consistent
👍 looks good |
@@ -11,12 +11,6 @@ | |||
#include <g3log/logworker.hpp> | |||
#include "tester_sharedlib.h" | |||
|
|||
#if (defined(CHANGE_G3LOG_DEBUG_TO_DBUG)) |
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.
Thanks
IF (${VERSION}.x STREQUAL ".x") | ||
IF (MSVC) | ||
IF ( NOT VERSION ) | ||
IF ( "${CMAKE_HOST_SYSTEM_NAME}" STREQUAL "Windows" ) |
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.
nice change
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) | ||
ELSE() | ||
message( FATAL "Need CMake version >=3.4 to build shared windows library!" ) | ||
IF( WIN32 ) |
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 WIN32
check and not?:
IF ( "${CMAKE_HOST_SYSTEM_NAME}" STREQUAL "Windows" )
👍 @AndreasSchoenle it looks good. I had only one question that I was curious about. After you answer that I'll go ahead and merge it. |
Tested and needed for cmake-changes4 #190 |
Small addition to cmake-changes2: When cross compiling MSVC is of course not set on windows. Also it just checks whether using the MS compiler - will also not be set for mingw. Thus checking for the host OS and determining on which system the command is actually run on seems the way to go.