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_copy_tool_dependencies] Switch to cmake function file(GET_RUNTIME_DEPENDENCIES) #26585

Conversation

JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented Aug 29, 2022

Currently, vcpkg_copy_tool_dependencies calls the ps1 script applocal.ps1 on Windows to perform the function of analyzing dependencies and copying dependencies, which is affected by the powershell version and the Visual Studio English language package.
Switch to cmake functions to avoid these issues.

  • vcpkg_copy_tool_dependencies
  • magnumdeploy.ps1
  • k4adeploy.ps1
  • openni2deploy.ps1
  • qtdeploy.ps1
  • vcpkg.cmake

Related official document: https://cmake.org/cmake/help/latest/command/file.html#get-runtime-dependencies

Resolve: #17448 #17890 #16721 #14339 #10619 #12849

@JackBoosY JackBoosY added info:internal This PR or Issue was filed by the vcpkg team. category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Aug 29, 2022
@JackBoosY JackBoosY marked this pull request as draft August 29, 2022 14:16
find_program(Z_VCPKG_POWERSHELL_CORE pwsh)
if (NOT Z_VCPKG_POWERSHELL_CORE)
message(FATAL_ERROR "Could not find PowerShell Core; please open an issue to report this.")
if (CMAKE_MAJOR_VERSION VERSION_LESS 3 OR CMAKE_MINOR_VERSION VERSION_LESS 16)
Copy link
Contributor

Choose a reason for hiding this comment

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

broken with eventual cmake 4.0

Choose a reason for hiding this comment

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

There should be no need to test the individual version components separately. Operators like VERSION_LESS handle full version numbers:

Suggested change
if (CMAKE_MAJOR_VERSION VERSION_LESS 3 OR CMAKE_MINOR_VERSION VERSION_LESS 16)
if(CMAKE_VERSION VERSION_LESS 3.16)

Copy link
Contributor Author

@JackBoosY JackBoosY Aug 31, 2022

Choose a reason for hiding this comment

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

CMAKE_VERSION is only exists in and after cmake 2.6.3, see https://cmake.org/cmake/help/latest/variable/CMAKE_VERSION.html

Copy link
Contributor

Choose a reason for hiding this comment

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

are we supporting such an old CMake version in downstream projects? I don't think so!
also this script runs in internal cmake, which means we can use the latest and greatest, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are we supporting such an old CMake version in downstream projects? I don't think so! also this script runs in internal cmake, which means we can use the latest and greatest, right?

Good question! Therefore, we must choose one of the following ways:

  1. Always use cmake from vcpkg, which will go against VCPKG_FORCE_BINARIES.
  2. Keep the original code and enable the new code when the current cmake version is higher than the version that can support this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, we are working on directly incorporating this function into vcpkg-tool, including parsing PE, elf and mach-O formats.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question!

No, it was a rhetorical question. vcpkg requires at least CMake 3.7 in user projects, and script may rely on the version which is downloaded by vcpkg.

Copy link
Contributor

Choose a reason for hiding this comment

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

@JackBoosY
Copy link
Contributor Author

Currently stuck at upstream issue: https://gitlab.kitware.com/cmake/cmake/-/issues/23906

@JackBoosY JackBoosY closed this Oct 8, 2022
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:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
6 participants