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

add GDAL builder #293

Merged
merged 9 commits into from
Jan 19, 2020
Merged
74 changes: 74 additions & 0 deletions G/GDAL/build_tarballs.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
using BinaryBuilder

name = "GDAL"
version = v"3.0.2"

# Collection of sources required to build GDAL
sources = [
"https://github.com/OSGeo/gdal/releases/download/v$version/gdal-$version.tar.gz" =>
"787cf150346e58bff0ccf8c131b333139273e35d2abd590ad7196a9ee08f0039",
]

# Bash recipe for building across all platforms
script = raw"""
cd $WORKSPACE/srcdir/gdal-*/

if [[ ${target} == *mingw* ]]; then
export LDFLAGS="-L${libdir}"
cp ${libdir}/libproj_6_2.dll ${libdir}/libproj.dll
Copy link
Member

Choose a reason for hiding this comment

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

The more I think about this the more I'm convinced that we shouldn't do this here: in any case ${libdir}/libproj.dll must not be in the tarball generated by this builder, but if we keep this line and remove the file at the end of the builder, this file will be a missing dependency for whatever needs it. I think we should either copy the file in PROJ_jll or do something to convince GDAL to link against the right file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libgdal needs libproj. However, due to PROJ_jll with it's libproj_6_2.dll being a dependency of GDAL_jll, won't libproj_6_2.dll already be dlopened before libgdal and thus everything is fine?

Copy link
Member

@giordano giordano Dec 17, 2019

Choose a reason for hiding this comment

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

That's not a matter of being dlopened, when the linker is passed the option -lfoo it'll look for a file called libfoo.${dlext} (I think on Windows the "lib" is not necessary), with some variations to allow for the version number. Here the file is called libproj_6_2.dll, since the Makefile is compiling with -lproj the linker can't find libproj.dll (probably it would allow for libproj-62.dll). I think the dynamic loader follows similar rules as the linker: if the library is linked to libproj.dll, libproj_6_2.dll won't be an acceptable replacement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting. But in GDALBuilder we used the same trick and dit not distribute libproj.dll, only libproj_6_1.dll. And it does find it:
image

Copy link
Member

Choose a reason for hiding this comment

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

As far as I can see you're not deleting the file at the end, so it's probably in GDAL tarball, which is definitely something that we don't want. As I said before, the linker should allow for the version of the package to be in the file name, but I've never seen it as libproj_6_1, I'm surprised the loader can find it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm in the old GDAL/lib there is libproj.dll.a, but that's all, and if I move that file away it still finds it just fine.

So at least we should add a rm ${libdir}/libproj.dll after compilation in this build script to prevent getting that file out of this build? And if we get issues we try to fix the name in the PROJ build script?

Copy link
Member

Choose a reason for hiding this comment

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

The libraries for Windows are under bin, not lib

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I know, I just meant that under bin there is only libproj_6_1.dll, nothing else. So I gues the loader on Windows is sufficiently flexible, just not during the build, which is why we needed to rename it before the build only.

Copy link
Member

Choose a reason for hiding this comment

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

Have a look at b7eae71, if you don't like we can hard-reset it. However, I think this is a better way to deal with this, instead of copying files around.

In an ideal world, PROJ would provide a pkgconfig file and we would get from there the name of the library, instead of doing this mess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Have you considered submitting this patch upstream to https://github.com/OSGeo/gdal/commits/master/gdal/configure.ac?

fi

# Show options in the log
./configure --help
visr marked this conversation as resolved.
Show resolved Hide resolved
./configure --prefix=$prefix --host=$target \
--with-geos=${bindir}/geos-config \
--with-proj=$prefix \
--with-libz=$prefix \
--with-sqlite3=$prefix \
--with-curl=${bindir}/curl-config \
--with-python=no \
--enable-shared \
--disable-static

make -j${nproc}
make install
"""

platforms = supported_platforms()
visr marked this conversation as resolved.
Show resolved Hide resolved
platforms = expand_cxxstring_abis(platforms)

# The products that we will ensure are always built
products = [
LibraryProduct("libgdal", :libgdal),
ExecutableProduct("gdal_contour", :gdal_contour_path),
ExecutableProduct("gdal_grid", :gdal_grid_path),
ExecutableProduct("gdal_rasterize", :gdal_rasterize_path),
ExecutableProduct("gdal_translate", :gdal_translate_path),
ExecutableProduct("gdaladdo", :gdaladdo_path),
ExecutableProduct("gdalbuildvrt", :gdalbuildvrt_path),
ExecutableProduct("gdaldem", :gdaldem_path),
ExecutableProduct("gdalinfo", :gdalinfo_path),
ExecutableProduct("gdallocationinfo", :gdallocationinfo_path),
ExecutableProduct("gdalmanage", :gdalmanage_path),
ExecutableProduct("gdalsrsinfo", :gdalsrsinfo_path),
ExecutableProduct("gdaltindex", :gdaltindex_path),
ExecutableProduct("gdaltransform", :gdaltransform_path),
ExecutableProduct("gdalwarp", :gdalwarp_path),
ExecutableProduct("nearblack", :nearblack_path),
ExecutableProduct("ogr2ogr", :ogr2ogr_path),
ExecutableProduct("ogrinfo", :ogrinfo_path),
ExecutableProduct("ogrlineref", :ogrlineref_path),
ExecutableProduct("ogrtindex", :ogrtindex_path),
]

# Dependencies that must be installed before this package can be built
dependencies = [
"GEOS_jll",
"PROJ_jll",
"Zlib_jll",
"SQLite_jll",
"LibCURL_jll",
]

# Build the tarballs, and possibly a `build.jl` as well.
build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies)