resolve the tbb library path against the installed package on windows

tbbLibraryPath() returned NULL on Windows when called from a pre-built
binary, and tbbRoot() reported a directory that need not exist on the
machine running it (#270).

Both consulted TBB_LIB, recorded when configure ran. On Windows that names
the Rtools tree of the build machine -- 'D:/rtools45/...' in the report --
which says nothing about the installed package. Answer with our own library
directory instead: TBB is linked statically into RcppParallel.dll there, so
the tbb.dll stub installed beside it is the only TBB library there is. This
is already how tbbLdFlags() and loadTbbLibrary() behave; tbbRoot() was the
last place still trusting the configure-time value.

The Windows candidate names were also only static archives ('libtbb12.a',
'libtbb.a'), which are never installed into the package -- so fixing
tbbRoot() alone would still have found nothing. Look for the stub first,
keeping the archives for a TBB_LIB pointed at an Rtools tree at runtime.

Simulating a Windows install shows tbbLibraryPath("tbb") previously returned
NULL for every configuration, including a bundled-oneTBB build, not just the
binary case in the report. tbbLibraryPath("tbbmalloc") now returns NULL on
Windows, which is honest: no separate tbbmalloc library is installed there.

Author: kevinushey

NEWS.md

@@ -1,5 +1,17 @@
 # RcppParallel (development version)
 
+* Fixed an issue on Windows where `RcppParallel::tbbLibraryPath()` returned
+  `NULL`, and the internal `tbbRoot()` a non-existent directory, when called
+  from a pre-built binary (e.g. the CRAN build). Both resolved against the
+  Rtools directory recorded when the package was configured, which is a path
+  on the machine that built it rather than on the machine running it. On
+  Windows they now always report the package's own library directory, which
+  is where the TBB library it actually uses is installed; note that this is
+  the `tbb.dll` stub, since TBB itself is linked statically into
+  `RcppParallel.dll`. As a result, `tbbLibraryPath("tbbmalloc")` returns
+  `NULL` on Windows, as no separate tbbmalloc library is installed there
+  (#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"))
    )
@@ -133,6 +137,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")