fix: local corpus paths were unreadable on Windows (backslashes eaten) #55

Merged
jared merged 1 commits from fix/windows-backslash-paths into main 2026-08-09 09:08:42 -04:00
Owner

The bug

gsub() in regex mode treats backslashes in the replacement string as escape sequences and silently drops them. A Windows corpus path is nothing but backslashes:

C:\Users\RUNNER~1\AppData\Local\Temp\Rtmp123/   (in)
C:UsersRUNNER~1AppDataLocalTempRtmp123/         (substituted into the SQL)

Every DuckDB read then failed with IO Error: No files found that match the pattern.

What it actually broke

  • A local corpus was unreadable on Windows. That includes the bundled fixture, so the entire test suite failed there.
  • cog_mirror() was unusable on Windows — the exact escape hatch the new README recommends to anyone who would rather not depend on a third party for their data.
  • Remote https URLs were fine, having no backslashes. Part of why this stayed hidden.

This is not new

The bug lived in the original gsub("\\{url\\}", url, sql, fixed = FALSE) since that line was written; it was inherited when .render_view_sql() was extracted in #41. Nothing ever ran on Windows until the mirror's check matrix existed in #53 — which found it on its first run.

Of the 15 Windows failures, 14 were this directly; the 15th was a downstream consequence of view registration failing, so no views existed to assert on.

The fix

fixed = TRUE on both substitutions. It treats pattern and replacement as literal text, which is what a filesystem path needs.

Test plan

  • devtools::test() — 972 passing, 0 failures
  • New regression test asserts a Windows-style path survives both the {url} and {long_files} substitutions, and that C:Users (the mangled form) never appears
  • The regression test reproduces on any platform — this is string handling, not filesystem behaviour, so catching it needs no Windows runner
  • Windows matrix green — verified once this reaches the mirror

Note on versioning

Folding this into 0.3.0 rather than cutting 0.3.1: nothing is published yet, no one has installed anything, and the first public release should not ship with a known-broken platform. #47 (tag + r-universe) should wait for the Windows matrix to go green.

## The bug `gsub()` in regex mode treats backslashes **in the replacement string** as escape sequences and silently drops them. A Windows corpus path is nothing but backslashes: ``` C:\Users\RUNNER~1\AppData\Local\Temp\Rtmp123/ (in) C:UsersRUNNER~1AppDataLocalTempRtmp123/ (substituted into the SQL) ``` Every DuckDB read then failed with `IO Error: No files found that match the pattern`. ## What it actually broke - **A local corpus was unreadable on Windows.** That includes the bundled fixture, so the **entire test suite** failed there. - **`cog_mirror()` was unusable on Windows** — the exact escape hatch the new README recommends to anyone who would rather not depend on a third party for their data. - **Remote https URLs were fine**, having no backslashes. Part of why this stayed hidden. ## This is not new The bug lived in the original `gsub("\\{url\\}", url, sql, fixed = FALSE)` since that line was written; it was inherited when `.render_view_sql()` was extracted in #41. Nothing ever ran on Windows until the mirror's check matrix existed in #53 — which found it on its **first run**. Of the 15 Windows failures, **14 were this directly**; the 15th was a downstream consequence of view registration failing, so no views existed to assert on. ## The fix `fixed = TRUE` on both substitutions. It treats pattern and replacement as literal text, which is what a filesystem path needs. ## Test plan - [x] `devtools::test()` — **972 passing**, 0 failures - [x] New regression test asserts a Windows-style path survives both the `{url}` and `{long_files}` substitutions, and that `C:Users` (the mangled form) never appears - [x] The regression test **reproduces on any platform** — this is string handling, not filesystem behaviour, so catching it needs no Windows runner - [ ] Windows matrix green — verified once this reaches the mirror ## Note on versioning Folding this into `0.3.0` rather than cutting `0.3.1`: nothing is published yet, no one has installed anything, and the first public release should not ship with a known-broken platform. #47 (tag + r-universe) should wait for the Windows matrix to go green.
jared added 1 commit 2026-08-09 09:04:04 -04:00
fix: local corpus paths were unreadable on Windows (backslashes eaten)
R-CMD-check / check (push) Successful in 3m35s
R-CMD-check / check (pull_request) Successful in 3m49s
72b2cc3a26
gsub() in regex mode treats backslashes in the REPLACEMENT string as
escape sequences and silently drops them. A Windows corpus path is full of
them, so C:\Users\RUNNER\AppData\... was substituted into the view SQL
as C:UsersRUNNERAppData... and every DuckDB read failed with 'No files
found that match the pattern'.

Effect: uscogdata could not read a LOCAL corpus on Windows at all -- the
bundled fixture included, so the entire test suite failed there, and any
cog_mirror() copy was unusable. Remote https URLs were unaffected, having
no backslashes, which is part of why it stayed hidden.

The bug predates the {long_files} token; it lived in the original {url}
substitution since that code was written. Nothing ever ran on Windows
until the GitHub mirror's check matrix existed, which found it on its
first run: 14 of 15 Windows failures were this, the 15th a downstream
consequence of view registration failing.

fixed = TRUE treats pattern and replacement as literal text. The
regression test reproduces on any platform -- it is string handling, not
filesystem behaviour, so it needs no Windows runner.
jared merged commit 698812a25c into main 2026-08-09 09:08:42 -04:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Civilytics/uscogdata#55