From 054719675b559b910b62a7322a01af4bac8f77c0 Mon Sep 17 00:00:00 2001 From: Albert Bausili Date: Sat, 3 Oct 2026 05:00:17 +0200 Subject: [PATCH 1/2] fix(mage): Tools installs h2spec 2.6.0 on every platform and prints the version it ended with; pin the mage and benchstat hints (celeris#838) mage Tools fell back to `go install .../h2spec@latest`, which resolves to v2.2.1+incompatible (it reports Version: 2.0.0, one test case fewer), and its download path fetched the amd64 tarball on every architecture, so an arm64 Mac or Linux host got a binary that cannot run, with rc=0. It also accepted any h2spec already on PATH. Tools now builds the commit the v2.6.0 tag points at, as a pseudo-version the module proxy serves, with the -ldflags of h2spec's own Makefile, on every GOOS/GOARCH. It checks the version the result reports and the one PATH resolves, fails with the release URL when either is not 2.6.0, and prints the version and path it ended with. CONTRIBUTING pins mage to v1.17.2 (what CI installs) and names CI's golangci-lint v2.13; bench-ab.sh runs the benchstat that .github/tools pins with `go tool`, and Dependabot bumps it there. --- .github/dependabot.yml | 5 +- .github/scripts/bench-ab.sh | 6 ++- .github/tools/go.mod | 17 ++++-- .github/tools/go.sum | 4 ++ CONTRIBUTING.md | 8 +-- magefile.go | 100 +++++++++++++++++++++++++++--------- 6 files changed, 106 insertions(+), 34 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 18577d95a..668837885 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -32,7 +32,8 @@ updates: # ----- CI tools module ----- # .github/tools pins the govulncheck that the Vulnerability Check job - # runs, through its go.sum (celeris#827). Go records every requirement + # runs, and the benchstat that .github/scripts/bench-ab.sh runs, through + # its go.sum (celeris#827, celeris#838). Go records every requirement # of a module that only holds `tool` directives as `// indirect`, # golang.org/x/vuln included. Version updates cover direct # dependencies only unless `allow` says otherwise, so without the rule @@ -49,6 +50,8 @@ updates: allow: - dependency-name: golang.org/x/vuln dependency-type: all + - dependency-name: golang.org/x/perf + dependency-type: all schedule: interval: daily groups: diff --git a/.github/scripts/bench-ab.sh b/.github/scripts/bench-ab.sh index 5b139c38f..5f3e7754e 100755 --- a/.github/scripts/bench-ab.sh +++ b/.github/scripts/bench-ab.sh @@ -38,7 +38,8 @@ # Environment: BENCHTIME (default 1s, as CodSpeed runs it), OUT (a directory # to keep the raw outputs in; it must not hold an earlier run's A.txt, B.txt # or A2.txt; default a temporary one, removed on exit). -# Needs benchstat: go install golang.org/x/perf/cmd/benchstat@latest +# benchstat is the one .github/tools/go.mod pins (go.sum, Dependabot); `go tool` +# builds it on first use, so there is nothing to install (celeris#838). set -euo pipefail if [ "$#" -lt 4 ]; then @@ -47,9 +48,10 @@ if [ "$#" -lt 4 ]; then fi base=$1 head=$2 pkg=$3 re=$4 rounds=${5:-20} benchtime=${BENCHTIME:-1s} -command -v benchstat > /dev/null || { echo "benchstat not found: go install golang.org/x/perf/cmd/benchstat@latest" >&2; exit 2; } repo=$(git rev-parse --show-toplevel) +benchstat() { go tool -modfile="$repo/.github/tools/go.mod" benchstat "$@"; } +go tool -modfile="$repo/.github/tools/go.mod" -n benchstat > /dev/null || { echo "cannot build benchstat from $repo/.github/tools/go.mod" >&2; exit 2; } tmp=$(mktemp -d) cleanup() { git -C "$repo" worktree remove --force "$tmp/a" 2> /dev/null || true diff --git a/.github/tools/go.mod b/.github/tools/go.mod index f48ddf74c..cde4e6b81 100644 --- a/.github/tools/go.mod +++ b/.github/tools/go.mod @@ -1,16 +1,23 @@ -// Versions of the Go tools CI runs, kept out of the celeris module so they -// add nothing to its dependency graph. Each is a `tool` directive, pinned by -// go.sum and bumped by Dependabot (.github/dependabot.yml); a workflow runs -// one with `go tool -modfile=$GITHUB_WORKSPACE/.github/tools/go.mod `. +// Versions of the Go tools CI and the repo's scripts run, kept out of the +// celeris module so they add nothing to its dependency graph. Each is a +// `tool` directive, pinned by go.sum and bumped by Dependabot +// (.github/dependabot.yml); a workflow runs one with +// `go tool -modfile=$GITHUB_WORKSPACE/.github/tools/go.mod `, and +// .github/scripts/bench-ab.sh runs benchstat the same way (celeris#838). // This module is never imported or released (celeris#827). module celeris-ci-tools go 1.27.0 -tool golang.org/x/vuln/cmd/govulncheck +tool ( + golang.org/x/perf/cmd/benchstat + golang.org/x/vuln/cmd/govulncheck +) require ( + github.com/aclements/go-moremath v0.0.0-20210112150236-f10218a38794 // indirect golang.org/x/mod v0.41.0 // indirect + golang.org/x/perf v0.0.0-20260929162123-406019bb8b68 // indirect golang.org/x/sync v0.23.0 // indirect golang.org/x/sys v0.48.0 // indirect golang.org/x/telemetry v0.0.0-20260908163034-4bcc4b2ee518 // indirect diff --git a/.github/tools/go.sum b/.github/tools/go.sum index a354dd078..25e8b610f 100644 --- a/.github/tools/go.sum +++ b/.github/tools/go.sum @@ -1,3 +1,5 @@ +github.com/aclements/go-moremath v0.0.0-20210112150236-f10218a38794 h1:xlwdaKcTNVW4PtpQb8aKA4Pjy0CdJHEqvFbAnvR5m2g= +github.com/aclements/go-moremath v0.0.0-20210112150236-f10218a38794/go.mod h1:7e+I0LQFUI9AXWxOfsQROs9xPhoJtbsyWcjJqDd4KPY= github.com/google/go-cmdtest v0.4.1-0.20220921163831-55ab3332a786 h1:rcv+Ippz6RAtvaGgKxc+8FQIpxHgsF+HBzPyYL2cyVU= github.com/google/go-cmdtest v0.4.1-0.20220921163831-55ab3332a786/go.mod h1:apVn/GCasLZUVpAJ6oWAuyP7Ne7CEsQbTnc0plM3m+o= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= @@ -6,6 +8,8 @@ github.com/google/renameio v0.1.0 h1:GOZbcHa3HfsPKPlmyPyN2KEohoMXOhdMbHrvbpl2QaA github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c= golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o= +golang.org/x/perf v0.0.0-20260929162123-406019bb8b68 h1:nyQ/b4GTH65GoH7T/JFxssBadcBB57DTy6JNyQEfZBA= +golang.org/x/perf v0.0.0-20260929162123-406019bb8b68/go.mod h1:Pth32a9JhKKavemj73LtFqHHyyMhqz+K7tUZcG5tTWM= golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c3b1a81bc..998d9f16c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,10 +16,12 @@ Thank you for your interest in contributing to celeris! - Go 1.27.0+ — the `go` directive in `go.mod` is the single source of truth for the floor, and CI pins the same version. Quote it from there rather than restating it here, so the two cannot drift apart again. -- [Mage](https://magefile.org) build tool: `go install github.com/magefile/mage@latest` +- [Mage](https://magefile.org) build tool: `go install github.com/magefile/mage@v1.17.2`, the version CI + installs (`.github/workflows/ci.yml`) - Linux (for io_uring/epoll engine tests) or macOS (std engine only) -- [golangci-lint](https://golangci-lint.run/) v2.9+ -- [h2spec](https://github.com/summerwind/h2spec) (installed automatically by `mage tools`) +- [golangci-lint](https://golangci-lint.run/) v2.13, the version CI's lint job runs +- [h2spec](https://github.com/summerwind/h2spec) 2.6.0: `mage tools` installs it and prints the version it + ended with ### Build & Test diff --git a/magefile.go b/magefile.go index dce1d4534..fff9bcc90 100644 --- a/magefile.go +++ b/magefile.go @@ -77,37 +77,91 @@ func CleanBenchmarks() error { return nil } -// Tools installs external test tools (h2spec). +// The h2spec that `mage Tools` installs (celeris#838). h2spec's tags after +// v2.2.1 carry a go.mod without the /v2 suffix, so the module proxy has no +// version for them: `go install .../h2spec@v2.6.0` is refused, and `@latest` +// resolves to v2.2.1+incompatible, an older test set. The commit the v2.6.0 +// tag points at is still reachable as a pseudo-version, and building it with +// the -ldflags of h2spec's own Makefile gives the release's code and its +// version string, checked against the Go checksum database like any module, for +// every GOOS/GOARCH (the release ships amd64 binaries only). +const ( + h2specVersion = "2.6.0" + h2specCommit = "70ac2294010887f48b18e2d64f5cccd48421fad1" + h2specModule = "github.com/summerwind/h2spec/cmd/h2spec@v1.5.1-0.20200804131034-70ac22940108" + h2specRelease = "https://github.com/summerwind/h2spec/releases/tag/v2.6.0" +) + +// Tools installs external test tools (h2spec 2.6.0) and prints the version +// it ended with. It fails, rather than settle for another version, when it +// cannot install 2.6.0 or when PATH finds a different h2spec first. func Tools() error { - if _, err := exec.LookPath("h2spec"); err == nil { - fmt.Println("h2spec: already installed") - return nil + if path, err := exec.LookPath("h2spec"); err == nil { + v := h2specVersionOf(path) + if v == h2specVersion { + fmt.Printf("h2spec: %s at %s (already installed)\n", v, path) + return nil + } + fmt.Printf("h2spec: %s reports version %q, not %s; installing %s\n", path, v, h2specVersion, h2specVersion) } - fmt.Println("Installing h2spec v2.6.0...") - - goos := runtime.GOOS - goarch := "amd64" - - if goos != "linux" && goos != "darwin" { - fmt.Println("No prebuilt h2spec binary; trying go install...") - return run("go", "install", "github.com/summerwind/h2spec/cmd/h2spec@latest") + fmt.Printf("Installing h2spec %s: go install %s\n", h2specVersion, h2specModule) + ldflags := fmt.Sprintf("-ldflags=-X main.VERSION=%s -X main.COMMIT=%s", h2specVersion, h2specCommit) + if err := run("go", "install", ldflags, h2specModule); err != nil { + return fmt.Errorf("h2spec %s: go install failed (%w); download the release binary from %s and put it on PATH", h2specVersion, err, h2specRelease) } - - gopath, err := output("go", "env", "GOPATH") + binDir, err := goBinDir() + if err != nil { + return err + } + exe := "h2spec" + if runtime.GOOS == "windows" { + exe += ".exe" + } + installed := filepath.Join(binDir, exe) + if v := h2specVersionOf(installed); v != h2specVersion { + return fmt.Errorf("h2spec: %s reports version %q after the install, not %s; download the release binary from %s", installed, v, h2specVersion, h2specRelease) + } + path, err := exec.LookPath("h2spec") if err != nil { - return fmt.Errorf("GOPATH: %w", err) + return fmt.Errorf("h2spec %s is at %s, but %s is not on PATH, so TestH2Spec would skip; add it to PATH", h2specVersion, installed, binDir) } - binDir := filepath.Join(gopath, "bin") + if v := h2specVersionOf(path); v != h2specVersion { + return fmt.Errorf("h2spec %s is at %s, but PATH finds %s (version %q) first; remove it or put %s first on PATH", h2specVersion, installed, path, v, binDir) + } + fmt.Printf("h2spec: %s at %s\n", h2specVersion, path) + return nil +} - tarball := fmt.Sprintf("h2spec_%s_%s.tar.gz", goos, goarch) - url := fmt.Sprintf("https://github.com/summerwind/h2spec/releases/download/v2.6.0/%s", tarball) +// h2specVersionOf returns the version an h2spec binary reports +// ("Version: 2.6.0 ()" gives "2.6.0"), or "" when it cannot run. +func h2specVersionOf(path string) string { + out, err := exec.Command(path, "--version").Output() + if err != nil { + return "" + } + v, ok := strings.CutPrefix(strings.TrimSpace(string(out)), "Version: ") + if !ok { + return "" + } + v, _, _ = strings.Cut(v, " ") + return v +} - if err := run("bash", "-c", - fmt.Sprintf("curl -fsSL '%s' | tar xz -C '%s' h2spec", url, binDir)); err != nil { - fmt.Println("Binary download failed; trying go install...") - return run("go", "install", "github.com/summerwind/h2spec/cmd/h2spec@latest") +// goBinDir returns the directory `go install` writes to: GOBIN, or the bin +// directory of GOPATH's first entry. +func goBinDir() (string, error) { + gobin, err := output("go", "env", "GOBIN") + if err != nil { + return "", fmt.Errorf("go env GOBIN: %w", err) } - return nil + if gobin != "" { + return gobin, nil + } + gopath, err := output("go", "env", "GOPATH") + if err != nil { + return "", fmt.Errorf("go env GOPATH: %w", err) + } + return filepath.Join(filepath.SplitList(gopath)[0], "bin"), nil } // H2Spec runs HTTP/2 conformance tests using h2spec across all engines. From fe509c870f499151a1ee68df43180dc21ec9a6c0 Mon Sep 17 00:00:00 2001 From: Albert Bausili Date: Sat, 3 Oct 2026 06:29:46 +0200 Subject: [PATCH 2/2] fix(mage): Tools checks the installed h2spec's build info against the pinned module version; bench-ab.sh runs benchstat with GOWORK=off (celeris#838) The 2.6.0 version string is the -ldflags stamp Tools itself applies, so it said 2.6.0 for any code. Tools now reads the installed binary's build info and requires github.com/summerwind/h2spec@v1.5.1-0.20200804131034-70ac22940108, and prints the module it was built from. go refuses -modfile in workspace mode, so bench-ab.sh stopped for a contributor with a go.work; both go tool calls now run with GOWORK=off. golang.org/x/perf has no tagged release and Dependabot's version updates propose tagged versions only: the tools go.mod and dependabot.yml now say the benchstat pin moves by hand. --- .github/dependabot.yml | 4 ++++ .github/scripts/bench-ab.sh | 10 ++++---- .github/tools/go.mod | 15 +++++++----- magefile.go | 48 +++++++++++++++++++++++++++++++------ 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 668837885..2adb18542 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -50,6 +50,10 @@ updates: allow: - dependency-name: golang.org/x/vuln dependency-type: all + # benchstat (celeris#838). golang.org/x/perf has no tagged release and + # version updates propose tagged versions only, so this entry opens + # nothing until x/perf tags one; .github/tools/go.mod says how to bump + # it by hand meanwhile. - dependency-name: golang.org/x/perf dependency-type: all schedule: diff --git a/.github/scripts/bench-ab.sh b/.github/scripts/bench-ab.sh index 5f3e7754e..a5504128f 100755 --- a/.github/scripts/bench-ab.sh +++ b/.github/scripts/bench-ab.sh @@ -38,8 +38,10 @@ # Environment: BENCHTIME (default 1s, as CodSpeed runs it), OUT (a directory # to keep the raw outputs in; it must not hold an earlier run's A.txt, B.txt # or A2.txt; default a temporary one, removed on exit). -# benchstat is the one .github/tools/go.mod pins (go.sum, Dependabot); `go tool` -# builds it on first use, so there is nothing to install (celeris#838). +# benchstat is the one .github/tools/go.mod pins; `go tool` builds it on first +# use, so there is nothing to install (celeris#838). GOWORK=off because go +# refuses -modfile in workspace mode, which a contributor's go.work (.gitignore +# lists it) turns on. set -euo pipefail if [ "$#" -lt 4 ]; then @@ -50,8 +52,8 @@ base=$1 head=$2 pkg=$3 re=$4 rounds=${5:-20} benchtime=${BENCHTIME:-1s} repo=$(git rev-parse --show-toplevel) -benchstat() { go tool -modfile="$repo/.github/tools/go.mod" benchstat "$@"; } -go tool -modfile="$repo/.github/tools/go.mod" -n benchstat > /dev/null || { echo "cannot build benchstat from $repo/.github/tools/go.mod" >&2; exit 2; } +benchstat() { GOWORK=off go tool -modfile="$repo/.github/tools/go.mod" benchstat "$@"; } +GOWORK=off go tool -modfile="$repo/.github/tools/go.mod" -n benchstat > /dev/null || { echo "cannot build benchstat from $repo/.github/tools/go.mod" >&2; exit 2; } tmp=$(mktemp -d) cleanup() { git -C "$repo" worktree remove --force "$tmp/a" 2> /dev/null || true diff --git a/.github/tools/go.mod b/.github/tools/go.mod index cde4e6b81..3a66c4fac 100644 --- a/.github/tools/go.mod +++ b/.github/tools/go.mod @@ -1,10 +1,13 @@ -// Versions of the Go tools CI and the repo's scripts run, kept out of the -// celeris module so they add nothing to its dependency graph. Each is a -// `tool` directive, pinned by go.sum and bumped by Dependabot -// (.github/dependabot.yml); a workflow runs one with -// `go tool -modfile=$GITHUB_WORKSPACE/.github/tools/go.mod `, and -// .github/scripts/bench-ab.sh runs benchstat the same way (celeris#838). +// Versions of the Go tools CI runs, kept out of the celeris module so they +// add nothing to its dependency graph. Each is a `tool` directive, pinned by +// go.sum and bumped by Dependabot (.github/dependabot.yml); a workflow runs +// one with `go tool -modfile=$GITHUB_WORKSPACE/.github/tools/go.mod `. // This module is never imported or released (celeris#827). +// +// benchstat is here for .github/scripts/bench-ab.sh, which runs it the same +// way (celeris#838). golang.org/x/perf has no tagged release, and Dependabot's +// version updates propose tagged versions only, so that pin moves by hand: +// `GOWORK=off go -C .github/tools get golang.org/x/perf@latest`. module celeris-ci-tools go 1.27.0 diff --git a/magefile.go b/magefile.go index fff9bcc90..10b7e606b 100644 --- a/magefile.go +++ b/magefile.go @@ -3,6 +3,7 @@ package main import ( + "debug/buildinfo" "fmt" "os" "os/exec" @@ -85,11 +86,17 @@ func CleanBenchmarks() error { // the -ldflags of h2spec's own Makefile gives the release's code and its // version string, checked against the Go checksum database like any module, for // every GOOS/GOARCH (the release ships amd64 binaries only). +// +// The version string is the -ldflags stamp, so it would say 2.6.0 for any +// code. What proves the code is the module version the binary records in +// its build info, which Tools checks against h2specModVersion. const ( - h2specVersion = "2.6.0" - h2specCommit = "70ac2294010887f48b18e2d64f5cccd48421fad1" - h2specModule = "github.com/summerwind/h2spec/cmd/h2spec@v1.5.1-0.20200804131034-70ac22940108" - h2specRelease = "https://github.com/summerwind/h2spec/releases/tag/v2.6.0" + h2specVersion = "2.6.0" + h2specCommit = "70ac2294010887f48b18e2d64f5cccd48421fad1" + h2specMod = "github.com/summerwind/h2spec" + h2specModVersion = "v1.5.1-0.20200804131034-70ac22940108" + h2specPkg = h2specMod + "/cmd/h2spec@" + h2specModVersion + h2specRelease = "https://github.com/summerwind/h2spec/releases/tag/v2.6.0" ) // Tools installs external test tools (h2spec 2.6.0) and prints the version @@ -104,9 +111,9 @@ func Tools() error { } fmt.Printf("h2spec: %s reports version %q, not %s; installing %s\n", path, v, h2specVersion, h2specVersion) } - fmt.Printf("Installing h2spec %s: go install %s\n", h2specVersion, h2specModule) + fmt.Printf("Installing h2spec %s: go install %s\n", h2specVersion, h2specPkg) ldflags := fmt.Sprintf("-ldflags=-X main.VERSION=%s -X main.COMMIT=%s", h2specVersion, h2specCommit) - if err := run("go", "install", ldflags, h2specModule); err != nil { + if err := run("go", "install", ldflags, h2specPkg); err != nil { return fmt.Errorf("h2spec %s: go install failed (%w); download the release binary from %s and put it on PATH", h2specVersion, err, h2specRelease) } binDir, err := goBinDir() @@ -121,6 +128,13 @@ func Tools() error { if v := h2specVersionOf(installed); v != h2specVersion { return fmt.Errorf("h2spec: %s reports version %q after the install, not %s; download the release binary from %s", installed, v, h2specVersion, h2specRelease) } + built, err := h2specBuiltFrom(installed) + if err != nil { + return fmt.Errorf("h2spec: cannot read the build info of %s: %w", installed, err) + } + if want := h2specMod + "@" + h2specModVersion; built != want { + return fmt.Errorf("h2spec: %s was built from %s, not %s (the v%s commit); its version string is only the -ldflags stamp", installed, built, want, h2specVersion) + } path, err := exec.LookPath("h2spec") if err != nil { return fmt.Errorf("h2spec %s is at %s, but %s is not on PATH, so TestH2Spec would skip; add it to PATH", h2specVersion, installed, binDir) @@ -128,10 +142,30 @@ func Tools() error { if v := h2specVersionOf(path); v != h2specVersion { return fmt.Errorf("h2spec %s is at %s, but PATH finds %s (version %q) first; remove it or put %s first on PATH", h2specVersion, installed, path, v, binDir) } - fmt.Printf("h2spec: %s at %s\n", h2specVersion, path) + if sameFile(path, installed) { + fmt.Printf("h2spec: %s at %s (built from %s)\n", h2specVersion, path, built) + } else { + fmt.Printf("h2spec: %s at %s, first on PATH (the install at %s is built from %s)\n", h2specVersion, path, installed, built) + } return nil } +func sameFile(a, b string) bool { + fa, errA := os.Stat(a) + fb, errB := os.Stat(b) + return errA == nil && errB == nil && os.SameFile(fa, fb) +} + +// h2specBuiltFrom returns the main module path@version that a Go-built +// binary records in its build info. +func h2specBuiltFrom(path string) (string, error) { + bi, err := buildinfo.ReadFile(path) + if err != nil { + return "", err + } + return bi.Main.Path + "@" + bi.Main.Version, nil +} + // h2specVersionOf returns the version an h2spec binary reports // ("Version: 2.6.0 ()" gives "2.6.0"), or "" when it cannot run. func h2specVersionOf(path string) string {