resolve the tbb library path against the installed package on windows

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

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)
 

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)

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")