wesl.toml symlinks and wesl.toml nesting#193
Merged
Merged
Conversation
webgpu-tools#185 includes a test case for a pathological wesl.toml config with overlapping globs previously, we deduplicated for that by using canonical fs paths, following symlinks to absolute paths. But pnpm uses symlinks inside node_modules/, so absolute paths might not match user exclude paths in wesl.toml. This broke in lygia when I was using a link: dependency for development. In this commit we switch to using lexical path normalization for deduplication, processing '.' and '..' in paths. That way user exclude paths match. And if a user wants to intentionally duplicate a file or path with a symlink, they can now do that too.
k2d222
requested changes
Mar 23, 2026
| r#" | ||
| edition = "2026_pre" | ||
| root = "./shaders/" | ||
| include = ["./shaders/**/*.wesl", "./shaders/../shaders/**/*.wesl"] |
Collaborator
There was a problem hiding this comment.
is this no longer a valid test case?
Contributor
Author
There was a problem hiding this comment.
It's always been a somewhat pathological user configuration, but it's still a valid test case..
The scanning approach fixes it architecturally though,
so it would be reasonable to remove the test case to remove a little test bulk.
preference?
Collaborator
There was a problem hiding this comment.
would be good to have at least one test to check that you can't access files outside of the project root using ... Maybe one too for valid uses of ... Or perhaps forbid .. entirely, as you said it's pathological.
intead of manually stripping ./
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
the first commit fixes an issue I found testing lygia.
basically, the effort to handle pathological wesl.toml configs with overlapping glob patterns was overeager. It followed symlinks as a way to identify overlaps, but exclude patterns are in the language of sources, not symlink targets.
the second commit mostly obsoletes the first commit by manually walking the fs tree. The second commit also implements sub directory wesl.toml files as proposed in spec webgpu-tools/wesl-spec#172. We haven't discussed that one yet, so the implementation here is speculative.