Restrict walking sibling folders when using a pattern#20263
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Confidence Score: 5/5Safe to merge — the change is well-scoped, the logic is correct, and the extensive test suite covers the described edge cases including interleaved sources, wildcard intermediate segments, negation ordering, and coexistence with broad auto-detect sources. The No files require special attention. Reviews (4): Last reviewed commit: "update changelog" | Re-trigger Greptile |
ee3b3b9 to
60d8a95
Compare
60d8a95 to
5abd92d
Compare
This PR fixes an issue where a
@sourcepointing to a concrete file could result in scanning the entire parent folder instead of only looking for the file we are actually interested in.When we optimize a
@source, we move all the static parts of the pattern to thebase. This means that a@sourcelike this:Resolves to:
When walking the
base, we would only emit an!app.config.tsrule. This means that everything in the/Usersfolder is still walked, and the result is then thrown away. If you take a look at thegitignoreequivalent (which is what we build behind the scenes), then we would essentially create the following:But if you know how
gitignorefiles work, then you know that this does forceapp.config.tsto not be ignored, but it doesn't say anything about all the other files/folders.The fix is to restrict the
baseso that we ignore everything in the folder, and then explicitly re-include only the pattern we care about. Fixing this would result in the following:In the reproduction from #20255 this takes the build from appearing to hang (~22s) down to ~20ms on my machine.
There are a few edge cases we have to be careful about:
Multiple patterns for the same base. If we have multiple
@sourcedirectives for the same folder:Then blindly emitting
*for each one would result in this.gitignoreequivalent:Notice that the second
*would end up ignoringfoo.tsagain. To avoid this, we only emit the*rule once perbase.Dynamic parts in intermediate folders. If the pattern still contains a
*in one of its folders:This resolves to:
If we now inject the
*rule for/src, then we would never walk theba*folders (e.g.barorbaz). To fix this, we add inverse rules for each parent segment of the pattern:Bases already covered by a broader source. If the
baseis already included (or nested) under an unrestricted source (anAuto/Externalsource, or aPatterncontaining**), then we leave it alone. Restricting it would incorrectly hide siblings that the broader source is supposed to pick up. For example:Here the
**/*source should keep auto-detecting every file, so we must not restrictsrc/componentsjust because there's a more specific@sourcepointing to it.Source order is preserved throughout, so later
@source not …rules can still override an earlier restricted source.Fixes: #20255
Test plan
sourceslogic