retain PKG_* user-supplied env values (#4735)

Author: mattdowle

.dev/CRAN_Release.cmd

@@ -240,6 +240,12 @@ require(data.table)
 test.data.table()
 q("no")
 
+# User supplied PKG_CFLAGS and PKG_LIBS passed through, #4664
+# Next line from https://mac.r-project.org/openmp/. Should see the arguments passed through and then fail with gcc on linux.
+PKG_CFLAGS='-Xclang -fopenmp' PKG_LIBS=-lomp R CMD INSTALL data.table_1.13.1.tar.gz
+# Next line should work on Linux, just using superfluous and duplicate but valid parameters here to see them retained and work 
+PKG_CFLAGS='-fopenmp' PKG_LIBS=-lz R CMD INSTALL data.table_1.13.1.tar.gz
+
 R
 remove.packages("xml2")    # we checked the URLs; don't need to do it again (many minutes)
 require(data.table)

NEWS.md

@@ -28,6 +28,10 @@
 
 3. `test.data.table()` failed in non-English locales such as `LC_TIME=fr_FR.UTF-8` due to `Jan` vs `janv.` in tests 168 and 2042, [#3450](https://github.com/Rdatatable/data.table/issues/3450). Thanks to @shrektan for reporting, and @tdhock for making the tests locale-aware.
 
+4. User-supplied `PKG_LIBS` and `PKG_CFLAGS` are now retained and the suggestion in https://mac.r-project.org/openmp/; i.e.,
+    `PKG_CPPFLAGS='-Xclang -fopenmp' PKG_LIBS=-lomp R CMD INSTALL data.table_<ver>.tar.gz`
+has a better chance of working on Mac.
+
 
 # data.table [v1.13.0](https://github.com/Rdatatable/data.table/milestone/17?closed=1)  (24 Jul 2020)
 

configure

@@ -85,7 +85,7 @@ EOF
 
 if [ "$R_NO_OPENMP" = "1" ]; then
   # Compilation failed -- try forcing -fopenmp instead.
-  "${CC}" "${CFLAGS}" -fopenmp test-omp.c || R_NO_OPENMP=1
+  ${CC} ${CFLAGS} -fopenmp test-omp.c || R_NO_OPENMP=1
 fi
 
 # Clean up.
@@ -103,5 +103,8 @@ else
   echo "OpenMP supported"
   sed -e "s|@openmp_cflags@|\$(SHLIB_OPENMP_CFLAGS)|" src/Makevars.in > src/Makevars
 fi
+# retain user supplied PKG_ env variables, #4664. See comments in Makevars.in too.
+sed -i "s|@PKG_CFLAGS@|$PKG_CFLAGS|" src/Makevars
+sed -i "s|@PKG_LIBS@|$PKG_LIBS|" src/Makevars
 
 exit 0

src/Makevars.in

@@ -1,5 +1,10 @@
-PKG_CFLAGS = @openmp_cflags@
-PKG_LIBS = @openmp_cflags@ -lz
+PKG_CFLAGS = @PKG_CFLAGS@ @openmp_cflags@
+PKG_LIBS = @PKG_LIBS@ @openmp_cflags@ -lz
+# See WRE $1.2.1.1. But retain user supplied PKG_* too, #4664.
+# WRE states ($1.6) that += isn't portable and that we aren't allowed to use it.
+# Otherwise we could use the much simpler PKG_LIBS += @openmp_cflags@ -lz.
+# Can't do PKG_LIBS = $(PKG_LIBS)...  either because that's a 'recursive variable reference' error in make
+# Hence the onerous @...@ substitution. Is it still appropriate in 2020 that we can't use +=?
 
 all: $(SHLIB)
 	if [ "$(SHLIB)" != "datatable$(SHLIB_EXT)" ]; then mv $(SHLIB) datatable$(SHLIB_EXT); fi