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

BLD/RLS: ensure wheels are including GDAL built with openssl #506

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

jorisvandenbossche
Copy link
Member

I am running into the issue that pyogrio (or geopandas with the pyogrio engine) cannot read a file from Google Cloud Storage.
I get intermittently either a DataSourceError: CPLRSASHA256Sign() not implemented: GDAL must be built against libcrypto++ or libcrypto (openssl) or DataSourceError: HTTP response code: 403.
This is specifically with the wheels (with conda it works), and using fiona it also works. The first error message seems to indicate we don't properly built with openssl, although vcpkg does install openssl. For the Fiona wheels (and the conda libgdal), I can verify with ldd that the libgdal.so included in the wheel is linking against libcrypto (which comes from openssl). For our own wheel I can't check that the same way because we statically link the gdal dependencies into libgdal.so.

Now, based on the fact that sometimes the message indicates GDAL is not built against libcrypto, I am assuming this is indeed the case.
Looking a bit further into this, the logs of building GDAL with vcpkg on CI clearly shows that vcpkg is also installing openssl. However, the vcpkg port of GDAL has an explicit "feature" for openssl (https://github.com/microsoft/vcpkg/blob/14542c8ad9b6bcb9da755884ab823605c3300b68/ports/gdal/vcpkg.json#L227-L232), and that we are not explicitly enabling. And while GDAL's cmake default is to built against OpenSSL if it is found (docs), I am assuming that vcpkg might override this default to disable GDAL_USE_OPENSSL if you don't explicitly enable the feature through the vcpkg install of gdal (https://github.com/microsoft/vcpkg/blob/14542c8ad9b6bcb9da755884ab823605c3300b68/ports/gdal/portfile.cmake#L47C26-L47C42, not entirely sure how vcpkg_check_features works).

So testing this hypothesis here by explicitly adding "openssl" as a vcpkg feature for gdal. I should be able to test the generated wheel. Although not entirely sure if we also have a way we can test this on CI.

@jorisvandenbossche
Copy link
Member Author

And to be clear, the curl that is included in our wheels that gdal uses for other cloud interaction is built with openssl (as far as I can see in the logs (example log), at least), it's only for a few other things (like google cloud authorization) that GDAL needs itself to be directly built with openssl

@jorisvandenbossche
Copy link
Member Author

jorisvandenbossche commented Dec 6, 2024

And based on my comment at #161 (comment), we did correctly build with libcrypto in the past, but I assume because of how vcpkg refactored the gdal port to allow better selection of individual features, we accidentally lost that (that also means we could now remove the recommended-features to not include png and jpeg)

@jorisvandenbossche
Copy link
Member Author

Great, this passes, and can confirm locally that with the built wheel from CI my test data can be read from GCS!

Copy link
Member

@brendan-ward brendan-ward left a comment

Choose a reason for hiding this comment

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

Good catch! Thanks @jorisvandenbossche

@brendan-ward brendan-ward merged commit e96c532 into geopandas:main Dec 6, 2024
55 checks passed
@jorisvandenbossche jorisvandenbossche deleted the wheels-openssl branch December 7, 2024 12:15
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.

2 participants