Repository navigation
Description
Discussion, not a work item — please don't open a PR off this. The mechanical change is three lines; what's worth deciding first is what "mirror" is supposed to mean for this dataclass, because the answer decides whether exe gets deleted, gets a different justification, or gets joined by two fields it's currently missing.
Found while fixing #996 (PR #999), in the module next door. Not folded into that PR — it's out of #996's stated scope, and a previous pass evidently looked straight at this field and chose to keep it, so overruling that silently inside an unrelated fix seemed wrong.
What's there
internals/distcheck/src/distcheck/node_flow/platforms.py:21 carries exe: bool on the Platform dataclass. Nothing in internals/distcheck reads it outside its own tests — three assertions, at platforms_test.py:30, :42, and :65.
Two of those tests state, in comments, why it's still there:
def test_find_host_platform_windows_addon_has_no_exe_suffix():
# The `.exe` suffix belonged to the standalone binary, which is no longer
# published (#739); an addon is a `.node` on every platform.def test_only_windows_rows_flag_exe():
# `exe` still marks the Windows row (it mirrors packages/ts/src/platforms.ts),
# but nothing derives a filename from it now that no binary is published.So the field is knowingly dead weight, kept on one stated ground: that it mirrors the TypeScript source of truth.
The problem with that ground
It doesn't hold. packages/ts/src/platforms.ts has no exe field, and going by git log -S, it never has:
$ git log --all -S'exe' -- packages/ts/src/platforms.ts
(no output)
Its Platform interface is triple, nodePlatform, nodeArch, libName, os, cpu, libc?. There is nothing on the TypeScript side for exe to mirror, which means the second comment's justification has been false since it was written.
For provenance: platforms.py was created whole in 6502217, on claude/739-npm-in-process — the #739 branch itself. So exe didn't survive #739, it was born on it, carried over from the pre-#739 world where a standalone binary really did need an .exe suffix on Windows. It arrived already vestigial.
Why this is a question and not just a deletion
The mirror is loose in both directions, which is the part I don't think should be settled by whoever happens to touch the file next:
platforms.ts |
platforms.py |
|
|---|---|---|
triple |
✅ | ❌ missing |
libc |
✅ (Linux rows) | ❌ missing |
exe |
❌ never existed | ✅ present, unused |
platforms.py's own docstring says it is "A Python port of the host-row lookup in packages/ts/src/platforms.ts" holding "enough of each published target to reconstruct the host's @dirsql/lib-<slug> sub-package", and PLATFORMS is commented "Keep in lockstep with that file". Those two framings pull opposite ways — "enough of" licenses the omissions, "lockstep" forbids them — and exe is what falls out of the gap.
So the question is which framing wins:
- "Enough of" — a deliberate subset. Then
exeshould go (it isn't needed to reconstruct anything), the missingtriple/libcare correct as-is, and the "Keep in lockstep" comment is the thing that's misleading and should be reworded. Cheapest option, and my guess at the right one. - True lockstep. Then
exestill goes, buttripleandlibcshould be added, and it's worth asking whether a check should enforce the correspondence rather than a comment asserting it — a drifting hand-maintained mirror is what produced both this and distcheck: node flow's@dirsql/cli-<slug>prerequisite left behind by #739 (integration test raises, stale gate docstring + README) #996. That's a real cost; the payoff is that a new published target can't be half-added. - Keep
exe. Fine by me if there's a reason I can't see — but the comment giving that reason needs to be replaced, because as written it points at a field that isn't there.
The reason I'd rather not just push (1) is that a mirror nobody checks is the shared root cause of #972, #996, and this. Deleting one dead field tidies the symptom and leaves the mechanism intact, and whether that mechanism is worth hardening is a maintainer call about cost, not a cleanup.
Not urgent
Nothing is broken. exe is inert — no filename, path, or package name derives from it, and the distcheck flows pass. This is a correctness-of-the-record issue, and the sharpest part is that a comment currently explains a decision by citing something untrue, which is exactly the kind of thing a future reader would take at face value.
Activity
test: pin the platforms.py <-> platforms.ts mirror (RED)
feat(checks): enforce the platforms.py <-> platforms.ts mirror
test(checks): kill the platforms-mirror mutation survivors