Scan git-ignored directories inside explicit glob sources - #20405
Conversation
The extension rule emitted for glob sources already whitelists matching files against .gitignore, but it cannot stop the walker from pruning git-ignored directories on the way to those files. Directories that ignore themselves with '*' (like Laravel's storage/ directories) were never entered, so explicit sources like '@source "../storage/app/private/cms_content/**/*.html"' found nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughThe scanner now tracks extension-based glob-source bases and reopens ignored directories beneath those bases during traversal. Automatic directory ignores and 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Confidence Score: 5/5The PR appears safe to merge, with no concrete correctness or security defects identified. The new directory whitelist has the intended precedence over on-disk gitignore rules, while later built-in and explicit source exclusions continue to prune protected directories. Reviews (1): Last reviewed commit: "Add changelog entry" | Re-trigger Greptile |
| // Re-open directories under glob sources. These rules are registered before | ||
| // the auto source detection rules and the `@source` rules so both still take | ||
| // precedence, e.g. `node_modules` inside a glob source stays pruned unless | ||
| // targeted explicitly. | ||
| for base in reopened_bases { | ||
| let mut ignore_builder = GitignoreBuilder::new(base); | ||
| ignore_builder.add_line(None, "!*/").unwrap(); | ||
| let ignore = ignore_builder.build().unwrap(); | ||
| builder.add_gitignore(ignore); | ||
| } |
There was a problem hiding this comment.
This part opens a new can of worms that I only want to deal with if we can prove that there are issues that can't be solved by simpler glob patterns. More info: #18870 (comment)
In this case, let's say you have a setup like this:
# .gitignore
dist/Then you have your CSS file that contains:
@source './src';
@source './src/**/*.html';If you then, for any reason, have a file like ./src/dist/index.js then this file will be scanned because the src/ folder was re-opened. It was re-opened for the .html use case, not the .js use case.
|
Thanks for the PR! Were you trying to fix #18870, or did you run into an issue yourself with this kind of setup? If you ran into this issue yourself, can you open an issue with a minimal reproduction repo attached so we can take a look? This PR does improve some of the bugs involved, but it introduces new bugs as well. I just want to make sure that if there is a bug that can't be solved in user land with a different Thanks again! |
2 participants
Fixes #18870
Summary
@source "../storage/app/private/cms_content/**/*.html"finds nothing when the target directory is ignored from the inside, i.e. a.gitignorecontaining*+!.gitignore, the way Laravel ships everystorage/directory. The reporter's testcase also shows the two shapes that already work: pointing@sourceat the exact file, and the same glob once the nested.gitignoreis deleted.The asymmetry is in how the walker treats files vs. directories. Glob sources emit an
!*.htmlrule, and manually registered rules rank above on-disk.gitignorefiles, so a git-ignored file matched by an explicit glob is scanned today (this PR adds a test pinning that). That rule can't save anything inside a git-ignored directory though:*matches the child directories themselves, the walker prunes them, and no file rule ever runs. An ignore in an ancestor.gitignore(say/storagein the project root) doesn't trigger the bug, because the normalized source base starts the walk inside the ignored tree. That's what makes the failure look random unless you know where the ignore line lives.Glob sources now also register a directory re-include (
!*/) for their base, ordered so that everything that should still win, does:node_modulesetc. inside a glob source stay pruned unless targeted explicitly (the concern raised in the issue about accidentally walking huge trees)@sourcerules, so@source notstill excludes directories inside a re-opened source.gitignorefiles rank below manually registered rules, which is what lets the walker descendBare directory sources (
@source "../storage/app/private/cms_content") are unchanged: they emit no file rules, so.gitignoresemantics inside them still apply, as pinned byit_respects_gitignore_in_workspace_root.Test plan
crates/oxide/tests/scanner.rs: the issue's Laravel shape, a glob through an ancestor-ignored directory, git-ignored files matched by a glob (pins existing behavior),node_modulesstaying pruned inside a re-opened source, and@source notinside one.cargo test --workspacepasses.src/, relative../storage/...sources,source(none)): before this change the scanner returns no files fromcms_content; with it, the pre-rendered HTML is scanned and its candidates extracted.[ci-all]