[CI] Add `ShellCheck` workflow to Buildkite
ClosedPublic
Actions

Authored by • abosh on Jul 8 2022, 12:13 PM.

Details

Reviewers

atul
tomek
• jacek
• karol
varun

Commits

rCOMM05a909e71e37: [CI] Add `ShellCheck` workflow to Buildkite

Summary

Relevant Linear issue here.

This diff adds ShellCheck to the CI, a static analysis tool for shell scripts that provides warnings and errors.

We install ShellCheck and also set globstar so we can use ** to recursively match all .sh files in the repository. These files are then evaluated by ShellCheck.

Thank you to @atul for your help on this!

Test Plan

Ran in Buildkite and worked as expected. Currently the build is failing since we have so many shell scripts that contain problematic code, but that's expected! You can run it here on Buildkite as well.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• abosh created this revision.Jul 8 2022, 12:13 PM

Herald added subscribers: • adrian, ashoat. · View Herald TranscriptJul 8 2022, 12:13 PM

• abosh edited the test plan for this revision. (Show Details)Jul 8 2022, 12:15 PM

• abosh edited the summary of this revision. (Show Details)

Fix typo by removing dash in front of SCRIPTDIR

• abosh added inline comments.Jul 8 2022, 12:24 PM

.buildkite/shellcheck.yml
5 ↗	(On Diff #14362)	This command allows us to use `**` in the next command so we can recursively match files. Read more on StackExchange.
6 ↗	(On Diff #14362)	These flags are documented on the ShellCheck man page. The `-x` flag allows following any file the script may `source`. The `-P` flag specifies paths to search for sourced files. `SCRIPTDIR` is also explained on the page: The special path SCRIPTDIR can be used to specify the currently checked script's directory, as in source-path=SCRIPTDIR or source-path=SCRIPTDIR/../libs. Multiple paths accumulate, and -P takes precedence over them. Also, these flags are recommended by ShellCheck on the Buildkite page on their wiki:

Harbormaster completed remote builds in B10419: Diff 14361.Jul 8 2022, 12:26 PM

Harbormaster completed remote builds in B10420: Diff 14362.Jul 8 2022, 12:32 PM

• abosh requested review of this revision.Jul 8 2022, 12:32 PM

• abosh attached a referenced file: F96342: image.png. (Show Details)Jul 8 2022, 12:41 PM

Good to go, two things to address (specifying stable is the only one that really matters)

.buildkite/shellcheck.yml
2 ↗	(On Diff #14362)	we can probably just cut this to be consistent with other workflows
9 ↗	(On Diff #14362)	Let's change this to `debian:stable`

This revision is now accepted and ready to land.Jul 8 2022, 12:41 PM

Specify stable for Debian

.buildkite/shellcheck.yml
2 ↗	(On Diff #14362)	The emoji is necessary....🐚🐚 Just kidding, we resolved this in real life. I'll add a `label` to the other Buildkite pipelines.