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 {