Please test for minimum version libraries in configure #36

Closed
eddelbuettel opened this Issue Oct 27, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@eddelbuettel

Congrats on getting sf onto CRAN.

With it being there, it got caught up in a reverse dependency check for Rcpp which promptly failed leading me to suspect we had broken something.

But we didn't. Your build system is too lenient. Your README.md makes it clear (via the pointer to the travis.yml) that certain minimal versions are required. Apparently my recent-ish Ubuntu machine with 16.04 is not good enough. But you can actually know what version your are using so I think you should use this and test for a minimal version. I can probably help you with some configure.in snippets if you need them.

@edzer

This comment has been minimized.

Show comment
Hide comment
@edzer

edzer Oct 27, 2016

Member

Thanks for the heads up, and continuous reverse checking!

sf wants to be gdal 2.x compliant, and the whole of CRAN has (I believe) migrated to gdal 2.x (thanks to @rsbivand). Hence, I made the code that depended on 2.1 but broke on CRAN with 2.0.2 conditional, instead of checking the version at install time.

Checking for GDAL >= 2.0.0 at install time also seems a good idea; I will look into how rgdal does this. Most of the errors we see now should have been addressed by a resubmission that is already on Kurt's desk.

Member

edzer commented Oct 27, 2016

Thanks for the heads up, and continuous reverse checking!

sf wants to be gdal 2.x compliant, and the whole of CRAN has (I believe) migrated to gdal 2.x (thanks to @rsbivand). Hence, I made the code that depended on 2.1 but broke on CRAN with 2.0.2 conditional, instead of checking the version at install time.

Checking for GDAL >= 2.0.0 at install time also seems a good idea; I will look into how rgdal does this. Most of the errors we see now should have been addressed by a resubmission that is already on Kurt's desk.

@rsbivand

This comment has been minimized.

Show comment
Hide comment
@rsbivand

rsbivand Oct 27, 2016

Member

rgdal does a ballet stretch and branches on GDAL1 or GDAL2. It only checks a minimal GDAL version. rgdal/configure.ac first checks for a viable gdal-config, then asks it for the installed version. I don't see a configure.in|ac or configure in sf now - am I looking in the wrong place? The system requirements only say GDAL, GEOS++, which isn't specific even descriptively. I guess these both need guarding - the rgeos configure also works at this, as extra functionality is exposed in the C API in successive releases. The main problem is that all the packagers on Linux are way behind:

fedora
ubuntu
debian

for stable OS releases, so that users need to do something extra to get recent builds. Further, the most frequently asked question on rgdal installs on Linux is multiple installs of different versions of GDAL pulled in by binary installs of other software (often QGIS), which then leads to chaos as one version is used to install rgdal and another version's *.so is found at run time. Right now the release of GEOS 3.6.0 and 3.5.1 on the same day triggered a discussion on which to go for on Debian - some downstream packages use the C++ API (which was always strongly discouraged and banned from 3.6.0), so Debian will only go to 3.5.1.

