Skip to content

Commit 836b3f9

Browse files
authored
Preload Windows runtime DLL dependencies (t-kalinowski#115)
* Preload Windows runtime DLL dependencies * Preserve PATH order for Windows DLL preload Finding: Preserve PATH order when preloading DLLs quickr_windows_add_dll_paths() now returns the final post-update PATH directory order instead of synthetic candidate order. Added a regression test that keeps two candidate directories in reversed candidate order on PATH and asserts the returned preload order follows PATH precedence. * Limit Windows DLL preload to candidate directories Finding: Avoid preloading unrelated PATH DLLs quickr_windows_add_dll_paths() now preserves post-update PATH order only among quickr candidate runtime/link directories, instead of returning every directory on PATH. Added a regression test proving unrelated PATH directories with matching DLL basenames are excluded from preload search. * Restore Windows runtime path discovery * Limit grandparent Windows bin discovery Finding: Limit grandparent bin lookup to arch library dirs Restrict the ../../bin probe to -L directories shaped like .../lib/x64 or .../lib/i386, while keeping the regular ../bin probe for ordinary library paths. * set `TESTTHAT_CPUS` on CI
1 parent 773a2bc commit 836b3f9

3 files changed

Lines changed: 508 additions & 28 deletions

File tree

.github/workflows/R-CMD-check.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ jobs:
2929
env:
3030
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
3131
R_KEEP_PKG_SOURCE: yes
32+
TESTTHAT_CPUS: 5
3233

3334
steps:
3435
- uses: actions/checkout@v4

R/quick.R

Lines changed: 95 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,8 @@ compile <- function(fsub, build_dir = tempfile(paste0(fsub@name, "-build-"))) {
296296
stop("Compilation Error", call. = FALSE)
297297
}
298298

299-
quickr_windows_add_dll_paths(link_flags)
299+
dll_dirs <- quickr_windows_add_dll_paths(link_flags)
300+
quickr_windows_load_dll_dependencies(dll_dirs)
300301

301302
# tryCatch(dyn.unload(dll_path), error = identity)
302303
dll <- tryCatch(
@@ -330,35 +331,48 @@ quickr_windows_add_dll_paths <- function(
330331
which = Sys.which
331332
) {
332333
if (!identical(os_type, "windows")) {
333-
return(invisible(FALSE))
334+
return(invisible(character()))
334335
}
336+
337+
config_path <- function(value) {
338+
value <- trimws(value)
339+
if (!nzchar(value)) {
340+
return("")
341+
}
342+
value <- sub("^\"([^\"]+)\".*", "\\1", value)
343+
value <- sub("^'([^']+)'.*", "\\1", value)
344+
strsplit(value, "\\s+")[[1L]][[1L]]
345+
}
346+
335347
dirs <- flags[grepl("^-L", flags)]
336348
dirs <- sub("^-L", "", dirs)
349+
dirs <- vapply(dirs, config_path, character(1))
337350
dirs <- dirs[nzchar(dirs)]
338351

339-
bin_siblings <- file.path(dirs, "..", "bin")
352+
arch_lib_dirs <- dirs[
353+
tolower(basename(dirs)) %in%
354+
c("x64", "i386") &
355+
tolower(basename(dirname(dirs))) == "lib"
356+
]
357+
bin_siblings <- c(
358+
file.path(dirs, "..", "bin"),
359+
file.path(arch_lib_dirs, "..", "..", "bin")
360+
)
340361

362+
config_binpref <- config_path(config_value("BINPREF"))
363+
if (nzchar(config_binpref) && !dir.exists(config_binpref)) {
364+
config_binpref <- dirname(config_binpref)
365+
}
341366
config_values <- c(
342-
config_value("BINPREF"),
343367
config_value("FC"),
344368
config_value("F77"),
345369
config_value("CC"),
346370
config_value("CXX")
347371
)
348-
config_paths <- vapply(
349-
config_values,
350-
function(value) {
351-
value <- trimws(value)
352-
if (!nzchar(value)) {
353-
return("")
354-
}
355-
value <- sub("^\"([^\"]+)\".*", "\\1", value)
356-
value <- sub("^'([^']+)'.*", "\\1", value)
357-
strsplit(value, "\\s+")[[1L]][[1L]]
358-
},
359-
character(1)
360-
)
361-
config_bins <- unique(dirname(config_paths[nzchar(config_paths)]))
372+
config_paths <- vapply(config_values, config_path, character(1))
373+
config_bins <- dirname(config_paths[nzchar(config_paths)])
374+
config_bins <- config_bins[nzchar(config_bins) & config_bins != "."]
375+
config_bins <- unique(c(config_binpref, config_bins))
362376

363377
r_bin <- R.home("bin")
364378
r_bin_x64 <- file.path(r_bin, "x64")
@@ -373,6 +387,16 @@ quickr_windows_add_dll_paths <- function(
373387
"RTOOLS_HOME"
374388
))
375389
rtools_roots <- rtools_roots[nzchar(rtools_roots)]
390+
link_dirs <- gsub("\\", "/", dirs, fixed = TRUE)
391+
rtools_link_roots <- sub(
392+
"/(?:x86_64|aarch64)-w64-mingw32(?:\\.static(?:\\.posix)?)?/.*$",
393+
"",
394+
link_dirs
395+
)
396+
rtools_link_roots <- rtools_link_roots[
397+
nzchar(rtools_link_roots) & rtools_link_roots != link_dirs
398+
]
399+
rtools_roots <- unique(c(rtools_roots, rtools_link_roots))
376400
rtools_bins <- unique(c(
377401
file.path(rtools_roots, "usr", "bin"),
378402
file.path(rtools_roots, "mingw64", "bin"),
@@ -399,8 +423,9 @@ quickr_windows_add_dll_paths <- function(
399423
dirs <- dirs[nzchar(dirs)]
400424
dirs <- dirs[dir.exists(dirs)]
401425
if (!length(dirs)) {
402-
return(invisible(FALSE))
426+
return(invisible(character()))
403427
}
428+
dirs <- normalizePath(dirs, winslash = "\\", mustWork = FALSE)
404429

405430
path <- Sys.getenv("PATH", unset = "")
406431
existing <- strsplit(path, ";", fixed = TRUE)[[1]]
@@ -410,14 +435,61 @@ quickr_windows_add_dll_paths <- function(
410435
winslash = "\\",
411436
mustWork = FALSE
412437
))
413-
dirs_norm <- tolower(normalizePath(dirs, winslash = "\\", mustWork = FALSE))
438+
dirs_norm <- tolower(dirs)
414439
to_add <- dirs[!dirs_norm %in% existing_norm]
440+
path_entries <- existing
415441
if (length(to_add)) {
416-
Sys.setenv(PATH = paste(c(to_add, existing), collapse = ";"))
417-
return(invisible(TRUE))
442+
path_entries <- c(to_add, existing)
443+
Sys.setenv(PATH = paste(path_entries, collapse = ";"))
444+
}
445+
path_entries <- path_entries[nzchar(path_entries)]
446+
path_entries <- path_entries[dir.exists(path_entries)]
447+
path_entries <- normalizePath(
448+
path_entries,
449+
winslash = "\\",
450+
mustWork = FALSE
451+
)
452+
path_entries <- path_entries[tolower(path_entries) %in% dirs_norm]
453+
454+
invisible(path_entries)
455+
}
456+
457+
458+
quickr_windows_load_dll_dependencies <- function(
459+
dirs,
460+
os_type = .Platform$OS.type,
461+
dyn_load = base::dyn.load
462+
) {
463+
if (!identical(os_type, "windows")) {
464+
return(invisible(character()))
465+
}
466+
stopifnot(is.character(dirs), is.function(dyn_load))
467+
468+
patterns <- c(
469+
"libgcc_s*.dll",
470+
"libwinpthread*.dll",
471+
"libquadmath*.dll",
472+
"libgfortran*.dll",
473+
"libopenblas*.dll",
474+
"Rblas.dll",
475+
"Rlapack.dll"
476+
)
477+
dlls <- unlist(
478+
lapply(patterns, \(pattern) Sys.glob(file.path(dirs, pattern))),
479+
use.names = FALSE
480+
)
481+
dlls <- dlls[file.exists(dlls)]
482+
if (!length(dlls)) {
483+
return(invisible(character()))
484+
}
485+
486+
dlls <- normalizePath(dlls, winslash = "\\", mustWork = FALSE)
487+
dlls <- dlls[!duplicated(tolower(basename(dlls)))]
488+
for (dll in dlls) {
489+
dyn_load(dll)
418490
}
419491

420-
invisible(FALSE)
492+
invisible(dlls)
421493
}
422494

423495

0 commit comments

Comments
 (0)