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

[vcpkg] Controlling top-level find_package #27950

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

[vcpkg] Controlling top-level find_package #27950

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7f08f0
Add VCPKG_FIND_PACKAGE_<Pkg>
dg0yt Nov 22, 2022
46ed295
Merge remote-tracking branch 'origin/master' into HEAD
BillyONeal Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/users/buildsystems/cmake-integration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,3 +229,12 @@ See the `--feature-flags=` command line option for more information.

When this option is turned on, every call to `find_package` is printed.
Nested calls (e.g. via `find_dependency`) are indented according to nesting depth.

### `VCPKG_FIND_PACKAGE_<Pkg>`

When this option is turned set, non-nested calls to `find_package` are either
required (`VCPKG_FIND_PACKAGE_<Pkg>=ON`) or disabled (`VCPKG_FIND_PACKAGE_<Pkg>=OFF`).

This variable is a tool to control direct dependencies and related features in vcpkg ports
which use the CMake build system. It can be used with `vcpkg_check_features` and avoids
unintented effects on transitive dependencies.
28 changes: 27 additions & 1 deletion scripts/buildsystems/vcpkg.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,29 @@ macro("${VCPKG_OVERRIDE_FIND_PACKAGE_NAME}" z_vcpkg_find_package_package_name)
set(z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_ARGN "${ARGN}")
set(z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_backup_vars "")

if(z_vcpkg_find_package_backup_id EQUAL "1")
if(VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name})
Comment on lines +785 to +786
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(z_vcpkg_find_package_backup_id EQUAL "1")
if(VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name})
if(z_vcpkg_find_package_backup_id EQUAL "1")
# This is the top-level find_package call
if(VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name})

# Avoid CMake warning when both REQUIRED and CMAKE_REQUIRE_FIND_PACKAGE_<Pkg> are used
if(NOT "REQUIRED" IN_LIST z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_ARGN)
Copy link
Member

Choose a reason for hiding this comment

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

Stupid question: Why is "REQUIRED" in quotes but z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_ARGN not?

list(APPEND z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_backup_vars "CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name}")
set(z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_backup_CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} "${CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name}}")
Copy link
Member

Choose a reason for hiding this comment

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

Stupid question: How does this do the right thing if CMAKE_REQUIRE_FIND_PACKAGE_Blah is undefined? It looks like the backup/restore thingy below would make it be empty string instead?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think this is wrong and it should be:

Suggested change
set(z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_backup_CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} "${CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name}}")
if(DEFINED "CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name}")
set(z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_backup_CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} "${CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name}}")
endif()

Ditto below for the 'disable' arm

Copy link
Member

Choose a reason for hiding this comment

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

Also, should we always backup and restore both the REQUIRED and DISABLE one? So that
-DCMAKE_DISABLE_FIND_PACKAGE_Blah=1 -DVCPKG_FIND_PACKAGE_Blah=1 chooses in favor of the VCPKG_ one. Or maybe we should just make such a case an error?

set(CMAKE_REQUIRE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} 1)
endif()
if(VCPKG_TRACE_FIND_PACKAGE)
message(STATUS " (required by VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name}=${VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name}})")
endif()
elseif(DEFINED VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name})
list(APPEND z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_backup_vars "CMAKE_DISABLE_FIND_PACKAGE_${z_vcpkg_find_package_package_name}")
set(z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_backup_CMAKE_DISABLE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} "${CMAKE_DISABLE_FIND_PACKAGE_${z_vcpkg_find_package_package_name}}")
set(CMAKE_DISABLE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} 1)
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this would result in the value being still disabled in transitive find_package calls? What clears this when we're down a level?

Oh, Bill is an idiot:

Suggested change
set(CMAKE_DISABLE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} 1)
# We don't need to worry about clearing this for transitive users because
# once this top level find_package is disabled, we immediately will return
# not found and not try to visit transitive dependencies in the first place.
set(CMAKE_DISABLE_FIND_PACKAGE_${z_vcpkg_find_package_package_name} 1)

if(VCPKG_TRACE_FIND_PACKAGE)
message(STATUS " (disabled by VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name}=${VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name}})")
endif()
elseif(VCPKG_TRACE_FIND_PACKAGE)
message(STATUS " (could be controlled by VCPKG_FIND_PACKAGE_${z_vcpkg_find_package_package_name})")
endif()
endif()

# Workaround to set the ROOT_PATH until upstream CMake stops overriding
# the ROOT_PATH at apple OS initialization phase.
# See https://gitlab.kitware.com/cmake/cmake/merge_requests/3273
Expand All @@ -783,7 +806,10 @@ macro("${VCPKG_OVERRIDE_FIND_PACKAGE_NAME}" z_vcpkg_find_package_package_name)
string(TOLOWER "${z_vcpkg_find_package_package_name}" z_vcpkg_find_package_lowercase_package_name)
set(z_vcpkg_find_package_vcpkg_cmake_wrapper_path
"${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/share/${z_vcpkg_find_package_lowercase_package_name}/vcpkg-cmake-wrapper.cmake")
if(EXISTS "${z_vcpkg_find_package_vcpkg_cmake_wrapper_path}")
if(CMAKE_DISABLE_FIND_PACKAGE_${z_vcpkg_find_package_package_name})
# Skip wrappers, fail if REQUIRED.
_find_package("${z_vcpkg_find_package_package_name}" ${z_vcpkg_find_package_${z_vcpkg_find_package_backup_id}_ARGN})
elseif(EXISTS "${z_vcpkg_find_package_vcpkg_cmake_wrapper_path}")
if(VCPKG_TRACE_FIND_PACKAGE)
string(REPEAT " " "${z_vcpkg_find_package_backup_id}" z_vcpkg_find_package_indent)
message(STATUS "${z_vcpkg_find_package_indent}using share/${z_vcpkg_find_package_lowercase_package_name}/vcpkg-cmake-wrapper.cmake")
Expand Down