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

Up Minimum CMake Version to 3.20 #86530

Merged
merged 15 commits into from
Jul 4, 2023
Merged

Up Minimum CMake Version to 3.20 #86530

merged 15 commits into from
Jul 4, 2023

Conversation

ivdiazsa
Copy link
Member

The PR for #74027. All the necessary changes to increase the version of CMake will go here.

@ghost
Copy link

ghost commented May 19, 2023

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

The PR for #74027. All the necessary changes to increase the version of CMake will go here.

Author: ivdiazsa
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

@ivdiazsa
Copy link
Member Author

@jkoritzinsky Here's the PR we will be using for the CMake updates.

@ivdiazsa ivdiazsa added this to the Future milestone May 19, 2023
@ivdiazsa ivdiazsa modified the milestones: Future, 8.0.0 May 19, 2023
@ivdiazsa
Copy link
Member Author

FYI @trylek

@EgorBo
Copy link
Member

EgorBo commented May 19, 2023

Don't you need to update build instructions too? I see they contain * CMake 3.14.5 or newer

@ivdiazsa
Copy link
Member Author

Don't you need to update build instructions too? I see they contain * CMake 3.14.5 or newer

This is just the beginning. That's why I marked it as a Draft PR. There are still other adjustments to make, including updating the docs.

@ivdiazsa
Copy link
Member Author

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Comment on lines 51 to 56
**NOTE**: As of now, `apt` only has until CMake version 3.16.3 if you're using Ubuntu 20.04 LTS (or less in older Ubuntu versions), which is lower than the required 3.20. For this case, we use the `snap` package manager, which has version 3.26.4 at the time of writing:

```bash
sudo snap install cmake
```

Copy link
Member

Choose a reason for hiding this comment

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

We should also mention the Kitware APT feed, which always has the most recent version available as an official build from Kitware directly.

@@ -51,7 +51,7 @@ These steps are required only in case the tools have not been installed as Visua
* Install [CMake](https://cmake.org/download) for Windows.
* Add its location (e.g. C:\Program Files (x86)\CMake\bin) to the PATH environment variable. The installation script has a check box to do this, but you can do it yourself after the fact following the instructions at [Adding to the Default PATH variable](#adding-to-the-default-path-variable).

The _dotnet/runtime_ repository recommends using CMake 3.16.4 or newer, but it may work with CMake 3.15.5.
The _dotnet/runtime_ repository requires using CMake 3.20 or newer.
Copy link
Member

Choose a reason for hiding this comment

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

We should mention something in the section about the -msbuild flag that CMake 3.21 is required to use that flag (maybe we can add validation of that at some point?)

Copy link
Member Author

@ivdiazsa ivdiazsa May 23, 2023

Choose a reason for hiding this comment

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

That sounds good to me. Do you mean that on Windows, if you have CMake 3.21 or later, you have to always use the -msbuild flag? As in:

.\build.cmd <args go here> -msbuild

Otherwise it will fail?

Copy link
Member

Choose a reason for hiding this comment

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

Not quite. I mean if you want to use the -msbuild flag on Windows, you need at least CMake 3.21 as the VS2022 generator doesn't exist in CMake until that version.

…the corehost and libunwind CMake versions to 3.20
@jkoritzinsky
Copy link
Member

The wasm build images have finally been updated, so they can have the same 3.20 minimum as the rest of our targets.

@ivdiazsa
Copy link
Member Author

The wasm build images have finally been updated, so they can have the same 3.20 minimum as the rest of our targets.

Awesome! Let me remove the conditionings I added to rerun the CI.

@ivdiazsa ivdiazsa marked this pull request as ready for review June 21, 2023 17:38
@ivdiazsa
Copy link
Member Author

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -40,12 +40,24 @@ Install the following packages for the toolchain:
* zlib1g-dev
* ninja-build (optional, enables building native code with ninja instead of make)

**NOTE**: If you have an Ubuntu version older than 22.04 LTS, don't install `cmake` using `apt` directly. Follow the note written down below.
Copy link
Contributor

Choose a reason for hiding this comment

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

for reference, it looks like Debian 11 Bullseye (oldstable) has cmake 3.18.4 and Debian 12 Bookworm (stable) has cmake 3.25.1. So it may be worth also noting that Debian older than 12 also has an outdated cmake in its package repository since the same 'use snap or Kitware APT' solution applies there too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great thanks for pointing this out. I'll update the doc.

@kg
Copy link
Contributor

kg commented Jun 21, 2023

Mono changes look fine.

@@ -1,16 +1,9 @@
cmake_minimum_required(VERSION 3.6.2)
cmake_minimum_required(VERSION 3.20)

cmake_policy(SET CMP0042 NEW) # MACOSX_RPATH is enabled by default.
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to set these policies individually after specifying minimum required version 3.20?

cmake_minimum_required(VERSION 3.20) should set these policies to NEW according to the docs: https://cmake.org/cmake/help/latest/command/cmake_minimum_required.html

Copy link
Member

Choose a reason for hiding this comment

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

We plan to update the cmake_policy statements, remove redundant ones, as well as other build improvements after doing the initial minimum version upgrade. We wanted to keep the PR that is most likely to break workflows as small as possible to limit the risk of the change (and risk of having to revert it).

Copy link
Member

Choose a reason for hiding this comment

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

If you want to keep this PR as minimal as possible and keep cleanup for later, this block can stay as is. The if (CMAKE_VERSION VERSION_GREATER ...) conditions are harmless.

@ivdiazsa ivdiazsa merged commit cd26e63 into dotnet:main Jul 4, 2023
175 of 179 checks passed
akoeplinger added a commit to akoeplinger/runtime that referenced this pull request Jul 5, 2023
The prebuild on Codespaces broke after the minimum CMake version was bumped in dotnet#86530 because the container was still using Ubuntu 20.04 (Focal) which only has CMake 3.16.1.

Upgrade to Ubuntu 22.04 (Jammy) so we have a new enough CMake.

Also fix a missing policy in the Mono CMakeLists.txt that was accidentally removed.
hoyosjs pushed a commit that referenced this pull request Jul 5, 2023
…88410)

The prebuild on Codespaces broke after the minimum CMake version was bumped in #86530 because the container was still using Ubuntu 20.04 (Focal) which only has CMake 3.16.1.

Upgrade to Ubuntu 22.04 (Jammy) so we have a new enough CMake.

Also fix a missing policy in the Mono CMakeLists.txt that was accidentally removed.
@ghost ghost locked as resolved and limited conversation to collaborators Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Update minimum CMake version required to build the dotnet/runtime repo to 3.20
5 participants