Support --with-data-copy as environment variable

[jeroen]

Can I do the same as --with-data-copy by setting an environment variable rather than a configure flag?

When building thousands of packages, in a complex pipeline, it is very tricky to customize the build commands for individual packages. But we can easily set global environment variables on the build server.

I am already setting PROJ_LIB but from what I understand, there is no variable that does the equivalent of --with-data-copy? I was first assuming that --with-data-copy=yes was implicit when static linking but I see that is not the case. cf: #1583 (comment)

[rsbivand]

Shall I try this for rgdal to test a template for others? Do you have a suggested name, like:
_WITH_PROJ_GDAL_DATA_COPY ? Do you have a standard prefix?

[jeroen]

Thank you yes that would be great. The name doesn't matter for me, I think the autoconf convention is to omit the with part for envvar names, e.g. just PROJ_GDAL_DATA_COPY=TRUE

[rsbivand]

Here is a local copy of the rgdal source package, built with PROJ_LIB=/usr/local/share/proj PROJ_GDAL_DATA_COPY=TRUE R CMD build pkg. I watched the /tmp install needed for building the vignettes, and /gdal and /proj were there. Should be revision 1124 when R-Forge runs.

rgdal_1.5-24.tar.gz

[jeroen]

Hmm I trigged a build from the svn, but I don't think it worked. You can see the output here: https://github.com/r-universe/r-forge/runs/1919687390 (on the right hand under "Inspect check output").

Maybe the test autoconf line should simply be if test ${PROJ_GDAL_DATA_COPY} ; so without the = TRUE so that we simply test if the variable is set, because I am not sure if shell equates "TRUE" to TRUE.

[jeroen]

Actually on Linux it shows this, not sure if that is related

checking for proj_context_create in -lproj... yes
checking Using GDAL < 3 with PROJ >= 6... no
checking PROJ version >= 4.8.0... yes
checking PROJ: proj.db found and readable... yes
./configure: line 3348: test: =: unary operator expected

[rsbivand]

No, I don't think so. The section with data_copy starts at 4014. Maybe you picked up a transient state, not 1124?

On the package source, running:

PROJ_LIB=/usr/local/share/proj PROJ_GDAL_DATA_COPY=TRUE ./configure

I see:

...
checking proj.h usability... yes
checking proj.h presence... yes
checking for proj.h... yes
checking for proj_context_create in -lproj... yes
checking Using GDAL < 3 with PROJ >= 6... no
checking PROJ version >= 4.8.0... yes
checking PROJ: proj.db found and readable... yes
configure: "PROJ_GDAL_DATA_COPY used."
configure: Copy data for:
cp: cannot create directory '': No such file or directory
configure:   PROJ.4: /usr/local/share/proj
cp: cannot create directory '': No such file or directory
configure:   GDAL: /usr/local/share/gdal
configure: Package CPP flags:  -I/usr/local/include -DPROJ_H_API
configure: Package LIBS:  -L/usr/local/lib -lgdal -L/usr/local/lib -lproj 
configure: creating ./config.status
config.status: creating src/Makevars

which is as expected, as ${R_PACKAGE_DIR} is not set.

I just committed revision 1125, using test -n ${PROJ_GDAL_DATA_COPY} instead, and 1126 removing a bashism elsewhere.

[jeroen]

Thank you, it seems to work now:

checking Using GDAL < 3 with PROJ >= 6... no
checking PROJ version >= 4.8.0... yes
checking PROJ: proj.db found and readable... yes
configure: "PROJ_GDAL_DATA_COPY used."
configure: Copy data for:
configure:   PROJ.4: /usr/local/share/proj
configure:   GDAL: /usr/local/share/gdal
configure: Package CPP flags:  -I/usr/local/include -I/usr/local/lib/pkgconfig/../../include -DPROJ_H_API
configure: Package LIBS:  -L/usr/local/lib -lgdal -lbz2 -ljson-c -lfreexl -lgeos_c -lgeos -lwebp -lodbc -lodbcinst -lltdl -lexpat -lpthread -lopenjp2 -lnetcdf -lhdf5_hl -lhdf5 -lsz -lgif -ljpeg -lgeotiff -lpng -lpq -lpgport -lpgcommon -lssl -lcrypto -lldap -lzstd -llzma -lsqlite3 -lproj -lsqlite3 -ltiff -lz -lpthread -ldl -lspatialite -lrttopo -ldap -ldapserver -ldapclient -lpcre -lminizip -lcurl -lxml2 -liconv -lpthread -dynamic -Wl,-search_paths_first -lkrb5 -framework CoreFoundation -framework Security -framework OpenCL -L/usr/local/lib/pkgconfig/../../lib -lproj -lsqlite3 -ltiff -lcurl -ljpeg -lz
configure: creating ./config.status
config.status: creating src/Makevars

And the binary package now contains the gdal and proj directories.

[jeroen]

@edzer could you copy the rgdal configure script to sf so that I can test if that resolves the issues?

[rsbivand]

Up to revision 1128 - the message said PROJ.4, which is misleading, now PROJ.

[edzer]

This gives a problem on macos R-oldrel here; the line where this happened was last edited by @rsbivand here - Roger would you mind taking a look at what might be happening there?

[rsbivand]

No idea at all. What GDAL/PROJ versions are involved - GEOS 3.8.1, GDAL 3.1.4 and PROJ 6.3.1 (Nick Tate had trouble in class with a MacOS sf - is this release or oldrel? sf always fails on check for me locally (I never trust remote CI as you know, the links are effectively just noise because I cannot drill down to the specific failing test) because of lagging spatstat merges (my system is spatstat >= 2.0.0). If you tell me the GDAL/PROJ, I'll try to replicate.

[jeroen]

I'll try to reproduce it locally, but if it only happens for macos-oldrel on r-universe then maybe there is something out of sync on my end, and it may not happen on cran.

Note that r-universe builds and checks a package using dependencies from that same universe if available. You can see this in the check log under "Build and Check for MacOS" that some of the dependency packages are downloaded from r-spatial.r-universe.dev, and the remaining ones from cran.

[edzer]

The tab "Inspect check output" gives the output of the ./configure step: GDAL 3.2.0, PROJ 7.2.1, GEOS 3.9.0; strange enough I can't reproduce locally with 3.2.1/7.2.1/3.9.0.

[rsbivand]

Nor any problem here running source("crs.R", echo=TRUE) with:

> sf_extSoftVersion()
          GEOS           GDAL         proj.4 GDAL_with_GEOS     USE_PROJ_H 
       "3.9.1"        "3.2.1"        "7.2.1"         "true"         "true"