require cmake on all platforms; tbb is always enabled - #275
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DESCRIPTIONhas always declaredSystemRequirements: CMake (>= 3.5), and amissing 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.
This treats it as the unmet system requirement it is and fails the install.
TBB_ENABLEDis then alwaysTRUE, which makes several branches unreachable,so they are dropped:
configure.R, and itscounterpart in
install.libs.RNULLpkgLibsbranch for "no TBB to link at all"TBB_ENABLED = FALSE/-DRCPP_PARALLEL_USE_TBB=0configurationThe tinythread backend is unaffected as a runtime choice -- it is still
selectable via
RCPP_PARALLEL_BACKEND=tinythread. It simply is no longersomething a build can silently land on.
Testing
all 12 test files pass on macOS
verified the failure path end to end, with the user profile suppressed and
cmake off the PATH:
verified all three diagnostic variants (not found / unparseable version / too
old) select correctly
Note
This was written on top of #274 but pushed after that PR had already been
squash-merged, so it is cherry-picked onto current master here. The diff is
identical to the commit that was on that branch.