diff --git a/NEWS.md b/NEWS.md index 2f91ac66..5362cb71 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,6 +8,24 @@ loaded into the process, and failed with "Library not loaded: @rpath/libtbb.dylib" otherwise. (#209) +* Fixed `RcppParallel::tbbLibraryPath()` returning `NULL` on Windows. It looked + for the static archives `libtbb12.a` / `libtbb.a`, which are never installed + with the package: TBB is linked statically into `RcppParallel.dll` there, and + the only TBB library installed alongside it is the `tbb.dll` stub, which is + what it now finds. Previously it succeeded only when the package happened to + be running on the machine that built it against an Rtools TBB, and even then + answered with a path into Rtools -- a build dependency -- rather than with + anything the installed package uses. Note that `tbbLibraryPath("tbbmalloc")` + returns `NULL` on Windows, as no separate tbbmalloc library is installed + there. (#270) + +* `RcppParallel::tbbLibraryPath()` called with no arguments, and the internal + `tbbRoot()`, no longer report the Rtools directory recorded when the package + was configured. Unlike the named-library form, these return their answer + without checking that it exists, so for a pre-built binary (e.g. the CRAN + build) they named a directory on the machine that built the package. On + Windows both now report the package's own library directory. (#270) + * Fixed an issue where compiling code including `tbb/parallel_for_each.h` could fail with toolchains that accept `-std=c++20` but provide a pre-C++20 standard library, with errors of the form "no member named diff --git a/R/tbb.R b/R/tbb.R index cdf79c99..60ac7036 100644 --- a/R/tbb.R +++ b/R/tbb.R @@ -22,9 +22,13 @@ tbbLibraryPath <- function(name = NULL) { return(tbbRoot) # form library names + # + # on Windows the library we install is the 'tbb.dll' stub -- TBB itself is + # inside RcppParallel.dll -- so look for that first. the static archives are + # still tried afterwards, for a TBB_LIB pointed at an Rtools tree at runtime tbbLibNames <- list( "Darwin" = paste0("lib", name, ".dylib"), - "Windows" = paste0("lib", name, c("12", ""), ".a"), + "Windows" = c(paste0(name, ".dll"), paste0("lib", name, c("12", ""), ".a")), "SunOS" = paste0("lib", name, ".so"), "Linux" = paste0("lib", name, c(".so.2", ".so")) ) @@ -140,6 +144,15 @@ tbbLdFlags <- function() { tbbRoot <- function() { + # on Windows, always answer with our own library directory. TBB is linked + # statically into RcppParallel.dll there, and the only TBB library we + # install is the tbb.dll stub, so that directory is the whole story. TBB_LIB + # is worse than useless here: it records the Rtools tree of whichever + # machine ran configure, which for a pre-built binary is a path that need + # not exist on the user's machine at all (#270) + if (is_windows()) + return(archSystemFile("lib")) + if (nzchar(TBB_LIB)) return(TBB_LIB) diff --git a/R/zzz.R b/R/zzz.R index a263d089..98f941e7 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -21,8 +21,9 @@ loadTbbLibrary <- function(name) { if (is_windows()) { # NOTE: resolve against the package's own library directory, where - # install.libs.R places the stub; tbbRoot() would prefer TBB_LIB, - # which on Windows points at Rtools and holds only static libraries + # install.libs.R places the stub. this is what tbbRoot() reports on + # Windows too; spell it out here so that loading during .onLoad does + # not depend on the configured TBB_LIB in any way path <- archSystemFile("lib", paste0(name, ".dll")) if (!file.exists(path)) { verboseMessage("tbb library '%s' not found in package 'lib' folder; skipping", name) diff --git a/tests/test-tbb-library-path.R b/tests/test-tbb-library-path.R new file mode 100644 index 00000000..ec1a8156 --- /dev/null +++ b/tests/test-tbb-library-path.R @@ -0,0 +1,70 @@ +# Unit tests for tbbRoot() and tbbLibraryPath(), which resolve the TBB library +# that the *installed* package uses. +# +# Regression tests for #270: on Windows these consulted TBB_LIB, the library +# directory recorded when configure ran. For a package built from source that +# happens to be a real path, but for a pre-built binary it names the Rtools +# tree of the build machine -- so tbbRoot() reported a directory that need not +# exist on the user's machine (a 'D:/rtools45/...' path, in the report), and +# tbbLibraryPath() then found nothing and returned NULL. +# +# The invariant these tests pin down is that both functions describe the +# installation actually in use: whatever they return must exist. + +RcppParallel:::test_init() + +# tbbRoot() must never report a directory that isn't there. system.file() +# returns "" when it cannot resolve a path, so that sentinel is allowed; any +# non-empty answer has to be a real directory. +root <- tbbRoot() +assert(is.character(root)) +assert(length(root) == 1L) +if (nzchar(root)) + assert(dir.exists(root)) + +# on Windows the answer is always the package's own library directory, since +# TBB is statically linked into RcppParallel.dll and the tbb.dll stub is the +# only TBB library installed. in particular it must not depend on TBB_LIB. +if (is_windows()) { + + assert(identical(root, archSystemFile("lib"))) + + saved <- Sys.getenv("TBB_LIB", unset = NA) + Sys.setenv(TBB_LIB = "Z:/nonexistent/rtools/lib") + assert(identical(tbbRoot(), archSystemFile("lib"))) + if (is.na(saved)) Sys.unsetenv("TBB_LIB") else Sys.setenv(TBB_LIB = saved) + +} + +# tbbLibraryPath(NULL) reports the directory, and agrees with tbbRoot() when +# TBB_LIB is not set in the environment +if (is.na(Sys.getenv("TBB_LIB", unset = NA))) + assert(identical(tbbLibraryPath(), root)) + +# a named library either resolves to a file that exists, or isn't found at all. +# it must never name a file that isn't there -- that was the shape of #270. +for (name in c("tbb", "tbbmalloc", "tbbmalloc_proxy")) { + + path <- tbbLibraryPath(name) + if (is.null(path)) + next + + assert(is.character(path)) + assert(length(path) == 1L) + assert(file.exists(path)) + +} + +# the user-visible symptom from #270: tbbLibraryPath("tbb") returned nothing +# from a perfectly good installation. assert this only on Windows, where the +# installed layout is known exactly -- install.libs.R always builds the +# tbb.dll stub when TBB is enabled, and fails the install if it cannot. off +# Windows the library may legitimately be unresolvable, e.g. a system TBB +# whose distro package ships no unversioned 'libtbb.so' development symlink. +if (is_windows() && TBB_ENABLED) { + tbb <- tbbLibraryPath("tbb") + assert(!is.null(tbb)) + assert(file.exists(tbb)) +} + +writeLines("all tbbLibraryPath() tests passed")