Commit Graph
2 Commits
Author SHA1 Message Date
Neil ad0ccf83a8 fix(windows): run the node-pty addon gates on the Windows job that can
`rebuild-native-deps-node-pty.test.mjs` carries four `skipIf(platform !== 'win32')`
tests. The full suite runs on ubuntu, and the Windows PR job runs an explicit
file list that never named this file -- so those tests were skipped on Linux and
never reached anywhere else. Three of them predate this branch. The Windows job
is added the four node-pty addon suites plus the module-walker one; the comment
above that list already says why it is the right place, which is that the addon
assertions only hold once natives have been rebuilt. Running the path-joining
suites there also covers the separator this gate's candidate list is built from.

The rest is round-three review:

- the rebuild-time arch assertion told a reader "node-gyp did not honour --arch"
  about a file that was not a PE image at all, which is a truncated or
  quarantined artifact and a different command to run. The two now read
  differently, and neither claims the other's cause. Same fix the packaged gate
  had one commit ago, in the place that had not had it yet.
- the missing-addon error said node-pty "would load" a prebuild without checking
  it is there. It says "fall through to" now, which is true either way.
- `isLoadableByArch` had no caller left once the packaged gate started needing
  the raw machine field for its message. Removed rather than kept warm.
- each candidate's header is read once instead of up to three times.
- the module walker's comment claimed every shape that reaches a co-located
  module; it does not follow `projectRequire`/`requireLocal`, and it must not --
  those specifiers resolve against the project root, so following one stages the
  wrong path and the copy fails. Proven by trying: widening the pattern to
  require-shaped names broke nine tests on
  `projectRequire('./config/scripts/...')`. The comment now says what it follows
  and why it stops there.
- a new test resolved a file URL with `.pathname`, which keeps the drive-letter
  slash on Windows -- the very job this commit adds it to.
2026-09-15 22:37:01 -07:00
Neil 2a73e6b03a fix(windows): say why the packaged ConPTY fell back, not just that it did
The previous commit resolved the addon by architecture but still had one message
for every way the resolution could land on the published prebuild. Those ways
want opposite remedies, and the one it printed was the remedy the commit before
it had just called wrong:

- no source build in the package at all — the slice has to be built somewhere
  that can build node-pty for the target arch.
- a source build that is there but is the packaging host's architecture, because
  the cross-arch rebuild did not honour `--arch` — re-running that rebuild is the
  fix, and "package on a Windows arm64 host" is neither necessary nor possible.

The second is the common one, since node-pty publishes a prebuild for both
Windows arches and prune keeps the target's on every cross-arch package. So the
old text fired mostly on the case it described least. It now reports which source
builds were skipped and the machine field each carried, and names the rebuild
command.

"Nothing the target can load" had the same problem in reverse: a zero-length or
truncated `conpty.node` got a cross-architecture diagnosis. Every candidate is
now named with what was actually read, including "not a PE image".

The rebuild path asserts the architecture too. A rebuild that ignored `--arch`
was otherwise only visible at packaging, two steps from the command that fixes
it. Arches with no known machine value are left unjudged rather than guessed at.

Two things the extraction broke or nearly broke, both found by mutation:

- the shared PE reader answers `null` where the relay builder's private copy
  returned a number, which would have turned its "node-gyp ignored --arch" error
  into a `TypeError`. Both callers now go through `describePeMachine`.
- the rebuild fixtures stage a script's co-located modules by walking its
  imports, and the walker only understood `from '...'` — so the gate's new
  `require('./windows-pe-machine.cjs')` was left behind and every subprocess test
  failed with a resolution error, which is the exact failure its own comment
  warns about. It now follows `require` and bare side-effect `import` as well,
  and has tests; the fixture stages the gate by walking it rather than by naming
  one file.

Fixtures write real PE headers through one shared builder instead of three
hand-rolled ones.
2026-09-15 22:19:46 -07:00