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_configure_cmake] Introduce REQUIRE_ALL_PACKAGES #15808

Closed
wants to merge 3 commits into from
Closed

[vcpkg_configure_cmake] Introduce REQUIRE_ALL_PACKAGES #15808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52dfb19
[docs/regenerate.ps1] Fix on Linux
ras0219 Jan 21, 2021
49402d1
[vcpkg_configure_cmake] Add VCPKG_REQUIRE_ALL_PACKAGES
ras0219 Jan 21, 2021
aaadf6f
[many ports] Apply REQUIRE_ALL_PACKAGES
ras0219 Jan 21, 2021
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
3 changes: 3 additions & 0 deletions docs/maintainers/vcpkg_configure_cmake.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ Configure CMake for Debug and Release builds of a project.
vcpkg_configure_cmake(
SOURCE_PATH <${SOURCE_PATH}>
[PREFER_NINJA]
[REQUIRE_ALL_PACKAGES]
[OPTIONAL_PACKAGES <ZLIB>...]
[DISABLE_PACKAGES <PNG>...]
[DISABLE_PARALLEL_CONFIGURE]
[NO_CHARSET_FLAG]
[GENERATOR <"NMake Makefiles">]
Expand Down
5 changes: 0 additions & 5 deletions ports/armadillo/CONTROL
View file
Open in desktop

This file was deleted.

8 changes: 2 additions & 6 deletions ports/armadillo/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,11 @@ file(REMOVE ${SOURCE_PATH}/cmake_aux/Modules/ARMA_FindOpenBLAS.cmake)
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
DISABLE_PARALLEL_CONFIGURE
REQUIRE_ALL_PACKAGES
DISABLE_PACKAGES SuperLU ACML ACMLMP ARPACK ATLAS MKL PkgConfig
PREFER_NINJA
OPTIONS
-DDETECT_HDF5=false
-DCMAKE_DISABLE_FIND_PACKAGE_SuperLU=ON
-DCMAKE_DISABLE_FIND_PACKAGE_ACML=ON
-DCMAKE_DISABLE_FIND_PACKAGE_ACMLMP=ON
-DCMAKE_DISABLE_FIND_PACKAGE_ARPACK=ON
-DCMAKE_DISABLE_FIND_PACKAGE_ATLAS=ON
-DCMAKE_DISABLE_FIND_PACKAGE_MKL=ON
)

vcpkg_install_cmake()
Expand Down
11 changes: 11 additions & 0 deletions ports/armadillo/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "armadillo",
"version": "10.2.0",
"port-version": 1,
"description": "Armadillo is a high quality linear algebra library (matrix maths) for the C++ language, aiming towards a good balance between speed and ease of use",
"homepage": "http://arma.sourceforge.net",
"dependencies": [
"blas",
"lapack"
]
}
5 changes: 0 additions & 5 deletions ports/cppkafka/CONTROL
View file
Open in desktop

This file was deleted.

4 changes: 3 additions & 1 deletion ports/cppkafka/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ endif()
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
PREFER_NINJA
OPTIONS
REQUIRE_ALL_PACKAGES
DISABLE_PACKAGES Doxygen
OPTIONS
-DCPPKAFKA_BUILD_SHARED=${CPPKAFKA_BUILD_SHARED}
-DCPPKAFKA_DISABLE_TESTS=ON
-DCPPKAFKA_DISABLE_EXAMPLES=ON
Expand Down
11 changes: 11 additions & 0 deletions ports/cppkafka/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "cppkafka",
"version": "0.3.1",
"port-version": 3,
"description": "cppkafka allows C++ applications to consume and produce messages using the Apache Kafka protocol. The library is built on top of librdkafka, and provides a high level API that uses modern C++ features to make it easier to write code while keeping the wrapper's performance overhead to a minimum.",
"homepage": "https://github.com/mfontanini/cppkafka",
"dependencies": [
"boost-program-options",
"librdkafka"
]
}
45 changes: 45 additions & 0 deletions ports/gdal/gdal-config.in.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_DIR}" DIRECTORY)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" DIRECTORY)

add_library(GDAL::GDAL UNKNOWN IMPORTED)

set(GDAL_INCLUDE_DIRS "${_IMPORT_PREFIX}/include")
set(GDAL_INCLUDE_DIR "${GDAL_INCLUDE_DIRS}" CACHE STRING "")

