Skip to content
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

Note that cuda support requires cmake 3.9 #3497

Merged
merged 1 commit into from Oct 24, 2018
Merged

Note that cuda support requires cmake 3.9 #3497

merged 1 commit into from Oct 24, 2018

Conversation

biddisco
Copy link
Contributor

No description provided.

@hkaiser
Copy link
Member

hkaiser commented Oct 18, 2018

@biddisco is this enforced in the CMake files?

@biddisco
Copy link
Contributor Author

Updated PR with quick check in CMake to raise minimal version if HPX_WITH_CUDA is enabled

Copy link
Contributor

@chinz07 chinz07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO conditionally setting the minimum version is a very bad idea. cmake_minimum_required does not only check the version but also affects the default value of all policies. Let's not make it harder to maintain the build system by introducing multiple CMake dialects.

I would use a separate check:

if (HPX_WITH_CUDA AND CMAKE_VERSION VERSION_LESS 3.9)
    message(FATAL_ERROR "CUDA support requires at least CMake 3.9.")
endif() 

This solution has the additional benefit that the error message spells out the actual problem.

@biddisco
Copy link
Contributor Author

Updated to do it using your suggestion. I was being lazy and I'm glad you didn't let me get away with it.

@@ -15,6 +15,10 @@
# We require at least CMake V3.3.2
cmake_minimum_required(VERSION 3.3.2 FATAL_ERROR)

if (HPX_WITH_CUDA AND CMAKE_VERSION VERSION_LESS 3.9)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't HPX_WITH_CUDA defined only further down in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but the first time someone runs cmake, they will either use cmake -DHPX_WITH_CUDA on the command line, or they will run it once with ccmake then change the gui setting to enable HPX_WITH_CUDA and then when they configure, they'll get the error.

@msimberg msimberg merged commit 03277ac into master Oct 24, 2018
@msimberg msimberg deleted the cuda_cmake_doc branch October 24, 2018 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants