r-dbi / RSQLite

R interface for SQLite
https://rsqlite.r-dbi.org
GNU Lesser General Public License v2.1
327 stars 79 forks source link

System-wide sqlite 3.36 breaks installation of R package (which comes with sqlite 3.46) #512

Open shanesturrock opened 4 months ago

shanesturrock commented 4 months ago

OS - Rocky Linux 8 x86_64, R 4.4.0 (built from source) using standard gcc 8.5.0

Installing 2.3.6 from CRAN is fine.

Installing 2.3.7 from CRAN fails with this error:

In file included from ext-series.c:5:
vendor/extensions/series.c: In function ‘seriesBestIndex’:
vendor/extensions/series.c:500:13: error: ‘SQLITE_INDEX_CONSTRAINT_LIMIT’ undeclared (first use in this function); did you mean ‘SQLITE_INDEX_CONSTRAINT_LIKE’?
     if( op>=SQLITE_INDEX_CONSTRAINT_LIMIT
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             SQLITE_INDEX_CONSTRAINT_LIKE
vendor/extensions/series.c:500:13: note: each undeclared identifier is reported only once for each function it appears in
vendor/extensions/series.c:501:13: error: ‘SQLITE_INDEX_CONSTRAINT_OFFSET’ undeclared (first use in this function); did you mean ‘SQLITE_INDEX_CONSTRAINT_LIKE’?
      && op<=SQLITE_INDEX_CONSTRAINT_OFFSET
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             SQLITE_INDEX_CONSTRAINT_LIKE
vendor/extensions/series.c: At top level:
vendor/extensions/series.c:617:3: warning: excess elements in struct initializer
   0                          /* xIntegrity */
   ^
vendor/extensions/series.c:617:3: note: (near initialization for ‘seriesModule’)
make: *** [/opt/bioit/R-core/4.4.0/lib64/R/etc/Makeconf:195: ext-series.o] Error 1
ERROR: compilation failed for package ‘RSQLite’
krlmlr commented 4 months ago

Thanks. Can you pinpoint this to a specific commit? If this is due to a change in the bundled SQLite, we'd need to take that upstream.

shanesturrock commented 4 months ago

Those lines appeared in de3f0c2 affecting vendor/extensions/series.c when the bundled SQLite was upgraded to 3.46.0 from what I can tell.

krlmlr commented 4 months ago

I tried in Docker, works for me. Base: https://github.com/krlmlr/rig-rocky8

docker run --rm -ti --platform linux/amd64 ghcr.io/krlmlr/rig-rocky8:main R -q -e 'pak::pak("RSQLite")'
shanesturrock commented 3 months ago

Thanks for that. A little more context, we're on an HPC environment so we can't use Docker and we have multiple versions of R available for users all compiled from source and RSQLite 2.3.6 builds fine. To check if the issue was with sqlite itself, I downloaded the source for 3.46 and compiled that without issue. I then installed pak on R 4.4.0 to see if there was a difference in the behaviour but that also failed as follows:


✔ Updated metadata database: 2.93 MB in 8 files.
✔ Updating metadata database ... done

→ Will update 1 package.
→ Will download 1 CRAN package (4.25 MB).
+ RSQLite 2.3.6 → 2.3.7 [bld][cmp][dl] (4.25 MB)
? Do you want to continue (Y/n)
ℹ Getting 1 pkg (4.25 MB)
✔ Got RSQLite 2.3.7 (source) (4.27 MB)
✔ Downloaded 1 package (4.27 MB) in 2.8s
ℹ Building RSQLite 2.3.7
✖ Failed to build RSQLite 2.3.7 (33.4s)
Error:
! error in pak subprocess
Caused by error in `stop_task_build(state, worker)`:
! Failed to build source package RSQLite.
Type .Last.error to see the more details.
> .Last.error
<callr_error/rlib_error_3_0/rlib_error/error>
Error:
! error in pak subprocess
Caused by error in `stop_task_build(state, worker)`:
! Failed to build source package RSQLite.
---
Backtrace:
1. pak::pak("RSQLite")
2. pak::pkg_install(pkg, ...)
3. pak:::remote(function(...) get("pkg_install_do_plan", asNamespace("pak"))(...), …
4. err$throw(res$error)
---
Subprocess backtrace:
 1. base::withCallingHandlers(cli_message = function(msg) { …
 2. get("pkg_install_do_plan", asNamespace("pak"))(...)
 3. proposal$install()
 4. pkgdepends::install_package_plan(plan, lib = private$library, num_workers = nw, …
 5. base::withCallingHandlers({ …
 6. pkgdepends:::handle_events(state, events)
 7. pkgdepends:::handle_event(state, i)
 8. pkgdepends:::stop_task(state, worker)
 9. pkgdepends:::stop_task_build(state, worker)
10. base::throw(pkg_error("Failed to build source package {.pkg {pkg}}.", …
11. | base::signalCondition(cond)
12. global (function (e) …
krlmlr commented 3 months ago

Thanks. What does R CMD config --all show in your system? You mentioned gcc 8.5, can you please share the version output too?

This is not about running in Docker on your infrastructure, but about replicating your infrastructure in Docker for me to run.

shanesturrock commented 3 months ago

The gcc is the current 8.5.0 RPM that is available as standard on Rocky Linux 8. I've also tried the devtools versions of gcc 9 and also 11 both of which failed with the same error.

Here's the output of R CMD config --all


CFLAGS = -g -O2
CPICFLAGS = -fpic
CPPFLAGS = -I/usr/local/include
CC17 = gcc -std=gnu17
C17FLAGS = -g -O2
CC23 =
C23FLAGS =
CC90 = gcc -std=gnu90
C90FLAGS = -g -O2
CC99 = gcc -std=gnu99
C99FLAGS = -g -O2
CXX = g++ -std=gnu++17
CXXFLAGS = -g -O2
CXXPICFLAGS = -fpic
CXX11 = g++
CXX11STD = -std=gnu++11
CXX11FLAGS = -g -O2
CXX11PICFLAGS = -fpic
CXX14 = g++
CXX14STD = -std=gnu++14
CXX14FLAGS = -g -O2
CXX14PICFLAGS = -fpic
CXX17 = g++
CXX17STD = -std=gnu++17
CXX17FLAGS = -g -O2
CXX17PICFLAGS = -fpic
CXX20 =
CXX20STD =
CXX20FLAGS =
CXX20PICFLAG =
CXX23 =
CXX23STD =
CXX23FLAGS =
CXX23PICFLAGS =
DYLIB_EXT = .so
DYLIB_LD = gcc
DYLIB_LDFLAGS = -shared -fopenmp
FC = gfortran
FFLAGS = -g -O2
FPICFLAGS = -fpic
FLIBS = -lgfortran -lm -lquadmath
FCFLAGS = -g -O2
SAFE_FFLAGS = -g -O2 -msse2 -mfpmath=sse
OBJC =
OBJCFLAGS =
JAVA = /usr/bin/java
JAVAC = /usr/bin/javac
JAVAH = /usr/bin/javah
JAR = /usr/bin/jar
JAVA_HOME = /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.412.b08-2.el8.x86_64/jre
JAVA_LIBS = -L/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.412.b08-2.el8.x86_64/jre/lib/amd64/server -ljvm
JAVA_CPPFLAGS = -I/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.412.b08-2.el8.x86_64/jre/../include -I/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.412.b08-2.el8.x86_64/jre/../include/linux
LDFLAGS = -L/usr/local/lib64
SHLIB_CFLAGS =
SHLIB_CXXFLAGS =
SHLIB_CXXLD = g++ -std=gnu++17
SHLIB_CXXLDFLAGS = -shared
SHLIB_CXX11LD = g++ -std=gnu++11
SHLIB_CXX11LDFLAGS = -shared
SHLIB_CXX14LD = g++ -std=gnu++14
SHLIB_CXX14LDFLAGS = -shared
SHLIB_CXX17LD = g++ -std=gnu++17
SHLIB_CXX17LDFLAGS = -shared
SHLIB_CXX20LD =
SHLIB_CXX20LDFLAGS = -shared
SHLIB_CXX23LD =
SHLIB_CXX23LDFLAGS = -shared
SHLIB_EXT = .so
SHLIB_FFLAGS =
SHLIB_LD = gcc
SHLIB_LDFLAGS = -shared
TCLTK_CPPFLAGS = -I/usr/include -I/usr/include
TCLTK_LIBS = -L/usr/lib64 -ltcl8.6 -L/usr/lib64 -ltk8.6 -lX11
BLAS_LIBS = -lblas
LAPACK_LIBS = -llapack
MAKE = make
LIBnn = lib64
AR = ar
NM = /usr/bin/nm -B
RANLIB = ranlib
LTO =
LTO_FC =
LTO_LD =

## The following variables are defunct
CPP CXXCPP
shanesturrock commented 3 months ago

I think I've figured this out, it turns out that I had a build of sqlite 3.36 installed within our R environment to enable building of proj as it is a dependency and since we use modulefiles for the various builds on the HPC, this works fine and allows us to have versions of proj along with others for each R build to use rather than relying on RPMs. The downside is that old version of sqlite seems to be the cause of the problem as the build of RSQLite is picking up the header file from sqlite 3.36 rather than the bundled 3.46. I've recompiled the whole R suite but bumped the bundled sqlite to 3.46 which is still fine for proj to use and now RSQLite 2.3.7 also builds.

shanesturrock commented 3 months ago

Confirmed, I've been able to rebuild sqlite inside my R directory and now RSQLite 2.3.7 builds correctly.

krlmlr commented 3 months ago

Ewww, that's terrible. We need to work on header file priorities then. May I keep this open for a bit?

krlmlr commented 3 months ago

How do I install sqlite 3.36 from RPM? This should allow replicating in Docker.

shanesturrock commented 3 months ago

How do I install sqlite 3.36 from RPM? This should allow replicating in Docker.

It's not available for RL8, the newest one you can install via RPM is 3.26.0 but if the issue is with the central installed version that would trigger the issue. I don't think it is quite so simple because our HPC setup means we don't use that RPM. I've checked and while we have 3.26 RPM version installed that wasn't necessarily the issue. To test though, I've removed the compile of 3.46 that is inside my R build directory as well as the build of RSQLite and then installed again so this time it should use the code included inside the RSQLite build or the system installed one. I think the key issues is that the first place the compiler is looking is inside the lib64/R/include directory which had the local sqlite headers. Removing them means it falls back on the built in headers so it still builds. It doesn't see the system libs at all and it doesn't use the /usr/include/sqlite3.h

Long and short is this is a problem introduced by doing an HPC style build of R in its own directory and due to the issues with RPMs often being older versions than many R packages require, it is necessary to put newer versions than the system offers into lib64/R which are then used by packages to compile against. The issue I introduced was by having sqlite 3.36 included to allow proj to compile which is needed by the sp package for instance and that was fine but the move to using sqlite 3.46 in RSQLite was overridden by the older header file in lib64/R/include which are used in preference. The order of the includes results in this so if the first include could come after the bundled sqlite3 ones this might well solve the issue, but that first lib64/R/include and lib64/R/lib is the problem e.g.:

gcc -I"/opt/bioit/R-core/4.4.0/lib64/R/include" -DNDEBUG -I. -Ivendor -Ivendor/extensions -Ivendor/sqlite3 -DRSQLITE_USE_BUNDLED_SQLITE -DSQLITE_ENABLE_RTREE -DSQLITE_ENABLE_FTS3 -DSQLITE_ENABLE_FTS3_PARENTHESIS -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_JSON1 -DSQLITE_ENABLE_STAT4 -DSQLITE_SOUNDEX -DSQLITE_USE_URI=1 -DSQLITE_MAX_LENGTH=2147483647 -DHAVE_USLEEP=1 -I'/opt/bioit/R-core/4.4.0/lib64/R/library/plogr/include' -I'/opt/bioit/R-core/4.4.0/lib64/R/library/cpp11/include' -I/usr/local/include -fvisibility=hidden -fpic -g -O2 -c vendor/sqlite3/sqlite3.c -o vendor/sqlite3/sqlite3.o

krlmlr commented 3 months ago

Thanks. If I read this correctly, the bad header lives in /opt/bioit/R-core/4.4.0/lib64/R/include ?

Is there anything we can do about that here? Perhaps use

#include "./sqlite3.h"

instead of the current code without ./ ? Why are we relying on -I. anyway? 🤷‍♂️