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.
Differential D4819
[Chore] Make shellcheck happy • jon on Aug 11 2022, 5:04 PM. Authored by Tags None Referenced Files
Details
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. shellcheck CI
Diff Detail
Event TimelineComment 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:
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:
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:
Comment Actions
It's usually a small bash script that a distro provides. There's not a canonical "unix which" |