A further issue with GDAL binaries are the supported drivers - the external dependencies of GDAL itself (spatialite is typically a problem as its developers allow pre-beta releases - I've stopped building with it as it is too unstable for me).

Member

rsbivand commented Oct 27, 2016

rgdal does a ballet stretch and branches on GDAL1 or GDAL2. It only checks a minimal GDAL version. rgdal/configure.ac first checks for a viable gdal-config, then asks it for the installed version. I don't see a configure.in|ac or configure in sf now - am I looking in the wrong place? The system requirements only say GDAL, GEOS++, which isn't specific even descriptively. I guess these both need guarding - the rgeos configure also works at this, as extra functionality is exposed in the C API in successive releases. The main problem is that all the packagers on Linux are way behind:

fedora
ubuntu
debian

for stable OS releases, so that users need to do something extra to get recent builds. Further, the most frequently asked question on rgdal installs on Linux is multiple installs of different versions of GDAL pulled in by binary installs of other software (often QGIS), which then leads to chaos as one version is used to install rgdal and another version's *.so is found at run time. Right now the release of GEOS 3.6.0 and 3.5.1 on the same day triggered a discussion on which to go for on Debian - some downstream packages use the C++ API (which was always strongly discouraged and banned from 3.6.0), so Debian will only go to 3.5.1.

A further issue with GDAL binaries are the supported drivers - the external dependencies of GDAL itself (spatialite is typically a problem as its developers allow pre-beta releases - I've stopped building with it as it is too unstable for me).

@edzer

This comment has been minimized.

Show comment
Hide comment
@edzer

edzer Oct 27, 2016

Member

Thanks! here is sf's configure.in.

Member

edzer commented Oct 27, 2016

Thanks! here is sf's configure.in.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Oct 27, 2016

Which I also pointed to in my initial bug report to you:

But you can actually know what version your are using so

It also uses gdal-version and before I filed I checked for pkg-version:

edd@max:~$ gdal-config --version
1.11.3
edd@max:~$ pkg-config --modversion gdal
1.11.3
edd@max:~$ 

If you do not know the latter, study it a little. It allows eg for

edd@max:~$ pkg-config --atleast-version=2.1.0 gdal && echo "Yes we are good"
edd@max:~$ pkg-config --atleast-version=2.1.0 gdal || echo "Insufficient, cannot install"
Insufficient, cannot install
edd@max:~$ 

Having documentation pointing to adequate resources is nice. But I now test over 800 packages for Rcpp so I also appreciate explicit tests. It is easy for you to add them, so please do.

If you are confused by this I would be happy to send a PR. I have multiple configure.in scripts first testing for pkg-config and then using it. If it isn't found we can always fall back to current behaviour and just die later (as inelegant as it is). But if and when facilities exist we may as well use them.

Which I also pointed to in my initial bug report to you:

But you can actually know what version your are using so

It also uses gdal-version and before I filed I checked for pkg-version:

edd@max:~$ gdal-config --version
1.11.3
edd@max:~$ pkg-config --modversion gdal
1.11.3
edd@max:~$ 

If you do not know the latter, study it a little. It allows eg for

edd@max:~$ pkg-config --atleast-version=2.1.0 gdal && echo "Yes we are good"
edd@max:~$ pkg-config --atleast-version=2.1.0 gdal || echo "Insufficient, cannot install"
Insufficient, cannot install
edd@max:~$ 

Having documentation pointing to adequate resources is nice. But I now test over 800 packages for Rcpp so I also appreciate explicit tests. It is easy for you to add them, so please do.

If you are confused by this I would be happy to send a PR. I have multiple configure.in scripts first testing for pkg-config and then using it. If it isn't found we can always fall back to current behaviour and just die later (as inelegant as it is). But if and when facilities exist we may as well use them.

@edzer

This comment has been minimized.

Show comment
Hide comment
@edzer

edzer Oct 27, 2016

Member

Cool, I didn't know about pkg-config. I've put it in configure. sf 0.2-1 just appeared on CRAN so still doesn't have it, 0.2-2 should be OK.

Member

edzer commented Oct 27, 2016

Cool, I didn't know about pkg-config. I've put it in configure. sf 0.2-1 just appeared on CRAN so still doesn't have it, 0.2-2 should be OK.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Oct 27, 2016

You can test explicit for existence of the various foo-config scripts:

## check for pkg-config
AC_DEFUN([AC_PROG_PKGCONFIG], [AC_CHECK_PROG(PKGCONFIG,pkg-config,yes)])
AC_PROG_PKGCONFIG

That leads to (when running ./configure)

checking for pkg-config... yes

Also, this is just wrong: the updated source must be in the tarball as the other OS need them too. So run compileAttribute as part R CMD build (and oh-how-I-wished-we-had-a-hook-there....) Best thing may be calling from cleanup, as "wrong" as that is too. Else a Makefile. Or manually.

eddelbuettel commented Oct 27, 2016

You can test explicit for existence of the various foo-config scripts:

## check for pkg-config
AC_DEFUN([AC_PROG_PKGCONFIG], [AC_CHECK_PROG(PKGCONFIG,pkg-config,yes)])
AC_PROG_PKGCONFIG

That leads to (when running ./configure)

checking for pkg-config... yes

Also, this is just wrong: the updated source must be in the tarball as the other OS need them too. So run compileAttribute as part R CMD build (and oh-how-I-wished-we-had-a-hook-there....) Best thing may be calling from cleanup, as "wrong" as that is too. Else a Makefile. Or manually.

@rsbivand

This comment has been minimized.

Show comment
Hide comment
@rsbivand

rsbivand Oct 27, 2016

Member

Test for pkg-config only if gdal-config is not found. A gdal.pc is generated when installing recent GDAL from a source build, but pkg-config only looks in /usr/local with manual setting of an environment variable. GEOS does not generate a geos.pc file at all from a source build. Provide mechanisms for using configure arguments to say where gdal-config and geos-config live. You may also run foul of badly packaged PROJ4, which OGR/GDAL need - often missing extra NAD files, and even suffering from poorly packaged source releases of PROJ4 itself.

Member

rsbivand commented Oct 27, 2016

Test for pkg-config only if gdal-config is not found. A gdal.pc is generated when installing recent GDAL from a source build, but pkg-config only looks in /usr/local with manual setting of an environment variable. GEOS does not generate a geos.pc file at all from a source build. Provide mechanisms for using configure arguments to say where gdal-config and geos-config live. You may also run foul of badly packaged PROJ4, which OGR/GDAL need - often missing extra NAD files, and even suffering from poorly packaged source releases of PROJ4 itself.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Oct 27, 2016

Conditional: yes/no. Almost not worth it. But do test -- most distros support pk-config and its .pc files but not all, and there will always be a local user installing differently who does have it.

And for all the reasons @rsbivand outlines you want to test rigorously and before you compile as it allows you to issue at that configuration stage any type of detailed instructions about where to read more, download from etc pp

Dieing because the compiler cannot cope is the worst possible ending.

Edit: And it is not either/or or nested. Many systems may have gdal-config and pkg-config but only the latter makes it easy for you to test for minimum versions. Then again, you can test with simpler means for 1.* versions etc pp.

eddelbuettel commented Oct 27, 2016

Conditional: yes/no. Almost not worth it. But do test -- most distros support pk-config and its .pc files but not all, and there will always be a local user installing differently who does have it.

And for all the reasons @rsbivand outlines you want to test rigorously and before you compile as it allows you to issue at that configuration stage any type of detailed instructions about where to read more, download from etc pp

Dieing because the compiler cannot cope is the worst possible ending.

Edit: And it is not either/or or nested. Many systems may have gdal-config and pkg-config but only the latter makes it easy for you to test for minimum versions. Then again, you can test with simpler means for 1.* versions etc pp.

@edzer

This comment has been minimized.

Show comment
Hide comment
@edzer

edzer Nov 28, 2016

Member

I think this should be resolved now with 0.2-4 on CRAN.

Member

edzer commented Nov 28, 2016

I think this should be resolved now with 0.2-4 on CRAN.

@edzer edzer closed this Nov 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment