From 56aed2f8694c88a10fe6c7c98498682fafc9a3b0 Mon Sep 17 00:00:00 2001 From: jangorecki Date: Tue, 12 May 2020 14:43:18 +0100 Subject: [PATCH 1/3] dll name uses underscore --- NAMESPACE | 2 +- R/onLoad.R | 9 +++------ src/Makevars.in | 4 ++-- src/Makevars.win | 2 +- src/init.c | 5 ++--- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index c2c095a1d8..57cc6f63ef 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,4 +1,4 @@ -useDynLib(datatable, .registration=TRUE) +useDynLib("data_table", .registration=TRUE) ## For S4-ization import(methods) diff --git a/R/onLoad.R b/R/onLoad.R index cc667e65e1..ffdf73c4f6 100644 --- a/R/onLoad.R +++ b/R/onLoad.R @@ -19,13 +19,12 @@ # Runs when loaded but not attached to search() path; e.g., when a package just Imports (not Depends on) data.table if (!exists("test.data.table", .GlobalEnv, inherits=FALSE)) { # check when installed package is loaded but skip when developing the package with cc() - dllV = if (is.loaded("CdllVersion",PACKAGE="datatable")) .Call(CdllVersion) else "before 1.12.0" - # ^^ no dot as this is the name of the dll file, #3282 + dllV = if (is.loaded("CdllVersion",PACKAGE="data_table")) .Call(CdllVersion) else "before 1.12.0" RV = packageVersion("data.table") if (dllV != RV) { dll = if (.Platform$OS.type=="windows") "dll" else "so" # https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478 - stop("The datatable.",dll," version (",dllV,") does not match the package (",RV,"). Please close all R sessions to release the old ",toupper(dll)," and reinstall data.table in a fresh R session. The root cause is that R's package installer can in some unconfirmed circumstances leave a package in a state that is apparently functional but where new R code is calling old C code silently: https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478. Once a package is in this mismatch state it may produce wrong results silently until you next upgrade the package. Please help by adding precise circumstances to 17478 to move the status to confirmed. This mismatch between R and C code can happen with any package not just data.table. It is just that data.table has added this check.") + stop("The data_table.",dll," version (",dllV,") does not match the package (",RV,"). Please close all R sessions to release the old ",toupper(dll)," and reinstall data.table in a fresh R session. The root cause is that R's package installer can in some unconfirmed circumstances leave a package in a state that is apparently functional but where new R code is calling old C code silently: https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478. Once a package is in this mismatch state it may produce wrong results silently until you next upgrade the package. Please help by adding precise circumstances to 17478 to move the status to confirmed. This mismatch between R and C code can happen with any package not just data.table. It is just that data.table has added this check.") } builtUsing = readRDS(system.file("Meta/package.rds",package="data.table"))$Built$R if (!identical(base::getRversion()>="4.0.0", builtUsing>="4.0.0")) { @@ -136,9 +135,7 @@ getRversion = function(...) stop("Reminder to data.table developers: don't use g # 4) Defining getRversion with a stop() here helps prevent new switches on getRversion() being added in future. Easily circumvented but the point is to issue the message above. .onUnload = function(libpath) { - # fix for #474. the shared object name is different from package name - # So 'detach' doesn't find datatable.so, as it looks by default for data.table.so - library.dynam.unload("datatable", libpath) + library.dynam.unload("data_table", libpath) } # nocov end diff --git a/src/Makevars.in b/src/Makevars.in index 491b0afa0d..767daf1927 100644 --- a/src/Makevars.in +++ b/src/Makevars.in @@ -2,5 +2,5 @@ PKG_CFLAGS = @openmp_cflags@ PKG_LIBS = @openmp_cflags@ -lz all: $(SHLIB) - if [ "$(SHLIB)" != "datatable$(SHLIB_EXT)" ]; then mv $(SHLIB) datatable$(SHLIB_EXT); fi - if [ "$(OS)" != "Windows_NT" ] && [ `uname -s` = 'Darwin' ]; then install_name_tool -id datatable$(SHLIB_EXT) datatable$(SHLIB_EXT); fi + if [ "$(SHLIB)" != "data_table$(SHLIB_EXT)" ]; then mv $(SHLIB) data_table$(SHLIB_EXT); fi + if [ "$(OS)" != "Windows_NT" ] && [ `uname -s` = 'Darwin' ]; then install_name_tool -id data_table$(SHLIB_EXT) data_table$(SHLIB_EXT); fi diff --git a/src/Makevars.win b/src/Makevars.win index 3ea29da12d..a878b2fd09 100644 --- a/src/Makevars.win +++ b/src/Makevars.win @@ -2,4 +2,4 @@ PKG_CFLAGS = $(SHLIB_OPENMP_CFLAGS) PKG_LIBS = $(SHLIB_OPENMP_CFLAGS) -lz all: $(SHLIB) - mv $(SHLIB) datatable$(SHLIB_EXT) + mv $(SHLIB) data_table$(SHLIB_EXT) diff --git a/src/init.c b/src/init.c index aed2da3dbd..c619f9aa5a 100644 --- a/src/init.c +++ b/src/init.c @@ -234,8 +234,7 @@ static void setSizes() { // One place we need the largest sizeof is the working memory malloc in reorder.c } -void attribute_visible R_init_datatable(DllInfo *info) -// relies on pkg/src/Makevars to mv data.table.so to datatable.so +void attribute_visible R_init_data_table(DllInfo *info) { // C exported routines, see ?cdt for details R_RegisterCCallable("data.table", "CsubsetDT", (DL_FUNC) &subsetDT); @@ -356,7 +355,7 @@ inline long long DtoLL(double x) { // under clang 3.9.1 -O3 and solaris-sparc but not solaris-x86 or gcc. // There is now a grep in CRAN_Release.cmd; use this union method instead. // int64_t may help rather than 'long long' (TODO: replace all long long with int64_t) - // The two types must be the same size. That is checked in R_init_datatable (above) + // The two types must be the same size. That is checked in R_init_data_table (above) // where sizeof(int64_t)==sizeof(double)==8 is checked. // Endianness should not matter because whether big or little, endianness is the same // inside this process, and the two types are the same size. From 2763e37c1e8d723fa7d0011cb8e20c3993097aef Mon Sep 17 00:00:00 2001 From: jangorecki Date: Tue, 12 May 2020 15:16:08 +0100 Subject: [PATCH 2/3] update so name in cc.R --- .dev/cc.R | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.dev/cc.R b/.dev/cc.R index a908a6b3df..b35a24ae6c 100644 --- a/.dev/cc.R +++ b/.dev/cc.R @@ -50,7 +50,7 @@ cc = function(test=FALSE, clean=FALSE, debug=FALSE, omp=!debug, cc_dir, path=Sys # Make sure library .so is not loaded (neither installed package nor from dev) dll = unlist(do.call("rbind",getLoadedDLLs())[,"path"]) - dll = grep("datatable.so",dll,value=TRUE) + dll = grep("data_table.so",dll,value=TRUE) sapply(dll, dyn.unload) gc() @@ -61,18 +61,18 @@ cc = function(test=FALSE, clean=FALSE, debug=FALSE, omp=!debug, cc_dir, path=Sys if (clean) system("rm *.o *.so") OMP = if (omp) "" else "no-" if (debug) { - ret = system(sprintf("MAKEFLAGS='-j CC=%s PKG_CFLAGS=-f%sopenmp CFLAGS=-std=c99\\ -O0\\ -ggdb\\ -pedantic' R CMD SHLIB -d -o datatable.so *.c", CC, OMP)) + ret = system(sprintf("MAKEFLAGS='-j CC=%s PKG_CFLAGS=-f%sopenmp CFLAGS=-std=c99\\ -O0\\ -ggdb\\ -pedantic' R CMD SHLIB -d -o data_table.so *.c", CC, OMP)) } else { - ret = system(sprintf("MAKEFLAGS='-j CC=%s CFLAGS=-f%sopenmp\\ -std=c99\\ -O3\\ -pipe\\ -Wall\\ -pedantic\\ -fno-common' R CMD SHLIB -o datatable.so *.c", CC, OMP)) + ret = system(sprintf("MAKEFLAGS='-j CC=%s CFLAGS=-f%sopenmp\\ -std=c99\\ -O3\\ -pipe\\ -Wall\\ -pedantic\\ -fno-common' R CMD SHLIB -o data_table.so *.c", CC, OMP)) # TODO add -Wextra too? } if (ret) return() # clang -Weverything includes -pedantic and issues many more warnings than gcc - # system("R CMD SHLIB -o datatable.so *.c") + # system("R CMD SHLIB -o data_table.so *.c") if (any(sapply(objects(envir=.GlobalEnv),function(x){inherits(get(x,.GlobalEnv),"data.table")}))) { cat("ABOUT TO RELOAD .SO BUT THERE ARE DATA.TABLE OBJECTS IN .GLOBALENV SO FINALIZER MIGHT CRASH\n") } - dyn.load("datatable.so") + dyn.load("data_table.so") setwd(old) xx = getDLLRegisteredRoutines("datatable",TRUE) for (i in seq_along(xx$.Call)) From 40e9acdf2e9ac43de81dd04e9568056f5fbf7bc3 Mon Sep 17 00:00:00 2001 From: Matt Dowle Date: Wed, 4 Aug 2021 11:41:46 -0600 Subject: [PATCH 3/3] add news item --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index a3f043eddc..4d58f97c43 100644 --- a/NEWS.md +++ b/NEWS.md @@ -189,6 +189,10 @@ 12. `?merge` and `?setkey` have been updated to clarify that the row order is retained when `sort=FALSE`, and why `NA`s are always first when `sort=TRUE`, [#2574](https://github.com/Rdatatable/data.table/issues/2574) [#2594](https://github.com/Rdatatable/data.table/issues/2594). Thanks to Davor Josipovic and Markus Bonsch for the reports, and Jan Gorecki for the PR. +13. `datatable.[dll|so]` has changed name to `data_table.[dll|so]`, [#4442](https://github.com/Rdatatable/data.table/pull/4442). Thanks to Jan Gorecki for the PR. We had previously removed the `.` since `.` is not allowed by the following paragraph in the Writing-R-Extensions manual. Replacing `.` with `_` instead now seems more consistent with the last sentence. + + > ... the basename of the DLL needs to be both a valid file name and valid as part of a C entry point (e.g. it cannot contain ‘.’): for portable code it is best to confine DLL names to be ASCII alphanumeric plus underscore. If entry point R_init_lib is not found it is also looked for with ‘.’ replaced by ‘_’. + # data.table [v1.14.0](https://github.com/Rdatatable/data.table/milestone/23?closed=1) (21 Feb 2021)