find_library(GDAL_LIBRARY_RELEASE NAMES gdal_i gdal PATHS "${_IMPORT_PREFIX}/lib" NO_DEFAULT_PATH REQUIRED)
set(GDAL_LIBRARIES optimized "${GDAL_LIBRARY_RELEASE}")
set_property(TARGET GDAL::GDAL APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(
GDAL::GDAL PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${GDAL_INCLUDE_DIRS}"
IMPORTED_LOCATION_RELEASE "${GDAL_LIBRARY_RELEASE}"
)

find_library(GDAL_LIBRARY_DEBUG NAMES gdal_i_d gdal_d gdal_i gdal PATHS "${_IMPORT_PREFIX}/debug/lib" NO_DEFAULT_PATH)
if(GDAL_LIBRARY_DEBUG)
set_property(TARGET GDAL::GDAL APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
set_target_properties(
GDAL::GDAL PROPERTIES
IMPORTED_LOCATION_DEBUG "${GDAL_LIBRARY_DEBUG}"
)
list(APPEND GDAL_LIBRARIES debug "${GDAL_LIBRARY_DEBUG}")
endif()

include(CMakeFindDependencyMacro)

find_dependency(geos CONFIG)
set_property(TARGET GDAL::GDAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES GEOS::geos_c GEOS::geos)

find_dependency(OpenJPEG CONFIG)
set_property(TARGET GDAL::GDAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES openjp2)

if(@DEPENDENCY_TIFF@)
find_dependency(TIFF)
set_property(TARGET GDAL::GDAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES TIFF::TIFF)
endif()

find_dependency(geotiff CONFIG)
set_property(TARGET GDAL::GDAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES geotiff_library)

find_dependency(LibXml2)
set_property(TARGET GDAL::GDAL APPEND PROPERTY INTERFACE_LINK_LIBRARIES LibXml2::LibXml2)
128 changes: 119 additions & 9 deletions ports/gdal/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ vcpkg_extract_source_archive_ex(
PATCHES ${GDAL_PATCHES}
)

set(DEPENDENCY_TIFF OFF)

if (VCPKG_TARGET_IS_WINDOWS)
set(NATIVE_DATA_DIR "${CURRENT_PACKAGES_DIR}/share/gdal")
set(NATIVE_HTML_DIR "${CURRENT_PACKAGES_DIR}/share/gdal/html")
Expand Down Expand Up @@ -183,32 +185,140 @@ else()
set(BUILD_DYNAMIC no)
set(BUILD_STATIC yes)
endif()

set(CONF_OPTS --enable-shared=${BUILD_DYNAMIC} --enable-static=${BUILD_STATIC})
list(APPEND CONF_OPTS --with-proj=${CURRENT_INSTALLED_DIR} --with-libjson-c=${CURRENT_INSTALLED_DIR} --without-freexl)
list(APPEND CONF_OPTS --without-jasper)


vcpkg_find_acquire_program(PKGCONFIG)

set(backup_PKG_CONFIG_PATH "$ENV{PKG_CONFIG_PATH}")
set(ENV{PKG_CONFIG_PATH} "${CURRENT_INSTALLED_DIR}/lib/pkgconfig${VCPKG_HOST_PATH_SEPARATOR}$ENV{PKG_CONFIG_PATH}")
execute_process(
COMMAND "${PKGCONFIG}" --libs libtiff-4 libxml-2.0 libpng json-c netcdf libopenjp2 geos expat cfitsio
OUTPUT_VARIABLE release_libs
OUTPUT_STRIP_TRAILING_WHITESPACE
)
set(ENV{PKG_CONFIG_PATH} "${CURRENT_INSTALLED_DIR}/debug/lib/pkgconfig${VCPKG_HOST_PATH_SEPARATOR}${backup_PKG_CONFIG_PATH}")
execute_process(
COMMAND "${PKGCONFIG}" --libs libtiff-4 libxml-2.0 libpng json-c netcdf libopenjp2 geos expat cfitsio
OUTPUT_VARIABLE debug_libs
OUTPUT_STRIP_TRAILING_WHITESPACE
)
set(ENV{PKG_CONFIG_PATH} "${backup_PKG_CONFIG_PATH}")

# This is required to link proj4 statically.
if(VCPKG_TARGET_IS_OSX)
set(cppstdlib c++)
else()
set(cppstdlib stdc++)
endif()

vcpkg_configure_make(
SOURCE_PATH ${SOURCE_PATH}
AUTOCONFIG
COPY_SOURCE
OPTIONS ${CONF_OPTS}
OPTIONS_RELEASE
"LIBS=${release_libs} -l${cppstdlib} -lm -lpthread"
OPTIONS_DEBUG
--enable-debug
--without-fit # Disable cfitsio temporary
"LIBS=${debug_libs} -l${cppstdlib} -lm -lpthread"
OPTIONS
HAVE_GEOS=yes
HAVE_EXPAT=yes
--enable-shared=${BUILD_DYNAMIC}
--enable-static=${BUILD_STATIC}
--with-proj=${CURRENT_INSTALLED_DIR}
--with-liblzma=${CURRENT_INSTALLED_DIR}
--without-zstd
--without-pg
--without-grass
--without-libgrass
--with-cfitsio=${CURRENT_INSTALLED_DIR}
--with-pcraster=internal
--with-png=${CURRENT_INSTALLED_DIR}
--without-dds
--without-gta
--with-pcidsk=internal
--with-libtiff=${CURRENT_INSTALLED_DIR}
--with-geotiff=${CURRENT_INSTALLED_DIR}
--with-jpeg=${CURRENT_INSTALLED_DIR}
--without-charls
--without-jpeg12
--with-gif=internal
--without-ogdi
--without-fme
--without-sosi
--without-mongocxx
--without-boost-lib-path
--without-mongocxxv3
--without-hdf4
--without-hdf5
--without-kea
--with-netcdf=${CURRENT_INSTALLED_DIR}
--without-jasper
--with-openjpeg=${CURRENT_INSTALLED_DIR}
--without-fgdb
--without-ecw
--without-kakadu
--without-mrsid
--without-jp2mrsid
--without-mrsid_lidar
--without-j2lua
--without-msg
--without-oci
--without-mysql
--without-ingres
--without-xerces
--with-expat
--without-libkml
--without-odbc
--without-dods-root
--without-curl
--with-xml2=yes
--without-spatialite
--with-sqlite3=yes
--without-rasterlite2
--without-pcre
--without-teigha
--without-idb
--without-sde
--without-epsilon
--without-webp
--with-geos=yes
--without-qhull
--without-opencl
--without-freexl
--with-libjson-c=${CURRENT_INSTALLED_DIR}
--without-pam
--without-poppler
--without-podofo
--without-pdfium
--without-perl
--without-python
--without-java
--without-hdfs
--without-tiledb
--without-mdb
--without-rasdaman
--without-rdb
--without-armadillo
--without-cryptopp
--with-cypto=no
--without-lerc
--without-exr
)

vcpkg_install_make(MAKEFILE GNUmakefile)

file(REMOVE_RECURSE
${CURRENT_PACKAGES_DIR}/lib/gdalplugins
${CURRENT_PACKAGES_DIR}/debug/lib/gdalplugins
${CURRENT_PACKAGES_DIR}/debug/share
)

set(DEPENDENCY_TIFF ON)
endif()

file(INSTALL ${CMAKE_CURRENT_LIST_DIR}/usage DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT})
configure_file(${CMAKE_CURRENT_LIST_DIR}/vcpkg-cmake-wrapper.cmake ${CURRENT_PACKAGES_DIR}/share/${PORT}/vcpkg-cmake-wrapper.cmake @ONLY)
configure_file(${CMAKE_CURRENT_LIST_DIR}/gdal-config.in.cmake ${CURRENT_PACKAGES_DIR}/share/${PORT}/gdal-config.cmake @ONLY)

# Handle copyright
file(INSTALL ${SOURCE_PATH}/LICENSE.TXT DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT} RENAME copyright)
5 changes: 2 additions & 3 deletions ports/gdal/usage
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
The package GDAL provides CMake targets:
The package gdal:@TARGET_TRIPLET@ is compatible with built-in CMake Targets:

