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

[vcpkg] Simplify cmake invokation in bootsrap.sh #41997

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SchaichAlonso
Copy link
Contributor

@SchaichAlonso SchaichAlonso commented Nov 6, 2024

There's a [string escaping madness in bootstrap.sh's cmake invokation around line 217, making the script difficult to maintain.

I broke it before in PR #36828 which went into master undetected as it "worked over here" (tm) and isn't executed by the CI, but appearently it wouldn't work on arch linux' bash, resulting in PR #37047 .

microsoft/vcpkg-tool#1380 introduces a CMake Preset that can take over the cmake CLI variable setup.

This commit replaces the nested string composition by it's cmake-preset counterpart.

Fixes #41996.

There's a [string escaping madness in bootstrap.sh's cmake invokation around line 217, making the script difficult to maintain.

I broke it before in PR microsoft#36828 which went into master undetected as it "worked over here" (tm) and isn't executed by the CI, but appearently it wouldn't work on arch linux' bash, resulting in PR microsoft#37047 .

microsoft/vcpkg-tool#1380 introduces a CMake Preset that can take over the cmake CLI variable setup.

This commit replaces the nested string composition by it's cmake-preset counterpart.
@SchaichAlonso SchaichAlonso force-pushed the 41996-simplify-cmake-invokation-in-bootstrap branch from 14c4b9d to 780983b Compare November 6, 2024 22:49
@SchaichAlonso SchaichAlonso changed the title [vcpkg] Simplify cmake invokation (microsoft/vcpkg#41196) [vcpkg] Simplify cmake invokation in bootsrap.sh Nov 6, 2024
@LilyWangLL LilyWangLL added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Nov 7, 2024
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Nov 7, 2024
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

We can't use CMake presets because the current supported client CMake version is CMake 3.16.3, but CMake presets were added in 3.19. (I would be happy to take this in a couple years when our minimum is 3.19 though)

@SchaichAlonso
Copy link
Contributor Author

I'll set this to draft then and reset it when ubuntu 20.04 goes EOL in half a year :3

@SchaichAlonso SchaichAlonso marked this pull request as draft November 7, 2024 21:52
@vicroms vicroms added the depends:vm-update PR contains changes to the VM provisioning scripts label Nov 7, 2024
@BillyONeal
Copy link
Member

Unfortunately 3.19 is newer than several of the Linuxes we currently care about... though I don't have a complete table for more recent things since I made this table several years ago now.

image

@BillyONeal
Copy link
Member

Maybe we don't care since most of the time we ship binaries now?

@SchaichAlonso
Copy link
Contributor Author

SchaichAlonso commented Nov 8, 2024

Unfortunately 3.19 is newer than several of the Linuxes we currently care about... though I don't have a complete table for more recent things since I made this table several years ago now.

image

Isn't everything on that list, other then Ubuntu 20.04, 24.04, Debian 11 and Debian 12, End of Life?

So basically the only linux distribution which doesn't yet ship cmake-3.20 or newer is ubuntu-20.04, which will EoL in half a year.

I'm supprised we don't "care" for Ubuntu 22.04 though, despite there even being github hosted actions runners for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly depends:vm-update PR contains changes to the VM provisioning scripts info:reviewed Pull Request changes follow basic guidelines requires:discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vcpkg] simplify cmake invokation in bootstrap
4 participants