CI: Use a native version of Shellcheck instead of WASI

[akx]

Summary

Refs #17092 (comment), this switches the CI pipeline to use a native version of Shellcheck pulled from GitHub Releases instead of the WASI compilation.

cc @AlexWaygood

Test Plan

I tried this out in my fork of ruff (where it made a cold-cache pre-commit CI step faster than a hot-cache run), and we're probably about to find out how it works here in upstream land.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[AlexWaygood]

Nice, that's quite an impressive speedup... I knew that the actionlint step was slow (it's why it's only run if you pass --hook-stage=manual), but I didn't realise it was taking up so much time in the GitHub Actions run.

Did you verify that the shellcheck integration is still working as expected? I.e., does it still catch shell-scripting errors embedded in GitHub Actions run: steps?

A thing I don't really like about this approach is that it means you won't be able to run the pre-commit hook manually locally unless you have shellcheck preinstalled. (Or, well, you will be able to run the hook, but it won't actually check the same things as it checks in CI unless you have shellcheck preinstalled.) That seems confusing enough that we'd probably have to document it in our CONTRIBUTING.md file -- but I'd much rather contributors didn't have to install any other tools at all. Sorta the point of pre-commit, in my opinion, is that it makes it very easy to locally run exactly the same commands that are run in CI.

You mentioned in #17092 (comment) that a big reason this might be so slow in CI is because the GitHub Actions runners are starved of cores -- we could see if switching to the larger depot runners helps. We use them in some other jobs already, e.g.

ruff/.github/workflows/ci.yaml

Lines 201 to 203 in d0c8eaa

	cargo-test-linux:
	name: "cargo test (linux)"
	runs-on: depot-ubuntu-22.04-16

[akx]

Did you verify that the shellcheck integration is still working as expected? I.e., does it still catch shell-scripting errors embedded in GitHub Actions run: steps?

Yes – an earlier version of this had cd $(mktemp -d), which shellcheck dutifully did flag me to make cd "$(mktemp -d)" instead.

A thing I don't really like about this approach is that it means you won't be able to run the pre-commit hook manually locally unless you have shellcheck preinstalled.

I was thinking the same – but then again, this check is disabled by default anyway, so I doubt too many contributors would know to run --hook-stage=manual if they hadn't peeked in the pre-commit configuration to find the comment about this. 😅

You mentioned in #17092 (comment) that a big reason this might be so slow in CI is because the GitHub Actions runners are starved of cores

That was, tbh, before I noticed it's running WASM in a Go-based interpreter. 😄

[AlexWaygood]

Yes – an earlier version of this had cd $(mktemp -d), which shellcheck dutifully did flag me to make cd "$(mktemp -d)" instead.

Nice, thank you ❤️

I was thinking the same – but then again, this check is disabled by default anyway, so I doubt too many contributors would know to run --hook-stage=manual if they hadn't peeked in the pre-commit configuration to find the comment about this. 😅

hmm, true. It's still nice for me, personally, to be able to run exactly the same thing locally as is run in CI, though ;-)

I'd like to experiment with switching to the depot runners and see how much that buys us

[akx]

I'd like to experiment with switching to the depot runners and see how much that buys us

Sure, but I wouldn't be holding my breath for great gains there. I can whip up a sibling PR anyway, just a sec.

[AlexWaygood]

Thanks for your work on this!! #17120 got this down to 30s, which I think is "fast enough" now :-D