find_package(GDAL REQUIRED)
target_include_directories(main PRIVATE ${GDAL_INCLUDE_DIRS})
target_link_libraries(main PRIVATE ${GDAL_LIBRARIES})
target_link_libraries(main PRIVATE GDAL::GDAL)
13 changes: 1 addition & 12 deletions ports/gdal/vcpkg-cmake-wrapper.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1 @@
include(FindPackageHandleStandardArgs)
include(SelectLibraryConfigurations)

find_path(GDAL_INCLUDE_DIR NAMES gdal.h HINTS ${CURRENT_INSTALLED_DIR})

find_library(GDAL_LIBRARY_DEBUG NAMES gdal_d gdal_i_d gdal NAMES_PER_DIR PATH_SUFFIXES lib PATHS "${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug" NO_DEFAULT_PATH REQUIRED)
find_library(GDAL_LIBRARY_RELEASE NAMES gdal_i gdal NAMES_PER_DIR PATH_SUFFIXES lib PATHS "${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}" NO_DEFAULT_PATH REQUIRED)

select_library_configurations(GDAL)

set(GDAL_INCLUDE_DIRS ${GDAL_INCLUDE_DIR})
set(GDAL_LIBRARIES ${GDAL_LIBRARY})
_find_package(${ARGS} CONFIG)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is indirectly providing a FindGDAL.cmake via a vcpkg created -config.cmake. Maybe finally decide to support a CMake module directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why we don't use the module directory approach is twofold:

  1. Many projects do something like:
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake")

which would break the "obvious" way to provide a module path.
2. Because vcpkg packages are the "producers", they should provide -config.cmake files, which are essentially strictly superior to Find modules (notably they support find_dependency()).

I took this approach for GDAL to replace the official target with a config file, since I felt that we could provide the full official interface easily enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

