Copilot commented on code in PR #50317:
URL: https://github.com/apache/arrow/pull/50317#discussion_r3519953729
##########
r/inst/build_arrow_static.sh:
##########
@@ -114,7 +114,29 @@ ${CMAKE_WRAPPER} ${CMAKE} -DARROW_BOOST_USE_SHARED=OFF \
-G "${CMAKE_GENERATOR:-Unix Makefiles}" \
${SOURCE_DIR}
-${CMAKE} --build . --target install -- -j $N_JOBS
+sanitize_makeflags() {
+ printf '%s\n' "$1" |
+ sed -E \
+ -e 's/(^|[[:space:]])-j[[:space:]]*[0-9]*([[:space:]]|$)/ /g' \
+ -e 's/(^|[[:space:]])([[:alpha:]]*)j[0-9]*([[:alpha:]]*)($|[[:space:]])/
\2\3 /g' \
+ -e 's/[[:space:]]+/ /g' \
+ -e 's/^ //; s/ $//'
+}
+
+env | sort
+SANITIZED_MAKEFLAGS="$(sanitize_makeflags "${MAKEFLAGS:-}")"
+SANITIZED_MFLAGS="$(sanitize_makeflags "${MFLAGS:-}")"
+SANITIZED_GNUMAKEFLAGS="$(sanitize_makeflags "${GNUMAKEFLAGS:-}")"
+printf 'MAKEFLAGS before sanitize: %s\n' "${MAKEFLAGS:-}"
+printf 'MFLAGS before sanitize: %s\n' "${MFLAGS:-}"
+printf 'GNUMAKEFLAGS before sanitize: %s\n' "${GNUMAKEFLAGS:-}"
+printf 'MAKEFLAGS after sanitize: %s\n' "${SANITIZED_MAKEFLAGS}"
+printf 'MFLAGS after sanitize: %s\n' "${SANITIZED_MFLAGS}"
+printf 'GNUMAKEFLAGS after sanitize: %s\n' "${SANITIZED_GNUMAKEFLAGS}"
+MAKEFLAGS="${SANITIZED_MAKEFLAGS}" \
+MFLAGS="${SANITIZED_MFLAGS}" \
+GNUMAKEFLAGS="${SANITIZED_GNUMAKEFLAGS}" \
+ ${CMAKE} --build . --target install -- -j"${N_JOBS}"
Review Comment:
The MAKEFLAGS sanitization logic is fairly complex/regex-driven and still
risks missing or mangling make/jobserver-related flags. Since the goal is to
avoid parent make settings interfering with this nested CMake/make invocation,
it’s more robust (and matches the PR title) to explicitly unset
MAKEFLAGS/MFLAGS/GNUMAKEFLAGS for the build command and drop the extra debug
printing.
##########
r/inst/build_arrow_static.sh:
##########
@@ -114,7 +114,29 @@ ${CMAKE_WRAPPER} ${CMAKE} -DARROW_BOOST_USE_SHARED=OFF \
-G "${CMAKE_GENERATOR:-Unix Makefiles}" \
${SOURCE_DIR}
-${CMAKE} --build . --target install -- -j $N_JOBS
+sanitize_makeflags() {
+ printf '%s\n' "$1" |
+ sed -E \
+ -e 's/(^|[[:space:]])-j[[:space:]]*[0-9]*([[:space:]]|$)/ /g' \
+ -e 's/(^|[[:space:]])([[:alpha:]]*)j[0-9]*([[:alpha:]]*)($|[[:space:]])/
\2\3 /g' \
+ -e 's/[[:space:]]+/ /g' \
+ -e 's/^ //; s/ $//'
+}
+
+env | sort
Review Comment:
`env | sort` dumps the full environment to the build log. This is very noisy
and can inadvertently expose sensitive values (tokens, credentials, etc.)
present in CI environments. It’s safer to remove this and only print the
specific variables needed for debugging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
