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.cmake] Setup CMAKE_PROGRAM_PATH for the host if possible #23322

Merged
merged 8 commits into from
Mar 4, 2022

Conversation

Neumann-A
Copy link
Contributor

In most useful cases CMAKE_PROGRAM_PATH needs to be setup for the host not the target.
This PR sets it up for the host by default but also allows deactivating vcpkg setup completely or using the current setup.
This avoids hacks like: 4d6220f where CMAKE_PROGRAM_PATH is adjusted before vcpkg to actually be able to find host pkgconf and run it in the configure script.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 28, 2022

I was considering a similar change but the lack of a satisfying response to #17607 held me back. All problems are still valid. In particular, and not resolved with this PR:

  • autotools install to tools/<PORT>/bin.
  • in some cases you really need the debug configuration of a tool.

@Neumann-A
Copy link
Contributor Author

autotools install to tools//bin.

fixed. using GLOB_RECURSE now + some filtering.

in some cases you really need the debug configuration of a tool.

in this case the port can pass it by hand. There are probably only a handfull of ports really needing that (<10 ports)

@Cheney-W Cheney-W added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Mar 2, 2022
@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Mar 4, 2022
@vicroms vicroms merged commit bd60227 into microsoft:master Mar 4, 2022
@Neumann-A Neumann-A deleted the add_host_option branch March 4, 2022 22:17
if(IS_DIRECTORY "${Z_VCPKG_TOOLS_DIR}")
list(APPEND CMAKE_PROGRAM_PATH "${Z_VCPKG_TOOLS_DIR}")
option(VCPKG_SETUP_CMAKE_PROGRAM_PATH "Enable the setup of CMAKE_PROGRAM_PATH to vcpkg paths" ON)
cmake_dependent_option(VCPKG_USE_HOST_TOOLS "Setup CMAKE_PROGRAM_PATH to use host tools" ON "NOT VCPKG_HOST_TRIPLET STREQUAL \"\"" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

CMake Warning (dev) at F:/vcpkg/downloads/tools/cmake-3.22.2-windows/cmake-3.22.2-windows-i386/share/cmake-3.22/Modules/CMakeDependentOption.cmake:84 (message):
  Policy CMP0127 is not set: cmake_dependent_option() supports full Condition
  Syntax.  Run "cmake --help-policy CMP0127" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.
Call Stack (most recent call first):
  F:/vcpkg/scripts/buildsystems/vcpkg.cmake:521 (cmake_dependent_option)
  F:/vcpkg/buildtrees/libwebsockets/x64-windows-rel/CMakeFiles/3.22.2/CMakeSystem.cmake:6 (include)
  F:/vcpkg/buildtrees/libwebsockets/x64-windows-rel/CMakeFiles/CMakeTmp/CMakeLists.txt:5 (project)
This warning is for project developers.  Use -Wno-dev to suppress it.

Copy link

@tusharb86 tusharb86 Jul 22, 2022

Choose a reason for hiding this comment

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

@JackBoosY @Neumann-A - I'm seeing the same warning about CMP0127 in our project. I'm using CMake version 3.23.1. Because vcpkg.cmake is wrapped in a cmake_policy push/pop, I can't set a policy outside of the file as it doesn't have any effect ☹️

It seems like the best option is to place a call to cmake_policy(SET CMP0127 NEW) just before the call to cmake_dependent_option. Does that seem like a reasonable fix? If so, I can prepare the change. Thanks!

Choose a reason for hiding this comment

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

I've created an issue to track this here #25985.

if(VCPKG_SETUP_CMAKE_PROGRAM_PATH)
set(tools_base_path "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/tools")
if(VCPKG_USE_HOST_TOOLS)
set(tools_base_path "${VCPKG_INSTALLED_DIR}/${VCPKG_HOST_TRIPLET}/tools")
Copy link
Contributor

Choose a reason for hiding this comment

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

VCPKG_HOST_TRIPLET maybe empty here.
See #23395.

Comment on lines +529 to +531
file(GLOB_RECURSE Z_VCPKG_TOOLS_DIRS "${tools_base_path}/*")
file(GLOB_RECURSE Z_VCPKG_TOOLS_FILES LIST_DIRECTORIES false "${tools_base_path}/*")
list(REMOVE_ITEM Z_VCPKG_TOOLS_DIRS ${Z_VCPKG_TOOLS_FILES})
Copy link
Contributor

Choose a reason for hiding this comment

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

I can confirm that changing GLOB_RECURSE to GLOB will fix the problem of Z_VCPKG_TOOLS_DIRS being empty when the base path has folders. Is it a cmake bug?

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 info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants