Lint the bash inside the workflows, not only the bash in files
The shellcheck step added a few hours ago reads `git ls-files '*.sh'`. That is four files. It does not read the inline `run:` blocks, and a good deal of this repo's bash lives there: the release verification in build.yml, the emulator setup and teardown in status_check.yml and api37-debug.yml. "shellcheck runs in CI" was true of the files and not of the blocks, and CLAUDE.md said so rather than pretending otherwise. actionlint closes that half. It parses each workflow and runs shellcheck over every `run:`, on top of its own checks for expression syntax, `needs:` references, matrix keys and action input names. Pinned by digest, for the reason shellcheck is pinned -- a new rule making untouched files fail is a red build whose diff cannot explain it -- and for a second reason of its own. actionlint's documented install is bash <(curl -s https://raw.githubusercontent.com/.../download-actionlint.bash) off a moving branch. Running that in a repository that pins every action by SHA would contradict its own supply-chain posture more than the linter is worth. That is why #70 was filed instead of bolted onto the shellcheck commit. It reported exactly one finding, and it is fixed here rather than suppressed: build.yml parsed `ls` to pick the release APK (SC2012). The glob was already in the line, so a bash array reads it without the pipe. Gradle's output names have no spaces today, which is the kind of assumption that holds right up until it does not. Proved it catches something, rather than trusting a green run: planting `if [ $UNQUOTED = bad ]` into a build.yml `run:` block produces shellcheck reported issue in this script: SC2086:info:4:6: Removed again afterwards. A linter that cannot be shown to catch a plant is not wired in, it is just running -- and SC2086 in a `run:` block is invisible to the .sh-file step, which is the whole argument for this commit. CLAUDE.md loses the "does not cover inline run: blocks" caveat, because it no longer does. Both linters verified clean at their pinned digests. Closes #70.
This commit is contained in:
@@ -81,7 +81,11 @@ jobs:
|
||||
|
||||
- name: Verify the released artifacts
|
||||
run: |
|
||||
APK=$(ls app/build/outputs/apk/release/*.apk | head -1)
|
||||
# A glob, not `ls | head`: the glob is already here, and parsing ls is what
|
||||
# SC2012 is about. Gradle's names have no spaces today, which is exactly the
|
||||
# kind of assumption that holds until it does not.
|
||||
apks=(app/build/outputs/apk/release/*.apk)
|
||||
APK="${apks[0]}"
|
||||
# A release that shipped one ABI, or lost 16 KB alignment, would install
|
||||
# fine on a test device and fail for users or at Play submission. Both are
|
||||
# cheap to check and expensive to discover later.
|
||||
|
||||
@@ -182,6 +182,23 @@ jobs:
|
||||
docker run --rm "$SHELLCHECK" --version
|
||||
git ls-files -z '*.sh' | xargs -0 -r docker run --rm -v "$PWD:/mnt" "$SHELLCHECK"
|
||||
|
||||
# actionlint closes the half shellcheck cannot see. The step above reads .sh files;
|
||||
# a good deal of this repo's bash lives in inline `run:` blocks instead -- the release
|
||||
# verification here, the emulator setup and teardown in this file and in
|
||||
# api37-debug.yml. actionlint parses each workflow and runs shellcheck over every
|
||||
# `run:`, on top of its own checks for expression syntax, `needs:` references, matrix
|
||||
# keys and action input names.
|
||||
#
|
||||
# Pinned by digest for the same reason shellcheck is, and with a second reason of its
|
||||
# own: actionlint's documented install is `bash <(curl -s .../download-actionlint.bash)`
|
||||
# off a moving branch, which would sit badly in a repo that pins every action by SHA.
|
||||
- name: actionlint
|
||||
env:
|
||||
ACTIONLINT: rhysd/actionlint@sha256:9d36088643581e728c969f35141f88139fec77280b2be23c1f66f8e40e1025e7
|
||||
run: |
|
||||
docker run --rm "$ACTIONLINT" -version
|
||||
docker run --rm -v "$PWD:/repo" -w /repo "$ACTIONLINT" -color
|
||||
|
||||
# `!cancelled()` rather than a plain sequence: a shellcheck failure above must not
|
||||
# cost the ktlint/detekt/lint lists. Same reason this step passes --continue -- one
|
||||
# round trip should produce every list, not stop at the first.
|
||||
|
||||
@@ -185,11 +185,12 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
`podman run --rm -v "$PWD:/mnt:z" docker.io/koalaman/shellcheck@sha256:61862eba... <files>`
|
||||
(the digest is in `status_check.yml`; there is no shellcheck system package on this host).
|
||||
|
||||
**It does not cover inline `run:` blocks in the workflows**, and a good deal of this repo's bash
|
||||
lives there. `actionlint` does cover them — it runs shellcheck over each `run:` — and reports one
|
||||
pre-existing `info` finding in `build.yml`. It is not wired in because every action here is
|
||||
pinned by SHA, and actionlint's usual installer is a `curl | bash` off a moving branch; doing it
|
||||
properly means pinning a container digest. Tracked separately rather than bolted on.
|
||||
**`actionlint` covers the half shellcheck cannot see** — the inline `run:` blocks, where a good
|
||||
deal of this repo's bash lives. It runs shellcheck over each `run:` plus its own checks on
|
||||
expression syntax, `needs:` references, matrix keys and action inputs. It sits in the same job,
|
||||
**pinned by digest** for the reason above and one of its own: its documented installer is a
|
||||
`curl | bash` off a moving branch, which does not belong in a repo that pins every action by SHA.
|
||||
Locally: `podman run --rm -v "$PWD:/repo:z" -w /repo docker.io/rhysd/actionlint@sha256:9d360886... -color`.
|
||||
|
||||
## Dependency versions
|
||||
|
||||
|
||||
Reference in New Issue
Block a user