Repository navigation
Windows support - #135
Merged
Merged
Windows support#135
Conversation
Windows is a supported platform and nothing was measuring it, so every POSIX assumption in the tool reads as correct on a Mac and breaks there. A matrix gates both; fail-fast is off so one platform's failure never hides the other's. The pmd install is the one step that cannot be shared — the Linux leg is curl/sha256sum/unzip, the Windows leg the PowerShell equivalents against the same pinned digest.
Nothing named an encoding, so Python used the locale's. The guides are UTF-8; a cp1252 console is what Windows gives you. habit-hooks died at the first guide it rendered (#133), and the test suite did not even collect. It is not a Windows bug — LC_ALL=C reproduces it on a Mac. Both directions were open. Reads decoded in the locale, and the box-drawing characters in a printed block could not be encoded back out to a console that had no room for them. stdin is covered too: it carries the findings pipe, and only json.dumps' ensure_ascii default was keeping it upright. The child half goes with it. PYTHONIOENCODING reaches every helper this repo ships; a third-party tool cannot be told, which is why errors="replace" is the policy on our side of the pipe — one bad byte must not take a sensor down, and a replacement character in a quoted-back message is a visible loss rather than a silent one. An AST gate holds the line, over every read, write and text-mode spawn in the project. It is the half that stops this returning one call site at a time.
The file sat at exactly 200 lines against the threshold this project holds
everyone else to. The split it wanted was always the ruleset concern — which
config PMD runs with is a first-class question here, the same one knip.cjs and
jscpd.py each answer for their own tool.
What blocked it was said to be vendoring, per-file, so a helper could have no
neighbour. That was never true. `${dir}` expands to whichever directory won the
override chain, and a sensor already needs its toml, its helper and its ruleset
side by side there — a project vendoring the toml alone fails today, before this
change. Per-file is the override *chain*: one guide replaced without copying a
plugin. A different property, and not this one.
The import conflict was real and is gone. A helper is spawned as a loose script,
so its own directory is sys.path[0] and a neighbour is a plain top-level import.
The only load path under which that fails is importing it as a package module —
which no run does, and which two unit tests were doing. They now load it the way
the interpreter does.
PYTHONSAFEPATH is what takes that directory away again, and a consumer may have
it set. Pinned off beside PYTHONIOENCODING, or the helper answers a run with a
ModuleNotFoundError traceback instead of coaching.
…t does not A spec case's steps are POSIX shell run through bash. Windows has no such shell, so on the new Windows CI leg the documentation suite could only ever fail — 158 cases red for a reason that says nothing about the tool. It skips there now, with a reason naming issue #137, so the gap is a line in the summary rather than an absence. The parser tests still run everywhere: the skip sits in the helper that reaches a step, not on the module, and nothing that only parses needs a shell. Splitting test_harness.py fell out of it — the file was one line under the threshold and its own section comments had been naming three concerns for a while. Markers and their normalisation are what one step means; contexts are the rules between cases; the runner both drive is neither.
A venv keeps its executables in Scripts on Windows and bin everywhere else, so tool_search_path pointed at a directory that does not exist there — a project's own pinned tools invisible to every run, silently, which is the one thing that function exists to prevent. The wheel-install harness had the same fact wrong and took every installed-run test down with it. CreateProcess caps a whole command line at 32,767 characters. The budget was 100,000, reasoned about macOS ARG_MAX and Linux MAX_ARG_STRLEN — neither of which Windows has. A work-tree-sized scope overflowed the spawn, which is the "Argument list too long" in #134. host_platform answers the platform question and nothing else; each fact stays in the module that owns it. It is asked through the module rather than imported by name, so a test can flip it — and both branches of both facts are exercised from a Mac, which is the only way either could be trusted here.
Naming the directory was not enough: on Windows the interpreter is python.exe, and a path built from the directory alone pointed at nothing. Every installed-run test — the gate that catches "an installed run cannot locate its plugins" — errored on it, before and after the directory was corrected. Splitting one fact across two modules is what let it ship half-done, so the whole answer lives in project_paths and the test harness asks rather than assembles. The platform tests were reading their expected answer off the host. One was called ..._on_this_machine, which is the fault stated out loud: green on a Mac, red on Windows, and no use on either. Each now pins the platform through the seam and asserts that platform's answer, so the suite says the same thing wherever it runs — which is the only reason the seam is there.
Every part was spawned as bash -c. On Windows there is no POSIX shell, and bash
there is usually C:\Windows\System32\bash.exe — the WSL launcher. The CI runner
proves it: sensors came back with UTF-16 "Windows Subsystem for Linux has no
installed distributions" on stdout where findings JSON belongs. Reaching another
filesystem by accident is worse than not running.
Nine of the eleven shipped parts are a single argv already — no pipe, no glob,
no redirection. Only ruff and eslint need syntax a list cannot carry, and only
to reach jq. So a part now spells one of two recipes, and command_text is where
the difference is spent: an argv is spawned as it stands, a command becomes
["bash", "-c", the same quoted text as before]. spawn.py spawns an argv and
knows nothing about shells.
A placeholder is a string or a list. ${python} and ${dir} substitute inside an
element, so "${dir}/line-count.py" stays one argument. ${files}, ${args} and
${config} stand for whole lists, so an element must be exactly one of them —
"--paths=${files}" is refused, because joining a file list into one argument is
the bug this form exists to make impossible. Nothing is quoted: quoting keeps a
shell from reading a filename as syntax, and with no shell a quote character
would just become part of the name.
An argv spawn has no shell to say "command not found", so it says it itself —
the same words, the same code path, the same #114 message either way. A missing
tool reading as nothing to report is what #88 is for.
Nothing shipped moves yet; all eleven still spell command.
Each of them was already a single command with no pipe, no glob and no redirection — the shell was carrying nothing. pmd's literal -- separator survives as an element of its own, which is what keeps a project's PMD flags on one side of it and the scoped files on the other. A test now reads every shipped spec and fails on a part that spells command without being on a named list of two, each justified by the pipe into jq it cannot lose. A second test keeps that list honest: it fails if either stops needing a shell and quietly stays exempt. The failure notices in the plugin docs move with it. A failing part quotes its own recipe back, and an argv quotes as shlex spells it.
Windows has no killpg. A process group of its own buys the console isolation a session buys and nothing towards the deadline — there is no "kill this group" call there, only taskkill walking live parent links down from a pid, which an orphaned grandchild escapes. The table is in live_commands.py, stated as what each platform actually guarantees rather than as an equivalence. The names went with it. Nothing kills a group on Windows and the tracked number is a plain pid, so kill_group became kill_command and the registry lost the word it was asserting. its_own_process_group keeps it: that half is true on both. A command recipe on Windows now refuses instead of spawning. bash there is usually the WSL launcher, so spawning it either says nothing useful or — worse, with a distribution installed — measures a different filesystem and calls it this project. It fails as one part, not as the run: the other sensors still report, and the run is failed rather than clean. Asked of the platform, never of whether a bash can be found, because a findable bash is the trap.
Three times a platform test was written to assert whatever the author's machine
said — green on a Mac, red on the Windows leg, and no evidence either way. The
third arrived after the first two had been called out, which makes it a shape
rather than three slips, so it is written down in CLAUDE.md next to the seam it
is about.
Every pinning site now goes through platform_probe, so there is one way to do it
and one docstring saying why. Where the shell a test spawned was incidental to
what it proved — a scope narrowing, a budget split — the part is spelled as an
argv instead, and there is nothing platform-specific left to pin.
One was not a pinning fault at all: the expected text for ${dir} was missing the
quoting the code does, and passed on POSIX only because a POSIX path has nothing
in it to quote.
init's install prompt no longer runs through a shell. shell=True is cmd.exe on
Windows, which does not understand the single quotes in
uv tool install 'habit-hooks[python]' — so the one command init offers to run
for you was the one it could not. The printed line is unchanged; it is for a
human's own shell and still pastes.
…t managed to Windows appends .exe to a bare name and nothing else, so a Node tool installed as jscpd.CMD or a pmd.bat was found by shutil.which and then not found by the spawn. project_paths' own docstring already named that as the thing its two questions share one answer to prevent; now they do, through tool_executable, which asks shutil.which rather than copying it — which filenames a machine will run is that machine's question. No platform branch: off Windows it names the very file execvpe would have reached. This is not the whole of that bug. A helper spawns its own tool — jscpd, pmd, deptry, phpmd, knip — and those calls are still bare names. A timed-out command was losing its last words. Windows' _communicate raises TimeoutExpired carrying nothing at all and leaves the pipes open on purpose, so the notice quoted back an empty string where the tool had said why it was stuck. deadline.py kills, then drains what is left under a second ceiling. Same code both platforms, and strictly more of it on POSIX. Test fixtures that only ever ran on a Mac: a path interpolated into a TOML basic string, where a backslash is an escape; a killpg stubbed on an os that has none; an allowlist compared against a backslashed path; and tool stubs with a #!/bin/sh line, which Windows will not call an executable at all.
ruff and eslint were the last two parts spelling a command string, and both only because they piped their tool through jq. Each is now a helper beside its spec — ruff_sensor.py, eslint.cjs — the shape every other sensor here already had, and both spell argv. On Windows that is the difference between a Python or TypeScript project getting its findings and getting none. jq goes with them. It was a declared detector of two plugins whose only install hint we could give was brew install jq, which is no help to most of the people who needed it. Each helper was diffed against the recipe it replaces over the same sources — identical findings, identical exit codes, including the branch where a project has no eslint config of its own and the shipped one is lent to it. A helper spawns its own tool too, and those were bare names. The four Python plugins resolve through tool_spawn.py, which asks shutil.which along the PATH habit-hooks already handed them — the same question that cleared the tool at setup, adding only the spelling Windows needs. The TypeScript pair cannot: Node refuses a .cmd outright, so shutil.which's answer is the one thing unusable there. project_tool.cjs finds the package in node_modules and runs its own bin script with process.execPath, so there is no shim and no shell either way. An argument to a .bat or .cmd is refused if cmd.exe would read it as syntax. CreateProcess routes a batch file through cmd.exe, list2cmdline quotes for the C runtime rather than for cmd, and our arguments are filenames out of whatever branch is checked out — the case test_a_filename_can_never_execute_a_command already exists for on POSIX. The program's extension decides, never the platform, so the guarantee is provable here and live there.
… does eslint and knip were declared as commands, looked for on PATH. They are spawned out of the project's node_modules now, so a globally installed copy was being cleared at setup and then not reached at run time — the disagreement project_paths exists to prevent, arrived at from the other side. Both are node-module detectors, the kind ts-morph already was for the same reason. The reporter on #133 installed eslint globally and said it helped. It will not any more, and this is what tells them so, with the npm line to fix it. The shipped config could never have worked with a global install regardless: it resolves its plugins from the project. jq is gone from the README, and the gotcha about indexing a jq object with null now describes where that hazard actually lives — a Python dict lookup that returns None, and a JS Map rather than an object literal, because a plain object answers map["constructor"] off Object.prototype and the finding silently loses its smell. Why there are two spawn seams rather than one is written down, because the next person will try to merge them: shutil.which answers with a .cmd, and Node refuses to spawn one at all.
kill_command named signal.SIGKILL, which a Windows signal module has not got — so an AttributeError came out of the handler whose whole job was reporting a timeout. The number is read the way CREATE_NEW_PROCESS_GROUP already was. A spawn refused in a project directory that no longer exists said "[WinError 2] The system cannot find the file specified", naming neither the program nor the directory, and sent a reader off to install a tool that was never the problem. We know which directory it was, so it says so, in the same words on both. The rest were tests holding a POSIX-shaped answer. ts-morph reports TypeScript's own spelling — forward slashes everywhere — where eslint reports the platform's, so one expected-path helper could not serve both and there are now two. Neither escapes the sensor boundary: anchoring reads either. jscpd cannot scan from a config on Windows. It resolves a config's path to an absolute one and globs it, and fast-glob reads the backslashes as escapes, so nothing matches. Bare jscpd does the same thing in the same project, which is what our precedence rule asks for — so the cases skip there rather than the sensor working around it, and the tool's own bug is filed as one. part_output was at exactly 200 lines and could not take the new message. Quoting a tool's own output back was the concern to move out whole.
~=1.4 is >=1.4,<2, and by PEP 440 ordering 1.4.0rc1 sorts below 1.4 — so a release candidate declared floors its own plugins could not satisfy, and pip install habit-hooks==1.4.0rc1 died naming a version sitting in the list it had just printed. No pre-release flag lifts it: --pre and --prerelease=allow are policy about which candidates a resolver may consider, and this is the specifier excluding the version outright. >=1.4.dev0,<2 differs from ~=1.4 on exactly three versions — the pre-releases of 1.4.0 itself — and agrees on every other. 1.3.1 is still refused, 1.4.1rc1 and 1.5.0rc1 were already admitted, 2.0.0rc1 is still refused. One spelling serves the candidate and the release, so nothing is rewritten at the tag, which is where a rewrite would never be tested. The gate asks packaging's own SpecifierSet whether this release satisfies the floors this release declares. Nothing asked that before, which is why a floor that could not ship was only ever going to be found by trying to ship it. The packaging gate could not test a candidate at all: it handed uv six wheel paths, and a direct-URL package is not lifted by a pre-release flag either. It installs from a wheelhouse that is the only index now, and asserts the versions it ends up with are the versions it built — which caught the by-name case quietly measuring 1.3.1 downloaded from PyPI while reporting itself green.
This branch was successfully deployed
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
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.
Windows support: issues #133 and #134.
Draft, and open only so CI runs — this workflow triggers on
mainand on pullrequests, so a work branch has no other way to reach a Windows runner.
First commit adds the runner. Expect it red: that red is the baseline the fix
is written against.