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

gdal: update to 3.6.4 #18077

Merged
merged 5 commits into from Apr 30, 2023
Merged

gdal: update to 3.6.4 #18077

merged 5 commits into from Apr 30, 2023

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Mar 24, 2023

Description

Updates GDAL to version 3.6.3.

GDAL has dropped support for autotools, thus this is a complete remake to cmake.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 12.6.3 21G419 arm64
Xcode 14.2 14C18

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@macportsbot
Copy link

Notifying maintainers:
@Veence for port gdal.
@petrrr for port py-gdal.

@macportsbot macportsbot added type: update maintainer: open Affects an openmaintainer port labels Mar 24, 2023
@nilason nilason force-pushed the gdal_update_3.6.3 branch 2 times, most recently from f0500e4 to 7757cdc Compare March 25, 2023 21:29
@nilason
Copy link
Contributor Author

nilason commented Mar 26, 2023

Just noticed that there are two Python packages installed outside MacPorts directory in the CI build log:

/Library/Python/3.9/site-packages/osgeo/
/Library/Python/3.9/site-packages/osgeo_utils/

This didn't happen locally (it went to /opt/local/Library/Frameworks/Python.framework/Versions/3.11/...)

It is possible to configure which Python to use:
see GDAL docs for options.

I'm not sure what would be the correct or preferable way to detect and set specific Python version. I'd be grateful for any help on this!

@nilason
Copy link
Contributor Author

nilason commented Mar 26, 2023

Just noticed that there are two Python packages installed outside MacPorts directory in the CI build log:

/Library/Python/3.9/site-packages/osgeo/
/Library/Python/3.9/site-packages/osgeo_utils/

This didn't happen locally (it went to /opt/local/Library/Frameworks/Python.framework/Versions/3.11/...)

It is possible to configure which Python to use: see GDAL docs for options.

I'm not sure what would be the correct or preferable way to detect and set specific Python version. I'd be grateful for any help on this!

How silly I am, this is what py-gdal is for.

Copy link
Contributor

@ryandesign ryandesign left a comment

Choose a reason for hiding this comment

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

Just noticed that there are two Python packages installed outside MacPorts directory in the CI build log:

/Library/Python/3.9/site-packages/osgeo/
/Library/Python/3.9/site-packages/osgeo_utils/

You'll need to figure out how to get the build system to stop doing that.

gis/gdal-grass/Portfile Outdated Show resolved Hide resolved
gis/gdal-grass/Portfile Outdated Show resolved Hide resolved
gis/gdal-grass/Portfile Outdated Show resolved Hide resolved
gis/gdal-grass/Portfile Outdated Show resolved Hide resolved
gis/gdal-grass/Portfile Outdated Show resolved Hide resolved
gis/gdal/Portfile Outdated Show resolved Hide resolved
gis/gdal/Portfile Outdated Show resolved Hide resolved
gis/gdal/Portfile Outdated Show resolved Hide resolved
gis/gdal/Portfile Outdated Show resolved Hide resolved
gis/gdal/Portfile Show resolved Hide resolved
@nilason nilason force-pushed the gdal_update_3.6.3 branch 2 times, most recently from bdb41b2 to 099c88c Compare March 28, 2023 19:20
@nilason
Copy link
Contributor Author

nilason commented Mar 28, 2023

@ryandesign Thanks a lot for taking time on this! I think I addressed all you suggestions, issues.
I added a minor liblas update which I may remove if not appropriate in one PR.

@nilason
Copy link
Contributor Author

nilason commented Mar 28, 2023

Just noticed that there are two Python packages installed outside MacPorts directory in the CI build log:

/Library/Python/3.9/site-packages/osgeo/
/Library/Python/3.9/site-packages/osgeo_utils/

You'll need to figure out how to get the build system to stop doing that.

Setting -DBUILD_PYTHON_BINDINGS was needed to drop them.

@nilason
Copy link
Contributor Author

nilason commented Mar 31, 2023

@ryandesign What is the common practice here, shall I add a commit with revision bump of dependants?

@nilason
Copy link
Contributor Author

nilason commented Apr 3, 2023

@Veence Please take a look at this. GDAL update is long overdue.

Comment on lines +33 to +37
set grass_gisbase [exec ${prefix}/bin/grass --config path]

configure.args-append --with-gdal=${prefix}/bin/gdal-config
configure.args-append --with-grass=${prefix}/share/grass-7.4.0
configure.args-append --with-grass=${grass_gisbase}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"
}