Concerning 1:
The reason projects does this is because the cmake docs tell them to set it. In my opinion, this is a case of outdated/Wrong docs since CMAKE_MODULE_PATH can be provided either by a toolchain or by a superproject/superbuild so only setting the value is conceptually wrong and needs correction. (i.e. list(PREPEND) or similar is the correct way to go. See e.g. VTK). Modifying&backuping the CMAKE_MODULE_PATH is for example also done by VTK-Config.cmake

Concerning 2:
If the information is provided by vcpkg it does not matter here if it is <package>-config.cmake or Find<package>.cmake because you can encode the same information in both. The config way would only be superior if you also supply a <configfile>Version.cmake. Although the find_dependency part is correct you just can wrap it into a minimal function in a Find<package>.cmake to workaround that.

My main nitpick here ist that with a CMAKE_MODULE_PATH you wouldn't need to use the indirection with the vcpkg-cmake-wrapper.cmake here. Furthermore redirection of all calls to FindGDAL.cmake is possibly breaking downstream users with a custom FindGDAL.cmake which does not define the same targets and/or variables as the vcpkg one without a way to bail out. So chose your poison very carefully.

Copy link
Contributor

@ras0219-msft ras0219-msft Feb 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed that the current practice of many projects is "wrong"/"bad", but patching the world is never a good solution.

It is true that we are possibly overriding a package's custom FindGDAL.cmake, however these custom find scripts are (in general) fundamentally broken in the face of static libraries, because there's no way for them to know the precise dependencies that were used during the build. Additionally, as long as we adhere to "only use vcpkg-cmake-wrapper to wrap built-in CMake modules", the probability of custom FindXYZ.cmake scripts is much lower (but non-zero -- we've both seen it happen).

Copy link
Contributor

Choose a reason for hiding this comment

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

because there's no way for them to know the precise dependencies that were used during the build

Unless they are using the FindPkgConfig module + pkg_check_modules in the Find<Package>.cmake. If they also use the IMPORTED_TARGET option and link against the created target it should be always correct (unless there is something wrong with the pc files generated by vcpkg). Unfortunately, I have also seen that the results of pkg_check_modules is sometimes only used as hinting for find_library calls etc. (Source: Qt6)

Since I saw a mention of changes to FindGDAL in the CMake 3.20 release notes you might want to have look at in the CMake RC to make sure everything is ok with the config here.

40 changes: 31 additions & 9 deletions ports/gdal/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,51 @@
{
"name": "gdal",
"version-string": "3.1.3",
"port-version": 3,
"version-semver": "3.1.3",
"port-version": 4,
"description": "The Geographic Data Abstraction Library for reading and writing geospatial raster and vector data.",
"homepage": "hhttps://gdal.org/",
"homepage": "https://gdal.org/",
"dependencies": [
"cfitsio",
"curl",
"expat",
{
"name": "curl",
"platform": "windows"
},
{
"name": "expat",
"platform": "windows"
},
"geos",
"hdf5",
{
"name": "hdf5",
"platform": "windows"
},
{
"name": "json-c",
"platform": "!windows"
},
"libgeotiff",
{
"name": "libjpeg-turbo",
"platform": "!windows"
},
"liblzma",
"libpng",
"libpq",
"libwebp",
{
"name": "libpq",
"platform": "windows"
},
{
"name": "libwebp",
"platform": "windows"
},
"libxml2",
"netcdf-c",
"openjpeg",
"proj4",
"sqlite3",
{
"name": "tiff",
"platform": "!windows"
},
"zlib"
],
"features": {
Expand Down
4 changes: 0 additions & 4 deletions ports/geos/CONTROL
View file
Open in desktop

This file was deleted.

23 changes: 23 additions & 0 deletions ports/geos/pc.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
index 8a81a2a..b3e3da1 100644
--- a/tools/CMakeLists.txt
+++ b/tools/CMakeLists.txt
@@ -12,7 +12,7 @@
#################################################################################


-if(NOT MSVC)
+if(TRUE)
# Consider CMAKE_INSTALL_PREFIX with spaces
string(REPLACE " " "\\ " ESCAPED_INSTALL_PREFIX ${CMAKE_INSTALL_PREFIX})
configure_file(
diff --git a/tools/geos.pc.cmake b/tools/geos.pc.cmake
index 0a9df7f..443b770 100644
--- a/tools/geos.pc.cmake
+++ b/tools/geos.pc.cmake
@@ -8,4 +8,4 @@ Description: Geometry Engine, Open Source - C API
Requires:
Version: @GEOS_VERSION@
Cflags: -I${includedir}
-Libs: -L${libdir} -lgeos_c
+Libs: -L${libdir} -lgeos_c@CMAKE_DEBUG_POSTFIX@ -lgeos@CMAKE_DEBUG_POSTFIX@
Loading