minimal required fix to fully compile'n'link on Visual Studio 2017 #4467
Conversation
| @@ -35,6 +35,7 @@ ExternalProject_Add(jsoncpp-project | |||
| -DJSONCPP_WITH_TESTS=OFF | |||
| -DJSONCPP_WITH_PKGCONFIG_SUPPORT=OFF | |||
| -DCMAKE_CXX_FLAGS=${JSONCPP_EXTRA_FLAGS} | |||
| -DCMAKE_BUILD_TYPE=Release | |||
| # Overwrite build and install commands to force Release build on MSVC. | |||
| BUILD_COMMAND cmake --build <BINARY_DIR> --config Release | |||
| INSTALL_COMMAND cmake --build <BINARY_DIR> --config Release --target install | |||
There was a problem hiding this comment.
Isn't this --config Release override the args above?
There was a problem hiding this comment.
@axic, nope, the --config part is for stage 2 (build) and stage 3 (install), but not for stage 1 (bootstrap). That is exactly why it took me so long to catch. :-)
There was a problem hiding this comment.
So CMake ignores CMAKE_BUILD_TYPE variable when generating for Visual Studio because it's multi-configuration generator and the config is requested with --config Release (or via dropdown menu in VS).
I expect the jsoncpp's CMake script incorrectly checks CMAKE_BUILD_TYPE on Windows and that's why this fix works.
There was a problem hiding this comment.
Do we want to leave a comment in there explaining this? Also anybody feels like checking jsoncpp's cmake file and fixing it? :)
There was a problem hiding this comment.
@chfast so I checked the build files of jsoncpp, and it's not reading the CMAKE_BUILD_TYPE var when compiling on Windows platform. So I am inclined to believe that VS2017 (at least) is interpreting this value when using the native builtin cmake support (FYI: I do not generate .sln files with cmake but open the CMakeLists.txt as solution file instead.)
I'll close this ticket however, because the other PR makes this PR redundant. Thanks anyways. :-)
Hi,
this is the minimal version of my fix to properly compile and link on Visual Studio 2017 (latest version).
I am not sure how on earth other people can properly
linkwithout that change, except, that my VS2017 is maybe to new and the fact, that I am using the native Visual Studio's support for CMake files (that is: I am not using generated .sln files).I will however provide a second PR (linking this one) with some more changes I'd like to see on top, closely related to building Solidity on Windows via Visual Studio.