fix(ignore): use case-sensitive matching to prevent BUILD file exclusion on macOS#1166
Conversation
…ion on macOS On case-insensitive file systems (macOS HFS+/APFS), the default ignore pattern `build/**` incorrectly matches Pants build tool's `BUILD` files. Setting `caseSensitiveMatch: true` in globby options ensures ignore patterns match with exact case, so `build/**` only matches lowercase `build/` directories — not `BUILD` source files. Fixes yamadashy#980
Summary of ChangesHello @lucamorettibuilds, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue on case-insensitive file systems, particularly macOS, where the default Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA single configuration option ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The changes successfully address the issue where BUILD files were incorrectly ignored on case-insensitive filesystems (like macOS) due to the default build/** ignore pattern. By setting caseSensitiveMatch: true in the globby options, the tool now ensures that ignore patterns are matched with exact case. This also resolves an existing inconsistency where globby followed OS case-sensitivity while minimatch (used in findEmptyDirectories) was case-sensitive by default. The implementation is consistent across all file searching functions and the tests have been appropriately updated to reflect this change.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/core/file/fileSearch.test.ts (1)
267-296: Test assertions correctly updated; consider adding a behavioral regression test for the bug.The two updated
expect.objectContaining/toMatchObjectassertions correctly verify thatcaseSensitiveMatch: trueis forwarded to globby. However, the test suite has no case that demonstrates the actual bug fix — i.e., that aBUILDfile is not excluded whenbuild/**(or**/build/**) is among the ignore patterns. Without a dedicated test, a future refactor that reverts or weakens the option would silently re-introduce the regression.Consider adding a test like:
test('should not exclude uppercase BUILD files when build/** is in ignore patterns', async () => { const mockConfig = createMockConfig({ include: ['**/*'], ignore: { useGitignore: false, useDefaultPatterns: false, // Simulate the default `build/**` ignore pattern customPatterns: ['build/**'], }, }); // Globby with caseSensitiveMatch: true should NOT exclude BUILD files vi.mocked(globby).mockResolvedValue(['src/BUILD', 'BUILD']); const result = await searchFiles('/mock/root', mockConfig); expect(result.filePaths).toContain('src/BUILD'); expect(result.filePaths).toContain('BUILD'); expect(globby).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ caseSensitiveMatch: true }), ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/file/fileSearch.test.ts` around lines 267 - 296, The test suite lacks a behavioral regression test proving that searchFiles forwards caseSensitiveMatch to globby so uppercase filenames like BUILD aren't excluded by ignore patterns such as "build/**"; add a new test in tests/core/file/fileSearch.test.ts that calls searchFiles with a mockConfig whose ignore.customPatterns includes 'build/**' (and useGitignore/useDefaultPatterns false), mock globby to resolve ['src/BUILD','BUILD'], call await searchFiles('/mock/root', mockConfig), assert the returned result.filePaths contains both 'src/BUILD' and 'BUILD', and assert globby was called with expect.objectContaining({ caseSensitiveMatch: true }) so the behavior of searchFiles (the function under test) is explicitly verified.src/core/file/fileSearch.ts (1)
306-320: Add an inline comment explaining whycaseSensitiveMatch: trueis set explicitly.
caseSensitiveMatchalready defaults totruein fast-glob, so without a comment, a future maintainer may read this as noise and remove it. The motivation — preventing the defaultbuild/**ignore pattern from matching PantsBUILDfiles on macOS case-insensitive (HFS+/APFS) filesystems — should be documented inline.Additionally, note that
caseSensitiveMatch: trueonly governs theignore:array patterns (processed by micromatch). Thegitignore: truecode path uses a separate parsing mechanism (theignorenpm module) and is not affected by this option, so case sensitivity for.gitignore-sourced patterns remains OS/git-controlled. This is not a new problem, but worth a brief note in the comment.✏️ Suggested comment
- caseSensitiveMatch: true, + // Force case-sensitive pattern matching regardless of OS filesystem case sensitivity. + // Without this, on macOS (HFS+/APFS case-insensitive), the default ignore pattern + // `build/**` would incorrectly exclude Pants `BUILD` files. Although fast-glob defaults + // to true, we set it explicitly to document the intent and prevent accidental removal. + // Note: this only affects patterns in the `ignore` array (via micromatch); patterns + // sourced from .gitignore files are handled separately and are not affected by this option. + caseSensitiveMatch: true,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/file/fileSearch.ts` around lines 306 - 320, Add an inline comment above the caseSensitiveMatch: true setting in createBaseGlobbyOptions explaining that we explicitly set caseSensitiveMatch to true to avoid glob ignore patterns like "build/**" matching files named BUILD on case-insensitive macOS filesystems (HFS+/APFS), and also note that this option only affects patterns passed via the ignore array (micromatch) and does not change case handling for gitignore=true (which is handled by the ignore npm module and remains OS/git-controlled). Ensure the comment references caseSensitiveMatch and clarifies the distinction from the gitignore code path so future maintainers won't remove this explicit setting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/core/file/fileSearch.ts`:
- Around line 306-320: Add an inline comment above the caseSensitiveMatch: true
setting in createBaseGlobbyOptions explaining that we explicitly set
caseSensitiveMatch to true to avoid glob ignore patterns like "build/**"
matching files named BUILD on case-insensitive macOS filesystems (HFS+/APFS),
and also note that this option only affects patterns passed via the ignore array
(micromatch) and does not change case handling for gitignore=true (which is
handled by the ignore npm module and remains OS/git-controlled). Ensure the
comment references caseSensitiveMatch and clarifies the distinction from the
gitignore code path so future maintainers won't remove this explicit setting.
In `@tests/core/file/fileSearch.test.ts`:
- Around line 267-296: The test suite lacks a behavioral regression test proving
that searchFiles forwards caseSensitiveMatch to globby so uppercase filenames
like BUILD aren't excluded by ignore patterns such as "build/**"; add a new test
in tests/core/file/fileSearch.test.ts that calls searchFiles with a mockConfig
whose ignore.customPatterns includes 'build/**' (and
useGitignore/useDefaultPatterns false), mock globby to resolve
['src/BUILD','BUILD'], call await searchFiles('/mock/root', mockConfig), assert
the returned result.filePaths contains both 'src/BUILD' and 'BUILD', and assert
globby was called with expect.objectContaining({ caseSensitiveMatch: true }) so
the behavior of searchFiles (the function under test) is explicitly verified.
…t for caseSensitiveMatch
1 participant
Summary
On case-insensitive file systems (macOS HFS+/APFS), the default ignore pattern
build/**incorrectly matches Pants build tool'sBUILDfiles. This one-line fix ensures ignore patterns match with exact case.Problem
build/**(defaultIgnore.ts:111)BUILDfiles (used by Pants) get matched bybuild/**!BUILDto include patternsFix
Sets
caseSensitiveMatch: trueincreateBaseGlobbyOptions(). This is a globby/fast-glob option that forces case-sensitive pattern matching regardless of the OS filesystem.build/**will only match lowercasebuild/directories — notBUILDsource files.This is the simplest and most correct approach from the potential solutions listed in #980 (option 1). It also matches the behavior users expect from their
.gitignoreand.repomixignorepatterns.Changes
src/core/file/fileSearch.ts: AddcaseSensitiveMatch: truetocreateBaseGlobbyOptions()tests/core/file/fileSearch.test.ts: Update globby options assertionsFixes #980