Add shellcheck across RAPIDS

[gforsyth]

Description

shellcheck is a fast, static analysis tool for shell scripts. It's good at
flagging up unused variables, unintentional glob expansions, and other potential
execution and security headaches that arise from the wonders of bash (and other shlangs).

https://www.shellcheck.net/

Any shell script that is running in CI in a RAPIDS repo should be passing shellcheck.

Benefits of this work

• Fewer chances for shell-scripts to go sideways, or for shell scripts that
  complete to be conflated with shell scripts that have executed correctly.
• Reduction in manual review for the build team of new shell scripts or updated
  shell scripts in PRs, automating "linty" PR suggestions.

Notes

For the initial scope, I propose that we only run shellcheck on scripts in the
ci/ directory, or on any scripts that are called in GHA.

There are some repos with a lot of shell-scripts more related to local
build-process, and these are less critical to "get right" (although that can be
done incrementally in the future).

Further, making several small tweaks to existing local build scripts across
these packages may break things in subtle ways, so it's better to have buy-in
from project maintainers before delving too deeply beyond ci/

Acceptance Criteria

• shellcheck is run in CI against the ci/ directory in all RAPIDS repos that have shell scripts
• All shell scripts in-scope pass shellcheck or have explicit (individual)
  exceptions noted. (No blanket exemptions)

Approach

These repos can be updated in any order, since these updates are all self-contained.

The goal is to have each repo running shellcheck in GHA, similar to https://github.com/rapidsai/gha-tools/blob/main/.github/workflows/prs.yaml#L20-L24

Tasks

Preview Give feedback

1. cucim (Add shellcheck to pre-commit and fix warnings cucim#814)
2. cudf (Add shellcheck to pre-commit and fix warnings cudf#17778)
3. cugraph-docs (Add shellcheck to pre-commit and fix warnings cugraph-docs#80)
4. cugraph-ops repo being archived
5. cuml
6. cuopt
7. cuspatial
8. cuvs
9. cuxfilter
10. dask-cuda (Add shellcheck to pre-commit and fix warnings dask-cuda#1427)
11. dependency-file-generator
12. jupyterlab-nvdashboard
13. kvikio
14. legate-boost
15. legate-dataframe
16. legate-raft
17. nx-cugraph
18. pre-commit-hooks
19. pynvjitlink
20. raft
21. rapids-cmake
22. rmm (Add shellcheck to pre-commit and fix warnings rmm#1788)
23. ucx-py
24. ucxx
25. wholegraph

[jameslamb]

Hey thanks for putting this up! I totally support it.

But instead of invoking a separate GitHub action, I think we should do this via the shellcheck-py pre-commit hook.

repos:
  - repo: https://github.com/shellcheck-py/shellcheck-py
    rev: v0.10.0.1
    hooks:
      - id: shellcheck

I've been sprinkling that around some repos for the last couple months and have found it works great... lightweight to install, runs fast, exactly matches the behavior of other shellcheck packages like those from conda-forge or Homebrew.

For example:

https://github.com/rapidsai/deployment/blob/65e6acf22b7ade745c8822bda76bc535e4b53857/.pre-commit-config.yaml#L28

https://github.com/rapidsai/legate-boost/blob/b9462f8d77c2540ba44393bc04b9f75ad3103d9a/.pre-commit-config.yaml#L10-L13

Doing that via pre-commit means you can get local feedback instead of needing to wait for a CI run.

[gforsyth]

Sounds good to me -- do we also run pre-commit in CI to ensure files are compliant?

[jameslamb]

Yep!

By convention, every RAPIDS project uses a workflow called checks:

  checks:
    secrets: inherit
    needs: telemetry-setup
    uses: rapidsai/shared-workflows/.github/workflows/[email protected]
    with:
      enable_check_generated_files: false
      ignored_pr_jobs: "telemetry-summarize"

(example from cudf)

Which looks for a script named ci/check_style.sh (code link in rapidsai/shared-workflows). (a bit of a misnomer because lots of static analysis catches more than just style, like correctness, efficiency, deprecations, etc., but anyway....)

Most repos then have content in that check_style.sh that runs pre-commit run --all-files (example from cudf).

So that all happens by convention but you can generally safely assume that every RAPIDS repo is running pre-commit run --all-files on every pull request.

[gforsyth]

I pushed up an example commit of what this might look like here for rmm: gforsyth/rmm@02f9758