Page MenuHomePhabricator

[CI] Add `ShellCheck` workflow to Buildkite
ClosedPublic

Authored by abosh on Jul 8 2022, 12:13 PM.
Tags
None
Referenced Files
F3504834: D4485.diff
Fri, Dec 20, 10:44 AM
Unknown Object (File)
Tue, Dec 17, 7:02 AM
Unknown Object (File)
Nov 10 2024, 10:05 PM
Unknown Object (File)
Nov 9 2024, 2:20 PM
Unknown Object (File)
Nov 9 2024, 11:58 AM
Unknown Object (File)
Nov 9 2024, 8:50 AM
Unknown Object (File)
Nov 9 2024, 8:50 AM
Unknown Object (File)
Nov 9 2024, 12:30 AM
Subscribers

Details

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
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

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

Fix typo by removing dash in front of SCRIPTDIR

.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:

image.png (758×1 px, 127 KB)

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
abosh marked 2 inline comments as done.

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.

(this is safe to land, it won't get triggered until it's been added to harbormaster/herald)