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

Require CMake >=3.16 #3679

Merged
merged 9 commits into from
Jan 13, 2021
Merged

Conversation

masterleinad
Copy link
Contributor

@masterleinad masterleinad commented Dec 18, 2020

In accordance with trilinos/Trilinos#8401, we also decided to require CMake 3.16 or higher.

cmake/Modules/CudaToolkit.cmake Outdated Show resolved Hide resolved
IF(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.12.0")
CMAKE_POLICY(SET CMP0074 NEW)
ENDIF()
CMAKE_POLICY(SET CMP0074 NEW)
Copy link
Member

Choose a reason for hiding this comment

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

I actually wonder if we still need that guy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://cmake.org/cmake/help/latest/policy/CMP0074.html still says

[...]
This policy was introduced in CMake version 3.12. CMake version 3.19.2 warns when the policy is not set and uses OLD behavior. Use the cmake_policy() command to set it to OLD or NEW explicitly.

Copy link
Member

Choose a reason for hiding this comment

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

Look at the comment above that block of code. The policy is being set in the main CMakeLists.txt of the package

CMakeLists.txt Outdated Show resolved Hide resolved
.jenkins Outdated Show resolved Hide resolved
scripts/docker/Dockerfile.clang Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@masterleinad masterleinad changed the title Require CMake >=3.17.1 Require CMake >=3.16 Dec 22, 2020
@masterleinad
Copy link
Contributor Author

Retest this please.

3 similar comments
@masterleinad
Copy link
Contributor Author

Retest this please.

@masterleinad
Copy link
Contributor Author

Retest this please.

@masterleinad
Copy link
Contributor Author

Retest this please.

@dalg24 dalg24 requested a review from jrmadsen January 12, 2021 13:54
@@ -45,6 +45,26 @@ matrix:
compiler: gcc
env: CMAKE_BUILD_TYPE=Release BACKEND="OPENMP"

# Install newer CMake. The distribution comes with CMake 3.12.4 but we require at least 3.16
install:
- CMAKE_VERSION=3.17.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not:

DISTRIB_CODENAME=$(cat /etc/lsb-release | grep DISTRIB_CODENAME | awk -F '=' '{print $NF}')
wget -O - https://apt.kitware.com/keys/kitware-archive-latest.asc | apt-key add -
apt-add-repository "deb https://apt.kitware.com/ubuntu/ ${DISTRIB_CODENAME} main"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copied from our Dockerfiles. We could try your suggestions but it doesn't matter as long as we don't activate Travis again.

Copy link
Contributor

@DavidPoliakoff DavidPoliakoff left a comment

Choose a reason for hiding this comment

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

Good and net negative lines

@masterleinad
Copy link
Contributor Author

Lots of approvals including @dalg24 and @crtrott. Awaiting merging. If requested I can squash before.

@crtrott crtrott merged commit 78beb9f into kokkos:develop Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants