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

find_package in vcpkg.cmake must honor CMAKE_DISABLE_FIND_PACKAGE_<NAME> #18631

Open
dg0yt opened this issue Jun 24, 2021 · 4 comments
Open

find_package in vcpkg.cmake must honor CMAKE_DISABLE_FIND_PACKAGE_<NAME> #18631

dg0yt opened this issue Jun 24, 2021 · 4 comments
Assignees
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly

Comments

@dg0yt
Copy link
Contributor

dg0yt commented Jun 24, 2021

Describe the bug

In pristine CMake, "every non-REQUIRED find_package() call in a project can be disabled by setting the variable CMAKE_DISABLE_FIND_PACKAGE_<PackageName> to TRUE." This means, there are no side effects.

vcpkg.cmake overrides cmake's original find_package. It always includes vcpkg-cmake-wrapper.cmake for <PackageName> if it exists:

if(EXISTS "${z_vcpkg_find_package_vcpkg_cmake_wrapper_path}")
list(APPEND z_vcpkg_find_package_backup_vars "ARGS")
if(DEFINED ARGS)
set(z_vcpkg_find_package_backup_ARGS "${ARGS}")
else()
set(z_vcpkg_find_package_backup_ARGS)
endif()
set(ARGS "${z_vcpkg_find_package_package_name};${z_vcpkg_find_package_ARGN}")
include("${z_vcpkg_find_package_vcpkg_cmake_wrapper_path}")

So all side effects from the wrapper always occur, e.g. failures to load transitive dependencies.

Actual behaviour:
An example of the actual behaviour is seen in current simage build errors for osx: The simage build system is configured with -DCMAKE_DISABLE_FIND_PACKAGE_TIFF=ON. Still, iff port tiff happens to be already installed (and quicktime is not available), the wrapper for TIFF is loaded, and it causes an configuration error because the dependencies of TIFF (JPEG, ZLIB) are disabled, too:

-- Configuring x64-osx-dbg
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:105 (message):
    Command failed: /Users/vagrant/Data/downloads/tools/cmake-3.20.2-osx/cmake-3.20.2-macos-universal/CMake.app/Contents/bin/cmake /Users/vagrant/Data/buildtrees/simage/src/mage-1.8.0-17f988a113.clean -DSIMAGE_BUILD_SHARED_LIBS=OFF -DSIMAGE_USE_AVIENC=OFF -DSIMAGE_USE_GDIPLUS=OFF -DCMAKE_DISABLE_FIND_PACKAGE_FLAC=ON -DCMAKE_DISABLE_FIND_PACKAGE_Jasper=ON -DCMAKE_DISABLE_FIND_PACKAGE_Doxygen=ON -DCMAKE_DISABLE_FIND_PACKAGE_ZLIB=ON -DCMAKE_DISABLE_FIND_PACKAGE_GIF=ON -DCMAKE_DISABLE_FIND_PACKAGE_JPEG=ON -DCMAKE_DISABLE_FIND_PACKAGE_PNG=ON -DCMAKE_DISABLE_FIND_PACKAGE_TIFF=ON -DCMAKE_MAKE_PROGRAM=/Users/vagrant/Data/downloads/tools/ninja-1.10.2-osx/ninja -DCMAKE_SYSTEM_NAME=Darwin -DBUILD_SHARED_LIBS=OFF -DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=/Users/vagrant/Data/work/1/s/scripts/toolchains/osx.cmake -DVCPKG_TARGET_TRIPLET=x64-osx -DVCPKG_SET_CHARSET_FLAG=ON -DVCPKG_PLATFORM_TOOLSET=external -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=ON -DCMAKE_FIND_PACKAGE_NO_SYSTEM_PACKAGE_REGISTRY=ON -DCMAKE_INSTALL_SYSTEM_RUNTIME_LIBS_SKIP=TRUE -DCMAKE_VERBOSE_MAKEFILE=ON -DVCPKG_APPLOCAL_DEPS=OFF -DCMAKE_TOOLCHAIN_FILE=/Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake -DCMAKE_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION=ON -DVCPKG_CXX_FLAGS= -DVCPKG_CXX_FLAGS_RELEASE= -DVCPKG_CXX_FLAGS_DEBUG= -DVCPKG_C_FLAGS= -DVCPKG_C_FLAGS_RELEASE= -DVCPKG_C_FLAGS_DEBUG= -DVCPKG_CRT_LINKAGE=dynamic -DVCPKG_LINKER_FLAGS= -DVCPKG_LINKER_FLAGS_RELEASE= -DVCPKG_LINKER_FLAGS_DEBUG= -DVCPKG_TARGET_ARCHITECTURE=x64 -DCMAKE_INSTALL_LIBDIR:STRING=lib -DCMAKE_INSTALL_BINDIR:STRING=bin -D_VCPKG_ROOT_DIR=/Users/vagrant/Data/work/1/s -D_VCPKG_INSTALLED_DIR=/Users/vagrant/Data/installed -DVCPKG_MANIFEST_INSTALL=OFF -DCMAKE_OSX_ARCHITECTURES=x86_64 -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=/Users/vagrant/Data/packages/simage_x64-osx/debug
CMake Error at /Users/vagrant/Data/installed/x64-osx/share/jpeg/vcpkg-cmake-wrapper.cmake:1 (_find_package):
  _find_package for module JPEG called with REQUIRED, but
  CMAKE_DISABLE_FIND_PACKAGE_JPEG is enabled.  A REQUIRED package cannot be
  disabled.
Call Stack (most recent call first):
  /Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake:745 (include)
  /Users/vagrant/Data/installed/x64-osx/share/tiff/vcpkg-cmake-wrapper.cmake:17 (find_package)
  /Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake:745 (include)
  CMakeLists.txt:222 (find_package)


CMake Error at /Users/vagrant/Data/work/1/s/scripts/buildsystems/vcpkg.cmake:791 (_find_package):
  _find_package for module ZLIB called with REQUIRED, but
  CMAKE_DISABLE_FIND_PACKAGE_ZLIB is enabled.  A REQUIRED package cannot be
  disabled.

(From #18393 (comment))

Expected behavior:
For consistency with CMake, no side effects are allowed for non-REQUIRED find_package() calls when the variable CMAKE_DISABLE_FIND_PACKAGE_<PackageName> is set to TRUE, i.e. find_package must return early, without loading wrappers.

In the simage example, the TIFF wrapper must not be loaded for the find_package(TIFF) call in simage's CMakeLists.txt, due to
CMAKE_DISABLE_FIND_PACKAGE_TIFF being set.

@dg0yt
Copy link
Contributor Author

dg0yt commented Jun 27, 2021

Expected behavior:
For consistency with CMake, no side effects are allowed for non-REQUIRED find_package() calls when the variable CMAKE_DISABLE_FIND_PACKAGE_<PackageName> is set to TRUE, i.e. find_package must return early, without loading wrappers.

Plus: For REQUIRED calls, find_package in vcpkg.cmake must fail early.

@ras0219-msft
Copy link
Contributor

Yes, this is definitely a bug

@ras0219-msft ras0219-msft added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) and removed requires:discussion labels Jun 29, 2021
ras0219-msft added a commit to ras0219-msft/vcpkg that referenced this issue Jul 26, 2021
@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 30, 2021

Fixed by #18961.

@dg0yt dg0yt closed this as completed Oct 30, 2021
@JackBoosY JackBoosY reopened this Nov 1, 2021
@JackBoosY
Copy link
Contributor

Please keep this issue open until #18961 merged.

@LilyWangLL LilyWangLL assigned Cheney-W and unassigned JackBoosY Dec 6, 2022
@Cheney-W Cheney-W added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly and removed category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) labels Dec 6, 2024
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants