CI: fix failing wheel tests from merge of #70

[brendan-ward]

Unfortunately we have different coverage of test environments in our CI tests vs wheel-building & testing steps, and it looks like some of the vcpkg builds don't have dependencies setup in the way we expect (have not yet figured out why they are complaining about lack of GEOS support when GEOS is build and then compiled into GDAL via vcpkg).

Among other things, looks like we should add vcpkg based tests to our standard test environments so that we can catch issues before merging PRs.

[brendan-ward]

We appear to be getting this error when getting the CRS of an SQL-based layer; unexpected place for that error to get raised.

pyogrio.errors.CRSError: GEOS support not enabled.

[brendan-ward]

Not sure if this matters, but noting for the record that our vcpkg build does not include libspatialite (we don't enable that as an add-on feature); it is LGPL licensed, which likely has license implications for the wheels we create. I believe Fiona also does not build against libspatialite.

[brendan-ward]

As far as I can tell, vcpkg should be compiling GDAL with GEOS support; everything I could find for the error we're seeing points to enabling GEOS support with that flag. This only seems to affect tests that use INDIRECT_SQLITE as the fallback for when spatialite is not availble.

Looks like our path forward is to also skip those tests when INDIRECT_SQLITE cannot be used. The implication is that we need to be careful in the documentation to indicate that the wheels (except windows??) don't support spatial operators in sql statements.

[jorisvandenbossche]

The "GEOS support not enabled." message might be a red herring?
There is also an error that happens before that one, which says: "ERROR 1: sqlite3_exec(CREATE VIRTUAL TABLE "rtree_naturalearth_lowres_geom" USING rtree(id, minx, maxx, miny, maxy)) failed: no such module: rtree" (although that comes from the "setup" stderr logging, so that error might be coming from writing the files in the fixture).

I am also assuming that we correctly compile with GEOS support, as other features like intersection tests (without specifying it through the sql parameter) do work (although there are probably fallbacks to envelope checks if GEOS is not available).
It would be good to come up with a test for which we are certain that it needs GEOS and otherwise would fail.

We can maybe expose OGRGetGEOSVersion to be sure about this (this can always be useful for debugging purposes to expose that information I think).

[brendan-ward]

Unfortunately OGRGetGEOSVersion is only available for GDAL >= 3.4, so we fail during compilation. We don't have other GDAL version specific functions that we call (yet), so I'd like to avoid the tricks that Fiona uses to splice in version-specific functionality.

I wonder if we can just log out gdal-config --dep-libs; this doesn't tell us which version of GEOS, but presumably it would only include -lgeos_c if GDAL was compiled against GEOS.

[jorisvandenbossche]

BTW, it seems that you can actually quite easily do something conditional on the GDAL version (if that is known at compile time, in which case we might need to add the requirement the specify the version again for Windows) without resorting to the shim tricks of Fiona. See https://stackoverflow.com/questions/27273302/cython-conditional-compilation-based-on-external-value-given-via-setuptools and http://docs.cython.org/en/latest/src/userguide/language_basics.html#conditional-statements (this is already being used in pyproj, and there is a PR to also do that in Fiona: Toblerity/Fiona#1021

And in that Fiona PR, there is exactly something similar but for a function to get the PROJ version: https://github.com/Toblerity/Fiona/pull/1021/files#diff-3c8a96cee0fd3b758655091e0e6bdd97786e926a9669d57f7ac5d9d1bed9f841R75-R82

[jorisvandenbossche]

I did a quick check of that approach, see #88 (that would still need updates in the setup.py to properly get the actual GDAL version)

[jorisvandenbossche]

So #88 confirms that GEOS is indeed not enabled for the Linux/Mac wheels (but is enabled for Windows) ..

For Linux / Mac we are using a custom triplet configuration to have dynamic builds (which means this specific combination of config options is not continuously tested upstream in vcpkg). So there might be something in the portfile that prevents proper GEOS linking in case of dynamic libraries (just to be sure: the actual GEOS so files are present in the wheel libs, so that in itself is not the issue).

I can try to reproduce this locally for Linux later. But short term, do we find lacking GEOS support a blocker for a release to have people start testing the wheels? Based on our tests, for basic reading/writing, you don't actually need GEOS (it's only for an advanced SQL query that we ran into issues).

[brendan-ward]

I don't think lack of GEOS support is a blocker for our typical use cases. We can xfail those tests in the short term (will add that soon) and probably should add a note to the SQL section fo the documentation warning that support might not be available within the wheels.

[brendan-ward]

Actually, better to detect GEOS is present and skip those tests otherwise, so will wait on #88

[brendan-ward]

Resolved by #87