hypertidy / vapour

GDAL API package for R
https://hypertidy.github.io/vapour/
83 stars 9 forks source link

Warn if GDAL 3.6.0 used #183

Closed rsbivand closed 1 year ago

rsbivand commented 1 year ago

See also https://github.com/r-spatial/sf/issues/2077 and https://github.com/rspatial/terra/issues/965.

Tomas suggests adding an on load warning if the GDAL version is "3.6.0" https://lists.osgeo.org/pipermail/gdal-dev/2022-December/056621.html. In rgdal I've added https://r-forge.r-project.org/scm/viewvc.php/pkg/R/AAA.R?root=rgdal&r1=1181&r2=1192:

  rver <- getGDALVersionInfo()
  if (strsplit(strsplit(rver, ",")[[1]][1], " ")[[1]][2] == "3.6.0") warning("GDAL 3.6.0 has been officially retracted; use a later release:\nhttps://lists.osgeo.org/pipermail/gdal-dev/2022-December/056621.html")

which for vapour would use something like:

 rver <- version_gdal_cpp()
  if (strsplit(strsplit(rver, ",")[[1]][1], " ")[[1]][2] == "3.6.0") warning("GDAL 3.6.0 has been officially retracted; use a later release:\nhttps://lists.osgeo.org/pipermail/gdal-dev/2022-December/056621.html")
rsbivand commented 1 year ago

Running R CMD check --as-cran under Fedora with 3.6.0, I get:

* checking whether the namespace can be loaded with stated dependencies
... NOTE
Warning in load_stuff() :
   GDAL 3.6.0 has been officially retracted; use a later release:
https://lists.osgeo.org/pipermail/gdal-dev/2022-December/056621.html

A namespace must be able to be loaded with just the base namespace
loaded: otherwise if the namespace gets loaded by a saved object, the
session will be unable to start.

Probably some imports need to be declared in the NAMESPACE file.

which is I think spurious as only the base namespace is used. With just INSTALL:

** testing if installed package can be loaded from temporary location
Warning in load_stuff() :
   GDAL 3.6.0 has been officially retracted; use a later release:
https://lists.osgeo.org/pipermail/gdal-dev/2022-December/056621.html
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
Warning in load_stuff() :
   GDAL 3.6.0 has been officially retracted; use a later release:
https://lists.osgeo.org/pipermail/gdal-dev/2022-December/056621.html

and just loading:

> rgdal::rgdal_extSoftVersion()
           GDAL GDAL_with_GEOS           PROJ             sp           EPSG
        "3.6.0"         "TRUE"        "9.1.1"        "1.5-2"      "v10.076"
Warning message:
In load_stuff() :
   GDAL 3.6.0 has been officially retracted; use a later release:
https://lists.osgeo.org/pipermail/gdal-dev/2022-December/056621.html

With GDAL 3.6.2, there are no complaints, and in principle nobody should be using 3.6.0 anyway. Tomas established that the methods package also uses a warning on load. Probably OK to proceed.

mdsumner commented 1 year ago

I was wondering about this and I'm not done checking yet, thanks for the feedback

rsbivand commented 1 year ago

I was considering this with Tomas Kalibera (who has worked with Brodie Gaslam on moving GDAL to >= 3.6 for the build train). They were ready to go with 3.6.0 for Rtools43, but I asked them to wait before committing to that choice. Tomas wants all the packages linking to GDAL to warn if retracted 3.6.0 is being used, and since != 3.6.0 pass check (for my adapation for retiring rgdal), and only == 3.6.0 throws notes, we feel that that noise/pushback is warranted - CRAN should not be using 3.6.0 anyway. Anyway, 3.6.2 was released this morning UTC, and Rtools 43 should ship with it (Tomas is still testing, but I'm optimistic). Please report if you see any problems with vapour.

mdsumner commented 1 year ago

I meant I was considering the issues with doing the warning in onLoad

fwiw, it would be nice if this entire discussion was in public and not in weird private emails to a partial group of GDAL R developers ... especially Tomas' criticism of the GDAL releases, that stuff needs to be share-able.

mdsumner commented 1 year ago

vapour doesn't write geopackage so I'm not going to include the warning

rsbivand commented 1 year ago

OK, fair enough.