Drop Windows support and clear the gosec backlog - #47
Merged
Conversation
CI shipped with two deliberate compromises: lint ran non-blocking because golangci-lint reported five pre-existing gosec findings, and Windows was kept out of the test matrix because the suite failed there. Resolve both. Windows support was only ever partial. IsProcessAlive was a stub that always returned false, so stale setup-state reconciliation was a no-op, and setup hooks run through a POSIX shell, so they never ran at all. The remaining failures were mode bits and path separators. Making Windows genuinely work is a project, not a test fix, so remove it: delete the _windows.go implementations, drop the now-redundant !windows tags, and stop building the target in goreleaser and CI. GOOS=windows no longer compiles, which is the honest outcome. The gosec findings all fire on the tool doing what it exists to do -- walking the user's own project tree and launching the editor they configured. G702 and G703 are the taint-analysis equivalents of G204 and G304, which .golangci.yml already excluded, so extend that list rather than scatter nolint comments. With the findings resolved, lint becomes blocking. Claude-Session: https://claude.ai/code/session_01DCt9dthckWH7pYtPfzaDwF
Making lint blocking surfaced G115 in internal/disk/stat.go: Bsize is int64 on Linux and uint32 on Darwin, so the uint64 conversion only looks lossy on one of them. A block size is never negative, so silence it at the site rather than excluding G115 everywhere -- it is a useful rule. The finding was pre-existing and hidden twice over: lint was non-blocking, and it only ran for one GOOS. Lint each supported target so platform-specific files stop going unchecked; reflink_darwin.go was never being linted at all. Claude-Session: https://claude.ai/code/session_01DCt9dthckWH7pYtPfzaDwF
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.
Closes the two follow-ups left open when CI landed in #46: lint was non-blocking, and Windows was excluded from the test matrix.
Windows
Windows support was only ever partial, so it is removed rather than repaired:
TestRunParallelSetupHooksConcurrencyTestIsProcessAlivefalse; stale setup-state reconciliation was a no-opTestNewTemplateVars\vs/)TestCopyFileContentAndMode,TestByteCopyFallbackTwo core behaviours were inert, not cosmetically broken. Making Windows genuinely work is a project, not a test fix.
cmd/process_windows.go,internal/project/process_windows.go,internal/disk/disk_windows.go!windowsbuild tags and renamed the files accordingly (process_unix.go→process.go,disk_unix.go→stat.go), viagit mvso history followswindowsfrom.goreleaser.yaml(releases are now 4 archives: linux/darwin × amd64/arm64) and from the CI cross-compile jobREADME.mdnow states macOS/Linux support explicitlyGOOS=windows go buildnow fails to compile. That is intended — it is more honest than shipping a binary whose hooks and process handling do not work.internal/project/fscopy/reflink_other.gostays: its!darwin && !linuxconstraint still covers the BSDs.gosec
All 5 findings fire on
wtdoing exactly what it exists to do — walking the user's own project tree and launching the editor they configured.G702/G703are the taint-analysis equivalents ofG204/G304, which.golangci.ymlalready excluded, so the existing exclude list is extended rather than scattering 5 inline//nolintcomments.G122flags symlink TOCTOU while walkingshared/, the user's own directory.Lint is now blocking —
name: lintand no--issues-exit-code=0.Verification
make vet→ 0 issues (this gated making lint blocking)make testgreen, and green again under fresh-runner conditions (HOMEempty,GIT_CONFIG_NOSYSTEM=1)actionlintpasses;goreleaser checkvalidates the configNote:
gofumpt -lflagscmd/add.goandinternal/git/git_test.go, but those are pre-existing onmainand untouched here — a version skew between the standalone gofumpt binary and the one golangci-lint bundles. golangci-lint itself reports 0 issues.https://claude.ai/code/session_01DCt9dthckWH7pYtPfzaDwF