Re: [Buildroot] [PATCH v5] package/gdal: new package

[Romain Naour <romain.naour@smile.fr>]

Hello Dominik

Le 07/05/2022 à 11:46, Dominik Michael Rauh a écrit :
> GDAL is a translator library for raster and vector geospatial data
> formats. As a library, it presents a single raster abstract data model
> and single vector abstract data model to the calling application for all
> supported formats. It also comes with a variety of useful command line
> utilities for data translation and processing.
> 
> https://gdal.org/
> 
> test-pkg shows that this package is affected by binutils bug 27597.
> 
> Signed-off-by: Dominik Michael Rauh <dmrauh@posteo.de>
> ---
> Changes v4 -> v5 (after review by Arnout Vandecappelle):
> - Bump version from 3.3.1 to 3.4.3
> - Rename libgdal to gdal (following upstream)
> - Add upstream status to patch
> - Remove binutils bug 21464 from Config.in
> - Rework license information
> - Rework configuration options
> 
> Changes v3 -> v4 (after review by Thomas Petazzoni):
> -  Bump version from 3.3.0 to 3.3.1
> -  Configure libgdal to use external libraries where possible
> -  Explicitely disable libraries not yet handled by Buildroot
> 
> Changes v2 -> v3:
> -  Bump version from 3.2.2 to 3.3.0
> 
> Changes v1 -> v2 (after review by Peter Seiderer):
> -  Disable NEON and VSX support when using libgdal's libpng
> -  Disable compilation for toolchains with binutils bug 21464 or 27597
> -  Add the proper "depends" demanded by proj
> -  Fix the comment in Config.in
> -  Hopefully add the complete LIBGDAL_LICENSE information
> 
>  package/Config.in                             |   1 +
>  ...ode_iconv.cpp-fix-invalid-cast-error.patch |  50 +++++++
>  package/gdal/Config.in                        |  30 +++++
>  package/gdal/gdal.hash                        |   7 +
>  package/gdal/gdal.mk                          | 127 ++++++++++++++++++
>  package/postgis/postgis.mk                    |   4 +-
>  6 files changed, 217 insertions(+), 2 deletions(-)
>  create mode 100644 package/gdal/0001-port-cpl_recode_iconv.cpp-fix-invalid-cast-error.patch
>  create mode 100644 package/gdal/Config.in
>  create mode 100644 package/gdal/gdal.hash
>  create mode 100644 package/gdal/gdal.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index 8892134133..ddf1936235 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -1945,6 +1945,7 @@ menu "Other"
>  	source "package/flatbuffers/Config.in"
>  	source "package/flatcc/Config.in"
>  	source "package/gconf/Config.in"
> +	source "package/gdal/Config.in"
>  	source "package/gflags/Config.in"
>  	source "package/gli/Config.in"
>  	source "package/glibmm/Config.in"
> diff --git a/package/gdal/0001-port-cpl_recode_iconv.cpp-fix-invalid-cast-error.patch b/package/gdal/0001-port-cpl_recode_iconv.cpp-fix-invalid-cast-error.patch
> new file mode 100644
> index 0000000000..39593ab2ee
> --- /dev/null
> +++ b/package/gdal/0001-port-cpl_recode_iconv.cpp-fix-invalid-cast-error.patch
> @@ -0,0 +1,50 @@
> +From c54ba241e4ed6dd43f3a63576f94169c4cafa798 Mon Sep 17 00:00:00 2001
> +From: Dominik Michael Rauh <dmrauh@posteo.de>
> +Date: Fri, 6 May 2022 21:29:49 +0200
> +Subject: [PATCH] port/cpl_recode_iconv.cpp: fix invalid cast error
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Fixes:
> +
> +cpl_recode_iconv.cpp: In function ‘char* CPLRecodeIconv(const char*, const char*, const char*)’:
> +cpl_recode_iconv.cpp:90:18: error: invalid cast from type ‘int’ to type ‘iconv_t’ {aka ‘long int’}
> +   90 |     if( sConv == reinterpret_cast<iconv_t>(-1) )
> +      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +cpl_recode_iconv.cpp: In function ‘char* CPLRecodeFromWCharIconv(const wchar_t*, const char*, const char*)’:
> +cpl_recode_iconv.cpp:239:18: error: invalid cast from type ‘int’ to type ‘iconv_t’ {aka ‘long int’}
> +  239 |     if( sConv == reinterpret_cast<iconv_t>(-1) )
> +      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Signed-off-by: Dominik Michael Rauh <dmrauh@posteo.de>
> +[Upstream: https://github.com/OSGeo/gdal/pull/5684]
Your patch has been merged upstream!

> +---
> + port/cpl_recode_iconv.cpp | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/port/cpl_recode_iconv.cpp b/port/cpl_recode_iconv.cpp
> +index 33c2e06..d752b75 100644
> +--- a/port/cpl_recode_iconv.cpp
> ++++ b/port/cpl_recode_iconv.cpp
> +@@ -87,7 +87,7 @@ char *CPLRecodeIconv( const char *pszSource,
> + 
> +     sConv = iconv_open( pszDstEncoding, pszSrcEncoding );
> + 
> +-    if( sConv == reinterpret_cast<iconv_t>(-1) )
> ++    if( sConv == (iconv_t)(-1) )
> +     {
> +         CPLError( CE_Warning, CPLE_AppDefined,
> +                   "Recode from %s to %s failed with the error: \"%s\".",
> +@@ -236,7 +236,7 @@ char *CPLRecodeFromWCharIconv( const wchar_t *pwszSource,
> + 
> +     sConv = iconv_open( pszDstEncoding, pszSrcEncoding );
> + 
> +-    if( sConv == reinterpret_cast<iconv_t>(-1) )
> ++    if( sConv == (iconv_t)(-1) )
> +     {
> +         CPLFree( pszIconvSrcBuf );
> +         CPLError( CE_Warning, CPLE_AppDefined,
> +-- 
> +2.35.1
> +
> diff --git a/package/gdal/Config.in b/package/gdal/Config.in
> new file mode 100644
> index 0000000000..55a8c7fb83
> --- /dev/null
> +++ b/package/gdal/Config.in
> @@ -0,0 +1,30 @@
> +config BR2_PACKAGE_GDAL
> +	bool "gdal"
> +	depends on BR2_INSTALL_LIBSTDCPP # proj, libjson
> +	# configure can't find proj, when linking statically
> +	depends on !BR2_STATIC_LIBS
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_7 # C++11, proj
> +	depends on !BR2_TOOLCHAIN_HAS_BINUTILS_BUG_27597
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # proj
> +	depends on BR2_USE_WCHAR # proj
> +	select BR2_PACKAGE_JPEG
> +	select BR2_PACKAGE_LIBGEOTIFF
> +	select BR2_PACKAGE_LIBJSON
> +	select BR2_PACKAGE_LIBPNG
> +	select BR2_PACKAGE_PROJ
> +	select BR2_PACKAGE_ZLIB
> +	help
> +	  GDAL is a translator library for raster and vector geospatial
> +	  data formats. As a library, it presents a single raster
> +	  abstract data model and single vector abstract data model to
> +	  the calling application for all supported formats. It also
> +	  comes with a variety of useful command line utilities for data
> +	  translation and processing.
> +
> +	  https://gdal.org/
> +
> +comment "gdal needs a toolchain w/ C++, dynamic library, gcc >= 4.7, not binutils bug 27597, threads, wchar"
> +	depends on !BR2_INSTALL_LIBSTDCPP || BR2_STATIC_LIBS || \
> +		!BR2_TOOLCHAIN_GCC_AT_LEAST_4_7 || \
> +		BR2_TOOLCHAIN_HAS_BINUTILS_BUG_27597 || \
> +		!BR2_TOOLCHAIN_HAS_THREADS || !BR2_USE_WCHAR
> diff --git a/package/gdal/gdal.hash b/package/gdal/gdal.hash
> new file mode 100644
> index 0000000000..e6f25f9c54
> --- /dev/null
> +++ b/package/gdal/gdal.hash
> @@ -0,0 +1,7 @@
> +# md5 from: https://download.osgeo.org/gdal/3.4.3/gdal-3.4.3.tar.xz.md5, sha256 locally computed:
> +md5  36576fa060bcde5be3bb18fd3ddfd151  gdal-3.4.3.tar.xz
> +sha256  02a27b35899e1c4c3bcb6007da900128ddd7e8ab7cd6ccfecf338a301eadad5a  gdal-3.4.3.tar.xz
> +
> +# Hashes of license files:
> +sha256  b82e6cca0b13f5db2f22ab667f22254fb1f4b135ea73d5bd6238ef89aff31f6c  LICENSE.TXT
> +sha256  8afa5b03af2e776c7e484768c01bbd4ceee89a3baf590e2092e4611ad4c78c7f  PROVENANCE.TXT

Is PROVENANCE.TXT file really useful as license file ? it contains a list of
contributors that will be likely be updated for each release.

Here we want to check the license file hash to "detect" any license change.

> diff --git a/package/gdal/gdal.mk b/package/gdal/gdal.mk
> new file mode 100644
> index 0000000000..346dc34dd2
> --- /dev/null
> +++ b/package/gdal/gdal.mk
> @@ -0,0 +1,127 @@
> +################################################################################
> +#
> +# gdal
> +#
> +################################################################################
> +
> +GDAL_VERSION = 3.4.3
> +GDAL_SITE = https://download.osgeo.org/gdal/$(GDAL_VERSION)
> +GDAL_SOURCE = gdal-$(GDAL_VERSION).tar.xz
> +GDAL_LICENSE = MIT (GDAL/OGR), BSD-3-Clause, APACHE-2.0

The liscense is really complex to describe here, gdal contains a lot of code
with different license.  At least add a comment saying that gdal bundle several
bundled libraries with different licenses.

https://git.buildroot.net/buildroot/tree/package/supertux/supertux.mk?h=2022.05.1#n11

It seems Info-ZIP license is missing.

> +GDAL_LICENSE_FILES = LICENSE.TXT PROVENANCE.TXT
> +GDAL_INSTALL_STAGING = YES
> +GDAL_CONFIG_SCRIPTS = gdal-config
> +# gdal at its core only needs host-pkgconf, libgeotiff, proj and tiff but since
> +# by default mrf driver support is enabled, it also needs jpeg, libpng and
> +# zlib. By default there are also many other drivers enabled but it seems, in
> +# contrast to mrf driver support, that they can be implicitly disabled, by
> +# configuring gdal without their respectively needed dependencies.
> +GDAL_DEPENDENCIES = host-pkgconf jpeg libgeotiff libpng proj tiff zlib
> +
> +ifeq ($(BR2_PACKAGE_POSTGRESQL),y)
> +GDAL_DEPENDENCIES += postgresql
> +GDAL_CONF_OPTS += --with-pg
> +else
> +GDAL_CONF_OPTS += --without-pg
> +endif
> +
> +ifeq ($(BR2_PACKAGE_LIBXML2),y)
> +GDAL_DEPENDENCIES += libxml2
> +GDAL_CONF_OPTS += --with-xml2
> +else
> +GDAL_CONF_OPTS += --without-xml2
> +endif
> +
> +GDAL_CONF_OPTS += \
> +	--with-geotiff \
> +	--with-libjson-c \
> +	--with-libtiff \
> +	--with-libtool \
> +	--with-libz \
> +	--with-jpeg \
> +	--with-png \
> +	--with-proj
> +
> +# disable not yet handled packages and thus their associated gdal drivers
> +GDAL_CONF_OPTS += \
> +	--without-armadillo \
> +	--without-blosc \
> +	--without-brunsli \
> +	--without-cfitsio \
> +	--without-crypto \
> +	--without-cryptopp \
> +	--without-curl \
> +	--without-dds \
> +	--without-dods-root \
> +	--without-ecw \
> +	--without-expat \
> +	--without-exr \
> +	--without-fgdb \
> +	--without-fme \
> +	--without-freexl \
> +	--without-geos \
> +	--without-gnm \
> +	--without-libkml \
> +	--without-lz4 \
> +	--without-grass \
> +	--without-libgrass \
> +	--without-gta \
> +	--without-hdf4 \
> +	--without-hdf5 \
> +	--without-hdfs \
> +	--without-heif \
> +	--without-idb \
> +	--without-ingres \
> +	--without-jp2lura \
> +	--without-jasper \
> +	--without-java \
> +	--without-jpeg12 \
> +	--without-jxl \
> +	--without-charls \
> +	--without-kakadu \
> +	--without-kea \
> +	--without-lerc \
> +	--without-gif \
> +	--without-liblzma \
> +	--without-libdeflate \
> +	--without-mdb \
> +	--without-mongocxx \
> +	--without-mongocxxv3 \
> +	--without-mrsid \
> +	--without-jp2mrsid \
> +	--without-macosx-framework \
> +	--without-mrsid_lidar \
> +	--without-msg \
> +	--without-mysql \
> +	--without-netcdf \
> +	--without-null \
> +	--without-oci \
> +	--without-odbc \
> +	--without-ogdi \
> +	--without-opencl \
> +	--without-openjpeg \
> +	--without-pam \
> +	--without-pcidsk \
> +	--without-pcraster \
> +	--without-pcre \
> +	--without-pcre2 \
> +	--without-pdfium \
> +	--without-perl \
> +	--without-podofo \
> +	--without-poppler \
> +	--without-python \
> +	--without-qhull \
> +	--without-rasdaman \
> +	--without-rasterlite2 \
> +	--without-rdb \
> +	--without-sfcgal \
> +	--without-sosi \
> +	--without-spatialite \
> +	--without-sqlite3 \
> +	--without-teigha \
> +	--without-tiledb \
> +	--without-webp \
> +	--without-xerces \
> +	--without-zstd
> +
> +$(eval $(autotools-package))
> diff --git a/package/postgis/postgis.mk b/package/postgis/postgis.mk
> index b77071d03b..e26693410e 100644
> --- a/package/postgis/postgis.mk
> +++ b/package/postgis/postgis.mk
> @@ -21,8 +21,8 @@ POSTGIS_CONF_OPTS += \
>  	--with-geosconfig=$(STAGING_DIR)/usr/bin/geos-config \
>  	--with-xml2config=$(STAGING_DIR)/usr/bin/xml2-config
>  
> -ifeq ($(BR2_PACKAGE_LIBGDAL),y)
> -POSTGIS_DEPENDENCIES += libgdal
> +ifeq ($(BR2_PACKAGE_GDAL),y)
> +POSTGIS_DEPENDENCIES += gdal
>  POSTGIS_CONF_OPTS += --with-raster
>  else
>  POSTGIS_CONF_OPTS += --without-raster

Actually this part should be a separate commit.

Best regards,
Romain

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot