Skip to content

Commit 906bc0a

Browse files
hiroyuki-satokou
andauthored
GH-50796: [CI][Dev] Fix shellcheck errors in the ci/scripts/r_valgrind.sh (#50798)
### Rationale for this change This is the sub issue #44748. * SC2046: Quote this to prevent word splitting. * SC2086: Double quote to prevent globbing and word splitting. * SC2223: This default assignment may cause DoS due to globbing. Quote it. ``` shellcheck ci/scripts/r_valgrind.sh In ci/scripts/r_valgrind.sh line 21: : ${R_BIN:=RDvalgrind} ^------------------^ SC2223 (info): This default assignment may cause DoS due to globbing. Quote it. In ci/scripts/r_valgrind.sh line 27: pushd ${source_dir} ^-----------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: pushd "${source_dir}" In ci/scripts/r_valgrind.sh line 31: ${R_BIN} CMD INSTALL ${INSTALL_ARGS} arrow*.tar.gz ^-------------^ SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: ${R_BIN} CMD INSTALL "${INSTALL_ARGS}" arrow*.tar.gz In ci/scripts/r_valgrind.sh line 42: if [ $(grep -c "ERROR SUMMARY: 0 errors" testthat.out) != 1 ]; then ^-- SC2046 (warning): Quote this to prevent word splitting. For more information: https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt... https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ... https://www.shellcheck.net/wiki/SC2223 -- This default assignment may cause... ``` ### What changes are included in this PR? * SC2046: Quote variable to prevent word splitting. * SC2086: Quote variable * SC2223: Quote default variable assignments. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: #50796 Lead-authored-by: Hiroyuki Sato <hiroysato@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent 669c374 commit 906bc0a

2 files changed

Lines changed: 12 additions & 7 deletions

File tree

.pre-commit-config.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,7 @@ repos:
345345
?^ci/scripts/r_revdepcheck\.sh$|
346346
?^ci/scripts/r_sanitize\.sh$|
347347
?^ci/scripts/r_test\.sh$|
348+
?^ci/scripts/r_valgrind\.sh$|
348349
?^ci/scripts/release_test\.sh$|
349350
?^ci/scripts/ruby_test\.sh$|
350351
?^ci/scripts/rust_build\.sh$|

ci/scripts/r_valgrind.sh

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,32 @@
1818

1919
set -ex
2020

21-
: ${R_BIN:=RDvalgrind}
21+
: "${R_BIN:=RDvalgrind}"
2222

23-
source_dir=${1}/r
23+
source_dir="${1}/r"
2424

2525
export CMAKE_BUILD_TYPE=RelWithDebInfo
2626

27-
pushd ${source_dir}
27+
pushd "${source_dir}"
28+
29+
# Convert the space-separated options into a Bash array.
30+
# This avoids ShellCheck SC2086 and preserves argument boundaries.
31+
read -r -a R_INSTALL_ARGS <<< "${INSTALL_ARGS:-}"
2832

2933
# build first so that any stray compiled files in r/src are ignored
30-
${R_BIN} CMD build --no-build-vignettes .
31-
${R_BIN} CMD INSTALL ${INSTALL_ARGS} arrow*.tar.gz
34+
"${R_BIN}" CMD build --no-build-vignettes .
35+
"${R_BIN}" CMD INSTALL "${R_INSTALL_ARGS[@]}" arrow*.tar.gz
3236

3337
pushd tests
3438

3539
# to generate suppression files run:
3640
# ${R_BIN} --vanilla -d "valgrind --tool=memcheck --leak-check=full --track-origins=yes --gen-suppressions=all --log-file=memcheck.log" -f testthat.R
37-
${R_BIN} --vanilla -d "valgrind --tool=memcheck --leak-check=full --track-origins=yes --suppressions=/${1}/ci/etc/valgrind-cran.supp" -f testthat.R |& tee testthat.out
41+
"${R_BIN}" --vanilla -d "valgrind --tool=memcheck --leak-check=full --track-origins=yes --suppressions=/${1}/ci/etc/valgrind-cran.supp" -f testthat.R |& tee testthat.out
3842

3943
# valgrind --error-exitcode=1 should return an erroring exit code that we can catch,
4044
# but R eats that and returns 0, so we need to look at the output and make sure that
4145
# we have 0 errors instead.
42-
if [ $(grep -c "ERROR SUMMARY: 0 errors" testthat.out) != 1 ]; then
46+
if ! grep -q "ERROR SUMMARY: 0 errors" testthat.out; then
4347
cat testthat.out
4448
echo "Found Valgrind errors"
4549
exit 1

0 commit comments

Comments
 (0)