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

PDAL: New version 2.8.2 #9986

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

eschnett
Copy link
Contributor

Closes #9985

@eschnett
Copy link
Contributor Author

@evetion Please comment on this PR. Is this useful? Is there anything missing?

@evetion
Copy link
Contributor

evetion commented Dec 10, 2024

It's certainly useful, thanks for the very quick action! ❤️

I'm only doubting the GDAL bounds, you choose the latest, which might be too new? I'm checking the sonames (this build has libgdal.36.dylib), so 36 = v3.10, and 35 = v3.9 (jll v301.902). This version is not used yet by GDAL.jl, which might lead to package conflicts on common downstream packages (GeoDataFrames, Rasters), so it might be safer to use v301.902 here. @visr what do you think?

@eschnett
Copy link
Contributor Author

eschnett commented Dec 10, 2024

I thought we had fixed this... All the GDAL_jll @301 are supposed to be compatible. Are you saying that there are GDAL_jll versions that differ only in their minor version number and which are actually incompatible? These should then be GDAL_jll @302!

@evetion
Copy link
Contributor

evetion commented Dec 10, 2024

I'm not entirely sure here (I've always felt a bit out of my depth on these matters), hence me calling on visr.

That said, GDAL typically moves their GDAL_SOVERSION forward by one each minor version. Whether their ABI is actually compatible matters little if we link to a specific version in these builds I guess:

[ Info: Linked library libgdal.36.dylib has been auto-mapped to @rpath/libgdal.36.dylib

@visr
Copy link
Contributor

visr commented Dec 10, 2024

Yeah you can also tell they bump the SOVERSION in the minor release here: JuliaBinaryWrappers/GDAL_jll.jl@5eb9df0#diff-0ed6f04e79778dda22f5565f89d1f6e95329a8d15c57c4efa586fbe7b7374bcd.

I wasn't aware of this. It doesn't hurt GDAL.jl, but it does break PDAL and the currently 7 other downstream builds, only a few of which set compat for GDAL.

@eschnett
Copy link
Contributor Author

If this is so, then GDAL_jll should move their major version number forward by one for each minor version number change of GDAL. This is why the build recipe for GDAL_jll has a variable version_offset. It seems we forgot to bump it for the latest GDAL_jll update.

I'm tempted to disable the current (probably unused?) GDAL_jll for GDAL 3.10 to avoid breaking packages that would otherwise try to use it. We can then quickly re-release a new GDAL_jll with a bumped major version number.

We should also add a comment to the beginning of GDAL_jll's build recipe. I like reading comments from my past self.

@evetion
Copy link
Contributor

evetion commented Dec 10, 2024

FWIW not that many packages use GDAL_jll directly, so I would consider 3.10 unused of now. Of the jll packages , only two set compat, so others can be assumed broken. Of direct deps GDAL.jl pins GDAL_jll, and ADRIA and GMT have compats via other packages (GeoDataFrames>ArchGDAL>GDAL, and via PROJ_jll respectively). Pinging @joa-quim to be sure.

@joa-quim
Copy link
Contributor

GMT_jll directly sets a GDAL version

Dependency(PackageSpec(name="GDAL_jll", uuid="a7073274-a066-55f0-b90d-d619367d196c"); compat="301.900")

I confess that I'm nothing at easy with this version/compat things. And I would like to move to GDAL3.10 soon (was not possible when I looked not long ago).

@eschnett
Copy link
Contributor Author

eschnett commented Dec 11, 2024

@giordano Would it be feasible to yank the current GDAL_jll 301.1000? It's incompatible with earlier GDAL_jll @301. Is that a proper usage of the yanking mechanism?

@joa-quim
Copy link
Contributor

Would it be feasible to yank the current GDAL_jll 301.1000? It's incompatible with earlier GDAL_jll @301

Why incompatible? They are two different version builds. And as I mentioned above, I will want to make GMT.jl release that links to GDAL 3.10. GMT links directly to GDAL_jll so it should works (though some versions have broke in the past).

@eschnett
Copy link
Contributor Author

They are incompatible because their shared library sonames don't match. If you build PDAL_jll against GDAL_jll 3.9 then you cannot use PDAL_jll with GDAL_jll 3.10. This is "cannot use" in the sense "Julia will not detect it, things will abort with a low-level error message at run time".

This isn't supposed to happen. Most packages behave differently. If you e.g. build against OpenSSL 3.0.9, then you can run with OpenSSL 3.0.15 just fine.

To tell Julia about this (the fact that the sonames differ) we will need to bump the major version number of GDAL_jll when we re-release it for GDAL 3.10.

@joa-quim
Copy link
Contributor

OK, I'm fine with the replacement (re-release). I'm in no hurry to use 3.10

@evetion
Copy link
Contributor

evetion commented Dec 15, 2024

I'm fine with merging this PR as is 👍🏻

@eschnett eschnett marked this pull request as draft December 15, 2024 23:04
@eschnett
Copy link
Contributor Author

Let's wait a day or two. I'm working on Arrow_jll and GDAL_jll at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PDAL_jll fails to load on 1.10-1.11
4 participants