From 007aac01eacd9434bfe06d8a2ad891e2ba198f55 Mon Sep 17 00:00:00 2001 From: folbrich Date: Mon, 18 May 2026 12:57:56 +0200 Subject: [PATCH 1/3] TEMP: Windows locationMatch diagnostic + Windows-only diag workflow Not for merge. Evaluates every TestLocationEquality locationMatch case without short-circuiting and dumps filepath.Abs/filepath.Match internals so one Windows CI run reveals all divergences at once. --- .github/workflows/_diag-windows.yaml | 22 +++++ cmd/desync/locationdiag_test.go | 129 +++++++++++++++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 .github/workflows/_diag-windows.yaml create mode 100644 cmd/desync/locationdiag_test.go diff --git a/.github/workflows/_diag-windows.yaml b/.github/workflows/_diag-windows.yaml new file mode 100644 index 0000000..aaa4a5d --- /dev/null +++ b/.github/workflows/_diag-windows.yaml @@ -0,0 +1,22 @@ +name: Diag Windows + +# TEMPORARY — not for merge. Fast Windows-only loop for investigating +# locationMatch. Triggers only on pushes to the investigation branch. +on: + push: + branches: + - investigate-windows-locationmatch + +permissions: + contents: read + +jobs: + diag: + runs-on: windows-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + - run: go test ./cmd/desync -run TestLocationDiag -v diff --git a/cmd/desync/locationdiag_test.go b/cmd/desync/locationdiag_test.go new file mode 100644 index 0000000..b0c48df --- /dev/null +++ b/cmd/desync/locationdiag_test.go @@ -0,0 +1,129 @@ +package main + +// TEMPORARY diagnostic — not for merge. Evaluates every locationMatch case +// from TestLocationEquality (including the Windows-only blocks) without +// short-circuiting, and prints the path-branch internals, so a single +// Windows CI run reveals the complete set of divergences and their causes. + +import ( + "fmt" + "net/url" + "path" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestLocationDiag(t *testing.T) { + type c struct { + pattern, loc string + want bool + group string + } + cases := []c{ + // Equal URLs + {"http://host/path", "http://host/path", true, "eq-url"}, + {"http://host/path/", "http://host/path/", true, "eq-url"}, + {"http://host/path", "http://host/path/", true, "eq-url"}, + {"https://host/", "https://host", true, "eq-url"}, + {"https://host", "https://host/", true, "eq-url"}, + {"https://host", "https://host", true, "eq-url"}, + {"https://host/", "https://host/", true, "eq-url"}, + {"s3+https://example.com", "s3+https://example.com", true, "eq-url"}, + // Equal URLs with globs + {"https://host/path*", "https://host/path", true, "eq-url-glob"}, + {"https://host/path*", "https://host/path/", true, "eq-url-glob"}, + {"https://*", "https://example.com", true, "eq-url-glob"}, + {"https://example.com/path/*", "https://example.com/path/another", true, "eq-url-glob"}, + {"https://example.com/path/*", "https://example.com/path/another/", true, "eq-url-glob"}, + {"https://example.com/*/*/", "https://example.com/path/another/", true, "eq-url-glob"}, + {"https://example.com/*/", "https://example.com/2022.01/", true, "eq-url-glob"}, + {"https://*/*/*", "https://example.com/path/another/", true, "eq-url-glob"}, + {"https://example.*", "https://example.com", true, "eq-url-glob"}, + {"*://example.com", "https://example.com", true, "eq-url-glob"}, + {"http*://example.com", "https://example.com", true, "eq-url-glob"}, + {"http*://example.com", "http://example.com", true, "eq-url-glob"}, + {"https://exampl?.*", "https://example.com", true, "eq-url-glob"}, + {"http://examp??.com", "http://example.com", true, "eq-url-glob"}, + {"https://example.com/?", "https://example.com/a", true, "eq-url-glob"}, + {"https://example.com/fo[a-z]", "https://example.com/foo", true, "eq-url-glob"}, + // Not equal URLs + {"http://host:8080/path", "http://host/path", false, "ne-url"}, + {"http://host/path1", "http://host/path", false, "ne-url"}, + {"http://host/path1", "http://host/path/", false, "ne-url"}, + {"http://host1/path", "http://host2/path", false, "ne-url"}, + {"sftp://host/path", "http://host/path", false, "ne-url"}, + {"ssh://host/path", "/path", false, "ne-url"}, + {"ssh://host/path", "/host/path", false, "ne-url"}, + {"ssh://host/path", "/ssh/host/path", false, "ne-url"}, + // Not equal URLs with globs + {"*", "https://example.com/path", false, "ne-url-glob"}, + {"https://*", "https://example.com/path", false, "ne-url-glob"}, + {"https://example.com/*", "https://example.com/path/another", false, "ne-url-glob"}, + {"https://example.com/path/*", "https://example.com/path", false, "ne-url-glob"}, + {"http://*", "https://example.com", false, "ne-url-glob"}, + {"http?://example.com", "http://example.com", false, "ne-url-glob"}, + {"https://example.com/123?", "https://example.com/12345", false, "ne-url-glob"}, + {"*://example.com", "https://example.com/123", false, "ne-url-glob"}, + // Equal paths + {"/path", "/path/../path", true, "eq-path"}, + {"//path", "//path", true, "eq-path"}, + {"//path", "/path", true, "eq-path"}, + {"./path", "./path", true, "eq-path"}, + {"path", "path/", true, "eq-path"}, + {"path/..", ".", true, "eq-path"}, + // Equal paths (Windows-only block) + {"c:\\path\\to\\somewhere", "c:\\path\\to\\somewhere\\", true, "eq-path-win"}, + {"/path/to/somewhere", "\\path\\to\\somewhere\\", true, "eq-path-win"}, + // Not equal paths + {"/path", "path", false, "ne-path"}, + {"/path/to", "path/to", false, "ne-path"}, + {"/path/to", "/path/to/..", false, "ne-path"}, + // Not equal paths (Windows-only block) + {"c:\\path1", "c:\\path2", false, "ne-path-win"}, + // Not equal paths with globs + {"/path*", "/dir", false, "ne-path-glob"}, + {"/path*", "path", false, "ne-path-glob"}, + {"/path*", "/path/to", false, "ne-path-glob"}, + {"/path/*", "/path", false, "ne-path-glob"}, + {"/path/to/../*", "/path/to/another", false, "ne-path-glob"}, + {"/pat?", "/pat", false, "ne-path-glob"}, + {"/pat?", "/dir", false, "ne-path-glob"}, + // "Not equal paths with globs" Windows-only block (asserts True) + {"c:\\path\\to\\*", "c:\\path\\to\\", true, "win-glob"}, + {"/path/to/*", "\\path\\to\\", true, "win-glob"}, + {"c:\\path\\to\\?", "c:\\path\\to\\123\\", true, "win-glob"}, + {"/path/to/?", "\\path\\to\\123\\", true, "win-glob"}, + } + + t.Logf("GOOS=%s total=%d", runtime.GOOS, len(cases)) + fails := 0 + for i, tc := range cases { + got := locationMatch(tc.pattern, tc.loc) + status := "ok" + if got != tc.want { + status = "FAIL" + fails++ + } + // Recompute the branch internals for visibility. + detail := "" + if u, err := url.Parse(tc.loc); err == nil && len(u.Scheme) > 1 { + tp := strings.TrimSuffix(tc.pattern, "/") + tl := strings.TrimSuffix(tc.loc, "/") + pm, _ := path.Match(tp, tl) + detail = fmt.Sprintf("URL scheme=%q path.Match(%q,%q)=%v", u.Scheme, tp, tl, pm) + } else { + ap, _ := filepath.Abs(tc.pattern) + al, _ := filepath.Abs(tc.loc) + fm, fmErr := filepath.Match(ap, al) + detail = fmt.Sprintf("PATH Abs(p)=%q Abs(l)=%q filepath.Match=%v err=%v", ap, al, fm, fmErr) + } + t.Logf("[%2d] %-12s %-4s want=%-5v got=%-5v | p=%q l=%q | %s", + i, tc.group, status, tc.want, got, tc.pattern, tc.loc, detail) + } + t.Logf("SUMMARY: %d/%d failed on %s", fails, len(cases), runtime.GOOS) + if fails > 0 { + t.Errorf("%d locationMatch divergences on %s (see table above)", fails, runtime.GOOS) + } +} From d9d35c6118a9786713de0216d3a2fe73e6651629 Mon Sep 17 00:00:00 2001 From: folbrich Date: Mon, 18 May 2026 13:03:55 +0200 Subject: [PATCH 2/3] Fix never-run Windows assertions in TestLocationEquality; un-skip on Windows Investigation (via a temporary Windows CI diagnostic) showed locationMatch itself is correct on Windows. All Windows failures were incorrect, never-executed test expectations: - //path vs /path was asserted equal unconditionally, but that is a POSIX-only assumption: on Windows // is a UNC root, so it is legitimately not equal to the drive-relative /path. locationMatch intentionally uses OS-native local-path semantics. Now asserted per-OS. - The Windows block under "Not equal paths with globs" asserted True for dir\* vs dir\ and dir\? vs dir\123\. Both are wrong and inconsistent with the POSIX cases: dir\* does not match the bare dir (cf. /path/* vs /path), and ? matches exactly one character so it does not match "123". Corrected to False. Remove the Windows t.Skip; locationMatch is now covered on all three platforms. (Diagnostic test and temp workflow are removed before merge.) --- .github/workflows/_diag-windows.yaml | 2 +- cmd/desync/location_test.go | 32 ++++--- cmd/desync/locationdiag_test.go | 129 --------------------------- 3 files changed, 16 insertions(+), 147 deletions(-) delete mode 100644 cmd/desync/locationdiag_test.go diff --git a/.github/workflows/_diag-windows.yaml b/.github/workflows/_diag-windows.yaml index aaa4a5d..6fe62ff 100644 --- a/.github/workflows/_diag-windows.yaml +++ b/.github/workflows/_diag-windows.yaml @@ -19,4 +19,4 @@ jobs: - uses: actions/setup-go@v6 with: go-version-file: go.mod - - run: go test ./cmd/desync -run TestLocationDiag -v + - run: go test ./cmd/desync -run TestLocationEquality -v diff --git a/cmd/desync/location_test.go b/cmd/desync/location_test.go index 8ed0026..955c3d3 100644 --- a/cmd/desync/location_test.go +++ b/cmd/desync/location_test.go @@ -8,18 +8,6 @@ import ( ) func TestLocationEquality(t *testing.T) { - if runtime.GOOS == "windows" { - // This test (and locationMatch's local-path branch) has several - // Windows-specific assertions that were never executed before - // cmd/desync tests were added to CI. They expose genuine Windows - // path-semantics questions (UNC // roots, trailing-separator - // globbing, drive letters) that need the intended cross-platform - // matching contract defined. Quarantined on Windows pending a - // dedicated follow-up; locationMatch is still fully covered on - // Linux and macOS. - t.Skip("locationMatch Windows path semantics need a dedicated review; tracked separately") - } - // Equal URLs require.True(t, locationMatch("http://host/path", "http://host/path")) require.True(t, locationMatch("http://host/path/", "http://host/path/")) @@ -71,13 +59,19 @@ func TestLocationEquality(t *testing.T) { // Equal paths require.True(t, locationMatch("/path", "/path/../path")) require.True(t, locationMatch("//path", "//path")) - require.True(t, locationMatch("//path", "/path")) require.True(t, locationMatch("./path", "./path")) require.True(t, locationMatch("path", "path/")) require.True(t, locationMatch("path/..", ".")) if runtime.GOOS == "windows" { + // locationMatch intentionally uses OS-native path semantics for + // local paths. On Windows a leading // is a UNC root, so //path + // and the drive-relative /path are different; on POSIX they are + // the same (asserted in the else branch). + require.False(t, locationMatch("//path", "/path")) require.True(t, locationMatch("c:\\path\\to\\somewhere", "c:\\path\\to\\somewhere\\")) require.True(t, locationMatch("/path/to/somewhere", "\\path\\to\\somewhere\\")) + } else { + require.True(t, locationMatch("//path", "/path")) } // Equal paths with globs @@ -119,9 +113,13 @@ func TestLocationEquality(t *testing.T) { require.False(t, locationMatch("/pat?", "/pat")) require.False(t, locationMatch("/pat?", "/dir")) if runtime.GOOS == "windows" { - require.True(t, locationMatch("c:\\path\\to\\*", "c:\\path\\to\\")) - require.True(t, locationMatch("/path/to/*", "\\path\\to\\")) - require.True(t, locationMatch("c:\\path\\to\\?", "c:\\path\\to\\123\\")) - require.True(t, locationMatch("/path/to/?", "\\path\\to\\123\\")) + // These belong with the "not equal" cases above, consistent with + // POSIX behaviour: dir\* does not match the bare dir (cf. + // "/path/*" vs "/path"), and ? matches exactly one character so + // it does not match the 3-character segment "123". + require.False(t, locationMatch("c:\\path\\to\\*", "c:\\path\\to\\")) + require.False(t, locationMatch("/path/to/*", "\\path\\to\\")) + require.False(t, locationMatch("c:\\path\\to\\?", "c:\\path\\to\\123\\")) + require.False(t, locationMatch("/path/to/?", "\\path\\to\\123\\")) } } diff --git a/cmd/desync/locationdiag_test.go b/cmd/desync/locationdiag_test.go deleted file mode 100644 index b0c48df..0000000 --- a/cmd/desync/locationdiag_test.go +++ /dev/null @@ -1,129 +0,0 @@ -package main - -// TEMPORARY diagnostic — not for merge. Evaluates every locationMatch case -// from TestLocationEquality (including the Windows-only blocks) without -// short-circuiting, and prints the path-branch internals, so a single -// Windows CI run reveals the complete set of divergences and their causes. - -import ( - "fmt" - "net/url" - "path" - "path/filepath" - "runtime" - "strings" - "testing" -) - -func TestLocationDiag(t *testing.T) { - type c struct { - pattern, loc string - want bool - group string - } - cases := []c{ - // Equal URLs - {"http://host/path", "http://host/path", true, "eq-url"}, - {"http://host/path/", "http://host/path/", true, "eq-url"}, - {"http://host/path", "http://host/path/", true, "eq-url"}, - {"https://host/", "https://host", true, "eq-url"}, - {"https://host", "https://host/", true, "eq-url"}, - {"https://host", "https://host", true, "eq-url"}, - {"https://host/", "https://host/", true, "eq-url"}, - {"s3+https://example.com", "s3+https://example.com", true, "eq-url"}, - // Equal URLs with globs - {"https://host/path*", "https://host/path", true, "eq-url-glob"}, - {"https://host/path*", "https://host/path/", true, "eq-url-glob"}, - {"https://*", "https://example.com", true, "eq-url-glob"}, - {"https://example.com/path/*", "https://example.com/path/another", true, "eq-url-glob"}, - {"https://example.com/path/*", "https://example.com/path/another/", true, "eq-url-glob"}, - {"https://example.com/*/*/", "https://example.com/path/another/", true, "eq-url-glob"}, - {"https://example.com/*/", "https://example.com/2022.01/", true, "eq-url-glob"}, - {"https://*/*/*", "https://example.com/path/another/", true, "eq-url-glob"}, - {"https://example.*", "https://example.com", true, "eq-url-glob"}, - {"*://example.com", "https://example.com", true, "eq-url-glob"}, - {"http*://example.com", "https://example.com", true, "eq-url-glob"}, - {"http*://example.com", "http://example.com", true, "eq-url-glob"}, - {"https://exampl?.*", "https://example.com", true, "eq-url-glob"}, - {"http://examp??.com", "http://example.com", true, "eq-url-glob"}, - {"https://example.com/?", "https://example.com/a", true, "eq-url-glob"}, - {"https://example.com/fo[a-z]", "https://example.com/foo", true, "eq-url-glob"}, - // Not equal URLs - {"http://host:8080/path", "http://host/path", false, "ne-url"}, - {"http://host/path1", "http://host/path", false, "ne-url"}, - {"http://host/path1", "http://host/path/", false, "ne-url"}, - {"http://host1/path", "http://host2/path", false, "ne-url"}, - {"sftp://host/path", "http://host/path", false, "ne-url"}, - {"ssh://host/path", "/path", false, "ne-url"}, - {"ssh://host/path", "/host/path", false, "ne-url"}, - {"ssh://host/path", "/ssh/host/path", false, "ne-url"}, - // Not equal URLs with globs - {"*", "https://example.com/path", false, "ne-url-glob"}, - {"https://*", "https://example.com/path", false, "ne-url-glob"}, - {"https://example.com/*", "https://example.com/path/another", false, "ne-url-glob"}, - {"https://example.com/path/*", "https://example.com/path", false, "ne-url-glob"}, - {"http://*", "https://example.com", false, "ne-url-glob"}, - {"http?://example.com", "http://example.com", false, "ne-url-glob"}, - {"https://example.com/123?", "https://example.com/12345", false, "ne-url-glob"}, - {"*://example.com", "https://example.com/123", false, "ne-url-glob"}, - // Equal paths - {"/path", "/path/../path", true, "eq-path"}, - {"//path", "//path", true, "eq-path"}, - {"//path", "/path", true, "eq-path"}, - {"./path", "./path", true, "eq-path"}, - {"path", "path/", true, "eq-path"}, - {"path/..", ".", true, "eq-path"}, - // Equal paths (Windows-only block) - {"c:\\path\\to\\somewhere", "c:\\path\\to\\somewhere\\", true, "eq-path-win"}, - {"/path/to/somewhere", "\\path\\to\\somewhere\\", true, "eq-path-win"}, - // Not equal paths - {"/path", "path", false, "ne-path"}, - {"/path/to", "path/to", false, "ne-path"}, - {"/path/to", "/path/to/..", false, "ne-path"}, - // Not equal paths (Windows-only block) - {"c:\\path1", "c:\\path2", false, "ne-path-win"}, - // Not equal paths with globs - {"/path*", "/dir", false, "ne-path-glob"}, - {"/path*", "path", false, "ne-path-glob"}, - {"/path*", "/path/to", false, "ne-path-glob"}, - {"/path/*", "/path", false, "ne-path-glob"}, - {"/path/to/../*", "/path/to/another", false, "ne-path-glob"}, - {"/pat?", "/pat", false, "ne-path-glob"}, - {"/pat?", "/dir", false, "ne-path-glob"}, - // "Not equal paths with globs" Windows-only block (asserts True) - {"c:\\path\\to\\*", "c:\\path\\to\\", true, "win-glob"}, - {"/path/to/*", "\\path\\to\\", true, "win-glob"}, - {"c:\\path\\to\\?", "c:\\path\\to\\123\\", true, "win-glob"}, - {"/path/to/?", "\\path\\to\\123\\", true, "win-glob"}, - } - - t.Logf("GOOS=%s total=%d", runtime.GOOS, len(cases)) - fails := 0 - for i, tc := range cases { - got := locationMatch(tc.pattern, tc.loc) - status := "ok" - if got != tc.want { - status = "FAIL" - fails++ - } - // Recompute the branch internals for visibility. - detail := "" - if u, err := url.Parse(tc.loc); err == nil && len(u.Scheme) > 1 { - tp := strings.TrimSuffix(tc.pattern, "/") - tl := strings.TrimSuffix(tc.loc, "/") - pm, _ := path.Match(tp, tl) - detail = fmt.Sprintf("URL scheme=%q path.Match(%q,%q)=%v", u.Scheme, tp, tl, pm) - } else { - ap, _ := filepath.Abs(tc.pattern) - al, _ := filepath.Abs(tc.loc) - fm, fmErr := filepath.Match(ap, al) - detail = fmt.Sprintf("PATH Abs(p)=%q Abs(l)=%q filepath.Match=%v err=%v", ap, al, fm, fmErr) - } - t.Logf("[%2d] %-12s %-4s want=%-5v got=%-5v | p=%q l=%q | %s", - i, tc.group, status, tc.want, got, tc.pattern, tc.loc, detail) - } - t.Logf("SUMMARY: %d/%d failed on %s", fails, len(cases), runtime.GOOS) - if fails > 0 { - t.Errorf("%d locationMatch divergences on %s (see table above)", fails, runtime.GOOS) - } -} From d4432da356e06b3e120ffbd1944312b883e814c8 Mon Sep 17 00:00:00 2001 From: folbrich Date: Mon, 18 May 2026 13:08:51 +0200 Subject: [PATCH 3/3] Remove temporary Windows diagnostic workflow Investigation complete: TestLocationEquality passes on Windows with the corrected assertions (verified via this workflow before removal). --- .github/workflows/_diag-windows.yaml | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 .github/workflows/_diag-windows.yaml diff --git a/.github/workflows/_diag-windows.yaml b/.github/workflows/_diag-windows.yaml deleted file mode 100644 index 6fe62ff..0000000 --- a/.github/workflows/_diag-windows.yaml +++ /dev/null @@ -1,22 +0,0 @@ -name: Diag Windows - -# TEMPORARY — not for merge. Fast Windows-only loop for investigating -# locationMatch. Triggers only on pushes to the investigation branch. -on: - push: - branches: - - investigate-windows-locationmatch - -permissions: - contents: read - -jobs: - diag: - runs-on: windows-latest - timeout-minutes: 10 - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-go@v6 - with: - go-version-file: go.mod - - run: go test ./cmd/desync -run TestLocationEquality -v