Skip to content

Commit 8288715

Browse files
committed
require cmake on all platforms; tbb is always enabled
DESCRIPTION has always declared 'SystemRequirements: CMake (>= 3.5)', and a missing cmake was already fatal everywhere except Windows, where it instead produced a package with no TBB backend at all. That was easy to end up with and hard to notice: the install succeeded, and the only symptom was RcppParallel quietly running everything on tinythread. Treat it as the unmet system requirement it is and fail the install. TBB_ENABLED is then always TRUE, which makes several branches unreachable, so drop them: - the Windows 'building without a tbb backend' notice in configure.R and its counterpart in install.libs.R - the NULL pkgLibs branch for 'no TBB to link at all' - the TBB_ENABLED = FALSE / -DRCPP_PARALLEL_USE_TBB=0 configuration The tinythread backend is unaffected as a runtime choice: it is still selectable via RCPP_PARALLEL_BACKEND=tinythread. It simply is no longer something a build can silently land on.
1 parent fdd181e commit 8288715

3 files changed

Lines changed: 31 additions & 41 deletions

File tree

NEWS.md

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,16 @@
1616
user's toolchain, and left downstream packages with no TBB library to link
1717
against. Building it ourselves gives every platform the same oneTBB and makes
1818
the ABI a property of RcppParallel. `TBB_LIB` / `TBB_INC` are still honoured
19-
for anyone supplying their own build. This requires cmake, which Rtools has
20-
shipped since Rtools42; toolchains without one continue to use the tinythread
21-
fallback.
19+
for anyone supplying their own build.
20+
21+
* Building RcppParallel now requires cmake (>= 3.5) on all platforms, as
22+
`SystemRequirements` has always declared. Previously a missing or unusable
23+
cmake was fatal everywhere except Windows, where it instead produced a
24+
package with no TBB backend at all -- a silently degraded install that was
25+
easy to end up with and hard to notice. TBB is now always enabled. Rtools has
26+
shipped cmake since Rtools42, so this should not affect Windows users in
27+
practice. Note that the tinythread backend remains selectable at runtime via
28+
`RCPP_PARALLEL_BACKEND=tinythread`; it is only no longer a build outcome.
2229

