Faster globbing by respecting root .gitignore. #1209
Merged
Conversation
This file contains 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
Instead of globbing everything for **/spago.yaml and then filtering the matches against .gitignore files, use every entry in the root .gitignore as a negative match (ignore) while globbing. The resulting matches still need to be checked seperately using `Git.isIgnored`, because the new function only considers the root .gitignore file, .gitignore files in subdirectories are ignored.
f-f
approved these changes
Apr 2, 2024
Blugatroff
added a commit
to Blugatroff/spago
that referenced
this pull request
Apr 5, 2024
Addresses purescript#1182 This implementation is using `micromatch` (a dependency of fast-glob) and `fs.walk` to traverse the file system itself, instead of relying on fast-glob. As opposed to the previous implementation (from purescript#1209) This implementation takes every .gitignore file into account, not just the root one. The callbacks `entryFilter` and `deepFilter` are used to control which directories to recurse into. When `entryFilter` encounters a .gitignore file, it's patterns are parsed into micromatch compatible ones and every subsequent call to these filter functions respect them. I'm not sure whether this should have been (partially) implemented in the registry-dev repo since that's where the FFI for fast-glob is located. I also did not remove the fast-glob npm dependency, since i'm not sure if it is still in use elsewhere and because the registry-dev dependency definitely still contains the foreign functions relying on it being available.
3 tasks
f-f
pushed a commit
that referenced
this pull request
Apr 19, 2024
Fixes #1182 This implementation is using `micromatch` (a dependency of fast-glob) and `fs.walk` to traverse the file system itself, instead of relying on fast-glob. As opposed to the previous implementation (from #1209) This implementation takes every .gitignore file into account, not just the root one. The callbacks `entryFilter` and `deepFilter` are used to control which directories to recurse into. When `entryFilter` encounters a .gitignore file, it's patterns are parsed into micromatch compatible ones and every subsequent call to these filter functions respect them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Addresses part of #1182 by implementing this snippet.
I'm not sure, whether this is the right place for the change, would it have been better to implement this purescript/registry-dev since that's where Glob.match' is implemented? Although then again, Glob.match' is just a wrapper for fast-glob.
On my laptop and repo mentioned in #1182 this change reduces the time spent globbing for spago.yaml from 600ms to 30ms :)
Description of the change
Instead of globbing everything for **/spago.yaml and then filtering the matches against .gitignore files, use every entry in the root .gitignore as a negative match (ignore) while globbing.
The resulting matches still need to be checked seperately using
Git.isIgnored, because the new function only considers the root .gitignore file, .gitignore files in subdirectories are ignored.Checklist:
README