MINOR: [R] Unset MAKEFLAGS for nested libarrow CMake build#50317
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the R static Arrow build script to pass the Make/Ninja parallelism flag in a single token (-jN) to avoid cases where the underlying build tool interprets -j as missing its integer argument.
Changes:
- Adjusted the
cmake --buildinvocation to use-j${N_JOBS}instead of-j $N_JOBS.
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 6d4a25b Submitted crossbow builds: ursacomputing/crossbow @ actions-a4227cb8da
|
6d4a25b to
1d835a6
Compare
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 1d835a6 Submitted crossbow builds: ursacomputing/crossbow @ actions-c381cbcd47
|
1d835a6 to
ef59f03
Compare
|
@github-actions crossbow submit test-r-wasm |
|
Revision: ef59f03 Submitted crossbow builds: ursacomputing/crossbow @ actions-52b31fc83d
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 4f135a1 Submitted crossbow builds: ursacomputing/crossbow @ actions-ea2f0ed863
|
|
@thisisnic this change might solve the other r-wasm issue. |
| ${SOURCE_DIR} | ||
|
|
||
| ${CMAKE} --build . --target install -- -j $N_JOBS | ||
| (unset MAKEFLAGS MFLAGS; ${CMAKE} --build . --target install --parallel "${N_JOBS}") |
There was a problem hiding this comment.
It seems that this
diff --git a/r/inst/build_arrow_static.sh b/r/inst/build_arrow_static.sh
index 349531b75f..02bc4a24c7 100755
--- a/r/inst/build_arrow_static.sh
+++ b/r/inst/build_arrow_static.sh
@@ -114,7 +114,7 @@ ${CMAKE_WRAPPER} ${CMAKE} -DARROW_BOOST_USE_SHARED=OFF \
-G "${CMAKE_GENERATOR:-Unix Makefiles}" \
${SOURCE_DIR}
-${CMAKE} --build . --target install -- -j $N_JOBS
+${CMAKE} --build . --target install -- -j$N_JOBS
if command -v sccache &> /dev/null; then
echo "=== sccache stats after the build ==="or
diff --git a/r/inst/build_arrow_static.sh b/r/inst/build_arrow_static.sh
index 349531b75f..0df5240888 100755
--- a/r/inst/build_arrow_static.sh
+++ b/r/inst/build_arrow_static.sh
@@ -114,7 +114,7 @@ ${CMAKE_WRAPPER} ${CMAKE} -DARROW_BOOST_USE_SHARED=OFF \
-G "${CMAKE_GENERATOR:-Unix Makefiles}" \
${SOURCE_DIR}
-${CMAKE} --build . --target install -- -j $N_JOBS
+${CMAKE} --build . --target install --parallel $N_JOBS
if command -v sccache &> /dev/null; then
echo "=== sccache stats after the build ==="is enough.
There was a problem hiding this comment.
So if change that I get:
+ /usr/bin/cmake --build . --target install --parallel 2
gmake: the '-j' option requires a positive integer argumentand this error on crossbow test-r-wasm. Will try other combinations.
There was a problem hiding this comment.
Oh no worries. Do you think we shouldn't use unset MAKEFLAGS MFLAGS;? I'm trying things but didn't find anything useful yet.
There was a problem hiding this comment.
Could you show env | sort to consider whether we can unset MAKEFLAGS MFLAGS or not?
There was a problem hiding this comment.
The issue was MAKEFLAGS=%s\n' -jNA, which was due to ncores <- parallel::detectCores() returning NA in r/tools/nixlibs.R. This should be resolved now.
|
Revision: ca8919f Submitted crossbow builds: ursacomputing/crossbow @ actions-c97534ddb8
|
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 4c7c074 Submitted crossbow builds: ursacomputing/crossbow @ actions-e09624349a
|
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 4c7c074 Submitted crossbow builds: ursacomputing/crossbow @ actions-c84a75e285
|
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 6aa0fe6 Submitted crossbow builds: ursacomputing/crossbow @ actions-2aad1c3029
|
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 5a5ea93 Submitted crossbow builds: ursacomputing/crossbow @ actions-80e6f4beca
|
|
@github-actions crossbow submit test-r-wasm |
|
Revision: 8f4ba35 Submitted crossbow builds: ursacomputing/crossbow @ actions-dd5e1d7710
|
| # CRAN policy says not to use more than 2 cores during checks | ||
| # If you have more and want to use more, set MAKEFLAGS or NOT_CRAN | ||
| ncores <- parallel::detectCores() | ||
| ncores <- max(1, parallel::detectCores(), na.rm = TRUE) |
There was a problem hiding this comment.
Is this a typo; should it be min()?
There was a problem hiding this comment.
In case parallel::detectCores() returns 2 we'd have max(1, 2, na.rm = TRUE) which would return 2 which would be preferable over min(1, 2, na.rm = TRUE) which would return 1. Or am I misunderstanding something?
Rationale for this change
See failure here.
What changes are included in this PR?
Bash command used is adjusted to prevent this error.
Are these changes tested?
By CI.
Are there any user-facing changes?
No.