Detect GDAL version at build time / remove version features

[jdroenner]

This PR changes the build.rs file for gdal-sys and introduces a build.rs for gdal. It makes conditional compilation for version handling more convenient. However, it is still not very pretty.

In gdal-sys, the lib version is now used to select the correct binding file. On Linux it is provided by pkg-config if the gdal lib is detected.
In gdal, the features for versions are removed. The actual GDAL version is requested from gdal-sys and the config parameters are generated.

The changes for gdal-sys are:

• version specification as environmental variable or auto-detection using pkg-config. (i guess windows will require the GDAL_VERSION variable to be set).
• no more features to select versions

The changes for gdal are:

• Features for versions are removed.
• The Current GDAL version is requested from gdal-sys.
• Build.rs generates config parameters based on the GDAL version.

The following parameters are generated in gdals build.rs:

gdal_x, gdal_x_y, gdal_x_y_z with x = major, y = minor, and z= patch. (this provides backward capability to the features)
mayor_is_x, minor_is_y, patch_is_z
mayor_ge_x, minor_ge_y, patch_ge_z  for all version numbers up to the actual version. (major starts with 2).

For example GDAL 3.1.2 will have: --cfg major_is_3 --cfg minor_is_1 --cfg patch_is_2 --cfg major_ge_2 --cfg major_ge_3 --cfg minor_ge_0 --cfg minor_ge_1 --cfg patch_ge_0 --cfg patch_ge_1 --cfg patch_ge_2

This allows to enable methods based on versions:

#[all(cfg("major_ge_3"), cfg(minor_ge_1" ))]
fn method()

[jdroenner]

I guess there are some other tasks which should be addressed as separate issues:

• rework gdal-sys build.rs and separate linux and windows as well as bindgen and predefined binding processing.
• improve gdal-sys build on windows
• check if gdal-sys was build against a lib with the same version

[michaelkirk]

This allows to enable methods based on versions:

#[any(cfg("major_ge_3"), cfg(minor_ge_1" ))]
fn method()

I think that's a typo, shouldn't it be:

This allows to enable methods based on versions:

-#[any(cfg("major_ge_3"), cfg(minor_ge_1" ))]
+#[all(cfg("major_ge_3"), cfg(minor_ge_1" ))]
fn method()

Otherwise it seems like method would be enabled for gdal 1.1+, 2.1+, 3.1+, etc.

To make sure I understand, this is equivalent to (but sometimes more ergonomic than) something like:

#[all(cfg("gdal_3"), not(cfg("gdal_3_0")))]
fn method()

Is my understanding correct?

It might be helpful to see them used in practice to evaluate their fitness. It looks like the code is still checking for the no-longer existing features, e.g.:

#[cfg(feature = "gdal_3_0")]

[jdroenner]

I think that's a typo, shouldn't it be:

This allows to enable methods based on versions:

-#[any(cfg("major_ge_3"), cfg(minor_ge_1" ))]
+#[all(cfg("major_ge_3"), cfg(minor_ge_1" ))]
fn method()

Otherwise it seems like method would be enabled for gdal 1.1+, 2.1+, 3.1+, etc.

yes this is a typo. #[all(cfg("major_ge_3"), cfg(minor_ge_1" ))] is correct.

To make sure I understand, this is equivalent to (but sometimes more ergonomic than) something like:

#[all(cfg("gdal_3"), not(cfg("gdal_3_0")))]
fn method()

Is my understanding correct?

This is correct. I don't think there is a more maintainable way to handle the versioning.

It might be helpful to see them used in practice to evaluate their fitness. It looks like the code is still checking for the no-longer existing features, e.g.:

#[cfg(feature = "gdal_3_0")]

oh I must have missed that

[jdroenner]

i replaced all the cfg(feature = gdal_ ... with the new ones

[michaelkirk]

$ cargo test
...
  thread 'main' panicked at 'No pre-build binding available for this GDAl version.', gdal-sys/build.rs:195:17

Which is accurate - there are no bindings for 3.1 currently. It might be nice to include the version number in the error message and some potential next steps.

Maybe something like:

No pre-build binding available for this GDAL version 3.1. Build with --features bindgen to generate your own bindings.

And when I run with bindgen:

$ cargo test --features bindgen
...
---- 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

Which are exactly the tests fixed by #86 (which I can rebase after this is merged).

So, LGTM

Nice work!