Fix multi-configuration on Windows CMake (CUDA) #18548
Closed
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
peterjc123
reviewed
Mar 28, 2019
peterjc123
reviewed
Mar 28, 2019
peterjc123
reviewed
Mar 28, 2019
|
Anything wrong with commit how it is now? |
|
@pytorchbot rebase this please |
|
Sorry, I can't merge this because there are conflicts. To merge this yourself, run the commands below: (To learn more about this bot, see Bot commands.) |
|
@xsacha Could you please rebase this first, please? |
peterjc123
approved these changes
Mar 31, 2019
|
Rebased, tests passed |
|
@pytorchbot retest this please |
|
@pytorchbot rebase this please |
|
Sorry, I can't merge this because there are conflicts. To merge this yourself, run the commands below: (To learn more about this bot, see Bot commands.) |
|
Looks like conflicts are still there. |
Multiple configurations is the default (eg. Release;Debug) on Windows and this check always broke this configuration as CMAKE_BUILD_TYPE was not set. The workaround was to always set CMAKE_BUILD_TYPE to Debug or Release, which was very unfortunate. The correct method is to use generator expressions that expand depending on the current CONFIG being processed.
|
What a silly bot. There was no conflicts the first time or this time.
|
|
@pytorchbot merge this please |
pytorchbot
added
the
merge-this-please
There was a problem hiding this comment.
@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
zdevito
pushed a commit
to zdevito/ATen
that referenced
this pull request
Apr 2, 2019
Summary: Multiple configurations is the default (eg. Release;Debug) on Windows and this check always broke this configuration as CMAKE_BUILD_TYPE was not set. The workaround was to always set CMAKE_BUILD_TYPE to Debug or Release, which was very unfortunate. The correct method is to use generator expressions that expand depending on the current CONFIG being processed. Side note: Anywhere else CMAKE_BUILD_TYPE is checked should probably be fixed too. Note that the CMakeLists.txt forces it in to Release mode. However, I came across this error when importing the prebuilt Config in to another project, where CMAKE_BUILD_TYPE was not set. > 3>CMake Error at pre_built/pytorch-1.0.1/share/cmake/Caffe2/public/cuda.cmake:380 (message): > 3> Unknown cmake build type: Proper support for configurations would mean we can build debug and release at the same time and as you can see, it is less CMake code. Pull Request resolved: pytorch/pytorch#18548 Differential Revision: D14730790 Pulled By: ezyang fbshipit-source-id: 70ae16832870d742c577c34a50ec7564c3da0afb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
Multiple configurations is the default (eg. Release;Debug) on Windows and this check always broke this configuration as CMAKE_BUILD_TYPE was not set. The workaround was to always set CMAKE_BUILD_TYPE to Debug or Release, which was very unfortunate.
The correct method is to use generator expressions that expand depending on the current CONFIG being processed.
Side note: Anywhere else CMAKE_BUILD_TYPE is checked should probably be fixed too.
Note that the CMakeLists.txt forces it in to Release mode. However, I came across this error when importing the prebuilt Config in to another project, where CMAKE_BUILD_TYPE was not set.
Proper support for configurations would mean we can build debug and release at the same time and as you can see, it is less CMake code.