Skip to content

[gdal] Update to 3.5.1, build with CMake#22392

Merged
vicroms merged 37 commits intomicrosoft:masterfrom
dg0yt:gdal-cmake
Jul 7, 2022
Merged

[gdal] Update to 3.5.1, build with CMake#22392
vicroms merged 37 commits intomicrosoft:masterfrom
dg0yt:gdal-cmake

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 6, 2022

  • What does your PR fix?

    GDAL will switch to CMake as single build system. At the moment it is experimental and not ready for production.
    This PR attempts to mirror the state of the new build system for testing in the context of vcpkg. In particular the static configuration in Linux isn't sufficiently tested upstream, so they will need feedback. Vcpkg eventually can get rid of divergent feature sets between Windows (nmake) and the rest of the world (autotools).

    Updates GDAL to 3.5.1 (currently RC1, waiting for official release).
    Switches to CMake.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    uwp unsupported (due to missing win32 function), no.
    (Actually the uwp issue only appeared very late. I have no idea why it worked before.)

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    yes

@Neumann-A
Copy link
Contributor

Why the hell are they inspecting INTERFACE_LINK_LIBRARIES and IMPORTED_LOCATION(_CONFIG) ? Just saying INTERFACE_LINK_LIBRARIES can and will contain a genex which commonly breaks such inspections.

@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jan 7, 2022
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 7, 2022

Why the hell ...

Too early for such negative phrases. The CMake build system is adopted from one of two forks that tried that as an individiual effort. Don't expect best practice. It will remain experimental with GDAL 3.5. This community is still learning CMake.

... but important to point out such issues now. For exporting linker flags to pc files, I don't even see a common practice in general. That's why we have so mucht work in getting pc files right in vcpkg.

FTR, I had to patch this section because the vcpkg wrapper for FindEXPAT.cmake turns EXPAT_LIBRARY in to a list (optimized/debug) while pristine CMake sets a file path. Like happened before e.g. with FindJPEG.cmake, this breaks get_filename_component on this variable.
(#21783 would help to resolve wrapper misbehaviour. We might even create a variable which can be added to the options of consuming ports to indicate that we need wrappers to provide file paths in PKG_LIBRARY.)

@Neumann-A
Copy link
Contributor

For exporting linker flags to pc files, I don't even see a common practice in general.

make some noise in
https://gitlab.kitware.com/cmake/cmake/-/issues/22621
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6363
and maybe cmake will get proper pkgconfig export support. Without it is is basically impossible to produce correct pc files in all cases.

@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 25, 2022

Initial build: https://dev.azure.com/vcpkg/public/_build/results?buildId=65617&view=results
Force-pushing PR update.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/gdal/vcpkg.json b/ports/gdal/vcpkg.json
index 87cc790..6d52201 100644
--- a/ports/gdal/vcpkg.json
+++ b/ports/gdal/vcpkg.json
@@ -6,8 +6,8 @@
   "supports": "!uwp",
   "dependencies": [
     {
-      "name": "json-c",
-      "$platform": "!windows | mingw"
+      "$platform": "!windows | mingw",
+      "name": "json-c"
     },
     "libgeotiff",
     "libiconv",
@@ -46,18 +46,18 @@
         }
       ]
     },
-    "geos": {
-      "description": "Enable GEOS support",
-      "dependencies": [
-        "geos"
-      ]
-    },
     "freexl": {
       "description": "Enable FREEXL support",
       "dependencies": [
         "freexl"
       ]
     },
