gdal3 ci

[michaelkirk]

Tests are currently failing for gdal 3.1 on both mac and linux.

Crate changes

• adds gdal_3_1 features
• adds prebuilt 3.1 bindings
• changes some test expactations for gdal_3_1
• extracts gdal_3 feature, which is shared by gdal_3_0 and gdal_3_1 for sharing test behavior.

CI Changes

• Adds a stable-rust gdal 3.0 target for linux
• Adds a stable-rust gdal 2.4 target for macos
• Adds a stable-rust gdal 3.1 target for macos

For now, I've maintained all the existing stable, beta, nightly gdal 2.0 test runners, though it might make sense to remove some of these older ones and emphasize more modern combinations if that better reflects the user base. The work in this PR makes it fairly easy to shuffle around the test matrix.

[rmanoka]

@michaelkirk Nice scripts! Looking through some of the proj tests that have failed, it seems the test case indeed covers gdal 3.0 also. See for example:

#[test]
fn from_epsg_to_wkt_proj4() {
    let spatial_ref = SpatialRef::from_epsg(4326).unwrap();
    let wkt = spatial_ref.to_wkt().unwrap();
    // TODO: handle proj changes on lib level
    #[cfg(not(feature = "gdal_3_0"))]
    assert_eq!("GEOGCS[\"WGS 84\",DATUM[\"WGS_1984\",SPHEROID[\"WGS 84\",6378137,298.257223563,AUTHORITY[\"EPSG\",\"7030\"]],AUTHORITY[\"EPSG\",\"6326\"]],PRIMEM[\"Greenwich\",0,AUTHORITY[\"EPSG\",\"8901\"]],UNIT[\"degree\",0.0174532925199433,AUTHORITY[\"EPSG\",\"9122\"]],AUTHORITY[\"EPSG\",\"4326\"]]", wkt);
    #[cfg(feature = "gdal_3_0")]
    assert_eq!("GEOGCS[\"WGS 84\",DATUM[\"WGS_1984\",SPHEROID[\"WGS 84\",6378137,298.257223563,AUTHORITY[\"EPSG\",\"7030\"]],AUTHORITY[\"EPSG\",\"6326\"]],PRIMEM[\"Greenwich\",0,AUTHORITY[\"EPSG\",\"8901\"]],UNIT[\"degree\",0.0174532925199433,AUTHORITY[\"EPSG\",\"9122\"]],AXIS[\"Latitude\",NORTH],AXIS[\"Longitude\",EAST],AUTHORITY[\"EPSG\",\"4326\"]]", wkt);
    let proj4string = spatial_ref.to_proj4().unwrap();
    assert_eq!("+proj=longlat +datum=WGS84 +no_defs", proj4string.trim());
}

It seems to expect the feature gdal_3_0 to be set. Could you add this feature tag in CI for 3.0? This might fix many of the issues.

Some of the raster tests are being run on a png file, which seems to have been fixed upstream to not give any GIS metadata (scale, offset). Might be prudent to use a geotiff here (see "tinymarble.png"). We could convert the png into a tiff as follows:

# from inside fixtures/ directory
gdalwarp -to SRC_METHOD=NO_GEOTRANSFORM -te 0 0 100 50 tinymarble.png tinymarble.tif

This will create a tinymarble.tif file; then we could modify the failing tests to use tinymarble.tif instead. Specifically, tests get_offset and get_scale in src/raster/tests.rs . should now pass on both gdal versions.

[michaelkirk]

An update:

GDAL 2.0 builds passing on linux (ubuntu trusty) and macos.

GDAL 3.0 build on macos is failing these two tests:

---- raster::tests::test_get_offset stdout ----
thread 'raster::tests::test_get_offset' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(0.0)`', src/raster/tests.rs:347:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- raster::tests::test_get_scale stdout ----
thread 'raster::tests::test_get_scale' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(1.0)`', src/raster/tests.rs:339:5
failures:
    raster::tests::test_get_offset
    raster::tests::test_get_scale
test result: FAILED. 69 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out

https://travis-ci.org/github/georust/gdal/jobs/722360412#L298

these the the problematic lines:
gdal_sys::GDALGetRasterScale(self.c_rasterband, &mut pb_success)
and
gdal_sys::GDALGetRasterOffset(self.c_rasterband, &mut pb_success)

On macos, both fail because pb_success != 1

Some of the raster tests are being run on a png file, which seems to have been fixed upstream to not give any GIS metadata (scale, offset).

Interestingly, GDAL 3.0 build on linux (ubuntu bionic) is passing these same tests:
https://travis-ci.org/github/georust/gdal/jobs/722360408#L857

Does that debunk the theory that it's just a matter of a bad PNG @rmanoka?

For context, I'm not familiar with GDAL, or even explicitly using it. This was all in pursuit of updating to the latest geo-types, but figured I should have a working test suite first.

[michaelkirk]

This will create a tinymarble.tif file; then we could modify the failing tests to use tinymarble.tif instead. Specifically, tests get_offset and get_scale in src/raster/tests.rs . should now pass on both gdal versions.

Also, just tried this, and tests still failing for me.

[rmanoka]

@michaelkirk Found this relevant issue on OSGeo/gdal. It seems like it changed with release 3.1.0. The linux CI might be passing because it is 3.x but < 3.1.0. The issue also suggests the same behavior for tif files, so I was wrong there. Unfortunately, we can't use the feature flag as the regression is between 3.0 and 3.1.0.

Could you try this: once you have the tinymarble.tif ; run:

gdal_edit -scale 1.2 -offset 12 file.tif

Then, in the test, we should look for these two values (instead of 1.0, 0.0). Now that we're setting explicit non-default values, I'm hoping it will reflect in the calls.

[michaelkirk]

Thanks for your guidance @rmanoka, I think this PR is now ready for review.

[jdroenner]

Hi, thank you for your contribution. This kind of issue is why i ceated the "Feature_gated_versions" branch which i intend to merge into master soon. It splits gdal 2 and 3 and add Features for different versions.
However, i am away for the next 10 days. Untill then think i can't review or merge PRs.

[michaelkirk]

This kind of issue is why i ceated the "Feature_gated_versions" branch which i intend to merge into master soon. It splits gdal 2 and 3 and add Features for different versions.

To be clear, the issue here was not one of a new feature being added or removed. Rather, the behavior of an existing method changed and we weren't running CI to test that version.

[jdroenner]

Oh sorry if i got it wrong. There are so many issues related to the versioning i thought it is one of them.

[jdroenner]

Could you update this PR to reflect the changes in version handling?

[michaelkirk]

Could you update this PR to reflect the changes in version handling?

Done! Provided CI passes this should be good to go.