TileDB-Inc / TileDB-R

R interface to TileDB: The Modern Database
https://tiledb-inc.github.io/TileDB-R
Other
103 stars 18 forks source link

Providing access to the TileDB shared libraries in downstream packages #780

Closed LTLA closed 1 week ago

LTLA commented 3 weeks ago

I have an R package (beachmat.tiledb) that vendors a C++ library that links to the TileDB C++ library. I would like my package to be installable even if TileDB is not installed in the system, so I was going to download the TileDB release binaries myself. However, I noticed that tiledb-R's installation directory already contains a copy of the binaries in its tiledb/ directory. I would like to be able to use these binaries to avoid having to duplicate tiledb-R's configure.ac in my own package. (This also avoids the creation of redundant copies of the TileDB binaries scattered throughout the user's R installation directory.)

Would you consider adding a little function that sets up the Makevars flags for downstream packages to link to these binaries? Based on the configure.ac, it would look something like this (for Linux/MacOS):

pkgconfig <- function(opt = c("PKG_CXX_FLAGS", "PKG_CXX_LIBS")) {
    opt <- match.arg(opt)
    pkgdir <- system.file(package='tiledb')
    vendored <- system.file('tiledb', package='tiledb')

    if (vendored != "") {
        if (opt == "PKG_CXX_FLAGS") {
            return(sprintf("-I%s/include -I%s/include", pkgdir, vendored))
        } else if (opt == "PKG_CXX_LIBS") {
            return(sprintf("-ltiledb -L%s/lib -L%s/lib", pkgdir, vendored))
        }
    }

    pc <- Sys.which("pkg-config")
    if (pc != "") {
        if (system2("pkg-config", c("--exists", "tiledb")) == 0) {
            if (opt == "PKG_CXX_FLAGS"){
                return(system2("pkg-config", c("--cflags", "tiledb"), stdout=TRUE))
            } else if (opt == "PKG_CXX_LIBS") {
                return(system2("pkg-config", c("--libs", "tiledb"), stdout=TRUE))
            }
        }
    }

    if (opt == "PKG_CXX_FLAGS"){
        return("")
    } else if (opt == "PKG_CXX_LIBS") {
        return("-ltiledb")
    }
}

Downstream packages could then create a little Makevars as shown below, allowing their C++ code to compile against tiledb-R's copy of the binaries (or the system libraries, if available).

CXX_STD = CXX17
PKG_LIBS=$(shell "${R_HOME}/bin${R_ARCH_BIN}/Rscript" -e 'tiledb::pkgconfig("PKG_CXX_LIBS")') 
PKG_CPPFLAGS=$(shell "${R_HOME}/bin${R_ARCH_BIN}/Rscript" -e 'tiledb::pkgconfig("PKG_CXX_FLAGS")') 

This could be (easily?) extended to Windows, if the newly built static library was deposited somewhere in inst/ so that pkgconfig() could reference it later in their Makevars.win.

FWIW this proposal is based on the strategy used by the Rhdf5lib and Rhtslib packages.

mojaveazure commented 1 week ago

Hi @LTLA; this functionality is now available in the development version of TileDB-R; you can install the development version of TileDB-R from GitHub with remotes::install_github() or from R-universe (once built in ~2 hours) with

install.packages("tiledb", repos = c("https://tiledb-inc.r-universe.dev", getOption("repos")))

Please let us know if you need more for this functionality to work with beachmat.tiledb

LTLA commented 1 week ago

Nice, thanks - I'll check it out tonight/tomorrow.

johnkerl commented 1 week ago

BTW/FYI we anticipate a 0.31.0 release of this package in the next couple weeks

LTLA commented 1 week ago

Works like a charm on Linux and MacOS. ~Will check on the Mac at work tomorrow.~

Unfortunately, on Windows, there are a few issues. The first is at:

https://github.com/TileDB-Inc/TileDB-R/blob/ffabb437db96148f8e61e657bbe1fb9c65626298/R/Version.R#L179

The quotes introduced by shQuote become part of the path and confuses the subsequent dir.exists(), causing Filter to remove all paths. An easy fix is to move the shQuote() call into the paste0 for -L.

After fixing the -L flags, I get a barrage of linking errors. Most of these can be resolved by copying all of the -l flags in the Makevars.win into the output of .pkg_config("PKG_CXX_LIBS"). I managed to whittle it down to the following:

$ /c/Program\ Files/R/R-4.4.1/bin/R CMD INSTALL beachmat.tiledb/
* installing to library 'C:/Users/aaron/AppData/Local/R/win-library/4.4'
* installing *source* package 'beachmat.tiledb' ...
** using staged installation
** libs
using C++ compiler: 'G__~1.EXE (GCC) 13.2.0'
using C++17
g++ -shared -s -static-libgcc -o beachmat.tiledb.dll tmp.def RcppExports.o initialize.o load_dense.o load_sparse.o -LC:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/lib/x64 -LC:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64 -LC:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64-ucrt -ltiledbstatic -ltiledb -lbz2 -lzstd -llz4 -lz -lspdlog -lfmt -laws-cpp-sdk-identity-management -laws-cpp-sdk-cognito-identity -laws-cpp-sdk-sts -laws-cpp-sdk-s3 -laws-cpp-sdk-core -llibmagic -lwebp -lpcre2-posix -lpcre2-8 -laws-crt-cpp -laws-c-mqtt -laws-c-event-stream -laws-c-s3 -laws-c-auth -laws-c-http -laws-c-io -lSecur32 -lCrypt32 -laws-c-compression -laws-c-cal -lNCrypt -laws-c-sdkutils -laws-checksums -laws-c-common -lBCrypt -lKernel32 -lRpcrt4 -lWininet -lWinhttp -lWs2_32 -lShlwapi -lUserenv -lversion -lws2_32 -lsharpyuv -LC:/rtools44/x86_64-w64-mingw32.static.posix/lib/x64 -LC:/rtools44/x86_64-w64-mingw32.static.posix/lib -LC:/PROGRA~1/R/R-44~1.1/bin/x64 -lR
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0xa0): undefined reference to `tiledb_query_get_field'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0xf7): undefined reference to `tiledb_field_datatype'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x14f): undefined reference to `tiledb_query_field_free'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query10field_typeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x1e4): undefined reference to `tiledb_query_field_free'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0xa0): undefined reference to `tiledb_query_get_field'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0xf7): undefined reference to `tiledb_field_cell_val_num'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x14f): undefined reference to `tiledb_query_field_free'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: initialize.o:initialize.cpp:(.text$_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE[_ZNK6tiledb5Query15field_var_sizedERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE]+0x1e4): undefined reference to `tiledb_query_field_free'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libtiledbstatic.a(array_directory.cc.obj):(.text+0xae57): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libtiledbstatic.a(array_schema.cc.obj):(.text+0x3693): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libtiledbstatic.a(attribute.cc.obj):(.text+0x1bb4): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libtiledbstatic.a(dimension.cc.obj):(.text+0x1882): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libtiledbstatic.a(dimension_label.cc.obj):(.text+0x477): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libtiledbstatic.a(domain.cc.obj):(.text+0xec7): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libtiledbstatic.a(enumeration.cc.obj):(.text+0xcb6): more undefined references to `__imp___iob_func' follow
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(EventStreamBuf.cpp.obj):(.text+0x68): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(EventStreamBuf.cpp.obj):(.text+0x335): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(ub_core.cpp.obj):(.text+0xcd23): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(ub_core.cpp.obj):(.text+0xe025): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(ub_core.cpp.obj):(.text+0x1624c): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(ub_core.cpp.obj):(.text+0x1795c): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(ub_core.cpp.obj):(.text+0x257e5): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(ub_core.cpp.obj):(.text+0x2e1f3): undefined reference to `std::istream::seekg(std::fpos<int>)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-cpp-sdk-core.a(ub_core.cpp.obj):(.rdata$_ZTVN3Aws5Utils6Crypto22SymmetricCryptoBufSinkE[_ZTVN3Aws5Utils6Crypto22SymmetricCryptoBufSinkE]+0x38): undefined reference to `std::basic_streambuf<char, std::char_traits<char> >::seekpos(std::fpos<int>, std::_Ios_Openmode)'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/liblibmagic.a(apprentice.c.obj):(.text+0x5bb): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/liblibmagic.a(apprentice.c.obj):(.text+0x628): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/liblibmagic.a(apprentice.c.obj):(.text+0x406d): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/liblibmagic.a(apprentice.c.obj):(.text+0x44e6): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/liblibmagic.a(funcs.c.obj):(.text+0x7fd): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/liblibmagic.a(funcs.c.obj):(.text+0xc42): more undefined references to `__imp___iob_func' follow
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-c-common.a(log_formatter.c.obj):(.text+0x246): undefined reference to `vsnprintf_s'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-c-common.a(log_writer.c.obj):(.text+0x8f): undefined reference to `__imp___iob_func'
C:\rtools44\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/Users/aaron/AppData/Local/R/win-library/4.4/tiledb/tiledb/lib/x64/libaws-c-common.a(log_writer.c.obj):(.text+0x10f): undefined reference to `__imp___iob_func'
collect2.exe: error: ld returned 1 exit status
no DLL was created
ERROR: compilation failed for package 'beachmat.tiledb'
* removing 'C:/Users/aaron/AppData/Local/R/win-library/4.4/beachmat.tiledb'

Dunno what's going on here, because the same -l flags work fine for the tiledb R package itself. The only difference is that I see tiledb's -L refers to a non-existent ../inst/tiledb/lib-13.2.0/x64 path, while beachmat.tiledb refers to the correct **/tiledb/lib/x64 path. Maybe this is consequential if linking to x64-ucrt is intended.

FWIW I'm using Rtools 4.4 on R 4.4.1, and I installed tiledb from GitHub (ffabb43).