Skip to content

Commit bb93142

Browse files
committed
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
1 parent ffabb43 commit bb93142

File tree

4 files changed

+129
-25
lines changed

4 files changed

+129
-25
lines changed

.gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ TileDB-R.Rproj
1111
src/*.o
1212
src/*.so
1313
src/*.dll
14+
src/connection/*.o
1415
# Output files from R CMD build
1516
/*.tar.gz
1617
# Output files from R CMD check
@@ -49,3 +50,5 @@ vignettes/md/
4950
# vim
5051
.sw?
5152
.*.sw?
53+
inst/tiledb/
54+
inst/lib/

DESCRIPTION

+20-5
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,30 @@ SystemRequirements: A C++17 compiler is required; on macOS compilation version 1
2424
build selected); on x86_64 and M1 platforms pre-built TileDB Embedded libraries
2525
are available at GitHub and are used if no TileDB installation is detected, and
2626
no other option to build or download was specified by the user.
27-
Imports:
27+
Depends:
28+
R (>= 4.2)
29+
Imports:
2830
methods,
29-
Rcpp (>= 1.0.8),
31+
nanoarrow,
3032
nanotime,
33+
Rcpp (>= 1.0.8),
3134
spdl,
32-
nanoarrow,
3335
tools
34-
LinkingTo: Rcpp, RcppInt64, nanoarrow
35-
Suggests: tinytest, simplermarkdown, curl, bit64, Matrix, palmerpenguins, nycflights13, data.table, tibble, arrow
36+
LinkingTo:
37+
nanoarrow,
38+
Rcpp,
39+
RcppInt64
40+
Suggests:
41+
arrow,
42+
bit64,
43+
curl,
44+
data.table,
45+
Matrix,
46+
nycflights13,
47+
palmerpenguins,
48+
simplermarkdown,
49+
tibble,
50+
tinytest
3651
VignetteBuilder: simplermarkdown
3752
Roxygen: list(markdown = TRUE)
3853
RoxygenNote: 7.3.2

R/Version.R

+103-19
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,20 @@ tiledb_version <- function(compact = FALSE) {
159159
EXPR = .Platform$OS.type,
160160
# Adapted from Makevars.win, which includes libdir/include/tiledb in
161161
# addition to libdir/include and pkgdir/include
162-
windows = sprintf(
163-
"-I%s/include -I%s/include -I%s/include/tiledb",
164-
shQuote(pkgdir, type = "cmd"),
165-
shQuote(lib, type = "cmd"),
166-
shQuote(lib, type = "cmd")
167-
),
162+
windows = {
163+
include <- file.path(
164+
c(pkgdir, lib, lib),
165+
c("include", "include", "include/tiledb")
166+
)
167+
# Windows likes spaces, but Make does not
168+
if (any(idx <- grepl("[[:space:]]", include))) {
169+
include[idx] <- shQuote(include[idx], type = "cmd")
170+
}
171+
paste(
172+
paste0("-I", include, collapse = " "),
173+
"-DTILEDB_STATIC_DEFINE -DTILEDB_SILENT_BUILD"
174+
)
175+
},
168176
sprintf("-I%s/include -I%s/include", pkgdir, lib)
169177
),
170178
PKG_CXX_LIBS = switch(
@@ -173,20 +181,96 @@ tiledb_version <- function(compact = FALSE) {
173181
# Unix-alikes; R 4.2 and higher require ucrt
174182
windows = {
175183
arch <- .Platform$r_arch
176-
libs <- as.vector(vapply(
177-
c(pkgdir, lib),
178-
FUN = \(x) c(
179-
sprintf("%s/lib/%s", shQuote(x, type = "cmd"), arch),
180-
ifelse(
181-
test = getRversion() > '4.2.0',
182-
yes = sprintf("%s/lib/%s-ucrt", shQuote(x, type = "cmd"), arch),
183-
no = ""
184-
)
184+
libs <- c(
185+
connection = sprintf("%s/lib/%s", pkgdir, arch),
186+
libtiledb = sprintf("%s/lib/%s-ucrt", lib, arch)
187+
)
188+
libs <- Filter(dir.exists, libs)
189+
# Windows requires additional linking flags, so we need flags for
190+
# rwinlib-tiledb DLLs and Windows system DLLs; these have to be in
191+
# a specific order so filter the rwinlib-tiledb DLLs to the ones
192+
# we're using and interleave them with the Windows system DLLs
193+
winlibs <- list(
194+
c("Secur32", "Crypt32"),
195+
"NCrypt",
196+
c(
197+
"BCrypt",
198+
"Kernel32",
199+
"Rpcrt4",
200+
"Wininet",
201+
"Winhttp",
202+
"Ws2_32",
203+
"Shlwapi",
204+
"Userenv",
205+
"version",
206+
"ws2_32"
207+
)
208+
)
209+
tiledblibs <- list(
210+
c(
211+
"tiledbstatic",
212+
"bz2",
213+
"zstd",
214+
"lz4",
215+
"z",
216+
"spdlog",
217+
"fmt",
218+
"aws-cpp-sdk-identity-management",
219+
"aws-cpp-sdk-cognito-identity",
220+
"aws-cpp-sdk-sts",
221+
"aws-cpp-sdk-s3",
222+
"aws-cpp-sdk-core",
223+
"libmagic",
224+
"webp",
225+
"pcre2-posix",
226+
"pcre2-8",
227+
"aws-crt-cpp",
228+
"aws-c-mqtt",
229+
"aws-c-event-stream",
230+
"aws-c-s3",
231+
"aws-c-auth",
232+
"aws-c-http",
233+
"aws-c-io"
234+
),
235+
c(
236+
"aws-c-compression",
237+
"aws-c-cal"
238+
),
239+
c(
240+
"aws-c-sdkutils",
241+
"aws-checksums",
242+
"aws-c-common"
185243
),
186-
FUN.VALUE = character(2L),
187-
USE.NAMES = FALSE
188-
))
189-
paste('-ltiledb', paste0('-L', Filter(dir.exists, libs), collapse = ' '))
244+
"sharpyuv"
245+
)
246+
flags <- if (!is.null(libs["libtiledb"])) {
247+
dlls <- sub(
248+
pattern = "^lib",
249+
replacement = "",
250+
tools::file_path_sans_ext(list.files(libs["libtiledb"]))
251+
)
252+
for (i in seq_along(tiledblibs)) {
253+
tiledblibs[[i]] <- intersect(tiledblibs[[i]], dlls)
254+
if (i > length(winlibs)) {
255+
next
256+
}
257+
tiledblibs[[i]] <- if (length(tiledblibs[[i]])) {
258+
c(tiledblibs[[i]], winlibs[[i]])
259+
} else {
260+
winlibs[[i]]
261+
}
262+
}
263+
paste0("-l", unlist(tiledblibs), collapse = " ")
264+
} else {
265+
""
266+
}
267+
# Windows likes spaces, but Make does not
268+
# This has to come after the flags bit as R does not like quotes
269+
# in directory names
270+
if (any(idx <- grepl("[[:space:]]", libs))) {
271+
libs[idx] <- shQuote(libs[idx], type = "cmd")
272+
}
273+
paste("-ltiledb", paste0("-L", libs, collapse = " "), flags)
190274
},
191275
sprintf("-ltiledb -L%s/lib -L%s/lib", pkgdir, lib)
192276
)

TileDB-R.Rproj

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ Version: 1.0
22

33
RestoreWorkspace: No
44
SaveWorkspace: No
5-
AlwaysSaveHistory: Yes
5+
AlwaysSaveHistory: Default
66

77
EnableCodeIndexing: Yes
88
UseSpacesForTab: Yes
@@ -19,3 +19,5 @@ BuildType: Package
1919
PackageUseDevtools: Yes
2020
PackageInstallArgs: --no-multiarch --with-keep.source --install-tests
2121
PackageRoxygenize: rd,namespace
22+
23+
QuitChildProcessesOnExit: Yes

0 commit comments

Comments
 (0)