From 3eb236f80c614af96c3a04409ed6188c488a9a67 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Mon, 11 Nov 2024 15:01:36 -0500 Subject: [PATCH 1/9] Provide access to TileDB shared libraries in downstream packages New helper function to provide access to `libtiledb` used by tiledb-r for downstream packages. This builds off the prototype code @LTLA provided resolves #780 --- R/Init.R | 5 ++++- R/Version.R | 42 ++++++++++++++++++++++++++++++++++++++++++ TileDB-R.Rproj | 3 +++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/R/Init.R b/R/Init.R index 3cb0894dba..8ae1f76329 100644 --- a/R/Init.R +++ b/R/Init.R @@ -53,6 +53,10 @@ ## set a preference for allocation size defaults .pkgenv[["allocation_size"]] <- load_allocation_size_preference() + # cache package name and path + .pkgenv[["pkgname"]] <- pkgname + .pkgenv[["libname"]] <- libname + ## call setter for Rcpp plugin support .set_compile_link_options() @@ -121,7 +125,6 @@ inlineCxxPlugin <- function(...) { } } - #' @importFrom utils read.table .getLinuxFlavor <- function() { res <- NA_character_ diff --git a/R/Version.R b/R/Version.R index 099d42c333..e6e6330838 100644 --- a/R/Version.R +++ b/R/Version.R @@ -41,3 +41,45 @@ tiledb_version <- function(compact = FALSE) { libtiledb_version() } } + +#' Compiler Arguments for Using \code{libtiledb} +#' +#' @param opt A single character value with the package configuration variable +#' to fetch; choose from +#' \itemize{ +#' \item \dQuote{\code{PKG_CXX_FLAGS}}: compiler flags for \code{libtiledb} +#' \item \dQuote{\code{PKG_CXX_LIBS}}: linking flags for \code{libtiledb} +#' } +#' +#' @return ... +#' +#' @keywords internal +#' +#' @export +#' +#' @examples +#' .pkg_config() +#' .pkg_config("PKG_CXX_LIBS") +#' +.pkg_config <- function(opt = c("PKG_CXX_FLAGS", "PKG_CXX_LIBS")) { + opt <- match.arg(opt) + if (nzchar(lib <- system.file("tiledb", package = .pkgenv$pkgname, lib.loc = .pkgenv$libname))) { + pkgdir <- system.file(package = .pkgenv$pkgname, lib.loc = .pkgenv$libname) + return(switch( + EXPR = opt, + PKG_CXX_FLAGS = sprintf("-I%s/include -I%s/include", pkgdir, lib), + PKG_CXX_LIBS = sprintf("-ltiledb -L%s/lib -L%s/lib", pkgdir, lib) + )) + } + if (nzchar(pkgconfig <- Sys.which("pkg-config"))) { + if (!system2(pkgconfig, args = c("--exists", "tiledb"))) { + flag <- switch(EXPR = opt, PKG_CXX_FLAGS = "--cflags", PKG_CXX_LIBS = "--libs") + return(system2(pkgconfig, args = c(flag, "tiledb"), stdout = TRUE)) + } + } + return(switch( + EXPR = opt, + PKG_CXX_FLAGS = "", + PKG_CXX_LIBS = "-ltiledb" + )) +} diff --git a/TileDB-R.Rproj b/TileDB-R.Rproj index e05cf5db3e..9fbba2861f 100644 --- a/TileDB-R.Rproj +++ b/TileDB-R.Rproj @@ -12,6 +12,9 @@ Encoding: UTF-8 RnwWeave: Sweave LaTeX: pdfLaTeX +AutoAppendNewline: Yes +StripTrailingWhitespace: Yes + BuildType: Package PackageUseDevtools: Yes PackageInstallArgs: --no-multiarch --with-keep.source --install-tests From ee2b20e997d4c1d899966e2f99df1ee48b204a54 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Wed, 13 Nov 2024 15:24:56 -0500 Subject: [PATCH 2/9] Update windows install to include libtiledb source --- NAMESPACE | 1 + man/dot-pkg_config.Rd | 28 ++++++++++++++++++++++++++++ src/Makevars.win | 3 ++- tools/winlibs.R | 23 +++++++++++++++-------- 4 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 man/dot-pkg_config.Rd diff --git a/NAMESPACE b/NAMESPACE index 50b308decf..8816c68201 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -24,6 +24,7 @@ export("return_as<-") export("selected_points<-") export("selected_ranges<-") export("strings_as_factors<-") +export(.pkg_config) export(allows_dups) export(array_consolidate) export(array_vacuum) diff --git a/man/dot-pkg_config.Rd b/man/dot-pkg_config.Rd new file mode 100644 index 0000000000..bc5eda5e94 --- /dev/null +++ b/man/dot-pkg_config.Rd @@ -0,0 +1,28 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/Version.R +\name{.pkg_config} +\alias{.pkg_config} +\title{Compiler Arguments for Using \code{libtiledb}} +\usage{ +.pkg_config(opt = c("PKG_CXX_FLAGS", "PKG_CXX_LIBS")) +} +\arguments{ +\item{opt}{A single character value with the package configuration variable +to fetch; choose from +\itemize{ +\item \dQuote{\code{PKG_CXX_FLAGS}}: compiler flags for \code{libtiledb} +\item \dQuote{\code{PKG_CXX_LIBS}}: linking flags for \code{libtiledb} +}} +} +\value{ +... +} +\description{ +Compiler Arguments for Using \code{libtiledb} +} +\examples{ +.pkg_config() +.pkg_config("PKG_CXX_LIBS") + +} +\keyword{internal} diff --git a/src/Makevars.win b/src/Makevars.win index 00fdfc7c4a..f8cb7829e0 100644 --- a/src/Makevars.win +++ b/src/Makevars.win @@ -1,5 +1,6 @@ CXX_STD = CXX17 -RWINLIB = ../windows/rwinlib-tiledb +# RWINLIB = ../windows/rwinlib-tiledb +RWINLIB = ../inst/tiledb PKG_CPPFLAGS = -I. -I../inst/include -I$(RWINLIB)/include -I$(RWINLIB)/include/tiledb -DTILEDB_STATIC_DEFINE -DTILEDB_SILENT_BUILD PKG_LIBS = \ diff --git a/tools/winlibs.R b/tools/winlibs.R index 080b44b3c1..a418cddb9c 100644 --- a/tools/winlibs.R +++ b/tools/winlibs.R @@ -5,14 +5,21 @@ if (!file.exists(dcffile)) stop("TileDB Version file not found.") dcf <- read.dcf(dcffile) ver <- dcf[[1, "version"]] -if (!file.exists("../windows/rwinlib-tiledb/include/tiledb/tiledb.h")) { +# if (!file.exists("../windows/rwinlib-tiledb/include/tiledb/tiledb.h")) { +if (!file.exists("../inst/tiledb/include/tiledb/tiledb.h")) { if (getRversion() < "4") stop("This package requires Rtools40 or newer") - op <- options() - options(timeout=180) # CRAN request to have patient download settings - download.file(sprintf("https://github.com/TileDB-Inc/rwinlib-tiledb/archive/v%s.zip", ver), "lib.zip", quiet = TRUE) + op <- options(timeout = 180) # CRAN request to have patient download settings + zipfile <- tempfile(tmpdir = tempdir(check = TRUE), fileext = '.zip') + download.file( + sprintf("https://github.com/TileDB-Inc/rwinlib-tiledb/archive/v%s.zip", ver), + destfile = zipfile, + quiet = TRUE + ) options(op) - dir.create("../windows", showWarnings = FALSE) - unzip("lib.zip", exdir = "../windows") - file.rename(sprintf("../windows/rwinlib-tiledb-%s", ver), "../windows/rwinlib-tiledb") - unlink("lib.zip") + # dir.create("../windows", showWarnings = FALSE) + # unzip("lib.zip", exdir = "../windows") + unzip(zipfile, exdir = "../inst") + # file.rename(sprintf("../windows/rwinlib-tiledb-%s", ver), "../windows/rwinlib-tiledb") + file.rename(sprintf("../inst/rwinlib-tiledb-%s", ver), "../inst/tiledb") + unlink(zipfile, force = TRUE) } From 680e8ab47a2ef8fe1be7473a44bdd19b25437437 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Wed, 13 Nov 2024 16:46:31 -0500 Subject: [PATCH 3/9] Clean up comments --- src/Makevars.win | 1 - tools/winlibs.R | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Makevars.win b/src/Makevars.win index f8cb7829e0..262ea9024b 100644 --- a/src/Makevars.win +++ b/src/Makevars.win @@ -1,5 +1,4 @@ CXX_STD = CXX17 -# RWINLIB = ../windows/rwinlib-tiledb RWINLIB = ../inst/tiledb PKG_CPPFLAGS = -I. -I../inst/include -I$(RWINLIB)/include -I$(RWINLIB)/include/tiledb -DTILEDB_STATIC_DEFINE -DTILEDB_SILENT_BUILD diff --git a/tools/winlibs.R b/tools/winlibs.R index a418cddb9c..3936cf4db2 100644 --- a/tools/winlibs.R +++ b/tools/winlibs.R @@ -5,7 +5,6 @@ if (!file.exists(dcffile)) stop("TileDB Version file not found.") dcf <- read.dcf(dcffile) ver <- dcf[[1, "version"]] -# if (!file.exists("../windows/rwinlib-tiledb/include/tiledb/tiledb.h")) { if (!file.exists("../inst/tiledb/include/tiledb/tiledb.h")) { if (getRversion() < "4") stop("This package requires Rtools40 or newer") op <- options(timeout = 180) # CRAN request to have patient download settings @@ -13,13 +12,10 @@ if (!file.exists("../inst/tiledb/include/tiledb/tiledb.h")) { download.file( sprintf("https://github.com/TileDB-Inc/rwinlib-tiledb/archive/v%s.zip", ver), destfile = zipfile, - quiet = TRUE + quiet = !isTRUE(getOption('verbose', default = FALSE)) ) options(op) - # dir.create("../windows", showWarnings = FALSE) - # unzip("lib.zip", exdir = "../windows") unzip(zipfile, exdir = "../inst") - # file.rename(sprintf("../windows/rwinlib-tiledb-%s", ver), "../windows/rwinlib-tiledb") file.rename(sprintf("../inst/rwinlib-tiledb-%s", ver), "../inst/tiledb") unlink(zipfile, force = TRUE) } From 3707724ab2d55cca079c3a75036cd03d8b23f22b Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Wed, 13 Nov 2024 16:46:51 -0500 Subject: [PATCH 4/9] Better Windows support --- R/Version.R | 36 +++++++++++++++++++++++++++++++++--- man/dot-pkg_config.Rd | 3 ++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/R/Version.R b/R/Version.R index e6e6330838..9a6ac5e9b4 100644 --- a/R/Version.R +++ b/R/Version.R @@ -51,7 +51,8 @@ tiledb_version <- function(compact = FALSE) { #' \item \dQuote{\code{PKG_CXX_LIBS}}: linking flags for \code{libtiledb} #' } #' -#' @return ... +#' @return A single string containing either the include directories or linking +#' directories for \code{libtiledb} #' #' @keywords internal #' @@ -67,8 +68,37 @@ tiledb_version <- function(compact = FALSE) { pkgdir <- system.file(package = .pkgenv$pkgname, lib.loc = .pkgenv$libname) return(switch( EXPR = opt, - PKG_CXX_FLAGS = sprintf("-I%s/include -I%s/include", pkgdir, lib), - PKG_CXX_LIBS = sprintf("-ltiledb -L%s/lib -L%s/lib", pkgdir, lib) + PKG_CXX_FLAGS = switch( + EXPR = .Platform$OS.type, + # Adapted from Makevars.win, which includes libdir/include/tiledb in addition + # to libdir/include and pkgdir/include + windows = sprintf( + "-I%s/include -I%s/include -I%s/include/tiledb", + pkgdir, + lib, + lib + ), + sprintf("-I%s/include -I%s/include", pkgdir, lib) + ), + PKG_CXX_LIBS = switch( + EXPR = .Platform$OS.type, + # rwinlib-tiledb is structred slightly differently than libtiledb for + # Unix-alikes; R 4.2 and higher require ucrt + windows = { + arch <- .Platform$r_arch + libs <- as.vector(vapply( + c(pkgdir, lib), + FUN = \(x) c( + sprintf("%s/lib/%s", x, arch), + ifelse(getRversion() > '4.2.0', sprintf("%s/lib/%s-ucrt", x, arch), "") + ), + FUN.VALUE = character(2L), + USE.NAMES = FALSE + )) + paste('-ltiledb', paste0('-L', Filter(dir.exists, libs), collapse = ' ')) + }, + sprintf("-ltiledb -L%s/lib -L%s/lib", pkgdir, lib) + ) )) } if (nzchar(pkgconfig <- Sys.which("pkg-config"))) { diff --git a/man/dot-pkg_config.Rd b/man/dot-pkg_config.Rd index bc5eda5e94..2d6b8e5753 100644 --- a/man/dot-pkg_config.Rd +++ b/man/dot-pkg_config.Rd @@ -15,7 +15,8 @@ to fetch; choose from }} } \value{ -... +A single string containing either the include directories or linking +directories for \code{libtiledb} } \description{ Compiler Arguments for Using \code{libtiledb} From d2dab5c400dda4e0f42eb91be538da3eb7b27487 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Thu, 14 Nov 2024 17:44:19 -0500 Subject: [PATCH 5/9] Add tests for .pkg_config() --- .github/workflows/pkgconfig.yaml | 73 +++++++++++++++++++++++++ R/Version.R | 21 +++++-- inst/pkgconfigtest/test_pkg_config.R | 82 ++++++++++++++++++++++++++++ tests/pkgconfig-test.R | 7 +++ 4 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/pkgconfig.yaml create mode 100644 inst/pkgconfigtest/test_pkg_config.R create mode 100644 tests/pkgconfig-test.R diff --git a/.github/workflows/pkgconfig.yaml b/.github/workflows/pkgconfig.yaml new file mode 100644 index 0000000000..4143353660 --- /dev/null +++ b/.github/workflows/pkgconfig.yaml @@ -0,0 +1,73 @@ +on: + push: + pull_request: + +name: pkgconfig + +permissions: read-all + +jobs: + pkgconfig: + runs-on: ${{ matrix.os }} + name: ${{ matrix.os }} (r-${{ matrix.r }}) (${{ matrix.vendored }} libtiledb) + + strategy: + fail-fast: false + matrix: + os: + - ubuntu-latest + r: + - release + # - devel + vendored: + - 'vendored' + - 'system' + include: + - os: ubuntu-latest + use-public-rspm: true + # - os: ubuntu-latest + # r: devel + # http-user-agent: 'release' + exclude: + - os: macos-latest + r: devel + + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + R_KEEP_PKG_SOURCE: yes + _R_CHECK_FORCE_SUGGESTS_: "FALSE" + _R_TILEDB_LIBTILEDB_: ${{ matrix.vendored }} + + steps: + - uses: actions/checkout@v4 + + - uses: r-lib/actions/setup-pandoc@v2 + + - uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.r }} + http-user-agent: ${{ matrix.http-user-agent }} + use-public-rspm: ${{ matrix.use-public-rspm }} + + - name: Setup libtiledb + if: ${{ matrix.vendored == 'system' }} + run: | + VERSION=$(grep version tools/tiledbVersion.txt | cut -f 2 -d ':' | tr -d '[:space:]') + SHA=$(grep sha tools/tiledbVersion.txt | cut -f 2 -d ':' | tr -d '[:space:]') + URL="https://github.com/TileDB-Inc/TileDB/releases/download/${VERSION}/tiledb-linux-x86_64-${VERSION}-${SHA}.tar.gz" + mkdir -vp libtiledb + cd libtiledb + wget -O libtiledb.tar.gz ${URL} + tar -xvzf libtiledb.tar.gz + /usr/bin/sudo cp -Rv include/* /usr/local/include/ + /usr/bin/sudo cp -Rv lib/* /usr/local/lib/ + cd .. + rm -rfv libtiledb + sudo ldconfig + + - uses: r-lib/actions/setup-r-dependencies@v2 + with: + extra-packages: local::. + + - name: Test tiledb::.pkg_config() + run: Rscript tests/pkgconfig-test.R diff --git a/R/Version.R b/R/Version.R index 9a6ac5e9b4..3a9c9a7d7b 100644 --- a/R/Version.R +++ b/R/Version.R @@ -64,14 +64,19 @@ tiledb_version <- function(compact = FALSE) { #' .pkg_config <- function(opt = c("PKG_CXX_FLAGS", "PKG_CXX_LIBS")) { opt <- match.arg(opt) - if (nzchar(lib <- system.file("tiledb", package = .pkgenv$pkgname, lib.loc = .pkgenv$libname))) { + lib <- system.file( + "tiledb", + package = .pkgenv$pkgname, + lib.loc = .pkgenv$libname + ) + if (nzchar(lib)) { pkgdir <- system.file(package = .pkgenv$pkgname, lib.loc = .pkgenv$libname) return(switch( EXPR = opt, PKG_CXX_FLAGS = switch( EXPR = .Platform$OS.type, - # Adapted from Makevars.win, which includes libdir/include/tiledb in addition - # to libdir/include and pkgdir/include + # Adapted from Makevars.win, which includes libdir/include/tiledb in + # addition to libdir/include and pkgdir/include windows = sprintf( "-I%s/include -I%s/include -I%s/include/tiledb", pkgdir, @@ -82,7 +87,7 @@ tiledb_version <- function(compact = FALSE) { ), PKG_CXX_LIBS = switch( EXPR = .Platform$OS.type, - # rwinlib-tiledb is structred slightly differently than libtiledb for + # rwinlib-tiledb is structured slightly differently than libtiledb for # Unix-alikes; R 4.2 and higher require ucrt windows = { arch <- .Platform$r_arch @@ -103,8 +108,12 @@ tiledb_version <- function(compact = FALSE) { } if (nzchar(pkgconfig <- Sys.which("pkg-config"))) { if (!system2(pkgconfig, args = c("--exists", "tiledb"))) { - flag <- switch(EXPR = opt, PKG_CXX_FLAGS = "--cflags", PKG_CXX_LIBS = "--libs") - return(system2(pkgconfig, args = c(flag, "tiledb"), stdout = TRUE)) + flag <- switch( + EXPR = opt, + PKG_CXX_FLAGS = "--cflags", + PKG_CXX_LIBS = "--libs" + ) + return(trimws(system2(pkgconfig, args = c(flag, "tiledb"), stdout = TRUE))) } } return(switch( diff --git a/inst/pkgconfigtest/test_pkg_config.R b/inst/pkgconfigtest/test_pkg_config.R new file mode 100644 index 0000000000..1d7672ecdd --- /dev/null +++ b/inst/pkgconfigtest/test_pkg_config.R @@ -0,0 +1,82 @@ + +if (!at_home()) { + exit_file("'.pkg_config()' tests run at home") +} + +if (!nzchar(libtype <- Sys.getenv("_R_TILEDB_LIBTILEDB_"))) { + exit_file("The libtiledb type is unset") +} + +expect_true( + libtype %in% c("system", "vendored"), + info = "`Sys.getenv('_R_TILEDB_LIBTILEDB_')` must be 'system' or 'vendored'" +) + +expect_error(.pkg_config('unknown'), info = "Invalid opts throw an error") +expect_true( + nzchar(include <- .pkg_config("PKG_CXX_FLAGS")), + info = "'PKG_CXX_FLAGS' are available" +) +expect_true( + nzchar(libs <- .pkg_config("PKG_CXX_LIBS")), + info = "'PKG_CXX_LIBS' are available" +) + +if (libtype == "system") { + # Check includes + expect_true( + grepl("^-I/", include), + info = "'PKG_CXX_FLAGS' points to an include directory" + ) + expect_length( + unlist(strsplit(include, split = "[[:space:]]")), + length = 1L, + info = "'PKG_CXX_FLAGS' has only one include directory" + ) + # Check libs + expect_true( + grepl("^-L", libs), + info = "'PKG_CXX_LIBS' starts with a linking directory" + ) + expect_true( + grepl("[[:space:]]-ltiledb$", libs), + info = "'PKG_CXX_LIBS' ends with the libtiledb directive" + ) +} else { + pkgdir <- system.file(package = "tiledb") + pattern <- paste0("^-%s", pkgdir) + # Check includes + include <- unlist(strsplit(include, split = "[[:space:]]")) + expect_true( + length(include) > 1L, + info = "'PKG_CXX_FLAGS' includes multiple include directories" + ) + for (i in seq_along(along.with = include)) { + expect_true( + grepl(sprintf(pattern, "I"), include[i]), + info = sprintf( + "All include directories are within the package directory (include %i)", + i + ) + ) + } + # Check libs + expect_true( + grepl('^-ltiledb[[:space:]]', libs), + info = "'PKG_CXX_LIBS' starts with the libtiledb directory" + ) + libs <- unlist(strsplit(libs, split = "[[:space:]]")) + expect_true( + length(libs) > 2L, + info = "'PKG_CXX_LIBS' includes multiple linking directories" + ) + for (i in seq.int(2L, length(libs))) { + expect_true( + grepl(sprintf(pattern, "L"), libs[i]), + info = sprintf( + "All linking directories are within the package directory (link %i)", + i - 1L + ) + ) + } +} diff --git a/tests/pkgconfig-test.R b/tests/pkgconfig-test.R new file mode 100644 index 0000000000..c4b838f26d --- /dev/null +++ b/tests/pkgconfig-test.R @@ -0,0 +1,7 @@ +if (requireNamespace("tinytest", quietly = TRUE)) { + tinytest::test_package( + "tiledb", + testdir = "pkgconfigtest", + at_home = nzchar(Sys.getenv("_R_TILEDB_LIBTILEDB_")) + ) +} From 119145461688b675ec9286a2416a02ce8406c9c3 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Fri, 15 Nov 2024 11:48:43 -0500 Subject: [PATCH 6/9] Add utility functions to check version of libtiledb a package was built with vs version package is loaded with --- DESCRIPTION | 8 ++- NAMESPACE | 2 + R/Version.R | 82 +++++++++++++++++++++++++++++ inst/pkgconfigtest/test_core_info.R | 61 +++++++++++++++++++++ man/dot-core_info.Rd | 53 +++++++++++++++++++ man/dot-pkg_config.Rd | 3 +- 6 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 inst/pkgconfigtest/test_core_info.R create mode 100644 man/dot-core_info.Rd diff --git a/DESCRIPTION b/DESCRIPTION index 71591793f8..d4e9281d83 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -24,7 +24,13 @@ SystemRequirements: A C++17 compiler is required; on macOS compilation version 1 build selected); on x86_64 and M1 platforms pre-built TileDB Embedded libraries are available at GitHub and are used if no TileDB installation is detected, and no other option to build or download was specified by the user. -Imports: methods, Rcpp (>= 1.0.8), nanotime, spdl, nanoarrow +Imports: + methods, + Rcpp (>= 1.0.8), + nanotime, + spdl, + nanoarrow, + tools LinkingTo: Rcpp, RcppInt64, nanoarrow Suggests: tinytest, simplermarkdown, curl, bit64, Matrix, palmerpenguins, nycflights13, data.table, tibble, arrow VignetteBuilder: simplermarkdown diff --git a/NAMESPACE b/NAMESPACE index 8816c68201..8b3649fe86 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -24,6 +24,8 @@ export("return_as<-") export("selected_points<-") export("selected_ranges<-") export("strings_as_factors<-") +export(.core_hash) +export(.core_info) export(.pkg_config) export(allows_dups) export(array_consolidate) diff --git a/R/Version.R b/R/Version.R index 3a9c9a7d7b..ff7d5e1284 100644 --- a/R/Version.R +++ b/R/Version.R @@ -42,8 +42,90 @@ tiledb_version <- function(compact = FALSE) { } } +#' \code{libtiledb} Information +#' +#' Get version and install information of the core \code{libtiledb} install +#' +#' @section Checking the \code{libtiledb} information in downstream packages: +#' These functions are designed to make it easy to test if the core +#' \code{libtiledb} install has changed. This is accomplished by adding a +#' build-time constant to cache the version of \code{libtiledb} was built with. +#' For example, in \code{zzz.R}, put the following line to cache the +#' \code{libtiledb} information during package build +#' \preformatted{ +#' .built_with <- list(libtiledb = tiledb::.core_hash()) +#' } +#' Then, in the \link[base:ns-hooks]{load hook}, add the following check +#' \preformatted{ +#' .onLoad <- function(libname, pkgname) { +#' if (.built_with$libtiledb != tiledb::.core_hash()) { +#' warning("Core libtiledb has changed, please reinstall ", pkgname) +#' } +#' } +#' } +#' This will throw a warning if \pkg{tiledb}, and therefore \code{libtiledb}, +#' has changed between downstream package install and load +#' +#' @return \code{.core_info()}: A named character vector with the following entries: +#' \itemize{ +#' \item \dQuote{\code{version}}: \code{libtiledb} version +#' \item \dQuote{\code{libtype}}: type of \code{libtiledb} install; will be one +#' of \dQuote{\code{vendored}}, \dQuote{\code{system}}, or \dQuote{\code{unknown}} +#' } +#' +#' @keywords internal +#' +#' @export +#' +#' @examples +#' .core_info() +#' +.core_info <- function() { + info <- c( + version = as.character(tiledb_version(TRUE)), + libtype = character(1L) + ) + lib <- system.file( + "tiledb", + package = .pkgenv$pkgname, + lib.loc = .pkgenv$libname + ) + if (nzchar(lib)) { + info['libtype'] <- 'vendored' + return(info) + } + if (nzchar(pkgconfig <- Sys.which("pkg-config"))) { + if (!system2(pkgconfig, args = c("--exists", "tiledb"))) { + info['libtype'] <- 'system' + return(info) + } + } + info['libtype'] <- 'unknown' + return(info) +} + +#' @return \code{.core_hash()}: The \link[tools:md5sum]{MD5 hash} of the core info +#' +#' @rdname dot-core_info +#' +#' @export +#' +#' @examples +#' .core_hash() +#' +.core_hash <- function() { + tmp <- tempfile() + on.exit(file.remove(tmp), add = TRUE, after = FALSE) + info <- .core_info() + writeLines(paste(names(info), info, sep = ':\t', collapse = '\n'), con = tmp) + return(unname(tools::md5sum(tmp))) +} + #' Compiler Arguments for Using \code{libtiledb} #' +#' Get compiler flags for using the core \code{libtiledb} install +#' used by \pkg{tiledb} +#' #' @param opt A single character value with the package configuration variable #' to fetch; choose from #' \itemize{ diff --git a/inst/pkgconfigtest/test_core_info.R b/inst/pkgconfigtest/test_core_info.R new file mode 100644 index 0000000000..e9c9a1cde5 --- /dev/null +++ b/inst/pkgconfigtest/test_core_info.R @@ -0,0 +1,61 @@ + +if (!at_home()) { + exit_file("'.pkg_config()' tests run at home") +} + +libtype <- Sys.getenv("_R_TILEDB_LIBTILEDB_") +if (nzchar(libtype)) { + expect_true( + libtype %in% c("system", "vendored"), + info = "`Sys.getenv('_R_TILEDB_LIBTILEDB_')` must be 'system' or 'vendored'" + ) +} + +# Check .core_info() +expect_inherits( + info <- .core_info(), + class = "character", + info = "'.core_info()' returns a character vector" +) +expect_length(info, length = 2L, info = "'.core_info()' returns a two-length vector") +expect_identical( + names(info), + target = c("version", "libtype"), + info = "'.core_info()' returns a named vector with names of 'version' and 'libtype'" +) +expect_identical( + info[["version"]], + target = as.character(tiledb_version(compact = TRUE)), + info = "'.core_info()' returns the correct core version" +) +libtarget <- ifelse(nzchar(libtype), yes = libtype, no = "unknown") +expect_identical( + info[["libtype"]], + target = libtarget, + info = sprintf("'.core_info()' returns the correct libtype ('%s')", libtarget) +) + +# Check .core_hash() +tmpfile <- tempfile(tmpdir = tempdir(check = TRUE)) +writeLines( + sprintf( + "version:\t%s\nlibtype:\t%s", + as.character(tiledb_version(compact = TRUE)), + ifelse(nzchar(libtype), yes = libtype, no = "unknown") + ), + con = tmpfile +) +target <- unname(tools::md5sum(tmpfile)) +hash <- .core_hash() +expect_inherits( + hash, + class = "character", + info = "'.core_hash()' returns a character value" +) +expect_length(hash, length = 1L, info = "'.core_hash()' returns a single value") +expect_null(names(hash), info = "'.core_hash()' returns an unnamed value") +expect_identical( + hash, + target = target, + info = "'.core_hash()' returns the correct hash value" +) diff --git a/man/dot-core_info.Rd b/man/dot-core_info.Rd new file mode 100644 index 0000000000..6201695581 --- /dev/null +++ b/man/dot-core_info.Rd @@ -0,0 +1,53 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/Version.R +\name{.core_info} +\alias{.core_info} +\alias{.core_hash} +\title{\code{libtiledb} Information} +\usage{ +.core_info() + +.core_hash() +} +\value{ +\code{.core_info()}: A named character vector with the following entries: +\itemize{ +\item \dQuote{\code{version}}: \code{libtiledb} version +\item \dQuote{\code{libtype}}: type of \code{libtiledb} install; will be one +of \dQuote{\code{vendored}}, \dQuote{\code{system}}, or \dQuote{\code{unknown}} +} + +\code{.core_hash()}: The \link[tools:md5sum]{MD5 hash} of the core info +} +\description{ +Get version and install information of the core \code{libtiledb} install +} +\section{Checking the \code{libtiledb} information in downstream packages}{ + +These functions are designed to make it easy to test if the core +\code{libtiledb} install has changed. This is accomplished by adding a +build-time constant to cache the version of \code{libtiledb} was built with. +For example, in \code{zzz.R}, put the following line to cache the +\code{libtiledb} information during package build +\preformatted{ +.built_with <- list(libtiledb = tiledb::.core_hash()) +} +Then, in the \link[base:ns-hooks]{load hook}, add the following check +\preformatted{ +.onLoad <- function(libname, pkgname) { + if (.built_with$libtiledb != tiledb::.core_hash()) { + warning("Core libtiledb has changed, please reinstall ", pkgname) + } +} +} +This will throw a warning if \pkg{tiledb}, and therefore \code{libtiledb}, +has changed between downstream package install and load +} + +\examples{ +.core_info() + +.core_hash() + +} +\keyword{internal} diff --git a/man/dot-pkg_config.Rd b/man/dot-pkg_config.Rd index 2d6b8e5753..a63bfd9722 100644 --- a/man/dot-pkg_config.Rd +++ b/man/dot-pkg_config.Rd @@ -19,7 +19,8 @@ A single string containing either the include directories or linking directories for \code{libtiledb} } \description{ -Compiler Arguments for Using \code{libtiledb} +Get compiler flags for using the core \code{libtiledb} install +used by \pkg{tiledb} } \examples{ .pkg_config() From a17a9f307347628fc911651beba872513a14b35f Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Mon, 18 Nov 2024 15:22:02 -0500 Subject: [PATCH 7/9] Rename `R/Init.R` to `R/zzz.R` --- R/{Init.R => zzz.R} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename R/{Init.R => zzz.R} (100%) diff --git a/R/Init.R b/R/zzz.R similarity index 100% rename from R/Init.R rename to R/zzz.R From 256c6fc381a1aca8ab2e17c613c31f08680fb104 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Mon, 18 Nov 2024 15:27:51 -0500 Subject: [PATCH 8/9] Add quoting for Windows --- R/Version.R | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/R/Version.R b/R/Version.R index ff7d5e1284..6734c57466 100644 --- a/R/Version.R +++ b/R/Version.R @@ -161,9 +161,9 @@ tiledb_version <- function(compact = FALSE) { # addition to libdir/include and pkgdir/include windows = sprintf( "-I%s/include -I%s/include -I%s/include/tiledb", - pkgdir, - lib, - lib + shQuote(pkgdir, type = "cmd"), + shQuote(lib, type = "cmd"), + shQuote(lib, type = "cmd") ), sprintf("-I%s/include -I%s/include", pkgdir, lib) ), @@ -176,8 +176,12 @@ tiledb_version <- function(compact = FALSE) { libs <- as.vector(vapply( c(pkgdir, lib), FUN = \(x) c( - sprintf("%s/lib/%s", x, arch), - ifelse(getRversion() > '4.2.0', sprintf("%s/lib/%s-ucrt", x, arch), "") + sprintf("%s/lib/%s", shQuote(x, type = "cmd"), arch), + ifelse( + test = getRversion() > '4.2.0', + yes = sprintf("%s/lib/%s-ucrt", shQuote(x, type = "cmd"), arch), + no = "" + ) ), FUN.VALUE = character(2L), USE.NAMES = FALSE From 2486b5662067a11e2f6af387a9190f0406a7d011 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Mon, 18 Nov 2024 15:33:23 -0500 Subject: [PATCH 9/9] Update changelog Bump develop version [ci skip] --- DESCRIPTION | 2 +- NEWS.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index d4e9281d83..3605d2a5a4 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: tiledb Type: Package -Version: 0.30.2.3 +Version: 0.30.2.4 Title: Modern Database Engine for Complex Data Based on Multi-Dimensional Arrays Authors@R: c( person("TileDB, Inc.", role = c("aut", "cph")), diff --git a/NEWS.md b/NEWS.md index c593759854..13e5ddef9f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,7 @@ * Support parentheses in query conditions * memory alloc: Accomodate zero buffer size estimate v2 * Apply `styler::style_pkg()` +* Expose include/linking flags for re-using `libtiledb` in downstream packages # tiledb 0.30.2