Decide what the GitHub Action is written in, before hardening it
- Lingua principale
- Rust
- Stelle
- 407
- Fork
- 12
- Merge medio
- 5h 45m
- PR unite (30g)
- 30
Descrizione
#393 lists work on `action/entrypoint.sh`, and #392 was one bug in it. Neither
asked the question underneath both: should this be a shell script at all? The
answer changes what #393 is — a list of fixes, or a list of things a rewrite
deletes — so it is worth settling first. Nobody has examined it, and #393
should not start until someone does.
This issue is for that decision. It is not a proposal; the options are laid out
so one can be chosen.
## What the script has to do
Download the release archive for the runner's platform, unpack it, and run the
binary with the caller's arguments. Five platforms, three archive layouts, and
a `version` input.
## Facts the options rest on
- GitHub's `ubuntu-24.04` image ships `shellcheck` 0.9.0, Node.js 22, and `gh`.
None of the three options needs a third-party action, which matters after
#388 pinned every one of them to a commit SHA.
- Local `shellcheck` here is 0.10.0 against CI's 0.9.0, so whichever way this
goes, a version worth pinning is a version worth pinning explicitly — as
`rust-toolchain.toml` and the `taplo` workflow already do.
- `shfmt` is *not* on the image; it would have to be installed.
- `shellcheck` currently reports 7 findings on `entrypoint.sh`, all `SC2086`
(quoting). The other repository scripts are covered by #396 and a follow-up.
## Option 1 — keep it a shell script, add `shellcheck` (and `shfmt`)
Run the runner's own `shellcheck` from a `run:` step, pin the version, fix the
7 findings, and do #393 by hand.
- Cheapest, no new language in a Rust repository, no new supply chain.
- Every item in #393 stays hand-written, including the part #393 itself flags
as needing a decision rather than a patch: keying the install by version and
platform, publishing it through `$GITHUB_PATH`, and translating that path
with `cygpath` on Windows.
- `shellcheck` finds quoting; it does not find "this guard tests the wrong
path", which is what #392 and the guard bug actually were.
## Option 2 — keep it a shell script, download with `gh` instead of `wget`
`gh release download "$VERSION" --pattern "$DOWNLOAD_FILE"` fails loudly when
nothing matches, which is exactly how #392 failed.
- Small diff, removes the failure mode that started this.
- Solves nothing else in #393: caching, `$GITHUB_PATH`, Windows, checksums.
- Adds an implicit dependency on `gh` and a token being present, which the
current script does not need.
## Option 3 — a JavaScript action using `@actions/tool-cache`
`runs: using: node20`, `tc.downloadTool` / `tc.extractTar` / `tc.extractZip` /
`tc.cacheDir`, `core.addPath`.
- `tc.cacheDir` is keyed by version and architecture, and `core.addPath`
publishes it. That is #393's "needs a decision, not just a patch" section,
already decided by a library everyone else uses.
- Errors throw by default, temporary directories are the library's problem, and
quoting stops existing — so `SC2086`, the missing `set -e`, the temp-dir item
and the guard item all go away rather than getting fixed.
- **It does not remove the architecture table.** `process.arch` reports `x64`
where the assets say `x86_64`, so a map is still needed. What it removes is
the *per-OS* difference that caused #392: `process.arch` is `arm64` on Linux
and macOS alike, where `uname -m` says `aarch64` on one and `arm64` on the
other. One table instead of a table plus a platform quirk.
- It does not verify checksums for free either; that stays hand-written under
every option.
- The cost is real: a Node toolchain, a bundled `dist/` committed to a Rust
repository, and another dependency surface for Dependabot.
## What this blocks
- #393 — its shape depends entirely on this. Under option 3 most of it closes
unimplemented.
- Whether `shellcheck` in CI is worth doing for `entrypoint.sh`, or only for
the development scripts under `scripts/`, where the stakes are much lower.
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.