Skip to content
Merged
26 changes: 25 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,32 @@ jobs:
cache: false
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
node-version: "20"
# Node 24 ships npm 11.x. npm Trusted Publishing
# requires npm >= 11.5; older CLIs silently fall back
# to token auth and the registry returns 404 for
# missing-credential publishes (404 instead of 401 so
# package existence isn't leaked).
node-version: "24"
registry-url: "https://registry.npmjs.org"
Comment thread
jeduden marked this conversation as resolved.
Comment on lines 174 to 182

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same response as on the line 201 thread: this is intentional. The PR description in this branch was updated when we replaced npm install -g npm@latest with the Node 24 bump — the trade-off is determinism (Node 24 ships a pinned, vendored npm 11.x) over self-healing (@latest re-introduces the non-determinism six earlier Copilot threads on this PR flagged).

The version-guard step that runs after setup-node does log npm --version and asserts >= 11.5, so the active npm is visible and asserted; we just don't replace it at runtime.


Generated by Claude Code

- name: Verify npm >= 11.5 for Trusted Publishing
# Defensive guardrail: even though Node 24 currently ships
# npm 11.x, a future Node 24 patch could bundle an older
# CLI. If npm < 11.5 the publish would silently 404.
run: |
actual=$(npm --version)
echo "npm version: $actual"
node -e '
const v = process.argv[1].split(".").map(Number);
const min = [11, 5, 0];
for (let i = 0; i < 3; i++) {
if (v[i] > min[i]) process.exit(0);
if (v[i] < min[i]) {
console.error("npm " + process.argv[1] +
" is too old for Trusted Publishing (need >= 11.5.0)");
process.exit(1);
}
}
' "$actual"
Comment thread
jeduden marked this conversation as resolved.
- name: Stamp tracked manifests with the tag
Comment thread
jeduden marked this conversation as resolved.
env:
VERSION: ${{ github.ref_name }}
Expand Down
42 changes: 39 additions & 3 deletions internal/release/buildwheels.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,31 @@ var wheelBuilds = []wheelBuild{
// hatchling build backend on PATH. Stamp must run first so
// pyproject.toml carries the published version.
func (t *Toolkit) BuildWheels(rootDir, artifactsDir, outDir string) error {
if err := t.fs.MkdirAll(outDir, 0o755); err != nil {
// Resolve outDir and artifactsDir to absolute paths up
// front. buildOneWheel runs `python -m build --outdir <…>`
// with cmd.Dir set to a staged temp tree, so a relative
// outDir would be interpreted by python relative to that
// temp dir — the wheel would land somewhere we never look,
// listWheels would return an empty slice, and (without the
// post-build guard) the workflow would silently move on
// with an empty python/dist before failing at publish time.
absOut, err := filepath.Abs(outDir)
if err != nil {
return fmt.Errorf("resolve outDir %q: %w", outDir, err)
}
absArtifacts, err := filepath.Abs(artifactsDir)
if err != nil {
return fmt.Errorf("resolve artifactsDir %q: %w", artifactsDir, err)
}
if err := t.fs.MkdirAll(absOut, 0o755); err != nil {
return err
}
src := filepath.Join(rootDir, "python")
if _, err := t.fs.Stat(src); err != nil {
return fmt.Errorf("python source missing: %w", err)
}
for _, wb := range wheelBuilds {
if err := t.buildOneWheel(src, artifactsDir, outDir, wb); err != nil {
if err := t.buildOneWheel(src, absArtifacts, absOut, wb); err != nil {
return err
}
}
Expand Down Expand Up @@ -85,14 +101,34 @@ func (t *Toolkit) buildOneWheel(src, artifactsDir, outDir string, wb wheelBuild)
defer func() { _ = t.fs.RemoveAll(stage) }()

staging := filepath.Join(outDir, ".staging-"+wb.PlatTag)
// Wipe before mkdir so a stale `.staging-<plat>/` left over
// from a killed previous run cannot fool the post-build
// empty-wheel guard. RemoveAll on a missing path is a no-op.
if err := t.fs.RemoveAll(staging); err != nil {
return fmt.Errorf("wipe staging %s: %w", staging, err)
}
if err := t.fs.MkdirAll(staging, 0o755); err != nil {
return err
return fmt.Errorf("mkdir staging %s: %w", staging, err)
}
defer func() { _ = t.fs.RemoveAll(staging) }()

if err := t.runPythonBuild(stage, staging, wb.PlatTag); err != nil {
return err
}
// `python -m build --wheel` exits 0 even when, for whatever
// reason, no wheel actually lands in the staging directory.
// We can't catch that via Run() alone, and the empty-loop
// silence in retagWheels / moveWheels would let the workflow
// continue with an empty outDir and only fail later at
// publish-time. Verify here instead.
staged, err := t.listWheels(staging)
if err != nil {
return err
}
if len(staged) == 0 {
Comment thread
jeduden marked this conversation as resolved.
Comment thread
jeduden marked this conversation as resolved.
return fmt.Errorf("python -m build (%s) produced no wheel in %s",
wb.PlatTag, staging)
}
Comment thread
jeduden marked this conversation as resolved.
Comment thread
jeduden marked this conversation as resolved.
Comment thread
jeduden marked this conversation as resolved.
if err := t.retagWheels(staging, wb.PlatTag); err != nil {
return err
}
Expand Down
221 changes: 201 additions & 20 deletions internal/release/fault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package release

import (
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
Expand Down Expand Up @@ -276,6 +277,26 @@ func TestBuildWheelsFailsOnStagingMkdir(t *testing.T) {
err := NewWithFS(ff).BuildWheels(root, artifacts, filepath.Join(root, "wheels"))
require.Error(t, err)
assert.ErrorIs(t, err, errInjected)
assert.Contains(t, err.Error(), "mkdir staging")
}

func TestBuildWheelsFailsOnStagingWipe(t *testing.T) {
// RemoveAll call order in BuildWheels (one buildOneWheel
// iteration):
// 1. buildOneWheel's wipe of outDir/.staging-<plat>
// (subsequent RemoveAll calls only fire on defer at function
// end; the first iteration's wipe is call #1.)
root := t.TempDir()
fixtureManifests(t, root)
artifacts := filepath.Join(root, "artifacts")
fakeArtifacts(t, artifacts)
ff := newFakeFS()
ff.failOnRemoveAllCall = 1

err := NewWithFS(ff).BuildWheels(root, artifacts, filepath.Join(root, "wheels"))
require.Error(t, err)
assert.ErrorIs(t, err, errInjected)
assert.Contains(t, err.Error(), "wipe staging")
}

func TestStagePythonTreeFailsOnMkdirTemp(t *testing.T) {
Expand Down Expand Up @@ -564,6 +585,138 @@ func TestCopyDirFailsOnEntryInfoError(t *testing.T) {
assert.ErrorIs(t, err, errInjected)
}

// recordingRunner captures the arguments of every RunCommand
// call so a test can assert that downstream callers (e.g.
// `python -m build --outdir <…>`) are invoked with absolute
// paths. python interprets `--outdir` relative to its own
// cwd, which we set to a staged temp tree; if outDir is
// relative, the wheel lands somewhere we never look.
type recordingRunner struct {
calls []recordedCall
}

type recordedCall struct {
dir string
name string
args []string
}

func (r *recordingRunner) RunCommand(dir, name string, args ...string) error {
r.calls = append(r.calls, recordedCall{dir: dir, name: name, args: append([]string{}, args...)})
return nil
}

// TestBuildWheelsPassesAbsoluteOutdirToPython is a regression
// for the "python/dist is empty after build" silent-failure.
// `python -m build --outdir <…>` with cmd.Dir pointing at a
// staged temp tree must receive an absolute --outdir;
// otherwise python writes the wheel under <stage>/<relative>/
// while the Go side reads from <repo-cwd>/<relative>/, finds
// nothing, and the workflow skips happily to publish.
func TestBuildWheelsPassesAbsoluteOutdirToPython(t *testing.T) {
root := t.TempDir()
fixtureManifests(t, root)
artifacts := filepath.Join(root, "artifacts")
fakeArtifacts(t, artifacts)
rec := &recordingRunner{}

// Run from a working directory where the relative outDir
// resolves predictably; chdir into a temp parent so the
// resulting absolute path is observable.
wd, err := os.Getwd()
require.NoError(t, err)
t.Cleanup(func() { _ = os.Chdir(wd) })
require.NoError(t, os.Chdir(root))

// Pass a relative outDir; the Toolkit must resolve it to
// absolute before invoking python.
err = NewWithDeps(osFS{}, rec).BuildWheels(root, artifacts, "rel-out")
// Build always errors since the recordingRunner doesn't
// actually create wheels — the empty-wheel guard fires on
// the first iteration. The recorded calls before that are
// what we verify.
require.Error(t, err)

require.NotEmpty(t, rec.calls, "no python invocations recorded")
// First recorded call should be `python -m build --wheel
// --outdir <abs>`. Find the --outdir arg and confirm it's
// absolute.
first := rec.calls[0]
idx := -1
for i, a := range first.args {
if a == "--outdir" {
idx = i
break
}
}
require.NotEqual(t, -1, idx, "no --outdir flag in first invocation: %v", first.args)
require.Greater(t, len(first.args), idx+1, "--outdir has no value: %v", first.args)
outdir := first.args[idx+1]
assert.True(t, filepath.IsAbs(outdir),
"python -m build received relative --outdir %q; "+
"wheel would land under cmd.Dir not requested outDir",
outdir)
}

// TestBuildOneWheelFailsWhenPythonProducesNoWheel pins the
// post-runPythonBuild guard: a Runner that exits 0 without
// writing any .whl into staging must still fail buildOneWheel,
// not silently move on. Earlier behaviour let an empty staging
// dir flow all the way to PyPI publish-time, where the
// pypi-publish action fails with "no distribution packages".
func TestBuildOneWheelFailsWhenPythonProducesNoWheel(t *testing.T) {
src := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(src, "pyproject.toml"),
[]byte("[project]\nname=\"x\"\n"), 0o644))
asset := filepath.Join(t.TempDir(), "asset")
require.NoError(t, os.WriteFile(asset, []byte("bin"), 0o755))
out := t.TempDir()
wb := wheelBuilds[0]

// Default fakeRunner exits 0 on every call without writing
// anything; perfect for the "build appeared to succeed but
// produced nothing" scenario.
tk := NewWithDeps(osFS{}, &fakeRunner{})
err := tk.buildOneWheel(src, filepath.Dir(asset), out, wheelBuild{
Asset: filepath.Base(asset), PlatTag: wb.PlatTag, Exe: wb.Exe,
})
require.Error(t, err)
assert.Contains(t, err.Error(), "produced no wheel")
}

// TestBuildOneWheelWipesStaleStaging is a regression for stale
// `.staging-<plat>/` left over by a killed previous run. Without
// the pre-build wipe, listWheels would see the stale wheel,
// the empty-wheel guard wouldn't fire, and retagWheels +
// moveWheels would relabel and ship the stale artifact.
func TestBuildOneWheelWipesStaleStaging(t *testing.T) {
src := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(src, "pyproject.toml"),
[]byte("[project]\nname=\"x\"\n"), 0o644))
asset := filepath.Join(t.TempDir(), "asset")
require.NoError(t, os.WriteFile(asset, []byte("bin"), 0o755))
out := t.TempDir()
wb := wheelBuilds[0]

// Plant a stale wheel where buildOneWheel will create its
// staging dir. If the wipe is missing, listWheels finds it
// and the empty-wheel guard would NOT fire — buildOneWheel
// would happily move on, retag the stale wheel, and ship it.
staleStaging := filepath.Join(out, ".staging-"+wb.PlatTag)
require.NoError(t, os.MkdirAll(staleStaging, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(staleStaging, "stale.whl"),
[]byte("stale"), 0o644))

// Default fakeRunner exits 0 without writing anything new.
tk := NewWithDeps(osFS{}, &fakeRunner{})
err := tk.buildOneWheel(src, filepath.Dir(asset), out, wheelBuild{
Asset: filepath.Base(asset), PlatTag: wb.PlatTag, Exe: wb.Exe,
})
require.Error(t, err)
assert.Contains(t, err.Error(), "produced no wheel",
"stale wheel must not satisfy the empty-wheel guard")
}

func TestBuildOneWheelPropagatesPythonFailure(t *testing.T) {
// Stage a real source tree so stagePythonTree succeeds, then
// fail on the first runner call (python -m build).
Expand All @@ -582,11 +735,41 @@ func TestBuildOneWheelPropagatesPythonFailure(t *testing.T) {
assert.ErrorIs(t, err, errInjected)
}

// preStageWheel sets up a real source tree, asset, and an
// outDir/.staging-<plat>/fake.whl so buildOneWheel can reach
// retagWheels and moveWheels without invoking real python.
// Returns (src, artifactsDir, outDir, wheelBuild).
func preStageWheel(t *testing.T) (string, string, string, wheelBuild) {
// wheelStagingRunner is a fakeRunner that, on the first
// `python -m build --outdir <dir>` invocation, drops a fake
// `*.whl` into <dir> — mimicking what the real interpreter
// produces. Lets tests reach retagWheels and moveWheels without
// pre-staging a wheel (which would now be wiped by the
// build-time RemoveAll).
type wheelStagingRunner struct {
fakeRunner
}

func (r *wheelStagingRunner) RunCommand(dir, name string, args ...string) error {
r.calls++
// First call (python -m build): drop a fake.whl under
// --outdir so listWheels finds it on the next pass.
if r.calls == 1 {
for i, a := range args {
if a == "--outdir" && i+1 < len(args) {
if err := os.WriteFile(filepath.Join(args[i+1], "fake.whl"),
[]byte("x"), 0o644); err != nil {
return fmt.Errorf("wheelStagingRunner: stage fake wheel: %w", err)
}
break
}
}
}
if r.failOnCall != 0 && r.calls == r.failOnCall {
return errInjected
}
return nil
}

// stageBuildOneWheelInputs sets up the (src, artifactsDir,
// outDir, wheelBuild) inputs buildOneWheel expects without
// pre-staging the staging dir — that's the runner's job now.
func stageBuildOneWheelInputs(t *testing.T) (string, string, string, wheelBuild) {
t.Helper()
src := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(src, "pyproject.toml"),
Expand All @@ -595,35 +778,33 @@ func preStageWheel(t *testing.T) (string, string, string, wheelBuild) {
require.NoError(t, os.WriteFile(asset, []byte("bin"), 0o755))
out := t.TempDir()
wb := wheelBuilds[0]
staging := filepath.Join(out, ".staging-"+wb.PlatTag)
require.NoError(t, os.MkdirAll(staging, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(staging, "fake.whl"),
[]byte("x"), 0o644))
return src, filepath.Dir(asset), out, wheelBuild{
Asset: filepath.Base(asset), PlatTag: wb.PlatTag, Exe: wb.Exe,
}
}

func TestBuildOneWheelPropagatesRetagFailure(t *testing.T) {
// runPythonBuild succeeds (call 1); retagWheels finds the
// pre-staged wheel and invokes the runner for `wheel tags`,
// which we fail (call 2).
src, artifacts, out, wb := preStageWheel(t)
tk := NewWithDeps(osFS{}, &fakeRunner{failOnCall: 2})
// runPythonBuild succeeds AND drops a wheel (call 1);
// retagWheels finds it and invokes the runner for `wheel
// tags`, which we fail (call 2).
src, artifacts, out, wb := stageBuildOneWheelInputs(t)
tk := NewWithDeps(osFS{}, &wheelStagingRunner{
fakeRunner: fakeRunner{failOnCall: 2},
})
err := tk.buildOneWheel(src, artifacts, out, wb)
require.Error(t, err)
assert.ErrorIs(t, err, errInjected)
}

func TestBuildOneWheelPropagatesMoveFailure(t *testing.T) {
// runner succeeds for both python invocations; FS.Rename
// fails when moveWheels tries to move the pre-staged wheel,
// covering the buildOneWheel branch that returns moveWheels'
// error.
src, artifacts, out, wb := preStageWheel(t)
// runner succeeds for both python invocations and drops a
// wheel during the first call; FS.Rename fails when
// moveWheels tries to move the now-real wheel, covering the
// buildOneWheel branch that returns moveWheels' error.
src, artifacts, out, wb := stageBuildOneWheelInputs(t)
ff := newFakeFS()
ff.failOnRenameCall = 1
tk := NewWithDeps(ff, &fakeRunner{})
tk := NewWithDeps(ff, &wheelStagingRunner{})
err := tk.buildOneWheel(src, artifacts, out, wb)
require.Error(t, err)
assert.ErrorIs(t, err, errInjected)
Expand Down
Loading