fix(patches): force RcppParallel to use its bundled oneTBB (#146)
Some checks failed
ci/crow/cron/process-updates/16 Pipeline failed
ci/crow/cron/process-updates/11 Pipeline failed
ci/crow/cron/process-updates/18 Pipeline failed
ci/crow/cron/process-updates/17 Pipeline failed
ci/crow/cron/process-updates/12 Pipeline failed
ci/crow/cron/process-updates/6 Pipeline failed
ci/crow/cron/process-updates/5 Pipeline was successful
Some checks failed
ci/crow/cron/process-updates/16 Pipeline failed
ci/crow/cron/process-updates/11 Pipeline failed
ci/crow/cron/process-updates/18 Pipeline failed
ci/crow/cron/process-updates/17 Pipeline failed
ci/crow/cron/process-updates/12 Pipeline failed
ci/crow/cron/process-updates/6 Pipeline failed
ci/crow/cron/process-updates/5 Pipeline was successful
## Why `RcppParallel` binaries built on ubuntu 22.04 still record `NEEDED libtbb.so.12` and cannot `dyn.load` without the distro TBB, although #145 landed the link-order patch and build-env-images dropped `TBB_INC`/`TBB_LIB` the same morning. The link-order fix does not survive a build image that exports those variables, and the published images still do: `.crow/images.yaml` in build-env-images triggers on `cron`/`manual` only, so every image in the registry is still the 2026-07-21 build. With `TBB_LIB` set, `configure.R` never reaches the bundled-oneTBB branch at all, `install.libs.R` fills `RcppParallel/lib` with absolute symlinks into the image's library dir, and the plain `-L` from #145 just points at the system TBB instead of the bundled one. Nothing on the pipeline side can undo this: R reads `~/.Renviron` **after** the process environment, so neither a Crow `environment:` block nor the registry `env` field wins. ``` # the image, not the Containerfile docker run --rm reg.devxy.io/rpkgs/build-env-ubuntu:jammy sh -lc 'grep TBB ~/.Renviron' TBB_INC=/usr/include/oneapi TBB_LIB=/usr/lib/x86_64-linux-gnu ``` Rather than couple correctness to image freshness for a third round-trip, the patch now forces the bundled TBB itself. ## Changes - `local/patches/RcppParallel/bundled-tbb-link-order.patch` -> `local/patches/RcppParallel/force-bundled-tbb.patch`. Keeps the `-Wl,-L` -> `-L` link-order hunk (the bundled branch has the same problem when a distro libtbb is installed) and adds an opt-out, `BINCRAFT_ALLOW_SYSTEM_TBB=TRUE`, for the ambient `TBB_ROOT`/`TBB_LIB`/`TBB_INC`. - All four read sites have to move together; patching only `configure.R` builds the bundled TBB and then **segfaults R on the post-install load test**: - `tools/config/configure.R`: branch selection. - `src/install.libs.R`: `R CMD INSTALL` runs it outside the `tbb` rule in `src/Makevars` that passes the configured values, so at install time it still sees the image environment. - `R/tbb.R` `tbbLibraryPath()`: run time. `.onLoad()` would `dyn.load` the system TBB into the global scope next to the bundled one that `RcppParallel.so` needs - two copies of the same symbols, hence the segfault. - `R/tbb.R` `tbbCxxFlags()` / `tbbLdFlags()`: otherwise dependents such as `rstan` get the system TBB handed back to them. - `local/patches/registry.json`: new patch path and a `reason` describing both failure modes. ## Verification Built through `bincraft:::prepare_patched_repo()` on the **current, unfixed** images (all three still export `TBB_INC`/`TBB_LIB`), with bincraft v5.0.1, i.e. what CI deploys: | image | `NEEDED` | rpath | `RcppParallel/lib` | load, all system `libtbb*` moved away | | --- | --- | --- | --- | --- | | `build-env-ubuntu:jammy` | `libtbb.so` | `$ORIGIN/../lib` | real `libtbb.so.2`, 5.0 MB | OK, 12 threads | | `build-env-redhat:9` | `libtbb.so` | `$ORIGIN/../lib` | real `libtbb.so.2`, 5.1 MB | OK, 12 threads | | `build-env-alpine:3.24` | `libtbb.so` | `$ORIGIN/../lib` | real `libtbb.so.2`, 4.7 MB | OK, 12 threads | For contrast, the same jammy build without this change: ``` NEEDED libtbb.so.12 RUNPATH /usr/lib/x86_64-linux-gnu:$ORIGIN/../lib lib/ libtbb.so.12.5 -> /usr/lib/x86_64-linux-gnu/libtbb.so.12.5 (dangling off the image) load libtbb.so.12: cannot open shared object file: No such file or directory ``` With `TBB_LIB` still exported, `RcppParallel::tbbLibraryPath()` resolves to the package's own `lib`, and `CxxFlags()` emits the package's own `include` plus `-DTBB_INTERFACE_NEW` - not `/usr/include/oneapi`. ## Behaviour change Published `RcppParallel` binaries carry their own oneTBB on every platform, whichever image version CI pulls. The patch content changed, so the patched-binary cache key changes with it and no stale entry is reused. The build-env-images fix is still worth rolling out (those images also carry the broken el8 `uvr` and uvr 0.4.1), but RcppParallel no longer waits on it. ## Follow-up, not in this PR The binaries already in B2 are the broken ones; they need a rebuild, and a Bunny `/purge` does not evict Perma-Cache. Reviewed-on: #146
This commit is contained in:
parent
98371fb9c5
commit
fe2289af56
3 changed files with 156 additions and 48 deletions
|
|
@ -1,46 +0,0 @@
|
||||||
diff --git a/tools/config/configure.R b/tools/config/configure.R
|
|
||||||
index 6293fe1..2f84337 100644
|
|
||||||
--- a/tools/config/configure.R
|
|
||||||
+++ b/tools/config/configure.R
|
|
||||||
@@ -186,12 +186,23 @@ define(
|
|
||||||
)
|
|
||||||
|
|
||||||
# set PKG_LIBS
|
|
||||||
+#
|
|
||||||
+# bincraft patch: the library directories below are passed as plain '-L', not
|
|
||||||
+# '-Wl,-L'. gcc expands its own search dirs (/usr/lib64, /usr/lib/<triplet>)
|
|
||||||
+# into explicit '-L' options ahead of anything forwarded verbatim with '-Wl,',
|
|
||||||
+# so with '-Wl,-L' a system libtbb.so wins over the one named here: on a build
|
|
||||||
+# host with a distro TBB installed, '-ltbb' resolves to that library and
|
|
||||||
+# RcppParallel.so records its SONAME (libtbb.so.12, or libtbb.so.2 for the
|
|
||||||
+# classic Intel TBB) instead of the bundled 'libtbb.so'. The binary then loads
|
|
||||||
+# the system TBB rather than the copy shipped in RcppParallel/lib, and fails
|
|
||||||
+# outright on a machine that has no system TBB. gcc places a plain '-L' before
|
|
||||||
+# its built-in dirs, so the intended library is found first.
|
|
||||||
pkgLibs <- if (!is.na(tbbLib)) {
|
|
||||||
|
|
||||||
# a TBB supplied via TBB_LIB / TBB_ROOT. an rpath is meaningless on Windows,
|
|
||||||
# where the loader has no equivalent -- see R/zzz.R for how we resolve there
|
|
||||||
c(
|
|
||||||
- "-Wl,-L\"$(TBB_LIB)\"",
|
|
||||||
+ "-L\"$(TBB_LIB)\"",
|
|
||||||
if (.Platform$OS.type != "windows")
|
|
||||||
sprintf("-Wl,-rpath,%s", shQuote(tbbLib)),
|
|
||||||
"-l$(TBB_NAME)",
|
|
||||||
@@ -201,14 +212,14 @@ pkgLibs <- if (!is.na(tbbLib)) {
|
|
||||||
} else if (R.version$os == "emscripten") {
|
|
||||||
|
|
||||||
c(
|
|
||||||
- "-Wl,-Ltbb/build/lib_release",
|
|
||||||
+ "-Ltbb/build/lib_release",
|
|
||||||
"-l$(TBB_NAME)"
|
|
||||||
)
|
|
||||||
|
|
||||||
} else {
|
|
||||||
|
|
||||||
c(
|
|
||||||
- "-Wl,-Ltbb/build/lib_release",
|
|
||||||
+ "-Ltbb/build/lib_release",
|
|
||||||
"-l$(TBB_NAME)",
|
|
||||||
"-l$(TBB_MALLOC_NAME)"
|
|
||||||
)
|
|
||||||
154
local/patches/RcppParallel/force-bundled-tbb.patch
Normal file
154
local/patches/RcppParallel/force-bundled-tbb.patch
Normal file
|
|
@ -0,0 +1,154 @@
|
||||||
|
diff --git a/R/aaa.R b/R/aaa.R
|
||||||
|
index 568a2aa..bfdab4c 100644
|
||||||
|
--- a/R/aaa.R
|
||||||
|
+++ b/R/aaa.R
|
||||||
|
@@ -5,4 +5,21 @@ TBB_LIB <- ""
|
||||||
|
TBB_INC <- ""
|
||||||
|
|
||||||
|
TBB_NAME <- "tbb"
|
||||||
|
-TBB_MALLOC_NAME <- "tbbmalloc"
|
||||||
|
\ No newline at end of file
|
||||||
|
+TBB_MALLOC_NAME <- "tbbmalloc"
|
||||||
|
+
|
||||||
|
+# bincraft patch: our build images (and plenty of user environments) export
|
||||||
|
+# TBB_ROOT / TBB_LIB / TBB_INC. A binary we publish always carries its own
|
||||||
|
+# oneTBB in RcppParallel/lib -- see the companion changes in
|
||||||
|
+# tools/config/configure.R and src/install.libs.R -- so honouring those
|
||||||
|
+# variables at run time is actively harmful: .onLoad() would dyn.load() a
|
||||||
|
+# second, unrelated TBB into the process next to the bundled one (two copies
|
||||||
|
+# of the same symbols in the global scope, which segfaults R on load), and
|
||||||
|
+# RcppParallelLibs() / CxxFlags() would hand that system TBB to dependents
|
||||||
|
+# such as rstan, putting NEEDED libtbb.so.12 back into their binaries. Read
|
||||||
|
+# the variables only when explicitly opted back in.
|
||||||
|
+bincraftGetenv <- function(name, unset = "") {
|
||||||
|
+ if (Sys.getenv("BINCRAFT_ALLOW_SYSTEM_TBB", unset = "FALSE") == "TRUE")
|
||||||
|
+ Sys.getenv(name, unset = unset)
|
||||||
|
+ else
|
||||||
|
+ unset
|
||||||
|
+}
|
||||||
|
\ No newline at end of file
|
||||||
|
diff --git a/R/tbb.R b/R/tbb.R
|
||||||
|
index 6f6a745..e407986 100644
|
||||||
|
--- a/R/tbb.R
|
||||||
|
+++ b/R/tbb.R
|
||||||
|
@@ -17,7 +17,7 @@ tbbLibraryPath <- function(name = NULL) {
|
||||||
|
sysname <- Sys.info()[["sysname"]]
|
||||||
|
|
||||||
|
# find root for TBB install
|
||||||
|
- tbbRoot <- Sys.getenv("TBB_LIB", unset = tbbRoot())
|
||||||
|
+ tbbRoot <- bincraftGetenv("TBB_LIB", unset = tbbRoot())
|
||||||
|
if (is.null(name))
|
||||||
|
return(tbbRoot)
|
||||||
|
|
||||||
|
@@ -58,7 +58,7 @@ tbbCxxFlags <- function() {
|
||||||
|
flags <- c("-DRCPP_PARALLEL_USE_TBB=1")
|
||||||
|
|
||||||
|
# if TBB_INC is set, apply those library paths
|
||||||
|
- tbbInc <- Sys.getenv("TBB_INC", unset = TBB_INC)
|
||||||
|
+ tbbInc <- bincraftGetenv("TBB_INC", unset = TBB_INC)
|
||||||
|
if (!file.exists(tbbInc)) {
|
||||||
|
tbbInc <- system.file("include", package = "RcppParallel")
|
||||||
|
}
|
||||||
|
@@ -117,7 +117,7 @@ tbbLdFlags <- function() {
|
||||||
|
}
|
||||||
|
|
||||||
|
# shortcut if TBB_LIB defined
|
||||||
|
- tbbLib <- Sys.getenv("TBB_LINK_LIB", Sys.getenv("TBB_LIB", unset = TBB_LIB))
|
||||||
|
+ tbbLib <- bincraftGetenv("TBB_LINK_LIB", bincraftGetenv("TBB_LIB", unset = TBB_LIB))
|
||||||
|
if (nzchar(tbbLib)) {
|
||||||
|
if (R.version$os == "emscripten") {
|
||||||
|
fmt <- "-L%1$s -l%2$s"
|
||||||
|
diff --git a/src/install.libs.R b/src/install.libs.R
|
||||||
|
index 3b3cfda..c0e6f3e 100644
|
||||||
|
--- a/src/install.libs.R
|
||||||
|
+++ b/src/install.libs.R
|
||||||
|
@@ -477,6 +477,18 @@ prependFlags <- function(prependFlags, toFlags) {
|
||||||
|
tbbLib <- Sys.getenv("TBB_LIB")
|
||||||
|
tbbInc <- Sys.getenv("TBB_INC")
|
||||||
|
|
||||||
|
+# bincraft patch: the companion change in tools/config/configure.R stops an
|
||||||
|
+# ambient TBB_LIB / TBB_INC from selecting a system TBB, but this script is
|
||||||
|
+# also run directly by `R CMD INSTALL` (not only through the `tbb` rule in
|
||||||
|
+# src/Makevars, which passes the configured values), so at install time it
|
||||||
|
+# still sees the image's environment and would symlink the system libraries
|
||||||
|
+# into RcppParallel/lib. Drop them here for the same reason, under the same
|
||||||
|
+# opt-out.
|
||||||
|
+if (Sys.getenv("BINCRAFT_ALLOW_SYSTEM_TBB", unset = "FALSE") != "TRUE") {
|
||||||
|
+ tbbLib <- ""
|
||||||
|
+ tbbInc <- ""
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
args <- commandArgs(trailingOnly = TRUE)
|
||||||
|
if (identical(args, "build")) {
|
||||||
|
if (nzchar(tbbLib) && nzchar(tbbInc)) {
|
||||||
|
diff --git a/tools/config/configure.R b/tools/config/configure.R
|
||||||
|
index 6293fe1..eae4aaa 100644
|
||||||
|
--- a/tools/config/configure.R
|
||||||
|
+++ b/tools/config/configure.R
|
||||||
|
@@ -40,6 +40,24 @@ tbbRoot <- Sys.getenv("TBB_ROOT", unset = NA)
|
||||||
|
tbbLib <- Sys.getenv("TBB_LIB", unset = NA)
|
||||||
|
tbbInc <- Sys.getenv("TBB_INC", unset = NA)
|
||||||
|
|
||||||
|
+# bincraft patch: ignore an ambient TBB_ROOT / TBB_LIB / TBB_INC. Several of
|
||||||
|
+# our build images export these (a leftover from RcppParallel 5.x, whose
|
||||||
|
+# bundled Intel TBB would not build on musl or with modern g++), and any of
|
||||||
|
+# them switches the branches below to a system TBB. The published binary then
|
||||||
|
+# records NEEDED libtbb.so.12 (or libtbb.so.2 for the classic Intel TBB) and
|
||||||
|
+# gets a RcppParallel/lib full of absolute symlinks into the image's library
|
||||||
|
+# dir, so it cannot dyn.load on a consumer machine without that exact TBB.
|
||||||
|
+# 6.x bundles oneTBB 2022 and builds it with cmake on every platform we ship,
|
||||||
|
+# so the bundled copy is always the right choice here; forcing it in the
|
||||||
|
+# package rather than relying on the image environment keeps the binary
|
||||||
|
+# correct whichever image version CI happens to pull. Set
|
||||||
|
+# BINCRAFT_ALLOW_SYSTEM_TBB=TRUE to restore the upstream behaviour.
|
||||||
|
+if (Sys.getenv("BINCRAFT_ALLOW_SYSTEM_TBB", unset = "FALSE") != "TRUE") {
|
||||||
|
+ tbbRoot <- NA
|
||||||
|
+ tbbLib <- NA
|
||||||
|
+ tbbInc <- NA
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
tbbName <- Sys.getenv("TBB_NAME", unset = "tbb")
|
||||||
|
tbbMallocName <- Sys.getenv("TBB_MALLOC_NAME", unset = "tbbmalloc")
|
||||||
|
|
||||||
|
@@ -186,12 +204,23 @@ define(
|
||||||
|
)
|
||||||
|
|
||||||
|
# set PKG_LIBS
|
||||||
|
+#
|
||||||
|
+# bincraft patch: the library directories below are passed as plain '-L', not
|
||||||
|
+# '-Wl,-L'. gcc expands its own search dirs (/usr/lib64, /usr/lib/<triplet>)
|
||||||
|
+# into explicit '-L' options ahead of anything forwarded verbatim with '-Wl,',
|
||||||
|
+# so with '-Wl,-L' a system libtbb.so wins over the one named here: on a build
|
||||||
|
+# host with a distro TBB installed, '-ltbb' resolves to that library and
|
||||||
|
+# RcppParallel.so records its SONAME (libtbb.so.12, or libtbb.so.2 for the
|
||||||
|
+# classic Intel TBB) instead of the bundled 'libtbb.so'. The binary then loads
|
||||||
|
+# the system TBB rather than the copy shipped in RcppParallel/lib, and fails
|
||||||
|
+# outright on a machine that has no system TBB. gcc places a plain '-L' before
|
||||||
|
+# its built-in dirs, so the intended library is found first.
|
||||||
|
pkgLibs <- if (!is.na(tbbLib)) {
|
||||||
|
|
||||||
|
# a TBB supplied via TBB_LIB / TBB_ROOT. an rpath is meaningless on Windows,
|
||||||
|
# where the loader has no equivalent -- see R/zzz.R for how we resolve there
|
||||||
|
c(
|
||||||
|
- "-Wl,-L\"$(TBB_LIB)\"",
|
||||||
|
+ "-L\"$(TBB_LIB)\"",
|
||||||
|
if (.Platform$OS.type != "windows")
|
||||||
|
sprintf("-Wl,-rpath,%s", shQuote(tbbLib)),
|
||||||
|
"-l$(TBB_NAME)",
|
||||||
|
@@ -201,14 +230,14 @@ pkgLibs <- if (!is.na(tbbLib)) {
|
||||||
|
} else if (R.version$os == "emscripten") {
|
||||||
|
|
||||||
|
c(
|
||||||
|
- "-Wl,-Ltbb/build/lib_release",
|
||||||
|
+ "-Ltbb/build/lib_release",
|
||||||
|
"-l$(TBB_NAME)"
|
||||||
|
)
|
||||||
|
|
||||||
|
} else {
|
||||||
|
|
||||||
|
c(
|
||||||
|
- "-Wl,-Ltbb/build/lib_release",
|
||||||
|
+ "-Ltbb/build/lib_release",
|
||||||
|
"-l$(TBB_NAME)",
|
||||||
|
"-l$(TBB_MALLOC_NAME)"
|
||||||
|
)
|
||||||
|
|
@ -6,8 +6,8 @@
|
||||||
"env": {},
|
"env": {},
|
||||||
"configure_args": [],
|
"configure_args": [],
|
||||||
"makevars": {},
|
"makevars": {},
|
||||||
"patch": "RcppParallel/bundled-tbb-link-order.patch",
|
"patch": "RcppParallel/force-bundled-tbb.patch",
|
||||||
"reason": "RcppParallel 6.x bundles oneTBB 2022 and builds it with cmake, which works on musl and g++ 8-15, so the system-TBB workaround needed for 5.x is gone. What remains broken is the link order: configure.R passes the TBB directory as '-Wl,-L', and gcc expands its own search dirs (/usr/lib64, /usr/lib/<triplet>) into '-L' options ahead of anything forwarded with '-Wl,'. On a build host with a distro TBB installed, '-ltbb' therefore resolves to the system library and RcppParallel.so records its SONAME (libtbb.so.12, or libtbb.so.2 for the classic Intel TBB on el8/el9) instead of the bundled 'libtbb.so'. The published binary then loads the system TBB rather than the copy in RcppParallel/lib and fails to dyn.load on a consumer machine without one, the same way fs did with libuv. Passing a plain '-L' puts the bundled build dir ahead of gcc's defaults; verified on alpine 3.24, el8 and ubuntu noble to produce NEEDED libtbb.so + RPATH $ORIGIN/../lib, loading with every system libtbb removed."
|
"reason": "RcppParallel 6.x bundles oneTBB 2022 and builds it with cmake on every platform we ship, so the system-TBB workaround needed for 5.x is gone, but two things still steer the build back to a system TBB. (1) Ambient TBB_ROOT/TBB_LIB/TBB_INC: several build images still export these (build-env-images dropped them, but the images are rebuilt only by cron/manual runs, so a stale image keeps them), and R reads ~/.Renviron *after* the process environment, so no pipeline-side env override can undo it. configure.R then takes the system-TBB branch, install.libs.R symlinks the image's libraries into RcppParallel/lib as absolute paths, and the binary records NEEDED libtbb.so.12 (libtbb.so.2 for the classic Intel TBB on el8/el9) -- it cannot dyn.load on a consumer machine without that exact TBB. Both files are patched to ignore those variables (opt out with BINCRAFT_ALLOW_SYSTEM_TBB=TRUE); install.libs.R needs it separately because R CMD INSTALL runs it outside the src/Makevars rule that passes the configured values. (2) Link order: the bundled branch passes its build dir as '-Wl,-Ltbb/build/lib_release', and gcc expands its own search dirs (/usr/lib64, /usr/lib/<triplet>) into '-L' options ahead of anything forwarded with '-Wl,', so '-ltbb' would still resolve to a distro TBB when one is installed; a plain '-L' puts the bundled dir first. Verified on the current build-env-ubuntu:jammy image (which still exports TBB_INC/TBB_LIB): NEEDED libtbb.so, RUNPATH $ORIGIN/../lib, real libtbb.so.2 in RcppParallel/lib, and the package loads with every system libtbb moved away."
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"package": "fs",
|
"package": "fs",
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue