diff --git a/NEWS.md b/NEWS.md index 4d7f7e89..0177b087 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,13 @@ # RcppParallel (development version) +* On macOS, `RcppParallel::LdFlags()` now also emits an `-rpath` entry for the + directory containing the TBB libraries. The libraries record an + `@rpath`-relative install name, so packages linking against them previously + produced binaries with no runtime search path for TBB; those binaries could + only be loaded when RcppParallel (and hence TBB) already happened to be + loaded into the process, and failed with "Library not loaded: + @rpath/libtbb.dylib" otherwise. (#209) + * 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 152fd163..1c21c477 100644 --- a/R/tbb.R +++ b/R/tbb.R @@ -108,8 +108,15 @@ tbbLdFlags <- function() { # explicitly link on macOS # https://github.com/RcppCore/RcppParallel/issues/206 + # + # the bundled libraries record an '@rpath'-relative install name (e.g. + # '@rpath/libtbb.dylib'), so '-L' alone is not enough: the client library + # ends up with no runtime search path for TBB at all, and only loads + # because RcppParallel -- and hence TBB -- normally happens to be loaded + # into the process first. Emit a matching '-rpath' so the client can + # resolve TBB on its own. (#209) if (is_mac()) { - fmt <- "-L%s -l%s -l%s" + fmt <- "-L%1$s -Wl,-rpath,%1$s -l%2$s -l%3$s" return(sprintf(fmt, asBuildPath(tbbLibraryPath()), TBB_NAME, TBB_MALLOC_NAME)) } diff --git a/tests/test-downstream-load.R b/tests/test-downstream-load.R new file mode 100644 index 00000000..06fe71ce --- /dev/null +++ b/tests/test-downstream-load.R @@ -0,0 +1,140 @@ + +# Check that a package linking against RcppParallel's TBB can be loaded on its +# own, in a process that has not already loaded RcppParallel. +# +# The TBB libraries we ship on macOS record an '@rpath'-relative install name, +# so LdFlags() must emit an '-rpath' alongside its '-L'. With only the '-L' the +# link still succeeds -- so the package builds, and loads fine via its +# NAMESPACE, whose importFrom(RcppParallel, ...) pulls TBB into the process +# first -- but the shared object itself carries no runtime search path, and +# dyld fails with "Library not loaded: @rpath/libtbb.dylib" for anything that +# loads it directly. Hence the load below runs in a fresh R process which never +# touches RcppParallel; loading it from here would prove nothing. +# +# See also tests/test-ld-flags.R, the cheap flag-level check that still runs +# where no toolchain is available. +# +# https://github.com/RcppCore/RcppParallel/issues/209 + +RcppParallel:::test_init() + +# building the test package requires a toolchain, so keep this test +# off of CRAN's machines +ci <- nzchar(Sys.getenv("CI")) || identical(Sys.getenv("NOT_CRAN"), "true") +if (!ci) { + writeLines("Not running on CI; skipping downstream load test.") + quit(save = "no") +} + +if (!TBB_ENABLED) { + writeLines("TBB is not enabled; skipping downstream load test.") + quit(save = "no") +} + +# Windows has no rpath: the loader resolves DLLs through the process search +# path, and downstream packages link '-lRcppParallel' rather than TBB itself +if (is_windows()) { + writeLines("Windows resolves DLLs via the search path; skipping.") + quit(save = "no") +} + +# A standalone load can only work where TBB is linked into the client +# explicitly: macOS (#206), and any platform configured against a system TBB. +# Elsewhere the TBB symbols are deliberately left undefined at link time and +# resolved from the libraries RcppParallel loads, so a standalone load is +# expected to fail. Note this mirrors the branches in tbbLdFlags() rather than +# inspecting its output: gating on the '-rpath' we are checking for would skip +# precisely the configuration that regressed. +tbbLib <- Sys.getenv("TBB_LINK_LIB", Sys.getenv("TBB_LIB", unset = TBB_LIB)) +if (!is_mac() && !nzchar(tbbLib)) { + writeLines("TBB is not linked into downstream packages here; skipping.") + quit(save = "no") +} + +# generate the test package +pkgRoot <- file.path(tempdir(), "downstreamtest") +dir.create(file.path(pkgRoot, "src"), recursive = TRUE, showWarnings = FALSE) + +writeLines(con = file.path(pkgRoot, "DESCRIPTION"), c( + "Package: downstreamtest", + "Type: Package", + "Title: Test Loading a Package Linked Against RcppParallel's TBB", + "Version: 0.1.0", + "Author: RcppParallel Authors", + "Maintainer: RcppParallel Authors ", + "Description: Confirms that such packages can be loaded standalone.", + "License: GPL-2", + "Imports: RcppParallel" +)) + +writeLines(con = file.path(pkgRoot, "NAMESPACE"), c( + "useDynLib(downstreamtest)", + "importFrom(RcppParallel, RcppParallelLibs)" +)) + +writeLines(con = file.path(pkgRoot, "src", "Makevars"), c( + 'PKG_CXXFLAGS = $(shell "${R_HOME}/bin/Rscript" -e "RcppParallel::CxxFlags()")', + 'PKG_LIBS = $(shell "${R_HOME}/bin/Rscript" -e "RcppParallel::RcppParallelLibs()")' +)) + +# call into TBB proper, so that the shared object records a real load-time +# dependency on the TBB library rather than merely being linked against it +writeLines(con = file.path(pkgRoot, "src", "concurrency.cpp"), c( + "#include ", + "", + "#include ", + "#include ", + "", + 'extern "C" SEXP downstream_max_concurrency(void)', + "{", + " return Rf_ScalarInteger(tbb::this_task_arena::max_concurrency());", + "}" +)) + +# install it, making sure child processes resolve this RcppParallel +libDir <- file.path(tempdir(), "library") +dir.create(libDir, recursive = TRUE, showWarnings = FALSE) +Sys.setenv(R_LIBS = paste(.libPaths(), collapse = .Platform$path.sep)) + +rExe <- file.path(R.home("bin"), "R") +args <- c("CMD", "INSTALL", "--no-multiarch", paste0("--library=", shQuote(libDir)), shQuote(pkgRoot)) + +output <- suppressWarnings(system2(rExe, args, stdout = TRUE, stderr = TRUE)) +writeLines(output) + +status <- attr(output, "status") +if (is.numeric(status) && status != 0L) + stop("error installing test package (status code ", status, ")") + +# locate the shared object that was just built +pattern <- paste0("^downstreamtest\\", .Platform$dynlib.ext, "$") +shlib <- list.files( + file.path(libDir, "downstreamtest", "libs"), + pattern = pattern, + recursive = TRUE, + full.names = TRUE +) + +assert(length(shlib) == 1L) + +# load it from a process which has never loaded RcppParallel; deparse() is used +# to embed the path as a properly-quoted R string literal +script <- file.path(tempdir(), "downstream-load.R") +writeLines(con = script, c( + 'if ("RcppParallel" %in% loadedNamespaces())', + ' stop("RcppParallel is loaded; this check would prove nothing")', + "", + sprintf("dll <- dyn.load(%s)", deparse(shlib)), + 'value <- .Call(getNativeSymbolInfo("downstream_max_concurrency", dll))', + 'cat("max concurrency:", value, "\\n")', + "", + 'if (!is.integer(value) || is.na(value) || value < 1L)', + ' stop("unexpected concurrency: ", value)' +)) + +rscript <- file.path(R.home("bin"), "Rscript") +output <- suppressWarnings(system2(rscript, c("--vanilla", shQuote(script)), stdout = TRUE, stderr = TRUE)) +writeLines(output) + +status <- attr(output, "status") +assert(is.null(status) || status == 0L) diff --git a/tests/test-ld-flags.R b/tests/test-ld-flags.R new file mode 100644 index 00000000..08d666f8 --- /dev/null +++ b/tests/test-ld-flags.R @@ -0,0 +1,38 @@ +# Unit tests for tbbLdFlags(), the linker flags handed to downstream packages +# via RcppParallel::LdFlags(). +# +# On macOS, the TBB libraries RcppParallel ships record an '@rpath'-relative +# install name (e.g. '@rpath/libtbb.dylib'), so a '-L' search path alone only +# satisfies the linker: the resulting binary carries no runtime search path for +# TBB, and dyld can resolve it only when RcppParallel has already pulled TBB +# into the process. Every '-L' we emit must therefore be paired with a matching +# '-rpath'. (#209) +# +# This is only a check on the shape of the flags, so that something still runs +# where no toolchain is available (e.g. CRAN). tests/test-downstream-load.R +# covers the behaviour itself: it builds a package against these flags and +# loads it in a process that never loads RcppParallel. + +RcppParallel:::test_init() + +flags <- tbbLdFlags() + +assert(is.character(flags)) +assert(length(flags) == 1L) +assert(!is.na(flags)) + +if (is_mac()) { + + parts <- strsplit(flags, "\\s+")[[1L]] + + libPaths <- sub("^-L", "", grep("^-L", parts, value = TRUE)) + rpaths <- sub("^-Wl,-rpath,", "", grep("^-Wl,-rpath,", parts, value = TRUE)) + + # we always link TBB explicitly on macOS, so there is a search path to check + assert(length(libPaths) > 0L) + assert(all(nzchar(libPaths))) + + # and each one must also be searched at load time + assert(all(libPaths %in% rpaths)) + +}