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.

Sign up for GitHub

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

Jump to bottom

[vcpkg] simplify cmake invokation in bootstrap #41996

Open
SchaichAlonso opened this issue Nov 6, 2024 · 0 comments · May be fixed by #41997
Open

[vcpkg] simplify cmake invokation in bootstrap #41996

SchaichAlonso opened this issue Nov 6, 2024 · 0 comments · May be fixed by #41997
Assignees
@JonLiu1993
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly

Comments

@SchaichAlonso
Copy link
Contributor

Describe the bug
There's a string escaping madness in bootstrap.sh's cmake invokation, 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

Environment

  • OS: FreeBSD 13.4-RELEASE-p1 GENERIC amd64 (anything unsupported will do)
  • Compiler: FreeBSD clang version 18.1.6 (https://github.com/llvm/llvm-project.git llvmorg-18.1.6-0-g1118c2e05e67) (anything will do)

To Reproduce
Steps to reproduce the behavior:

  1. sh bootstrap.sh

Expected behavior
vcpkg is bootstraped and without the need to handle nested string escapes in bash.

Failure logs
works over here(tm)

Additional context
We can probably ditch the branching for VCPKG_MAX_CONCURRENCY altogether and use

cmake --build ${buildDir} --parallel ${VCPKG_MAX_CONCURRENCY:1}`

instead of specifying ninja-specific options in the bootstrap.sh parallel build preset.
@SchaichAlonso SchaichAlonso changed the title [vcpkg-tool] simplify cmake invokation in bootstrap [vcpkg] simplify cmake invokation in bootstrap Nov 6, 2024
SchaichAlonso added a commit to PurpleFlowerGarden/vcpkg that referenced this issue Nov 6, 2024
@SchaichAlonso
[vcpkg] Simplify cmake invokation (microsoft#41996)
780983b 
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 linked a pull request Nov 6, 2024 that will close this issue
Draft
@JonLiu1993 JonLiu1993 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JonLiu1993 JonLiu1993

Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[vcpkg] Simplify cmake invokation in bootsrap.sh PurpleFlowerGarden/vcpkg
2 participants
@SchaichAlonso @JonLiu1993