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

Rev cmake minimum version from 3.14.2 to 3.14.5 #34757

Merged
merged 3 commits into from
Jun 1, 2020
Merged

Rev cmake minimum version from 3.14.2 to 3.14.5 #34757

merged 3 commits into from
Jun 1, 2020

Conversation

am11
Copy link
Member

@am11 am11 commented Apr 9, 2020

iOS requires 3.14.5 version and we have a special case for it. Since this is a minor version difference, and no distro is particularly providing cmake 3.14.2 package, it is safe to update to 3.14.5 to cover our supported platform matrix.

Discussion: #33959 (comment)
Fixes: #33976

cc @akoeplinger, @jkotas

@ghost
Copy link

ghost commented Apr 9, 2020

Tagging @ViktorHofer as an area owner

@am11
Copy link
Member Author

am11 commented Apr 10, 2020

For Windows, we would need to upload
https://github.com/Kitware/CMake/releases/download/v3.14.5/cmake-3.14.5-win64-x64.zip
at
https://netcorenativeassets.blob.core.windows.net/resource-packages/external/windows/cmake/

@jkotas, could we get cmake-3.14.5-win64-x64.zip file in that blob storage and rerun Windows legs?

@jkotas
Copy link
Member

jkotas commented Apr 10, 2020

This is a nice-to-have cleanup. The build infrastructure tool updates have fallout and create more work. I think we should save this for when we actually have to do this.

Some of the cleanup in this PR is fine.

@akoeplinger
Copy link
Member

The build infrastructure tool updates have fallout and create more work.

If the only work left to do is uploading a newer cmake .zip to netcorenativeassets.blob.core.windows.net then I'd still be in favor of doing it.

@akoeplinger
Copy link
Member

Looks like cmake-3.15.5 is already there: https://netcorenativeassets.blob.core.windows.net/resource-packages/external/windows/cmake/cmake-3.15.5-win64-x64.zip

Given that this is the version documented at https://github.com/dotnet/runtime/blob/master/docs/workflow/requirements/macos-requirements.md#toolchain-setup maybe we should bump the global.json to that (but still keep the cmake_minimum_version at 3.14.5)?

@am11 am11 changed the title Rev cmake minimum version from 3.14.2 to 3.14.5 Rev cmake minimum version from 3.14.2 to 3.15.5 Apr 10, 2020
@am11 am11 changed the title Rev cmake minimum version from 3.14.2 to 3.15.5 Rev cmake minimum version from 3.14.2 to 3.14.5 Apr 10, 2020
@am11
Copy link
Member Author

am11 commented Apr 10, 2020

@akoeplinger, 3.15.5 seems to have more problems on other platforms such as https://gitlab.kitware.com/cmake/cmake/-/issues/20236, around the major version change, so I have kept it to 3.14.5.
The remaining work is to upload the 3.14.5 zip (29 MB) to blob for windows.

@ViktorHofer
Copy link
Member

Should this change be part of the next batched rollout after the one in May: #35202?

@jkotas
Copy link
Member

jkotas commented Apr 29, 2020

FWIW, I do not think we need this update at this point. See #34757 (comment)

@am11
Copy link
Member Author

am11 commented Apr 29, 2020

This is a patch version update (not even a minor), which fixes iOS issue and aligns the versions across the board. All non-windows official CI build agents have higher (3.15+) versions installed for a while now. I cannot think of any breakage considering the job of cmake in case of Windows build is mainly to generate vcxproj files and we have CI coverage.

@akoeplinger
Copy link
Member

Yeah I'd still be in favor of doing this cleanup to consolidate the different cmake_minimum_required numbers. We can do it as part of the next batched rollout if it makes things easier.

@ViktorHofer do you have access to upload cmake: #34757 (comment)

@ViktorHofer
Copy link
Member

@ViktorHofer do you have access to upload cmake: #34757 (comment)

Unfortunately not. @MattGal should have access.

@MattGal
Copy link
Member

MattGal commented Apr 30, 2020

@ViktorHofer do you have access to upload cmake: #34757 (comment)

Unfortunately not. @MattGal should have access.

https://netcorenativeassets.blob.core.windows.net/resource-packages/external/windows/cmake/cmake-3.14.5-win64-x64.zip and https://netcorenativeassets.blob.core.windows.net/resource-packages/external/windows/cmake/cmake-3.14.5-win32-x86.zip are there.

@akoeplinger akoeplinger merged commit e2541c7 into dotnet:master Jun 1, 2020
@akoeplinger
Copy link
Member

@am11 thanks for the contribution and sorry for the long wait 👍

@am11 am11 deleted the feature/cmake-minimum-version-rev branch June 1, 2020 20:43
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
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.

Set minimum CMake version to 3.14.5
6 participants