Skip to content

GH-50796: [CI][Dev] Fix shellcheck errors in the ci/scripts/r_valgrind.sh - #50798

Merged
kou merged 3 commits into
apache:mainfrom
hiroyuki-sato:topic/shellchck-r_valgrind
Aug 5, 2026
Merged

GH-50796: [CI][Dev] Fix shellcheck errors in the ci/scripts/r_valgrind.sh#50798
kou merged 3 commits into
apache:mainfrom
hiroyuki-sato:topic/shellchck-r_valgrind

Conversation

@hiroyuki-sato

@hiroyuki-sato hiroyuki-sato commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

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-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #50796 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions Bot added the awaiting review Awaiting review label Aug 4, 2026
Comment thread ci/scripts/r_valgrind.sh
@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Aug 4, 2026
@hiroyuki-sato

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-r-linux-valgrind

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Revision: 397dd46

Submitted crossbow builds: ursacomputing/crossbow @ actions-4f596c72c9

Task Status
test-r-linux-valgrind GitHub Actions

@hiroyuki-sato

Copy link
Copy Markdown
Collaborator Author

https://github.com/ursacomputing/crossbow/actions/runs/30874057795/job/91881785985#step:6:23742

t looks like ${INSTALL_ARGS} is not set. I’ll try converting the variable into an array.

R CMD INSTALL '' arrow_25.0.0.9000.tar.gz

@hiroyuki-sato

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-r-linux-valgrind

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Revision: a41f6ca

Submitted crossbow builds: ursacomputing/crossbow @ actions-088bc54f02

Task Status
test-r-linux-valgrind GitHub Actions

Comment thread ci/scripts/r_valgrind.sh Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Aug 4, 2026
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 5, 2026
@hiroyuki-sato

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit test-r-linux-valgrind

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Revision: 3c3e577

Submitted crossbow builds: ursacomputing/crossbow @ actions-3134197238

Task Status
test-r-linux-valgrind GitHub Actions

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@kou
kou merged commit 906bc0a into apache:main Aug 5, 2026
44 checks passed
@kou kou removed the awaiting change review Awaiting change review label Aug 5, 2026
@github-actions github-actions Bot added the awaiting merge Awaiting merge label Aug 5, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 906bc0a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge Awaiting merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants