From 83174a45d732ae5266424b66ae9aac660332e9fd Mon Sep 17 00:00:00 2001 From: lex0c Date: Sun, 19 Apr 2026 23:53:27 -0300 Subject: [PATCH] Verify cross-platform claim in CI + fix filepath.Match on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related changes that close the gap between "cross-compiles" and "actually works on Windows/macOS": 1. CI now runs go vet + go test on ubuntu, macos, and windows runners via a matrix strategy. Release workflow has been publishing five OS/arch binaries, but CI only ever ran on ubuntu — any regression from filepath semantics, signal handling, or temp-file quirks would have surfaced in a user report, not a PR. Split into `test` (portable) and `dogfood` (linux-only, uses make + ./gitcortex) so the quality-gate steps don't need POSIX-tooling workarounds. 2. ShouldIgnore used filepath.Match, which is OS-aware and uses the platform separator ("\\" on Windows) for glob metacharacters. Paths from git arrive forward-slash on every OS, so a pattern like "src/generated/*.go" could silently fail to match "src/generated/types.go" on Windows. Swapped to path.Match (the URL/unix-style variant that always uses "/"). The parameter `path` was renamed to `p` to avoid shadowing the stdlib package name. Test fixture gained two cases covering the multi-segment glob behavior that motivated the fix; all existing cases still pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 32 ++++++++++++++++++++++++++++++-- internal/extract/extract.go | 19 ++++++++++++++----- internal/extract/extract_test.go | 7 +++++++ 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4995dc5..dfd4b11 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,8 +7,19 @@ on: branches: [main] jobs: - build: - runs-on: ubuntu-latest + # Cross-platform verification. The project cross-compiles cleanly to + # all three OSes via the release workflow, but "compiles" ≠ "runtime- + # correct": path-separator quirks, signal handling, temp-file + # semantics, and filepath.* OS-awareness are things that only surface + # when actually running. This matrix runs vet + unit tests on each + # platform so a regression is caught before release, not after a user + # reports it. + test: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 with: @@ -24,6 +35,23 @@ jobs: - name: Test run: go test ./... -count=1 + # Linux-only dogfood: build the binary, extract from this repo, and + # gate on churn-risk. The steps shell out to ./gitcortex and assume + # POSIX tooling (make), so keeping them on ubuntu avoids bifurcating + # the quality gate across OSes. The cross-platform story is covered + # by the `test` matrix above. + dogfood: + runs-on: ubuntu-latest + needs: test + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-go@v5 + with: + go-version: '1.21' + - name: Build run: make build diff --git a/internal/extract/extract.go b/internal/extract/extract.go index 47e2bfa..6ca0859 100644 --- a/internal/extract/extract.go +++ b/internal/extract/extract.go @@ -10,6 +10,7 @@ import ( "fmt" "log" "os" + "path" "path/filepath" "strconv" "strings" @@ -389,28 +390,36 @@ func loadDevEmails(path string) (map[string]struct{}, error) { return cache, scanner.Err() } -// ShouldIgnore reports whether path matches any of the ignore patterns. +// ShouldIgnore reports whether p matches any of the ignore patterns. // Exported so downstream packages (e.g. the stats suspect-warning) // can verify that the globs they emit actually match the paths they // describe — without a shared predicate the two surfaces can drift // and users end up with --ignore suggestions that don't do anything. -func ShouldIgnore(path string, patterns []string) bool { +// +// Uses path.Match and path.Base (not filepath.*) because the inputs +// here are git-emitted paths, which are always forward-slash +// regardless of the host OS. filepath.Match is OS-aware and would +// silently fail to apply glob metacharacters across "/" on Windows +// — e.g. "src/generated/*.go" against "src/generated/types.go" can +// return false because the Windows separator is "\\". path.Match +// fixes the semantics to match the input data. +func ShouldIgnore(p string, patterns []string) bool { if len(patterns) == 0 { return false } for _, pattern := range patterns { // Basename match: "*.min.js" matches "dist/app.min.js" - if matched, _ := filepath.Match(pattern, filepath.Base(path)); matched { + if matched, _ := path.Match(pattern, path.Base(p)); matched { return true } // Full path match: "src/generated/*.go" matches "src/generated/types.go" - if matched, _ := filepath.Match(pattern, path); matched { + if matched, _ := path.Match(pattern, p); matched { return true } // Directory prefix: "dist/*" or "dist/" matches "dist/foo/bar.js" prefix := strings.TrimSuffix(pattern, "*") prefix = strings.TrimSuffix(prefix, "/") - if prefix != "" && prefix != pattern && strings.HasPrefix(path, prefix+"/") { + if prefix != "" && prefix != pattern && strings.HasPrefix(p, prefix+"/") { return true } } diff --git a/internal/extract/extract_test.go b/internal/extract/extract_test.go index a2a4fc9..cda525f 100644 --- a/internal/extract/extract_test.go +++ b/internal/extract/extract_test.go @@ -124,6 +124,13 @@ func TestShouldIgnore(t *testing.T) { {"src/main.go", nil, false}, {"src/main.go", []string{}, false}, {"", []string{"*.go"}, false}, + // Portability regression: pattern with a slash glob. + // filepath.Match on Windows uses "\\" as separator and would + // silently fail to match "src/generated/types.go" against + // "src/generated/*.go". path.Match keeps forward-slash + // semantics and matches on every platform. + {"src/generated/types.go", []string{"src/generated/*.go"}, true}, + {"src/hand/types.go", []string{"src/generated/*.go"}, false}, } for _, tt := range tests {