diff --git a/.github/actions/npm/action.yml b/.github/actions/npm/action.yml index 35dd1c1..410a8fe 100644 --- a/.github/actions/npm/action.yml +++ b/.github/actions/npm/action.yml @@ -38,4 +38,7 @@ runs: shell: bash if: ${{ inputs.npm_ci }} run: | + # Unquoted on purpose: npm_ci_args is a list of flags and must word + # split into separate arguments. + # shellcheck disable=SC2086 npm ci ${{ inputs.npm_ci_args }} diff --git a/.github/actions/print_contexts/action.yml b/.github/actions/print_contexts/action.yml index 55218c0..995547b 100644 --- a/.github/actions/print_contexts/action.yml +++ b/.github/actions/print_contexts/action.yml @@ -30,14 +30,14 @@ runs: echo '${{ toJson(github) }}' echo "::endgroup::" EOF - + echo "**** env ****" cat <<'EOF' echo "::group::env" echo '${{ toJson(env) }}' echo "::endgroup::" EOF - + echo "**** vars ****" cat <<'EOF' echo "::group::vars" diff --git a/.github/actions/restore_npm_cache/action.yml b/.github/actions/restore_npm_cache/action.yml index a7adbc3..b2e3a87 100644 --- a/.github/actions/restore_npm_cache/action.yml +++ b/.github/actions/restore_npm_cache/action.yml @@ -20,7 +20,7 @@ runs: mkdir -p .nx - name: Restore cache - if: ${{ runner.environment != 'self-hosted' || inputs.restore-on-self-hosted == 'true' }} # don't restore cache on self-hosted runners by default + if: ${{ runner.environment != 'self-hosted' || inputs.restore-on-self-hosted == 'true' }} # don't restore cache on self-hosted runners by default uses: actions/cache/restore@v4 with: # must restore all cache dirs, and they must exist ahead of this! diff --git a/.github/actions/rust-toolchain/action.yml b/.github/actions/rust-toolchain/action.yml index 9dcc8f3..660238c 100644 --- a/.github/actions/rust-toolchain/action.yml +++ b/.github/actions/rust-toolchain/action.yml @@ -19,9 +19,11 @@ runs: echo "Components: $COMPONENTS" # Set outputs for use in subsequent steps - echo "toolchain=$TOOLCHAIN" >> $GITHUB_OUTPUT - echo "targets=$TARGETS" >> $GITHUB_OUTPUT - echo "components=$COMPONENTS" >> $GITHUB_OUTPUT + { + echo "toolchain=$TOOLCHAIN" + echo "targets=$TARGETS" + echo "components=$COMPONENTS" + } >> "$GITHUB_OUTPUT" - uses: dtolnay/rust-toolchain@v1 with: diff --git a/.github/scripts/lint_composite_shell.py b/.github/scripts/lint_composite_shell.py new file mode 100644 index 0000000..445a452 --- /dev/null +++ b/.github/scripts/lint_composite_shell.py @@ -0,0 +1,83 @@ +"""Shellcheck the `run:` blocks of every composite action in this repo. + +actionlint already does this for workflow files, but it ignores action.yml +entirely -- and composite actions are what this repo is made of, so without +this their shell would never be linted at all. + +Two things make a naive `shellcheck <(yq .run)` unusable: + +* `${{ ... }}` is not shell syntax. Left alone it produces a parse error in + every file, which drowns out real findings. It is rewritten to a plain + placeholder variable so the surrounding shell still parses and quoting + problems around the expression are still reported. +* A composite step may declare `shell: pwsh`/`python`/etc. Only bash and sh + steps are handed to shellcheck; the rest are skipped rather than + mis-analysed. + +Exits non-zero if shellcheck reports anything. +""" + +import pathlib +import re +import subprocess +import sys +import tempfile + +import yaml + +EXPRESSION = re.compile(r"\$\{\{.*?\}\}", re.DOTALL) +SHELLS = {"bash", "sh"} +ROOT = pathlib.Path(__file__).resolve().parents[2] + + +def check(path: pathlib.Path, index: int, shell: str, script: str) -> bool: + """Run shellcheck over one step's script. True if it is clean.""" + with tempfile.NamedTemporaryFile("w", suffix=".sh", delete=False) as handle: + handle.write(f"#!/usr/bin/env {shell}\n") + handle.write(EXPRESSION.sub("$GITHUB_EXPRESSION", script)) + temp = pathlib.Path(handle.name) + try: + result = subprocess.run( + ["shellcheck", "--shell", shell, "--color=always", str(temp)], + capture_output=True, + text=True, + ) + finally: + temp.unlink(missing_ok=True) + if result.returncode == 0: + return True + rel = path.relative_to(ROOT) + print(f"::group::{rel} (step {index})") + print(result.stdout.replace(str(temp), f"{rel}:step-{index}"), end="") + print(result.stderr, end="") + print("::endgroup::") + return False + + +def main() -> int: + clean = True + found = 0 + for path in sorted(ROOT.rglob("action.yml")) + sorted(ROOT.rglob("action.yaml")): + document = yaml.safe_load(path.read_text()) + if not isinstance(document, dict): + continue + runs = document.get("runs") + if not isinstance(runs, dict) or runs.get("using") != "composite": + continue + for index, step in enumerate(runs.get("steps") or []): + if not isinstance(step, dict) or "run" not in step: + continue + shell = str(step.get("shell", "bash")) + if shell not in SHELLS: + continue + found += 1 + clean &= check(path, index, shell, str(step["run"])) + + # An empty repo is a pass, not a silent no-op: say so, so a future change + # that stops the walk finding anything is visible rather than green. + print(f"shellchecked {found} composite step(s)") + return 0 if clean else 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..21e598a --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,64 @@ +name: lint + +on: + pull_request: + branches: [main] + push: + branches: [main] + # Lets the workflow be run by hand, which is the only way to get a `check` + # result onto a branch that predates this file. + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + # Named `check` deliberately: the `main` ruleset on this repo requires a status + # context of exactly that name. Until this workflow existed the repo reported + # no checks at all, so the required context could never be satisfied and every + # PR was permanently unmergeable. + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # actionlint validates workflow files and shellchecks the `run:` blocks + # inside them. It does not look at composite action.yml files at all -- + # that gap is what the shellcheck step below covers. + - name: actionlint + run: | + bash <(curl -fsSL https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash) + ./actionlint -color + + - name: yamllint + run: | + pipx install yamllint + yamllint --strict . + + # No toml in this repo today. Deliberate: rust-toolchain.toml, Cargo.toml + # and pyproject.toml all drive builds, and a malformed one fails late and + # confusingly, so the check exists before the first such file arrives. + - name: taplo (toml) + run: | + # Pinned, and not the "-full" asset: that variant is no longer + # published, so a `latest/download/taplo-full-...` URL 404s. + curl -fsSL https://github.com/tamasfe/taplo/releases/download/0.10.0/taplo-linux-x86_64.gz \ + | gunzip | sudo tee /usr/local/bin/taplo >/dev/null + sudo chmod +x /usr/local/bin/taplo + taplo lint + taplo fmt --check --diff + + # Composite actions are the substance of this repo, so their embedded + # shell is the code most worth linting. The extractor rewrites `${{ ... }}` + # to a placeholder first: those expressions are not shell syntax and + # shellcheck otherwise reports parse errors on every file. + - name: shellcheck composite action steps + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq shellcheck + pip install --quiet pyyaml + python3 .github/scripts/lint_composite_shell.py diff --git a/.yamllint.yaml b/.yamllint.yaml new file mode 100644 index 0000000..78bc865 --- /dev/null +++ b/.yamllint.yaml @@ -0,0 +1,20 @@ +# Deliberately loose. The point of linting yaml here is to catch what actually +# breaks a workflow -- duplicate keys, bad indentation, unparseable files -- +# not to enforce a house style on files that already work. +extends: default + +rules: + # GitHub's own schema uses `on:`, which yamllint reads as the boolean true. + truthy: + allowed-values: ["true", "false"] + check-keys: false + line-length: disable + comments-indentation: disable + document-start: disable + # Existing action.yml files mix 2- and 4-space nesting; reformatting them is + # not what this change is for. + indentation: disable + braces: + max-spaces-inside: 1 + empty-lines: + max: 2