Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Recursive globs fail too early on partial path match #14934

Open
refi64 opened this issue Nov 1, 2024 · 4 comments · May be fixed by #16824
Open

Recursive globs fail too early on partial path match #14934

refi64 opened this issue Nov 1, 2024 · 4 comments · May be fixed by #16824
Labels
bug Something isn't working bun:glob Related to Bun.Glob confirmed bug We can reproduce this issue

Comments

@refi64
Copy link
Contributor

refi64 commented Nov 1, 2024

What version of Bun is running?

1.1.31+e448c4cc3

What platform is your computer?

Linux 6.11.0-400.asahi.fc40.aarch64+16k aarch64 unknown

What steps can reproduce the bug?

console.log(new Bun.Glob(`**/*abc*`).match(`a/abc`))
console.log(new Bun.Glob(`**/*abc*`).match(`z/abc`))

What is the expected behavior?

true
true

What do you see instead?

false
true

Additional information

I'm pretty sure this is because the glob code sees the a and tries the parse the full abc, then bails out early when that fails?
@refi64 refi64 added bug Something isn't working needs triage labels Nov 1, 2024
@refi64
Copy link
Contributor Author

refi64 commented Nov 1, 2024

In glob_ascii.zig, we have:

bun/src/glob_ascii.zig

Lines 401 to 403 in 6933208

// If this is not a separator, lock in the previous globstar.
if (cc != '/')
state.globstar.path_index = 0;

I...don't think this is correct? In a/abc, it "locks in" at the first a, which makes no sense. Does this maybe need to maintain a stack or similar?

@Jarred-Sumner
Copy link
Collaborator

@zackradisic wdyt?

@Jarred-Sumner
Copy link
Collaborator

@dylan-conway spent some time investigating. This bug is present in both the Zig and Rust library our glob implementation was partially based on

@dylan-conway
Copy link
Member

Likely the same bug: devongovett/glob-match#9

@nektro nektro added the bun:glob Related to Bun.Glob label Nov 4, 2024
@RiskyMH RiskyMH added the confirmed bug We can reproduce this issue label Dec 3, 2024
@RiskyMH RiskyMH mentioned this issue Jan 20, 2025
Closed
@RiskyMH RiskyMH marked this as a duplicate of #16526 Jan 20, 2025
@probably-neb probably-neb linked a pull request Jan 28, 2025 that will close this issue
Open
4 tasks
@RiskyMH RiskyMH marked this as a duplicate of #16878 Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working bun:glob Related to Bun.Glob confirmed bug We can reproduce this issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Glob Match Rewrite probably-neb/bun
5 participants
@Jarred-Sumner @refi64 @nektro @dylan-conway @RiskyMH