variant mysql5 conflicts mysql57 mysql8 description {Enable MySQL 5 support} {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nilason nilason force-pushed the gdal_update_3.6.3 branch 2 times, most recently from 22b63fa to c1c1566 Compare April 20, 2023 11:48
Comment on lines 302 to 305
PortGroup boost 1.0

configure.args-delete --without-grass
configure.args-append --with-grass=${prefix}/share/grass78
depends_lib-append port:libkml
boost.depends_type build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Building with libkml driver requires include-path to boost. Not sure this is the best way to achieve this?

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure exactly what is needed here, but the boost PG does set quite a few cmake related variables already (see here). If the gdal port needs another variable name the PG probably will provide the necessary hints to do so.
Also, since you're setting the dependency type to build it would be good to check that nothing links to boost package (i.e., is required at runtime).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not GDAL that uses boost, but libkml. The GDAL driver code includes libkml header files which in turn includes boost. GDAL expects the header files to be in standard ${prefix}/include and has no way to know to look into eg. ${prefix}/libexec/boost/1.76/include.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, since you're setting the dependency type to build it would be good to check that nothing links to boost package (i.e., is required at runtime).

GDAL doesn't seem to link (as its not using boost directly):

otool -L /opt/local/lib/gdalplugins/ogr_LIBKML.dylib
/opt/local/lib/gdalplugins/ogr_LIBKML.dylib:
	@rpath/libgdal.32.dylib (compatibility version 32.0.0, current version 32.3.6)
	/opt/local/lib/libkmlbase.1.dylib (compatibility version 1.0.0, current version 1.3.0)
	/opt/local/lib/libkmldom.1.dylib (compatibility version 1.0.0, current version 1.3.0)
	/opt/local/lib/libkmlengine.1.dylib (compatibility version 1.0.0, current version 1.3.0)
	/opt/local/lib/libminizip.1.dylib (compatibility version 2.0.0, current version 2.0.0)
	/opt/local/lib/liburiparser.1.dylib (compatibility version 1.0.0, current version 1.0.30)
	/opt/local/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.13)
	/opt/local/lib/libexpat.1.dylib (compatibility version 10.0.0, current version 10.10.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)
	/opt/local/lib/openmpi-mp/libmpi_cxx.40.dylib (compatibility version 71.0.0, current version 71.1.0)
	/opt/local/lib/openmpi-mp/libmpi.40.dylib (compatibility version 71.0.0, current version 71.4.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1300.23.0)

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not GDAL that uses boost, but libkml. The GDAL driver code includes libkml header files which in turn includes boost. GDAL expects the header files to be in standard ${prefix}/include and has no way to know to look into eg. ${prefix}/libexec/boost/1.76/include.

that sounds strange to me... if gdal doesn't use boost then it shouldn't need to have it as a built dependency. If the libkml port uses boost then it should probably specify in that port where the boost headers are and gdal should be able to use those - or am I missing something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that sounds strange to me... if gdal doesn't use boost then it shouldn't need to have it as a built dependency.

There's no need for GDAL to have boost as dependency. With the GDAL variant (and port) libkml, which does have boost as dependency, however, GDAL's libkml-driver hooks to the libkml API (which in turn does, correctly, include boost API). libkml know where to look for boost header files, but GDAL doesn't. This would be all good and fine if they were located in "standard" location for include files (i.e. ${prefix}/include) .

If the libkml port uses boost then it should probably specify in that port where the boost headers are and gdal should be able to use those - or am I missing something here?

The boost PG defines the version used and ultimately the location of the include files. This is the reason, the only solution I could come up with, was the present by adding the boost PG to GDAL variant libkml. Note this only affects GDAL with the libkml variant. If there is a way to get the include path to boost, from boost PG, without adding the PG to GDAL, that would be an alternative and perhaps better solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In short: if it is possible to get:

