ci: use supported golangci-lint installer - #629
Closed
diegomrsantos wants to merge 2 commits into
Closed
Conversation
Contributor
Greptile SummaryThis PR pins
Confidence Score: 5/5Safe to merge — the change is limited to the lint toolchain setup and does not touch any Go source, generated files, or test behaviour. The only modified file is the Makefile, and both changes (pinning the version and correcting the -- separator) are straightforward improvements. No application logic, generated spec-test output, or CI workflow files are affected. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[make lint-prepare] --> B["curl golangci-lint install.sh"]
B --> C{"GOLANGCI_LINT_VERSION set?"}
C -- "Yes (env override)" --> D["Install specified version"]
C -- "No (default)" --> E["Install v2.11.4"]
D --> F["./bin/golangci-lint"]
E --> F
F --> G[make lint]
G --> H["./bin/golangci-lint run -v ./..."]
Reviews (1): Last reviewed commit: "ci: pin golangci-lint version" | Re-trigger Greptile |
ldez
reviewed
May 7, 2026
| .PHONY: lint-prepare | ||
| lint-prepare: | ||
| curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s latest | ||
| curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- $(GOLANGCI_LINT_VERSION) |
There was a problem hiding this comment.
Suggested change
| curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- $(GOLANGCI_LINT_VERSION) | |
| curl -sfL https://golangci-lint.run/install.sh | sh -s latest |
ldez
reviewed
May 7, 2026
| @@ -1,9 +1,10 @@ | |||
| GOPATH?=$(shell go env GOPATH) | |||
| TEST_PKG?=./... | |||
| GOLANGCI_LINT_VERSION?=v2.11.4 | |||
There was a problem hiding this comment.
Suggested change
| GOLANGCI_LINT_VERSION?=v2.11.4 |
diegomrsantos
changed the title
This was referenced May 7, 2026
vaclav-ssvlabs
requested review from
GalRogozinski and
MatheusFranco99
as code owners
May 25, 2026 15:15
vaclav-ssvlabs
force-pushed
the
spec-test-ci
branch
2 times, most recently
from
May 25, 2026 15:24
7379ff6 to
12e3438
Compare
|
This pull request has been marked as stale due to 60 days of inactivity. |
Contributor
|
Heads-up: #635 (branch The reason it came up there:
Leaving this open for now — if #635 lands first, this one should be closed rather than merged, since merging both would revert the pin to |
GalRogozinski
added a commit
that referenced
this pull request
Aug 6, 2026
The install.sh served from the golangci-lint master branch fails checksum verification for v2 releases, so 'make lint-prepare' pinned to v2.12.2 installed nothing: hash_sha256_verify checksum for golangci-lint-2.12.2-darwin-arm64.tar.gz did not verify https://golangci-lint.run/install.sh installs the same pinned version successfully. Same change as #629, kept together with the version pin so local and CI stay on one version.
Contributor
|
Superseded — closing rather than merging, so the version pin survives. The installer fix landed on Your diagnosis was right and it went further than the URL: pinning to the version Reopen if you'd rather land it from here instead — happy to revert that hunk and take yours with the pin on top. |
GalRogozinski
added a commit
that referenced
this pull request
Aug 11, 2026
The install.sh served from the golangci-lint master branch fails checksum verification for v2 releases, so 'make lint-prepare' pinned to v2.12.2 installed nothing: hash_sha256_verify checksum for golangci-lint-2.12.2-darwin-arm64.tar.gz did not verify https://golangci-lint.run/install.sh installs the same pinned version successfully. Same change as #629, kept together with the version pin so local and CI stay on one version.
GalRogozinski
added a commit
that referenced
this pull request
Aug 11, 2026
* move jsons outside to spec-tests folder * move spec tests outside * sync PRs across repos * create branch on demand * adjust the worflows to match the requirements * ci: add spec-tests sync workflows Adds GitHub Actions workflows to automatically sync generated spec test JSON files to a dedicated spec-tests repository on PR open and merge. - generate-spec-tests composite action: sets up Go, runs generators for ssv/qbft/types spectests, copies output to ../spec-tests - sync-spec-tests-pr: on PR open/sync, pushes generated files to a matching branch in spec-tests repo and creates/updates a PR - sync-spec-tests-merge: on push to main, regenerates files and merges the corresponding spec-tests PR - test.yaml: bumped to ubuntu-24.04, node24-compatible action versions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * applying requested reviews * add readme about the new pipeline * resolve no test has changed edge case * ci: use golangci-lint-action with pinned version The install.sh from golangci-lint master is broken (downloads SBOM but verifies against tarball checksum). Use the official action instead. * Anchor spec test generator paths * ci: harden spec-tests sync workflows Addresses review feedback on #610: - Split sync-spec-tests-pr into two jobs. `generate` runs the PR-controlled `go generate` and uploads a tarball; `sync` downloads it and pushes with the spec-tests App token. The token no longer shares a runner with code from the PR. - Enforce a strict shape on HEAD_REF before it reaches `git checkout -B`, `git push` and `gh pr create --head` (rejects leading `-`, `..`, `HEAD`, and anything outside [A-Za-z0-9._/-]). - Look up the mirror PR with `gh pr list --state open` instead of `gh pr view <branch>`, which can return a closed PR from the same branch. Same fix in sync-spec-tests-merge. - Comment the invariant that PR_TITLE/HEAD_REF must stay in `env:` and never be inlined as `${{ }}` in the shell script. - Use the full PR URL in the sync commit message so it does not auto-link to an unrelated spec-tests PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop in-repo JSON test vectors The merge in 5354bf5 resurrected 269 generated JSON files under ssv/spectest/generate/{tests,state_comparison} and types/spectest/generate/{tests,state_comparison}, which 860caf4 had deleted. main had regenerated them via #601. Because the composite action also copied any in-repo */spectest/generate/{tests,state_comparison} dir into the output, spec-tests received every vector twice, under two path shapes: types/tests/x.json <- anchored generator types/spectest/generate/tests/x.json <- copied from the repo Since cc74c02 the generators write straight to ../spec-tests/<module> via SpecTestsDirForModule and MkdirAll their own output, and readers resolve through SpecTestsDirFrom. Nothing reads the in-repo paths, so delete the vectors and the redundant copy step, keeping the guard that generation actually produced output. Verified: a from-scratch `go generate` yields only <module>/{tests, state_comparison} (2533 files, no spectest/generate shape), and `make test` passes on all 8 packages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: document local spec-test generation and sibling spec-tests dir Add a Running Locally section to the CI workflow docs covering the <parent-of-ssv-spec>/spec-tests/<module> layout, the generate-before-test order, and the shared-directory caveat for multiple checkouts. Point to it from the root README. * ci: pin local golangci-lint to the CI version make lint-prepare installed 'latest' while lint.yaml pins v2.12.2, so local and CI could run different linter versions. Pin the Makefile to the same version via GOLANGCI_LINT_VERSION. * ci: validate PR_HEAD_REF before syncing to spec-tests sync-spec-tests-merge.yaml used the merged PR's head ref unvalidated in git checkout -B, git push --force and gh pr list --head. head.ref is a bare branch name, so a fork PR opened from the fork's default branch yields 'main'; origin/main always exists, so the missing-mirror-branch check at line 130 cannot catch it and the run force-pushes generated fixtures onto spec-tests main outside the mirror PR. Apply the same shape check and main/master refusal that sync-spec-tests-pr.yaml already performs, at the point the ref is resolved. * ci: use the supported golangci-lint installer The install.sh served from the golangci-lint master branch fails checksum verification for v2 releases, so 'make lint-prepare' pinned to v2.12.2 installed nothing: hash_sha256_verify checksum for golangci-lint-2.12.2-darwin-arm64.tar.gz did not verify https://golangci-lint.run/install.sh installs the same pinned version successfully. Same change as #629, kept together with the version pin so local and CI stay on one version. * ci: add concurrency group to PR spec-tests sync Two pushes in quick succession force-push the same mirror branch with no ordering, so an older commit's fixtures can land last and leave the mirror stale until the next push. * ci: skip fork PRs in pre-merge sync, sync them after merge Fork PRs get no secrets on pull_request, so the pre-merge sync workflow never worked for them and the merge workflow failed loudly instead. Post-merge, fork code is already trusted: a human reviewed and merged it, and the merge workflow already runs go generate from main in the same job that holds the App token. Pre-merge mirroring for forks would let any GitHub user trigger App-token writes to spec-tests without review, which is the risk this change avoids. * ci: compute is-fork before the PR_HEAD_REF guard in merge sync The main/master and unsafe-shape checks on PR_HEAD_REF ran before is-fork was known, so a fork PR opened from its own default branch (head ref "main") tripped the protected-name refusal and never reached the fork sync path. Move the is-fork computation earlier and skip that validation for fork PRs, since their head ref is never used as a push/checkout target on the fork path. * ci: rely on whole-tree sync for fork fixtures instead of a fork push path Each sync in this step replaces the entire generated tree from main, so the next same-repo merge already carries a fork's fixtures into spec-tests. A dedicated fork push path only bought a shorter staleness window at the cost of a second, unaudited write path into spec-tests — skip the step for fork-originated merges instead. * docs: correct stale fork-path comments after simplification * ci: fail closed on unknown PR head repo in merge sync Codex review of the fork policy found two defects and one wrong claim: - jq's 'empty' suppresses the whole object, so head_repo: (.head.repo.full_name // empty) collapsed PR_INFO when the head repository is gone (deleted fork), making the run exit 'No PR found' and never reaching the fork check. Use "" instead. - Unknown head repo now means 'not this repo': skip the sync. There is no mirror branch to finalize in that case, and treating it as same-repo risked pushing to an unrelated mirror branch. - The push step gate is 'is-fork == false' rather than '!= true', so a missing output fails closed. README: the 'next in-repo merge' guarantee was too strong. An in-repo PR with no fixture diff never pushes a mirror branch, so if it merges next its merge run fails loudly instead of syncing; the sync lands on the following in-repo merge that has a mirror branch. * docs: fix composite action path and dead App setup link; fail tests cleanly - README Files table: ../actions/... not ../.github/actions/..., which resolved to .github/.github/actions. - Drop the link to a GITHUB_APP_SETUP.md that does not exist in this repo; point at GitHub's own App registration docs and name the two secrets. - qbft and types spectest TestJson: t.Fatalf instead of panic on a spec-tests dir resolution failure, matching ssv/spectest/run_test.go. * ci: make lint-prepare fail when the installer download fails Codex review of the branch found three issues in what we changed: - 'curl ... | sh' reports the exit status of sh, and sh with empty input exits 0, so a failed download looked like a successful install. Download to a temp file and run it as an &&-chain instead (verified: a 404 URL now exits 56 rather than 0). - The fork comment in the merge sync claimed fixtures reach spec-tests with 'the next same-repo merge', contradicting the gap documented in README.md. - The workflows README overview claimed every in-repo PR keeps a mirror PR open; PRs whose fixtures match spec-tests never create one. * ci: pin third-party actions to commit SHAs Tags are mutable refs: whoever controls the action repo can retag to different code, which then runs inside jobs holding the spec-tests App private key. Pin checkout, upload/download-artifact, create-github-app-token, setup-go, and golangci-lint-action to their resolved commit SHAs (with the version as a trailing comment). Add .github/dependabot.yml for the github-actions ecosystem so these pins keep getting bumped instead of going stale. * ci: let Dependabot see the composite action github-actions with directory '/' scans .github/workflows and a root action.yml only, so the setup-go pin in .github/actions/generate-spec-tests would never be bumped. Add that path explicitly. --------- Co-authored-by: Gal Rogozinski <galrogogit@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates the golangci-lint installer URL from the retired
masterbranch script to the supported installer URL recommended by upstream. This keeps the existinglatestbehavior while avoiding the checksum matching issue triggered by newer release assets.The v2.12.2 tarball checksum itself appears valid; the failure is in the old installer path used by this repo. Upstream closed the issue as a duplicate and recommended this installer URL here: golangci/golangci-lint#6572 (comment)
No sync workflow behavior or generated spec-test behavior is changed.
Verification: installed the current latest linter locally through the supported installer URL and ran the lint target successfully.