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

[geos] Relocatable geos-config; dynamic builds on Unix #17616

Merged
merged 9 commits into from
May 26, 2021
Merged

[geos] Relocatable geos-config; dynamic builds on Unix #17616

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
89893b6
Provide geos-config, and fix dynamic builds on Unix
dg0yt May 1, 2021
9a238b5
x-add-version
dg0yt May 1, 2021
8f8e29b
Fix debug suffix handling
dg0yt May 3, 2021
0fbdfa8
Update git-tree
dg0yt May 3, 2021
d222d85
Don't build benchmarks
dg0yt May 22, 2021
6c2680b
Revise pc file and geos-config handling
dg0yt May 22, 2021
9751a28
Port to non-deprecated cmake functions
dg0yt May 22, 2021
ab8c1d3
Add usage
dg0yt May 22, 2021
f90497f
Update git-tree
dg0yt May 22, 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
4 changes: 0 additions & 4 deletions ports/geos/CONTROL
View file
Open in desktop

This file was deleted.

31 changes: 20 additions & 11 deletions ports/geos/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ vcpkg_download_distfile(ARCHIVE
)
vcpkg_extract_source_archive_ex(
OUT_SOURCE_PATH SOURCE_PATH
ARCHIVE ${ARCHIVE}
ARCHIVE "${ARCHIVE}"
REF ${GEOS_VERSION}
PATCHES
dont-build-docs.patch
Expand All @@ -24,7 +24,7 @@ else()
endif()

vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
SOURCE_PATH "${SOURCE_PATH}"
PREFER_NINJA
OPTIONS
-DCMAKE_DEBUG_POSTFIX=d
Expand All @@ -34,19 +34,28 @@ vcpkg_configure_cmake(
vcpkg_install_cmake()
vcpkg_fixup_cmake_targets(CONFIG_PATH lib/cmake/GEOS)

if(VCPKG_LIBRARY_LINKAGE STREQUAL static)
file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/bin ${CURRENT_PACKAGES_DIR}/debug/bin)
if(EXISTS "${CURRENT_PACKAGES_DIR}/bin/geos-config")
file(READ "${CURRENT_PACKAGES_DIR}/bin/geos-config" GEOS_CONFIG)
string(REGEX REPLACE "(\nprefix=)[^\n]*" [[\1$(CDPATH= cd -- "$(dirname -- "$0")"/../.. && pwd -P)]] GEOS_CONFIG "${GEOS_CONFIG}")
file(WRITE "${CURRENT_PACKAGES_DIR}/bin/geos-config" "${GEOS_CONFIG}")
file(MAKE_DIRECTORY ${CURRENT_PACKAGES_DIR}/share/${PORT})
file(RENAME ${CURRENT_PACKAGES_DIR}/bin/geos-config ${CURRENT_PACKAGES_DIR}/share/${PORT}/geos-config)
endif()
if(EXISTS ${CURRENT_PACKAGES_DIR}/debug/bin/geos-config)
file(READ "${CURRENT_PACKAGES_DIR}/debug/bin/geos-config" GEOS_CONFIG)
string(REGEX REPLACE "(\nprefix=)[^\n]*" [[\1$(CDPATH= cd -- "$(dirname -- "$0")"/../.. && pwd -P)]] GEOS_CONFIG "${GEOS_CONFIG}")
string(REGEX REPLACE "(-lgeos(_c)?)d?( |\n)" "\\1d\\3" GEOS_CONFIG "${GEOS_CONFIG}")
file(WRITE "${CURRENT_PACKAGES_DIR}/debug/bin/geos-config" "${GEOS_CONFIG}")
file(MAKE_DIRECTORY ${CURRENT_PACKAGES_DIR}/share/${PORT})
file(RENAME ${CURRENT_PACKAGES_DIR}/debug/bin/geos-config ${CURRENT_PACKAGES_DIR}/share/${PORT}/geos-config-debug)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you give an example of how this would be consumed? My understanding is that xyz-config scripts like this are usually referred to like "${XYZ_ROOT}/xyz-config" and so having xyz-config-debug won't be directly usable. Instead, we should provide debug/share/geos/geos-config and set GEOS_ROOT to debug/share/geos when in debug mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gdal configure says:

  --with-geos=ARG         Include GEOS support (ARG=yes, no or geos-config
                          path)

and similar for curl, kea, sfcgal.

So this is to be consumed in the gdal portfile by:

    list(APPEND OPTIONS_RELEASE --with-geos=${CURRENT_INSTALLED_DIR}/share/geos/geos-config)
    list(APPEND OPTIONS_DEBUG --with-geos=${CURRENT_INSTALLED_DIR}/share/geos/geos-config-debug)

I tested this locally, but this just one out of a pile of pending changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

provide debug/share/geos/geos-config and set set GEOS_ROOT to debug/share/geos

If I adopt this: Outside vcpkg, the <package>_ROOT variable generally (e.g. in CMake find modules) means the installation prefix, i.e. ${CURRENT_INSTALLED_DIR}.
As a consumer in a port file, I would still have to assign the roots to per-build-type parameters because the build types are handled deeper down in the cmake call stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

provide debug/share/geos/geos-config

I tried that when preparing the PR, and I verified today: It leads into a build error:

-- Performing post-build validation
/debug/share should not exist. Please reorganize any important files, then use
    file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/share")

Copy link
Contributor

Choose a reason for hiding this comment

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

Which file is in ${CURRENT_PACKAGES_DIR}/debug/share?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which file is in ${CURRENT_PACKAGES_DIR}/debug/share?

None with the current PR. @ras0219-msft proposed to have debug/share/geos/geos-config, and I reported what would be the result.

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 alternative to fixing geos-config, curl-config etc. is forcing the consumers to use pkg-config where possible, by removing these files. Consumers also need to know whether to ask the scripts for static or standard flags, and this is solved for PKG_CONFIG.
E.g. port gdal will need a number of patches anyway, e.g. to use pkg-config for tiff.

endif()

file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/include)

