From ef6e33ed56580787f2f5ef7e61032207fa83fa13 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Fri, 24 Jul 2026 16:10:52 -0700 Subject: [PATCH] log mingw cpuid guard application during install patchTbbMachineHeader() previously exited silently in all cases: header missing, guard already present, unexpected header form, and even success. The unexpected-form case is dangerous -- it means the guard is absent and mingw builds may fail (the same way the #248 fix was silently incomplete, caught only by diffing a real Rtools45 build in #253). Log the normal outcomes in the style of the provenance logging added in #256, and emit a warning when the header does not have the expected form, so a reformatted Rtools/oneTBB header surfaces at install time instead of as a downstream build failure. Also add unit tests covering application, idempotency, and the malformed-header path. --- NEWS.md | 4 ++++ src/install.libs.R | 24 ++++++++++++++++++------ tests/test-install-libs.R | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index 1788c64d..466f5aba 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # RcppParallel (development version) +* The mingw cpuid guard applied to TBB's `_machine.h` header during + installation is now logged, and a warning is emitted if the header does + not have the expected form and the guard cannot be applied. + * On Linux, the bundled TBB libraries are once again installed with versioned names (e.g. `libtbb.so.2`) plus an unversioned `libtbb.so` symlink, matching the layout shipped by RcppParallel 5.1.11 and earlier. The oneTBB cmake build diff --git a/src/install.libs.R b/src/install.libs.R index 7f88be06..e770838d 100644 --- a/src/install.libs.R +++ b/src/install.libs.R @@ -270,16 +270,27 @@ useTbbPreamble <- function(tbbInc) { # applied to the copied headers, not just the bundled sources. patchTbbMachineHeader <- function(path) { - if (!file.exists(path)) - return() + if (!file.exists(path)) { + writeLines(sprintf("** no tbb header at '%s'; skipping mingw cpuid guard", path)) + return(invisible()) + } contents <- readLines(path) - if (any(grepl("push_macro", contents, fixed = TRUE))) - return() + if (any(grepl("push_macro", contents, fixed = TRUE))) { + writeLines(sprintf("** mingw cpuid guard already present in '%s'", path)) + return(invisible()) + } index <- which(contents == "#include ") - if (length(index) != 1L) - return() + if (length(index) != 1L) { + fmt <- paste( + "expected exactly one '#include ' line in '%s', but found %i;", + "the mingw cpuid guard was not applied, and mingw builds using these", + "headers may fail -- see patches/mingw_cpuid.diff" + ) + warning(sprintf(fmt, path, length(index))) + return(invisible()) + } replacement <- c( "// GCC's defines a function-like '__cpuid' macro that mangles the", @@ -297,6 +308,7 @@ patchTbbMachineHeader <- function(path) { contents <- append(contents[-index], replacement, after = index - 1L) writeLines(contents, path) + writeLines(sprintf("** applied mingw cpuid guard to '%s'", path)) } diff --git a/tests/test-install-libs.R b/tests/test-install-libs.R index 3b98806b..fe0ceebd 100644 --- a/tests/test-install-libs.R +++ b/tests/test-install-libs.R @@ -40,6 +40,7 @@ if (length(marker)) env <- new.env(parent = globalenv()) eval(parse(text = paste(lines, collapse = "\n")), envir = env) splitCompilerVar <- get("splitCompilerVar", envir = env) +patchTbbMachineHeader <- get("patchTbbMachineHeader", envir = env) # minimal assertion harness failures <- 0L @@ -112,6 +113,37 @@ Sys.unsetenv("TEST_CXX_UNSET") check(identical(splitCompilerVar("TEST_CXX_UNSET", "TEST_CXXFLAGS"), FALSE), "unset compiler variable returns FALSE") +# the mingw cpuid guard should be applied exactly once to a header with the +# expected form, and applying it again should leave the header untouched +header <- tempfile(fileext = ".h") +writeLines(c("#pragma once", "#include ", "int value;"), header) +patchTbbMachineHeader(header) +patched <- readLines(header) +check(any(grepl("push_macro", patched, fixed = TRUE)), + "cpuid guard is applied to a well-formed header") +patchTbbMachineHeader(header) +check(identical(readLines(header), patched), + "cpuid guard application is idempotent") + +# a header without exactly one matching include line must not be modified, +# and the skipped patch must be surfaced as a warning (a silent no-op here +# would quietly reintroduce the mingw __cpuid build failure) +malformed <- tempfile(fileext = ".h") +original <- c("#pragma once", " #include ") +writeLines(original, malformed) +warned <- FALSE +withCallingHandlers( + patchTbbMachineHeader(malformed), + warning = function(w) { + warned <<- TRUE + invokeRestart("muffleWarning") + } +) +check(warned, "unexpected header form emits a warning") +check(identical(readLines(malformed), original), + "unexpected header form is left unmodified") +unlink(c(header, malformed)) + if (failures > 0L) stop(sprintf("%d install.libs.R helper test(s) failed", failures))