+    "geos": {
+      "description": "Enable GEOS support",
+      "dependencies": [
+        "geos"
+      ]
+    },
     "hdf5": {
       "description": "Enable HDF5 support",
       "dependencies": [
@@ -98,6 +98,14 @@
       "description": "Core dependencies and features for which this port doesn't offer individual control at the moment",
       "dependencies": [
         "expat",
+        {
+          "name": "gdal",
+          "default-features": false,
+          "features": [
+            "curl",
+            "geos"
+          ]
+        },
         "giflib",
         "libjpeg-turbo",
         "liblzma",
@@ -108,15 +116,7 @@
         "pcre2",
         "sqlite3",
         "tiff",
-        "zstd",
-        {
-          "name": "gdal",
-          "default-features": false,
-          "features": [
-            "curl",
-            "geos"
-          ]
-        }
+        "zstd"
       ]
     },
     "supported-default-features": {
PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for osgearth but no changes to version or port version.
-- Version: 3.2#3
-- Old SHA: 2e3d8cf49728cdb23060f31152768a2292c24ab8
-- New SHA: c4672b4f8933cd72d6f6f3491a71992fbd8649d6
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout b18b17865cfb6bd24620a00f30691be6775abb96 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 89788d0..1f21f3d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2377,7 +2377,7 @@
       "port-version": 1
     },
     "gdal": {
-      "baseline": "3.4.1",
+      "baseline": "3.5.0",
       "port-version": 0
     },
     "gdcm": {
diff --git a/versions/g-/gdal.json b/versions/g-/gdal.json
index dc71637..1435834 100644
--- a/versions/g-/gdal.json
+++ b/versions/g-/gdal.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "a93f30f83c9c167330057e528a42985cabacd70b",
+      "version-semver": "3.5.0",
+      "port-version": 0
+    },
     {
       "git-tree": "a905d07fce3cebd0d1b52809cbdf1e37cce60cf3",
       "version-semver": "3.4.1",

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/gdal/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

@dg0yt dg0yt force-pushed the gdal-cmake branch 6 times, most recently from 603c19e to a53d65a Compare January 27, 2022 17:20
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 28, 2022

It seems that it reached a working state. The port still needs some work, but I plan to use this for gdal 3.5.0.

@dg0yt dg0yt changed the title [gdal] Build with CMake (experimental, WIP) [gdal] Build with CMake (WIP) Jan 28, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for osgearth but no changes to version or port version.
-- Version: 3.2#3
-- Old SHA: 2e3d8cf49728cdb23060f31152768a2292c24ab8
-- New SHA: 3f3e5b5739f5a36165b2ae9de30d4e9aeceeb94f
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 07e508359ca5488456988191d8709722484daccd -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 4058e15..2216f20 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2377,8 +2377,8 @@
       "port-version": 1
     },
     "gdal": {
-      "baseline": "3.4.1",
-      "port-version": 1
+      "baseline": "3.5.0",
+      "port-version": 0
     },
     "gdcm": {
       "baseline": "3.0.7",
diff --git a/versions/g-/gdal.json b/versions/g-/gdal.json
index 569ea9a..df72766 100644
--- a/versions/g-/gdal.json
+++ b/versions/g-/gdal.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "562f0d79d0d90a21d7ea13c559f22a6d87b29274",
+      "version-semver": "3.5.0",
+      "port-version": 0
+    },
     {
       "git-tree": "b3bff053bc6f972c2c19f88047f2a5ae53f21746",
       "version-semver": "3.4.1",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/gdal/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for osgearth but no changes to version or port version.
-- Version: 3.2#3
-- Old SHA: 2e3d8cf49728cdb23060f31152768a2292c24ab8
-- New SHA: 3f3e5b5739f5a36165b2ae9de30d4e9aeceeb94f
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout c93faafe6d892aca636765e3a7e67797736b0b09 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 0edbad2..aca7666 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2377,8 +2377,8 @@
       "port-version": 1
     },
     "gdal": {
-      "baseline": "3.4.1",
-      "port-version": 1
+      "baseline": "3.5.0",
+      "port-version": 0
     },
     "gdcm": {
       "baseline": "3.0.7",
diff --git a/versions/g-/gdal.json b/versions/g-/gdal.json
index 569ea9a..6fc7af2 100644
--- a/versions/g-/gdal.json
+++ b/versions/g-/gdal.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "d657b17c2ac934eb86831ac45c5486a8d0aed660",
+      "version-semver": "3.5.0",
+      "port-version": 0
+    },
     {
       "git-tree": "b3bff053bc6f972c2c19f88047f2a5ae53f21746",
       "version-semver": "3.4.1",

You have modified or added at least one vcpkg.json where a "license" field is missing.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/gdal/vcpkg.json

Valid values for the license field can be found in the documentation

@dg0yt
Copy link
Contributor Author

dg0yt commented Jul 1, 2022

@JackBoosY It is not clear to me which question "requires author response".

JackBoosY
JackBoosY previously approved these changes Jul 1, 2022
@JackBoosY
Copy link
Contributor

Please update the version info.

@dg0yt
Copy link
Contributor Author

dg0yt commented Jul 1, 2022

Please update the version info.

Still not the official release.

@m-kuhn
Copy link
Contributor

m-kuhn commented Jul 1, 2022

  • poppler will need a patch to respect transitive dependencies (fontconfig, freetype visible through pkgconfig) similar to spatialite, I'll follow up with a PR
  • 3.5.1- RC2 has been released

@dg0yt
Copy link
Contributor Author

dg0yt commented Jul 1, 2022

RC2 is included.
I will check for more PkgConfig usage, in the features via make available.

@dg0yt
Copy link
Contributor Author

dg0yt commented Jul 2, 2022

Additional link libs now for gdal[cfitsio,freexl,lerc,mysql-libmariadb,poppler]:x64-linux:

-lfreetype
-lbz2
-lbrotlidec-static
-lbrotlicommon-static

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for gdal have changed but the version was not updated
version: 3.5.1
old SHA: 1faa4583d3d9e2928d7d6d50a2937d8523340a00
new SHA: 22bf8c93a36f7421a203b633136218d2811a03f4
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

@dg0yt dg0yt requested a review from JackBoosY July 7, 2022 08:09
@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jul 7, 2022
@vicroms vicroms merged commit e6c8c2b into microsoft:master Jul 7, 2022
if(TARGET PROJ::proj)
set(PROJ_LIBRARIES PROJ::proj)
- string(APPEND CONFIG_DEPENDENCIES " find_dependency(PROJ CONFIG)\n")
+ string(APPEND CONFIG_PRIVATE_DEPENDENCIES " find_dependency(PROJ CONFIG)\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this is is incorrect:
In geotiff-depends-debug.cmake:

set_target_properties(geotiff_library PROPERTIES
  IMPORTED_IMPLIB_DEBUG "${_IMPORT_PREFIX}/debug/lib/geotiff_d_i.lib"
  IMPORTED_LINK_DEPENDENT_LIBRARIES_DEBUG "PROJ::proj"
  IMPORTED_LINK_INTERFACE_LIBRARIES_DEBUG "TIFF::TIFF"
  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/debug/bin/geotiff_d.dll"
  )

I'm going to fix this and the find_package name now.

@JackBoosY
Copy link
Contributor

JackBoosY commented Jul 8, 2022

When building gdal[core,curl,default-features,geos,hdf5,libspatialite,netcdf,postgresql,recommended-features]:x64-windows:

-- Could NOT find HDF5 (missing: HDF5_INCLUDE_DIRS) (found version "1.12.2")
CMake Error at cmake/helpers/CheckDependentLibraries.cmake:178 (message):
  Configured to use HDF5, but not found
Call Stack (most recent call first):
  cmake/helpers/CheckDependentLibraries.cmake:497 (gdal_check_package)
  gdal.cmake:254 (include)
  CMakeLists.txt:218 (include)

Can you please confirm this?

@dg0yt
Copy link
Contributor Author

dg0yt commented Jul 8, 2022

I can test x64-windows only in vcpkg CI. But wait, there was a CI run of this PR (i.e. implicitly merged against master) less than 24 hours ago, with
gdal[core,curl,default-features,geos,hdf5,libspatialite,netcdf,postgresql,recommended-features]:x64-windows:

2022-07-07T13:20:16.2619004Z Installing 225/254 gdal:x64-windows...
2022-07-07T13:20:16.2625417Z Building gdal[core,curl,default-features,geos,hdf5,libspatialite,netcdf,postgresql,recommended-features]:x64-windows...
2022-07-07T13:20:16.3055942Z -- Downloading https://github.com/OSGeo/gdal/archive/v3.5.1.tar.gz -> OSGeo-gdal-v3.5.1.tar.gz...
2022-07-07T13:20:17.1365822Z -- Extracting source D:/downloads/OSGeo-gdal-v3.5.1.tar.gz
2022-07-07T13:20:18.5243223Z -- Applying patch find-link-libraries.patch
2022-07-07T13:20:18.5599146Z -- Using source at D:/buildtrees/gdal/src/v3.5.1-2bdd297034.clean
2022-07-07T13:20:18.8831479Z -- Found external ninja('1.10.2').
2022-07-07T13:20:18.8879915Z -- Configuring x64-windows
2022-07-07T13:20:39.4806266Z -- Building x64-windows-dbg
2022-07-07T13:21:27.0721499Z -- Building x64-windows-rel
2022-07-07T13:22:40.2391329Z -- Fixing pkgconfig file: D:/packages/gdal_x64-windows/lib/pkgconfig/gdal.pc
2022-07-07T13:22:40.2432280Z -- Using cached msys-mingw-w64-i686-pkg-config-0.29.2-3-any.pkg.tar.zst.
2022-07-07T13:22:40.2458890Z -- Using cached msys-mingw-w64-i686-libwinpthread-git-9.0.0.6373.5be8fcd83-1-any.pkg.tar.zst.
2022-07-07T13:22:40.2521736Z -- Using msys root at D:/downloads/tools/msys2/9a1ec3f33446b195
2022-07-07T13:22:40.2652326Z -- Fixing pkgconfig file: D:/packages/gdal_x64-windows/debug/lib/pkgconfig/gdal.pc
2022-07-07T13:22:40.3174004Z -- Installing: D:/packages/gdal_x64-windows/share/gdal/vcpkg-cmake-wrapper.cmake
2022-07-07T13:22:40.3180788Z -- Installing: D:/packages/gdal_x64-windows/share/gdal/usage
2022-07-07T13:22:40.3187111Z -- Installing: D:/packages/gdal_x64-windows/share/gdal/copyright
2022-07-07T13:22:40.3234155Z -- Performing post-build validation
2022-07-07T13:22:41.0411182Z -- Performing post-build validation done
2022-07-07T13:22:59.1338809Z Uploaded binaries to 1 HTTP remotes.
2022-07-07T13:22:59.4764237Z Elapsed time to handle gdal:x64-windows: 2.72 min

https://dev.azure.com/vcpkg/public/_build/results?buildId=74747&view=logs&j=7922e5c4-0103-5f8f-ad17-45ce9bb98e80&t=66eef212-08d6-5e6c-a560-7d01149d0188

You should set CMAKE_FIND_DEBUG_MODE=1 in you local gdal build to see why it doesn't accept vcpkg's hdf5. It may depend on your hdf5's feature set, or even local modications.

if(NOT VCPKG_BUILD_TYPE)
vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/tools/gdal/debug/bin/gdal-config" "${CURRENT_INSTALLED_DIR}" "`dirname $0`/../../../..")
endif()
vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/include/cpl_config.h" "#define GDAL_PREFIX \"${CURRENT_INSTALLED_DIR}\"" "")
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 now broken

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be fixed by #25778

jorisvandenbossche added a commit to geopandas/pyogrio that referenced this pull request Oct 16, 2022
Updating GDAL to the latest version in vcpkg (microsoft/vcpkg#26676). 

Summary of the related changes:

- GDAL is updated to version 3.5.2
- GDAL is now built using its new CMake build config (microsoft/vcpkg#22392) 
- Using GDAL's cmake build, this now correctly includes GEOS also on Linux and MacOS
  - Using pyogrio (including GEOS) and shapely together fails on MacOS when using a different GEOS version and if pyogrio is imported first (this is because shapely (specifically for MacOS) will use an already loaded GEOS for ctypes, while its cython code still uses the GEOS included in the wheel, giving a GEOS version mismatch _within_ different parts of shapely itself) -> avoid this issue by trying to import shapely first in our `__init__.py`
- Further, the vcpkg setup of which GDAL dependencies are used has changed a bit in the refactor of the port to use GDAL's cmake build. This has some influence on which libraries are included (eg libxml2 and iconv are no longer included), see #161 (comment) for more details.
@dg0yt dg0yt deleted the gdal-cmake branch December 15, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants