Skip to content

Commit 0a650d9

Browse files
authored
log mingw cpuid guard application during install (#265)
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.
1 parent 2ff3695 commit 0a650d9

3 files changed

Lines changed: 54 additions & 6 deletions

File tree

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# RcppParallel (development version)
22

3+
* The mingw cpuid guard applied to TBB's `_machine.h` header during
4+
installation is now logged, and a warning is emitted if the header does
5+
not have the expected form and the guard cannot be applied.
6+
37
* On Linux, the bundled TBB libraries are once again installed with versioned
48
names (e.g. `libtbb.so.2`) plus an unversioned `libtbb.so` symlink, matching
59
the layout shipped by RcppParallel 5.1.11 and earlier. The oneTBB cmake build

src/install.libs.R

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -270,16 +270,27 @@ useTbbPreamble <- function(tbbInc) {
270270
# applied to the copied headers, not just the bundled sources.
271271
patchTbbMachineHeader <- function(path) {
272272

273-
if (!file.exists(path))
274-
return()
273+
if (!file.exists(path)) {
274+
writeLines(sprintf("** no tbb header at '%s'; skipping mingw cpuid guard", path))
275+
return(invisible())
276+
}
275277

276278
contents <- readLines(path)
277-
if (any(grepl("push_macro", contents, fixed = TRUE)))
278-
return()
279+
if (any(grepl("push_macro", contents, fixed = TRUE))) {
280+
writeLines(sprintf("** mingw cpuid guard already present in '%s'", path))
281+
return(invisible())
282+
}
279283

280284
index <- which(contents == "#include <intrin.h>")
281-
if (length(index) != 1L)
282-
return()
285+
if (length(index) != 1L) {
286+
fmt <- paste(
287+
"expected exactly one '#include <intrin.h>' line in '%s', but found %i;",
288+
"the mingw cpuid guard was not applied, and mingw builds using these",
289+
"headers may fail -- see patches/mingw_cpuid.diff"
290+
)
291+
warning(sprintf(fmt, path, length(index)))
292+
return(invisible())
293+
}
283294

284295
replacement <- c(
285296
"// GCC's <cpuid.h> defines a function-like '__cpuid' macro that mangles the",
@@ -297,6 +308,7 @@ patchTbbMachineHeader <- function(path) {
297308

298309
contents <- append(contents[-index], replacement, after = index - 1L)
299310
writeLines(contents, path)
311+
writeLines(sprintf("** applied mingw cpuid guard to '%s'", path))
300312

301313
}
302314

tests/test-install-libs.R

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ if (length(marker))
4040
env <- new.env(parent = globalenv())
4141
eval(parse(text = paste(lines, collapse = "\n")), envir = env)
4242
splitCompilerVar <- get("splitCompilerVar", envir = env)
43+
patchTbbMachineHeader <- get("patchTbbMachineHeader", envir = env)
4344

4445
# minimal assertion harness
4546
failures <- 0L
@@ -112,6 +113,37 @@ Sys.unsetenv("TEST_CXX_UNSET")
112113
check(identical(splitCompilerVar("TEST_CXX_UNSET", "TEST_CXXFLAGS"), FALSE),
113114
"unset compiler variable returns FALSE")
114115

116+
# the mingw cpuid guard should be applied exactly once to a header with the
117+
# expected form, and applying it again should leave the header untouched
118+
header <- tempfile(fileext = ".h")
119+
writeLines(c("#pragma once", "#include <intrin.h>", "int value;"), header)
120+
patchTbbMachineHeader(header)
121+
patched <- readLines(header)
122+
check(any(grepl("push_macro", patched, fixed = TRUE)),
123+
"cpuid guard is applied to a well-formed header")
124+
patchTbbMachineHeader(header)
125+
check(identical(readLines(header), patched),
126+
"cpuid guard application is idempotent")
127+
128+
# a header without exactly one matching include line must not be modified,
129+
# and the skipped patch must be surfaced as a warning (a silent no-op here
130+
# would quietly reintroduce the mingw __cpuid build failure)
131+
malformed <- tempfile(fileext = ".h")
132+
original <- c("#pragma once", " #include <intrin.h>")
133+
writeLines(original, malformed)
134+
warned <- FALSE
135+
withCallingHandlers(
136+
patchTbbMachineHeader(malformed),
137+
warning = function(w) {
138+
warned <<- TRUE
139+
invokeRestart("muffleWarning")
140+
}
141+
)
142+
check(warned, "unexpected header form emits a warning")
143+
check(identical(readLines(malformed), original),
144+
"unexpected header form is left unmodified")
145+
unlink(c(header, malformed))
146+
115147
if (failures > 0L)
116148
stop(sprintf("%d install.libs.R helper test(s) failed", failures))
117149

0 commit comments

Comments
 (0)