search

Rdatatable / data.table

R's data.table package extends data.frame:
http://r-datatable.com
Mozilla Public License 2.0
3.62k stars 986 forks source link

C API isFrame regression #6244

Closed ben-schwen closed 4 months ago

ben-schwen commented 4 months ago

6235 introduced a new error as shown by our GitLab CI

dogroups.c: In function ‘dogroups’:
dogroups.c:279:35: warning: implicit declaration of function ‘isDataFrame’; did you mean ‘isFrame’? [-Wimplicit-function-declaration]
  279 |         if (!isVector(thiscol) || isDataFrame(thiscol))
      |                                   ^~~~~~~~~~~
      |                                   isFrame

Error: package or namespace load failed for ‘data.table’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/builds/Rdatatable/data.table/data.table.Rcheck/00LOCK-data.table/00new/data.table/libs/data_table.so':
  /builds/Rdatatable/data.table/data.table.Rcheck/00LOCK-data.table/00new/data.table/libs/data_table.so: undefined symbol: isDataFrame
MichaelChirico commented 4 months ago

Hmm, I only see one other package attempting to define isDataFrame() and it's done in the same way AFAICT:

https://github.com/cran/QuickJSR/blob/0718dce159a9bdb8aa3abd08679418ebdabfffae/inst/include/quickjsr/SEXP_to_JSValue.hpp#L10-L13

@ben-schwen baby me a bit here, I tried checking out the GLCI logs and couldn't find that compilation error, could you help me find it?

ben-schwen commented 4 months ago
2024-07-13_00-38

You can download the artifacts.zip at the right hand side (see screenshot above) And then inside the artifacts.zip at /bus/test-lin-dev-gcc-strict-cran/data.table.Rcheck/00install.out

Or directly view the artifact via the webviewer

MichaelChirico commented 4 months ago

Oh, sorry. I managed to look at 00install.log thinking I was seeing 00install.out 😨

MichaelChirico commented 4 months ago

@ben-schwen what's going on here is the GCLI runner is on a slightly old version of r-devel. We see r86945 in the logs, but isDataFrame() was added in r86702: https://github.com/r-devel/r-svn/commit/4ef83b9dc3c6874e774195d329cbb6c11a71c414. So the GCLI issue will go away on its own. Still I think we can hold off on #6235 for a while as it's not generating a NOTE yet -- I think it makes sense to wait for isDataFrame() to make an R release first.