Skip to content

Commit

Permalink
retain PKG_* user-supplied env values (Rdatatable#4735)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattdowle authored and hongyuanjia committed Sep 2, 2023
1 parent cd0bd9c commit 6a6b5c5
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .dev/CRAN_Release.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 4 additions & 1 deletion configure
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
9 changes: 7 additions & 2 deletions src/Makevars.in
Original file line number Diff line number Diff line change
@@ -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
Expand Down

0 comments on commit 6a6b5c5

Please sign in to comment.