feat: install LLGo releases and refs across native platforms - #50
Conversation
There was a problem hiding this comment.
Review Summary
This is a substantial, well-executed rewrite of the install path. Strong improvements over the prior code: version resolution moved to a strict, well-tested allow-list (resolve.ts), git commands migrated from string-interpolated execSync to argument-array execFileSync (removing the shell-injection surface), SHA-256 checksum verification added for release downloads, HTTPS-only downloads with a redirect cap, and the old bug that rmSync'd the user's ~/workdir is gone. Test coverage in resolve.ts/install.test.ts/platform.test.ts is thorough, including command-injection-style inputs.
Findings are mostly low severity; none are blocking. The highest-value item is the download timeout gap (inline on src/download.ts).
Additional (no reliable inline anchor):
- Documentation —
README.md:63— "Hosted runners supply Node.js 20+" is an unverified/unenforced minimum. This same PR standardizes every CI/lint workflow on Node 24, and there's noenginesconstraint or runtime check enforcing the stated floor. Either verify/document the true minimum or align the wording with Node 24 so self-hosted-runner operators aren't misled. - Retry/backoff consistency —
download()(src/download.ts) and the release-metadatafetch(src/install.ts) have a timeout but no bounded retry, while the workflow invests heavily in retries elsewhere (curl --retry,pacman_with_retry). A single transient blip fails the whole multi-minute job. Consider a small bounded retry on these two network operations. (404/non-2xx correctly short-circuit and should not be retried.)
Nice work overall.
|
Addressed the two additional findings in the review summary in 16e5471:
All 60 tests pass on Node.js 20 and 24 across Linux, macOS and Windows. TypeScript, ESLint, formatting, actionlint and the rebuilt bundle check pass. The final upstream 27-job CI matrix passes, including all eight OS/architecture/ABI combinations through both release and source installation, plus the five selector jobs. llcppg CI also passes on macOS and Ubuntu using this exact action commit. |
LLGo installation previously built every selected version from source using hard-coded Go 1.20 and LLVM 18, and reused/deleted
~/workdir. This change installs current LLGo on native Linux, macOS and Windows hosts without touching existing user directories.releaseorsource.// llgodirectives in go.mod/go.work, plusgo-version-file, following setup-xgo input conventions. Verify the installed release patch version.Validation:
llgo testand a compiled executable; Windows also checks the target triple. Selector jobs exercise prefixes, patterns, ranges and commit forms.This action is consumed by llcppg#854, whose macOS and Ubuntu LLGo coverage tests both pass.
Compared with setup-xgo: preserve its existing SemVer/branch and version-file conventions while adding commit selection, precompiled releases, proper composite output mappings, and explicit native architecture/ABI execution checks.
The final upstream 27-job CI matrix passes: 16 native release/source installations, five selector jobs, and six Node.js 20/24 test jobs across Linux, macOS and Windows. Lint and generated-bundle checks also pass.