if(EXISTS ${CURRENT_PACKAGES_DIR}/bin/geos-config)
file(MAKE_DIRECTORY ${CURRENT_PACKAGES_DIR}/share/geos)
file(RENAME ${CURRENT_PACKAGES_DIR}/bin/geos-config ${CURRENT_PACKAGES_DIR}/share/geos/geos-config)
file(REMOVE ${CURRENT_PACKAGES_DIR}/debug/bin/geos-config)
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/debug/include")
if(VCPKG_LIBRARY_LINKAGE STREQUAL "static" OR NOT VCPKG_TARGET_IS_WINDOWS)
file(REMOVE_RECURSE "${CURRENT_PACKAGES_DIR}/bin" "${CURRENT_PACKAGES_DIR}/debug/bin")
endif()

# Handle copyright
configure_file(${SOURCE_PATH}/COPYING ${CURRENT_PACKAGES_DIR}/share/geos/copyright COPYONLY)
configure_file("${SOURCE_PATH}/COPYING" "${CURRENT_PACKAGES_DIR}/share/geos/copyright" COPYONLY)

vcpkg_copy_pdbs()
7 changes: 7 additions & 0 deletions ports/geos/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "geos",
"version": "3.9.1",
"port-version": 1,
"description": "Geometry Engine Open Source",
"homepage": "https://www.osgeo.org/projects/geos/"
}
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2198,7 +2198,7 @@
},
"geos": {
"baseline": "3.9.1",
"port-version": 0
"port-version": 1
},
"geotrans": {
"baseline": "3.8",
Expand Down
5 changes: 5 additions & 0 deletions versions/g-/geos.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "4827098f4cb8dad529b0b1b9a4d0b6db203acaa0",
"version": "3.9.1",
"port-version": 1
},
{
"git-tree": "97c3e5f08174b7f7b4634064f4f2ae6da4351e97",
"version-string": "3.9.1",
Expand Down