-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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 baseline][vcpkg toolchain] Fix toolchain usage with CMake <= 3.19 #23410
Conversation
* Use host tools by default. * use GLOB_RECURSE and filter the result * be positiv ;) * give me debug output * Revert "give me debug output" This reverts commit 17737bc. * remove unnecessary if(IS_DIRECTORY) Co-authored-by: Alexander Neumann <[email protected]>
BTW it should be |
AFAICS it was fixed in 3.20.0. My wording refers to the broken state, which is <= 3.19. |
Ping @vicroms @BillyONeal for merge this first. |
I'd like to fix #23395 in this PR. |
file(GLOB_RECURSE Z_VCPKG_TOOLS_FILES LIST_DIRECTORIES false "${tools_base_path}/*") | ||
list(REMOVE_ITEM Z_VCPKG_TOOLS_DIRS ${Z_VCPKG_TOOLS_FILES}) | ||
list(REMOVE_ITEM Z_VCPKG_TOOLS_DIRS ${Z_VCPKG_TOOLS_FILES} "") # need at least one item for REMOVE_ITEM if CMake <= 3.19 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to use ""
here, sorry for not having your consent.
@johnmcfarlane Can you please test this changes in your local machine? It should fix your issue. |
@JackBoosY thanks, it unblocks me and although the tools/ directory is still not present, the path being passed into the glob commands looks well-formed: /home/john/ws/wordle/vcpkg/vcpkg_installed/x64-linux/tools/. |
The tools directory only exists if you have a port installed which installs some sort of tool. Otherwise it does not exist by default. |
What does your PR fix?
Fixes using the toolchain with CMake <= 3.19: The recently added
list(REMOVE_ITEM ...)
needs at least one item to be removed. The proposed fix names the quirk and doesn't need any extra conditions.Fixes bd60227#commitcomment-68032427.
Fixes [cmake-user] Re-enable tests with minimum cmake version 3.7.2 #23315 (comment).
Fixes VCPKG fails to configure when running through cmake configuration on linux. #23402.
Which triplets are supported/not supported? Have you updated the CI baseline?
all, no
Does your PR follow the maintainer guide?
yes
If you have added/updated a port: Have you run
./vcpkg x-add-version --all
and committed the result?Not needed.