From 74ae1af3f1bbf1cfc9e52dcf3aabf05688ba8601 Mon Sep 17 00:00:00 2001 From: Paul Hoffman Date: Tue, 10 Dec 2024 11:07:31 -0500 Subject: [PATCH] Fix package config on Windows Fix issues with `.pkg_config()` on Windows; include new include and linking flags as suggested by @LTLA [SC-60076](https://app.shortcut.com/tiledb-inc/story/60076) resolves #780 --- .gitignore | 2 + DESCRIPTION | 23 ++++++++-- R/Version.R | 122 +++++++++++++++++++++++++++++++++++++++++-------- TileDB-R.Rproj | 4 +- 4 files changed, 127 insertions(+), 24 deletions(-) diff --git a/.gitignore b/.gitignore index 2d58d6fa36..0828282ccf 100644 --- a/.gitignore +++ b/.gitignore @@ -54,3 +54,5 @@ vignettes/md/ # vim .sw? .*.sw? +inst/tiledb/ +inst/lib/ diff --git a/DESCRIPTION b/DESCRIPTION index ab3bd115a0..4e6da37dff 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -23,15 +23,30 @@ 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. +Depends: + R (>= 4.2) Imports: methods, - Rcpp (>= 1.0.8), + nanoarrow, nanotime, + Rcpp (>= 1.0.8), spdl, - nanoarrow, tools -LinkingTo: Rcpp, RcppInt64, nanoarrow -Suggests: tinytest, simplermarkdown, curl, bit64, Matrix, palmerpenguins, nycflights13, data.table, tibble, arrow +LinkingTo: + nanoarrow, + Rcpp, + RcppInt64 +Suggests: + arrow, + bit64, + curl, + data.table, + Matrix, + nycflights13, + palmerpenguins, + simplermarkdown, + tibble, + tinytest VignetteBuilder: simplermarkdown Roxygen: list(markdown = TRUE) RoxygenNote: 7.3.2 diff --git a/R/Version.R b/R/Version.R index 6734c57466..54b710c318 100644 --- a/R/Version.R +++ b/R/Version.R @@ -159,12 +159,20 @@ tiledb_version <- function(compact = FALSE) { 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", - shQuote(pkgdir, type = "cmd"), - shQuote(lib, type = "cmd"), - shQuote(lib, type = "cmd") - ), + windows = { + include <- file.path( + c(pkgdir, lib, lib), + c("include", "include", "include/tiledb") + ) + # Windows likes spaces, but Make does not + if (any(idx <- grepl("[[:space:]]", include))) { + include[idx] <- shQuote(include[idx], type = "cmd") + } + paste( + paste0("-I", include, collapse = " "), + "-DTILEDB_STATIC_DEFINE -DTILEDB_SILENT_BUILD" + ) + }, sprintf("-I%s/include -I%s/include", pkgdir, lib) ), PKG_CXX_LIBS = switch( @@ -173,20 +181,96 @@ tiledb_version <- function(compact = FALSE) { # 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", shQuote(x, type = "cmd"), arch), - ifelse( - test = getRversion() > '4.2.0', - yes = sprintf("%s/lib/%s-ucrt", shQuote(x, type = "cmd"), arch), - no = "" - ) + libs <- c( + connection = sprintf("%s/lib/%s", pkgdir, arch), + libtiledb = sprintf("%s/lib/%s-ucrt", lib, arch) + ) + libs <- Filter(dir.exists, libs) + # Windows requires additional linking flags, so we need flags for + # rwinlib-tiledb DLLs and Windows system DLLs; these have to be in + # a specific order so filter the rwinlib-tiledb DLLs to the ones + # we're using and interleave them with the Windows system DLLs + winlibs <- list( + c("Secur32", "Crypt32"), + "NCrypt", + c( + "BCrypt", + "Kernel32", + "Rpcrt4", + "Wininet", + "Winhttp", + "Ws2_32", + "Shlwapi", + "Userenv", + "version", + "ws2_32" + ) + ) + tiledblibs <- list( + c( + "tiledbstatic", + "bz2", + "zstd", + "lz4", + "z", + "spdlog", + "fmt", + "aws-cpp-sdk-identity-management", + "aws-cpp-sdk-cognito-identity", + "aws-cpp-sdk-sts", + "aws-cpp-sdk-s3", + "aws-cpp-sdk-core", + "libmagic", + "webp", + "pcre2-posix", + "pcre2-8", + "aws-crt-cpp", + "aws-c-mqtt", + "aws-c-event-stream", + "aws-c-s3", + "aws-c-auth", + "aws-c-http", + "aws-c-io" + ), + c( + "aws-c-compression", + "aws-c-cal" + ), + c( + "aws-c-sdkutils", + "aws-checksums", + "aws-c-common" ), - FUN.VALUE = character(2L), - USE.NAMES = FALSE - )) - paste('-ltiledb', paste0('-L', Filter(dir.exists, libs), collapse = ' ')) + "sharpyuv" + ) + flags <- if (!is.null(libs["libtiledb"])) { + dlls <- sub( + pattern = "^lib", + replacement = "", + tools::file_path_sans_ext(list.files(libs["libtiledb"])) + ) + for (i in seq_along(tiledblibs)) { + tiledblibs[[i]] <- intersect(tiledblibs[[i]], dlls) + if (i > length(winlibs)) { + next + } + tiledblibs[[i]] <- if (length(tiledblibs[[i]])) { + c(tiledblibs[[i]], winlibs[[i]]) + } else { + winlibs[[i]] + } + } + paste0("-l", unlist(tiledblibs), collapse = " ") + } else { + "" + } + # Windows likes spaces, but Make does not + # This has to come after the flags bit as R does not like quotes + # in directory names + if (any(idx <- grepl("[[:space:]]", libs))) { + libs[idx] <- shQuote(libs[idx], type = "cmd") + } + paste("-ltiledb", paste0("-L", libs, collapse = " "), flags) }, sprintf("-ltiledb -L%s/lib -L%s/lib", pkgdir, lib) ) diff --git a/TileDB-R.Rproj b/TileDB-R.Rproj index 9fbba2861f..7b317a9de9 100644 --- a/TileDB-R.Rproj +++ b/TileDB-R.Rproj @@ -2,7 +2,7 @@ Version: 1.0 RestoreWorkspace: No SaveWorkspace: No -AlwaysSaveHistory: Yes +AlwaysSaveHistory: Default EnableCodeIndexing: Yes UseSpacesForTab: Yes @@ -19,3 +19,5 @@ BuildType: Package PackageUseDevtools: Yes PackageInstallArgs: --no-multiarch --with-keep.source --install-tests PackageRoxygenize: rd,namespace + +QuitChildProcessesOnExit: Yes