2330
* As a consequence, `RcppParallel::RcppParallelLibs()` now emits `-ltbb` and
2431
`-ltbbmalloc` on Windows, in addition to `-lRcppParallel` (which remains

src/install.libs.R

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -481,18 +481,15 @@ args <- commandArgs(trailingOnly = TRUE)
481481
if (identical(args, "build")) {
482482
if (nzchar(tbbLib) && nzchar(tbbInc)) {
483483
useSystemTbb(tbbLib, tbbInc)
484-
} else if (.Platform$OS.type == "windows" && !nzchar(Sys.getenv("CMAKE"))) {
485-
# configure found neither a usable TBB nor a cmake to build one with
486-
writeLines("** building RcppParallel without tbb backend")
487484
} else {
488485
useBundledTbb()
489486
}
490487
} else {
491488

492-
# prefer the configure-detected TBB_LIB when the environment variable
493-
# is unset; otherwise, e.g. on Windows (where configure detects the
494-
# Rtools copy of TBB), we'd wrongly take the bundled-TBB branch below,
495-
# and ship any stale artifacts present in tbb/build/lib_release
489+
# prefer the configure-detected TBB_LIB when the environment variable is
490+
# unset; otherwise a TBB configured via TBB_ROOT (which sets TBB_LIB only in
491+
# the generated tbb-autodetected.R) would wrongly take the bundled-TBB
492+
# branch below, and ship any stale artifacts in tbb/build/lib_release
496493
source("../R/tbb-autodetected.R")
497494
if (!nzchar(tbbLib))
498495
tbbLib <- TBB_LIB

tools/config/configure.R

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,12 @@ if (tryAutoDetect) {
111111

112112
}
113113

114-
# if we didn't find a TBB to use, we'll build the bundled copy from
115-
# sources -- which requires cmake. on Windows, a cmake that is missing,
116-
# unusable or too old is not fatal: toolchains that can't build oneTBB
117-
# (e.g. Rtools40, which provides neither cmake nor a TBB) fall back to
118-
# tinythread instead
114+
# if we didn't find a TBB to use, we'll build the bundled copy from sources --
115+
# which requires cmake, on every platform. this is what 'SystemRequirements:
116+
# CMake (>= 3.5)' in DESCRIPTION declares, so a missing or unusable cmake is an
117+
# unmet system requirement and fails the install rather than quietly producing
118+
# a package with no TBB backend. (the tinythread backend is still selectable at
119+
# runtime via RCPP_PARALLEL_BACKEND; it is only no longer a build outcome.)
119120
define(CMAKE = "")
120121
buildBundledTbb <- FALSE
121122

@@ -146,8 +147,8 @@ if (is.na(tbbLib)) {
146147
# (use the resolved path; cmake may not be on the PATH)
147148
#
148149
# any failure here -- a CMAKE pointing at nothing runnable, version output
149-
# we can't parse -- is treated as 'no usable cmake' rather than allowed to
150-
# propagate, so that Windows can still fall back to tinythread below
150+
# we can't parse -- is reported as 'no usable cmake', so that the error
151+
# names the actual problem rather than surfacing as a parse failure
151152
cmakeVersion <- NA
152153
if (!is.na(cmake)) {
153154
cmakeVersion <- tryCatch({
@@ -158,11 +159,7 @@ if (is.na(tbbLib)) {
158159

159160
buildBundledTbb <- !is.na(cmakeVersion) && cmakeVersion >= "3.5"
160161

161-
if (buildBundledTbb) {
162-
163-
define(CMAKE = cmake)
164-
165-
} else {
162+
if (!buildBundledTbb) {
166163

167164
reason <- if (is.na(cmake)) {
168165
"cmake was not found"
@@ -172,16 +169,12 @@ if (is.na(tbbLib)) {
172169
sprintf("cmake %s is too old (need >= 3.5)", cmakeVersion)
173170
}
174171

175-
# not fatal on Windows: toolchains that can build neither an oneTBB nor
176-
# anything else (e.g. Rtools40, which provides no cmake and no TBB) use
177-
# the tinythread backend instead
178-
if (.Platform$OS.type == "windows")
179-
writeLines(paste0(reason, "; building RcppParallel without a tbb backend"))
180-
else
181-
stop("error: RcppParallel requires cmake (>= 3.5); ", reason)
172+
stop("error: RcppParallel requires cmake (>= 3.5); ", reason)
182173

183174
}
184175

176+
define(CMAKE = cmake)
177+
185178
}
186179

187180
# now, define TBB_LIB and TBB_INC as appropriate
@@ -205,11 +198,6 @@ pkgLibs <- if (!is.na(tbbLib)) {
205198
"-l$(TBB_MALLOC_NAME)"
206199
)
207200

208-
} else if (!buildBundledTbb) {
209-
210-
# no TBB to link at all; the tinythread fallback is used instead
211-
NULL
212-
213201
} else if (R.version$os == "emscripten") {
214202

215203
c(
@@ -245,13 +233,11 @@ if (!is.na(tbbLib)) {
245233
}
246234

247235
# PKG_CXXFLAGS
248-
if (.Platform$OS.type == "windows" && is.na(tbbLib) && !buildBundledTbb) {
249-
define(TBB_ENABLED = FALSE)
250-
define(PKG_CXXFLAGS = "-DRCPP_PARALLEL_USE_TBB=0")
251-
} else {
252-
define(TBB_ENABLED = TRUE)
253-
define(PKG_CXXFLAGS = "-DRCPP_PARALLEL_USE_TBB=1")
254-
}
236+
#
237+
# TBB is always enabled: either one was supplied via TBB_LIB / TBB_ROOT, or we
238+
# built the bundled copy, and failing to do either is fatal above
239+
define(TBB_ENABLED = TRUE)
240+
define(PKG_CXXFLAGS = "-DRCPP_PARALLEL_USE_TBB=1")
255241

256242
# macOS needs some extra flags set
257243
if (Sys.info()[["sysname"]] == "Darwin") {

0 commit comments

Comments
 (0)