Fix absolute patterns with glob #10121
Conversation
|
❌ @zackradisic 4 files with test failures on linux-x64-baseline:
|
|
❌ @zackradisic 4 files with test failures on bun-darwin-aarch64:
|
|
❌ @zackradisic 4 files with test failures on linux-x64:
|
|
❌ @zackradisic 4 files with test failures on bun-darwin-x64:
|
test/js/bun/glob/scan.test.ts
Outdated
| @@ -445,6 +446,15 @@ test("glob.scan('.')", async () => { | |||
| expect(entries).toContain("README.md"); | |||
| }); | |||
|
|
|||
| test("absolute path pattern should ignore cwd and start at the proper path", async () => { | |||
There was a problem hiding this comment.
can we get more tests for this? is there a way to run all the tests that run against relative paths to also run against absolute paths?
|
tests timed out on Windows |
|
❌ @zackradisic 14 files with test failures on bun-windows-x86_64-haswell
|
|
conflict |
|
glob scan test is failing |
| if (pattern[component.start + component.len -| 1] == '/') { | ||
| component.trailing_sep = true; | ||
| } else if (comptime bun.Environment.isWindows) { | ||
| component.trailing_sep = pattern[component.start + component.len -| 1] == '\\'; |
There was a problem hiding this comment.
This should probably be any separator instead of specifically \
|
@zackradisic the next step for this PR is to fix the failing scan test |
| // `/Users/zackradisic/foo/bar` | ||
| // In that case we don't need to do any walking and can just open up the FS entry | ||
| if (component_idx >= this.walker.patternComponents.items.len) { | ||
| const path = try this.walker.arena.allocator().dupeZ(u8, path_without_special_syntax); |
There was a problem hiding this comment.
does this need to be duplicated twice
|
one nitpicky comment but lets merge |
| return Maybe(void).success; | ||
| } | ||
| const errpath = try this.walker.arena.allocator().dupeZ(u8, path); | ||
| return .{ .err = e.withPath(errpath) }; |
There was a problem hiding this comment.
I suspect there will be more errors to ignore here. Like EPERM, EACCESS, EBUSY etc
What does this PR do?
This fixes a bug where absolute patterns with Bun.Glob were not working
Fixes #8304
How did you verify your code works?
I included a test for the new code, or existing tests cover it
I ran my tests locally and they pass (
bun-debug test test-file-name.test)I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
I included a test for the new code, or an existing test covers it
JSValue used outside outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed
I wrote TypeScript/JavaScript tests and they pass locally (
bun-debug test test-file-name.test)