Shellcheck is failing on GitHub
Feel like I should have known this, as we use command -v in nixpkgs as well. Guess I just overlooked it because I wanted to follow the upstream patch as much as possible.
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- jonringer/fix-shellcheck
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Actually just saw this on GitHub, was going to send you a message about it, and saw this diff! Thanks for catching this.
For the sake of clarity, ShellCheck was complaining that which is non-standard. Use builtin command -v instead. The wiki also notes:
This is an optional suggestion. It must be explicitly enabled with a directive enable=deprecate-which in a # shellcheck comment or .shellcheckrc
I did not know which was non-standard, but it makes sense since its intended to be a human-readable external tool that uses each user's PATH. Not really familiar with command -v since I always use which, but I read these two resources and it made sense to me:
- https://github.com/koalaman/shellcheck/issues/1162
- https://stackoverflow.com/questions/37056192/which-vs-command-v-in-bash
Also you should probably add @atul as reviewer for all CI/GitHub Actions related things.
Comment Actions
What's more confusing to me is why this passed through lint-staged and Buildkite, but failed on GitHub Actions. They should be running the same shellcheck -x -P SCRIPTDIR [FILES...] command...
Comment Actions
Ah, investigated with @atul and figured out why this is happening. ShellCheck on lint-staged is installed by homebrew (locally, by developers), ShellCheck on Buildkite is installed using apt-get, and ShellCheck on GitHub Actions is preinstalled using ubuntu-latest. Because this check of using command -v over which is optional in versions of ShellCheck 0.7.1+ (which the homebrew and apt-get installations of ShellCheck are), this was not caught by those CI builds, but was caught by GitHub Actions since the version of ShellCheck on ubuntu-latest is 0.7.0, which is < 0.7.1. To fix this, we're bumping the ShellCheck job on GitHub Actions to run off ubuntu-22.04 instead of ubuntu-latest (which is 20.04). This is because the version of ShellCheck that comes with ubuntu-22.04 is 0.8.0-2, which is 0.7.1+.
Also, we could install ShellCheck through apt-get on GitHub Actions to keep it consistent with Buildkite. To do this, we'd just have to revert D4756. Edit: it's been done, see D4828.
More context from the ShellCheck wiki for this error:
This check is opt-in only in 0.7.1+, and you may choose to ignore it in earlier versions. which is very common, and some prefer its executable-or-nothing behavior over command -v's handling of builtins, functions and aliases.
Comment Actions
I did not know which was non-standard
It's usually a small bash script that a distro provides. There's not a canonical "unix which"