ci: test on the platforms releases actually ship - #3
Merged
Conversation
Releases ship Windows and macOS binaries. Neither had ever been executed by a job: CI ran on Linux only, so a portability break would have reached users before it reached us, and the Windows binary added last week was shipped on the strength of a type-check alone. A new `platform` job runs the suite and an end-to-end CLI smoke on windows-latest and macos-latest. `check` stays on Linux and keeps everything that does not vary by platform — formatting, clippy, and the network-egress gate, which needs iptables. Duplicating those three times would cost minutes per run and catch nothing. Two platform details are handled rather than discovered later. Windows runners convert LF to CRLF on checkout, which would test fixtures no user's repository actually contains and shift every byte offset the index records. `core.autocrlf false` is set in a step *before* checkout, because configuring it afterwards is too late. The smoke step runs under bash rather than the Windows default. PowerShell propagates only the last command's exit code, so a failing `init` followed by a passing `context` would have left the step green — a CI step that cannot fail is worse than no step. `fail-fast: false`, so a Windows failure does not hide a macOS one.
`process_is_alive` was implemented for unix and stubbed to `false` everywhere else. The comment called that the safer failure — erring toward reclaiming a lock rather than deadlocking a repository — but for the *current* process it means every lock looks stale, so the lock reclaims itself and stops excluding anything. Two `reify index` runs on Windows would both proceed against the same store. Windows CI found it in its first run, by failing to recognise its own process as alive. The Windows binary shipped last week has carried this since; it was added on the strength of a type-check, which is exactly what a type-check cannot catch. Implemented with `OpenProcess` + `WaitForSingleObject(handle, 0)`, declared by hand for one question asked once, the same way `kill` already is rather than taking a libc dependency. `WaitForSingleObject` is used in preference to `GetExitCodeProcess`, which reports the sentinel 259 for a running process and cannot distinguish it from one that genuinely exited with 259. `QUERY_LIMITED_INFORMATION` is the narrowest right that answers the question and is granted where `PROCESS_QUERY_INFORMATION` is not. The remaining `not(any(unix, windows))` arm now returns `true` rather than `false`. Without a liveness check the lock cannot be trusted, and refusing to reclaim a lock is a worse outcome for one user than letting two indexers share a store is for everyone.
The first attempt opened the process with `QUERY_LIMITED_INFORMATION` alone. `WaitForSingleObject` requires `SYNCHRONIZE`, so the wait did not return `WAIT_TIMEOUT` for a running process — it returned `WAIT_FAILED`, which the comparison read as "not running", restoring exactly the bug the function was written to fix. Windows CI caught it a second time, on the same assertion. Worth the comment it now carries: omitting an access right does not make the check stricter, it makes the call fail, and a failed liveness check that reads as "dead" is indistinguishable from the stub this replaced.
The install section listed a Windows binary; nothing else did. The two translated READMEs never mentioned Windows at all — they still offered macOS and Linux only — and the docs site said the same. A reader on Windows had no way to tell the tool was for them. All three READMEs and the site now carry a platform badge and name Windows alongside macOS and Linux, with what to do from PowerShell, where a `curl | sh` line is no help: take the msvc archive, verify its checksum, put `reify.exe` on PATH. The claim is only made because it is now true. Every listed platform runs the full suite in CI and has the CLI exercised end to end — the sentence saying so is in the README because it is the difference between this and the previous release, which published a Windows binary whose index lock did nothing.
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
1 participant
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.
Releases ship Windows and macOS binaries. Neither had ever been executed by a CI job — CI ran on Linux only, so a portability break would have reached users before it reached us, and the Windows binary added last week shipped on the strength of a type-check alone.
What this adds
A
platformjob running the full suite plus an end-to-end CLI smoke onwindows-latestandmacos-latest.checkstays on Linux and keeps everything that does not vary by platform — formatting, clippy, and the network-egress gate that needs iptables. Duplicating those across three runners would cost minutes per run and catch nothing.Two platform details handled rather than discovered later
Line endings. Windows runners convert LF to CRLF on checkout, which would test fixtures no user's repository actually contains and shift every byte offset the index records.
core.autocrlf falseis set in a step before checkout, because configuring it afterwards is too late.Exit codes. The smoke step runs under
bash, not the Windows default. PowerShell propagates only the last command's exit code, so a failinginitfollowed by a passingcontextwould have left the step green. A CI step that cannot fail is worse than no step.fail-fast: false, so a Windows failure does not hide a macOS one.Note
This PR's own CI run is the evidence. If either platform is red, that is a real portability bug this job was added to find — not a reason to weaken the job.