proc boost::include_dir {} {

without adding boost PG to gdal+libkml, then that would likely be enough.

@ryandesign
Copy link
Contributor

When I installed on my system in trace mode (sudo port -t install), it reported a lot of files provided by ports that were not declared as dependencies that the build would have accessed if I had not used trace mode:

  /opt/local/include/H5Cpp.h
  /opt/local/include/OpenEXR/ImfVersion.h
  /opt/local/include/bsoncxx/v_noabi/bsoncxx/config/version.hpp
  /opt/local/include/hdf5.h
  /opt/local/include/json-c/json.h
  /opt/local/include/libheif/heif.h
  /opt/local/include/mongocxx/v_noabi/mongocxx/client.hpp
  /opt/local/include/msodbcsql.h
  /opt/local/include/openjpeg-2.5/opj_config.h
  /opt/local/include/pcre.h
  /opt/local/include/pcre2.h
  /opt/local/include/sql.h
  /opt/local/lib/cmake/json-c/json-c-config.cmake
  /opt/local/lib/oracle/sdk/include/oci.h
  /opt/local/lib/pkgconfig/OpenEXR.pc
  /opt/local/lib/pkgconfig/json-c.pc
  /opt/local/lib/pkgconfig/libheif.pc
  /opt/local/lib/pkgconfig/libmongocxx.pc
  /opt/local/lib/pkgconfig/libopenjp2.pc

Often this indicates that opportunistic linking will occur, which we don't want, and if so the build should be changed so that it doesn't access those files unless it declares dependencies on those ports.

gdal 3.4.3 installed libgdal.30.dylib while gdal 3.6.3 installs libgdal.32.dylib so all ports that link with the library will be broken by this update and need to be rebuilt (via a version update or a revision increase) to avoid that. Here's the list of all ports that mention a gdal dependency though I don't know if all of them link with the library:

% port file all|sort -u|xargs grep -i ':gdal\b'
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/databases/postgis2/Portfile:    depends_lib-append      port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/databases/postgis3/Portfile:    depends_lib-append      port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/gdal-grass/Portfile:depends_lib         port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/grass/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/grass7/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/kealib/Portfile:depends_lib-append  port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/liblas/Portfile:depends_lib-append  port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/libosmium/Portfile:                        port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/mapnik/Portfile:    depends_lib-append      port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/mapserver/Portfile:    depends_lib-append      port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/mapserver/Portfile:    depends_lib-append      port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/orfeotoolbox/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/pdal/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/qgis3/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/qlandkartegt/Portfile:depends_lib      port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/rsgislib/Portfile:depends_lib-append  port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/saga/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/gis/t-rex/Portfile:    port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/graphics/OpenSceneGraph-devel/Portfile:                        port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/graphics/OpenSceneGraph/Portfile:                        port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/graphics/opencv3-devel/Portfile:                            port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/graphics/opencv3/Portfile:                            port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/graphics/opencv4-devel/Portfile:                    port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/graphics/opencv4/Portfile:                    port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/octave/octave-mapping/Portfile:depends_lib-append  port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/python/py-fiona/Portfile:                        port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/python/py-gdal/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/python/py-rasterio/Portfile:                        port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/science/gerbil/Portfile:                    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/science/gmt4/Portfile:    depends_lib-append      port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/science/gmt5/Portfile:    depends_lib-append      port:gdal
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/science/ncarg/Portfile:                            port:gdal       \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/science/sumo/Portfile:    port:gdal \
/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/science/xastir/Portfile:                    port:gdal \

@reneeotten
Copy link
Contributor

thanks @ryandesign for pointing that out! Unfortunately trace-mode is still broken on newer OSes so it's hard to test this locally for many people. There are also several Trac tickets for this port that I don't know if they have been resolved. Again, that's really the job of the maintainer of this port.

This PR updates the port and doesn't break it more than it is in the current state.
So yes, ideally everything would be fixed before merging - but at some point we probably just have to merge it and fix other issues step-by-step.

@Dave-Allured
Copy link
Contributor

Please update this PR from GDAL 3.6.3 to 3.6.4, a bug fix release on 2023 Apr 21. Thanks.

@nilason
Copy link
Contributor Author

nilason commented Apr 22, 2023

thanks @ryandesign for pointing that out! Unfortunately trace-mode is still broken on newer OSes so it's hard to test this locally for many people. There are also several Trac tickets for this port that I don't know if they have been resolved. Again, that's really the job of the maintainer of this port.

@ryandesign Thanks again for looking at this! Even though I regularly use port -vst I have not seen something similar to what you listed. I do check what drivers are enabled and not, and whether the related port is enabled or not. There are sooo many of them so I cannot guarantee there aren't mismatches, but I general it should be ok. I guess cmake does check up on them (causing the trace), but if not explicitly told to be included they will be disabled.

This PR updates the port and doesn't break it more than it is in the current state. So yes, ideally everything would be fixed before merging - but at some point we probably just have to merge it and fix other issues step-by-step.

I agree. I will make the bump to 3.6.4 (thanks @Dave-Allured!), and when there is an agreement on the boost issue, I'll add a dependencies-revision-bump-commit.

@Dave-Allured
Copy link
Contributor

@ryandesign said:

When I installed on my system in trace mode (sudo port -t install), it reported a lot of files provided by ports that were not declared as dependencies that the build would have accessed if I had not used trace mode:

... Often this indicates that opportunistic linking will occur, which we don't want, and if so the build should be changed so that it doesn't access those files unless it declares dependencies on those ports.

Ryan, this seems like false positives. I tested this with a default Macports build of GDAL 3.6.3, with custom prefix and no sudo privs. It built fine, other dependencies were installed, there were no complaints, and none of those 19 reported files were installed by anything. By name I associated 9 files out of 19 with inactive variants in @nilason's port file:

/opt/local/include/H5Cpp.h
/opt/local/include/hdf5.h
-DGDAL_USE_HDF5=OFF

/opt/local/include/OpenEXR/ImfVersion.h
/opt/local/lib/pkgconfig/OpenEXR.pc
-DGDAL_USE_OPENEXR=OFF

/opt/local/include/libheif/heif.h
/opt/local/lib/pkgconfig/libheif.pc
-DGDAL_USE_HEIF=OFF

/opt/local/include/openjpeg-2.5/opj_config.h
/opt/local/lib/pkgconfig/libopenjp2.pc
-DGDAL_USE_OPENJPEG=OFF

/opt/local/include/sql.h
-DGDAL_USE_MYSQL=OFF

So what is port install -t really doing here? Testing all variants, or what? Unless I am missing something, it seems that no new dependency declarations are needed for this port.

@nilason nilason changed the title gdal: update to 3.6.3 gdal: update to 3.6.4 Apr 24, 2023
@nilason
Copy link
Contributor Author

nilason commented Apr 24, 2023

Updated PR to bump to GDAL 3.6.4. Still not sure how to solve the boost issue differently.

@nilason
Copy link
Contributor Author

nilason commented Apr 24, 2023

The commits 41b2884 and f32e5c8 is an alternative to initial PG solution for boost.

py-gdal bumped to 3.6.4

GDAL 3.6.0 dropped support for Autotools and fully transitioned to CMake.
The gdal portfile is a major remake accordingly.

Variants added:
- openexr
- postgresql15
- proj9
- cryptopp

Variants removed:
- jasper
- grass (now available as a standalone driver with 'gdal-grass')
- mysql5

See https://trac.macports.org/ticket/43431
The "gdal-grass " GDAL driver has been split from GDAL and being made a
standalone driver at https://github.com/OSGeo/gdal-grass.

The version numbering is now separate from GDAL version and (re-)started from
1.0 (hence necessitated the setting of epoch).

Variants added:
- postgresql11-postgresql15
- grass

Variants removed:
- grass7

GRASS version detection is now done at build time.

Closes: https://trac.macports.org/ticket/64411
@nilason
Copy link
Contributor Author

nilason commented Apr 25, 2023

I believe this is ready for merge.
My solution for the boost problem was a minor patch to the libkml pc-file.

@nilason
Copy link
Contributor Author

nilason commented Apr 28, 2023

#18418 should preferably be merged before this.

@nilason
Copy link
Contributor Author

nilason commented Apr 28, 2023

If there are no more objections, suggestions, this may be merged now.
More than a whole year since last version bump of this port, now being very outdated, and a month since I put up this PR...

@i0ntempest
Copy link
Member

Are those failing checks just timeouts? If so I'll merge in 24 hours if no one objects.

@reneeotten
Copy link
Contributor

Are those failing checks just timeouts? If so I'll merge in 24 hours if no one objects.

You haven't commented or reviewed anything on this PR; so I'd suggest you leave merging of this to the people who did and can verify whether the comments have indeed been addressed.

@i0ntempest
Copy link
Member

Are those failing checks just timeouts? If so I'll merge in 24 hours if no one objects.

You haven't commented or reviewed anything on this PR; so I'd suggest you leave merging of this to the people who did and can verify whether the comments have indeed been addressed.

Cool, I won't touch this then. Just wanted to speed things up a bit since as the author said it's been a month ¯_(ツ)_/¯

@reneeotten
Copy link
Contributor

thanks @nilason for all your work on this! I revisited the PR and it seems all good to me, so I'll merge now and if issues show up we can address them!

PS if you're interested in helping to maintain and update these ports, you could consider adding yourself as co-maintainer as @Veence has been rather absent lately.

@reneeotten reneeotten merged commit 3d95877 into macports:master Apr 30, 2023
0 of 2 checks passed
@nilason nilason deleted the gdal_update_3.6.3 branch May 2, 2023 06:42
@nilason
Copy link
Contributor Author

nilason commented May 2, 2023

thanks @nilason for all your work on this! I revisited the PR and it seems all good to me, so I'll merge now and if issues show up we can address them!

Thank you all for getting this/these through!

PS if you're interested in helping to maintain and update these ports, you could consider adding yourself as co-maintainer as @Veence has been rather absent lately.

I've been thinking on this some time, unburden Veence in busy periods. What is the procedure for this?

@reneeotten
Copy link
Contributor

you can open a PR that adds yourself as a co-maintainer, if @Veence agrees with that (or doesn't respond in a reasonable amount of time) we can just merge it and move on. A slightly more drastic measure would be to file a "Port abandoned" ticket.

@nilason nilason mentioned this pull request Sep 8, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: update
8 participants