Visual Studio 2017 build-time (linking) fix and improvements #4470
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chfast
reviewed
Jul 10, 2018
cmake/jsoncpp.cmake
Outdated
| # 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 | ||
| BUILD_COMMAND ${JSONCPP_CMAKE_COMMAND} --build <BINARY_DIR> --config ${CMAKE_BUILD_TYPE} |
Contributor
There was a problem hiding this comment.
That will not work reliably. If you want to use Debug build of jsoncpp in VS, remove both BUILD_COMMAND and INSTALL_COMMAND.
Contributor
Author
There was a problem hiding this comment.
Thanks @chfast, this is an interesting point. I'm removing those two three then.
christianparpart
force-pushed
the
vs2017-build-fix-ideal
branch
from
734a1bb to
7d67a66
Compare
Contributor
|
Please merge if @chfast approves. |
chriseth
previously approved these changes
Jul 10, 2018
chfast
previously approved these changes
Jul 10, 2018
Contributor
chfast
left a comment
chfast
left a comment
There was a problem hiding this comment.
Looks ok to me, but I'll wait for Windows build.
Problems with emscripten?
christianparpart
force-pushed
the
vs2017-build-fix-ideal
branch
from
7d67a66 to
230317e
Compare
Contributor
Author
chfast
approved these changes
Jul 10, 2018
resonant-riches
added a commit
to ChorusOne/azimuth-cli
that referenced
this pull request
Mar 4, 2024
Hopefully fix compiler errors on Clang on Apple Silicon. Patches are based on the following upstream PRs: argotorg/solidity#4160 argotorg/solidity#4235 argotorg/solidity#4470 argotorg/solidity#5699 argotorg/solidity#7268 argotorg/solidity#7827 argotorg/solidity#9383 argotorg/solidity#14725
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a companion PR to #4467, this PR actually not just fixes building Release mode on VS2017, but also supports Debug modes by not hard-coding nor forcing to Release-mode.
This is actually necessary to properly develop'n'debug using VS2017.
cmakewas hard-coded too, some lines belowExternalProject_Add, so I adapted that one, too.