Repository navigation
Conversation
In tailwindlabs#15941, the intention was to prevent .gitignore files outside of a git repository from taking effect while ensuring global ignore files configured in git (like core.excludesFile) continue to work. However, builder.git_global(false) was explicitly set in the oxide scanner walker, which caused git's global ignore configuration (core.excludesFile / ~/.config/git/ignore) to be completely ignored. This change enables git_global(true) so that global ignore rules configured in git are honored as expected during source detection. Fixes tailwindlabs#20509
[Medium impact] The change affects which files Tailwind’s source scanner returns. The PR needs the remaining cross-repository global-ignore behavior fixed before merging. Reviews (7) · Last reviewed commit: "fix(scanner): resolve global ignore patt..." · Reviewed by Greptile |
|
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:
WalkthroughThe excludes-file parser now accepts quoted Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/oxide/tests/scanner.rs (1)
2961-2961: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSynchronize the process-wide
GIT_CONFIG_GLOBALmutation.
GIT_CONFIG_GLOBALis process-wide, and global Git-ignore handling reads it duringScanner::new. The configured pattern matches onlyignored-by-global.html. The inspected concurrent scanner fixtures do not contain that basename, and their Git commands are onlygit initcalls with ignored output. The race therefore does not currently change an asserted result or cause a command failure. It can affect a future scan that contains this basename. Isolate this test in a separate process or protect the mutation with a shared lock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 7f764ba3-4601-4e02-b611-e93f5ae31d2d
📒 Files selected for processing (2)
crates/oxide/src/scanner/mod.rscrates/oxide/tests/scanner.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
|
Hi maintainers, following up on this regression-test PR. It enables Git global ignore files during source scanning and adds coverage for core.excludesFile, addressing #20509. Greptile review is passing and the branch is mergeable; happy to adjust the implementation or test if there is any concern. |
The hooks are in git/hooks now, still found through core.hooksPath. That frees git/template to be an actual init.templateDir again. Sure, the hooks could've stayed behind, but then they'd be copied over on every `git init` and be the inert copies `git hooks-clean` removes. For now, this is only info/exclude with /.claude/ in it. But why, especially when ~/.gitignore (my core.excludesfile) already has it? Well, there's a bit of belt-and-suspenders action here, just super ensuring this doesn't get in unless I really, really want it to. Also, Tailwind v4 doesn't read the global excludes file, and it _does_ read .git/info/exclude. So something in some note can end up in the built CSS, even when it's not used in the app. Watch tailwindlabs/tailwindcss#20524 to see if/when that changes.
| // Don't respect global gitignore files | ||
| builder.git_global(false); | ||
| // Respect global gitignore files configured in git (e.g. `core.excludesFile`) | ||
| builder.git_global(true); |
There was a problem hiding this comment.
Global ignores use wrong directory
Global ignore patterns containing / are matched from the process working directory, not the scanned repository. If the CLI scans /projects/app from another directory, a global rule src/ignored.html does not exclude /projects/app/src/ignored.html. The scanner still reads that file and generates CSS from it, even though Git ignores it.
Build the global matcher relative to each repository root rather than the process working directory.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Accept valid core.excludesFile syntax at the parser boundary. · gitignore.rs:705
crates/ignore/src/gitignore.rs:705
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAccept valid
core.excludesFilesyntax at the parser boundary.The regex rejects valid Git values such as
core.excludesFile = " /foo/bar "and/foo/bar # comment. The parser then falls back to another configuration source or the default path, so the configured global ignore file is not applied during source scanning.Suggested fix
- .build(r#"(?im-u)^\s*excludesfile\s*=\s*(?:"(\S(?:[^"\r\n]*\S)?)"|(\S+))\s*$"#) + .build(r#"(?im-u)^\s*excludesfile\s*=\s*(?:"([^"\r\n]*)"|(\S+))(?:\s+#.*)?\s*$"#)
🟡 Minor · Apply Git-compatible pathname expansion in expand_tilde. · gitignore.rs:717-723
crates/ignore/src/gitignore.rs:717-723
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply Git-compatible pathname expansion in
expand_tilde.
expand_tildereplaces every~with the current user's home directory. Forcore.excludesFile = ~other-user/ignore, it selects a path under the current user's home. It also changes literal tildes in paths such as/config/~backup/ignore. The scanner can then miss patterns from the configured global ignore file.Expand only Git's
~/and~user/forms. Resolve~user/using that user's home directory, and preserve all other tildes. Do not usestr::replacefor pathname expansion.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
fb0000c8-a0fa-41c2-baee-07bb877c68d2
📒 Files selected for processing: 2
crates/oxide/src/scanner/mod.rscrates/oxide/tests/scanner.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
When scanning for source candidates, Oxide previously disabled git global ignore files via
builder.git_global(false).As noted in #15941, the intention was to prevent
.gitignorefiles in parent directories outside of a git repository from taking effect, while keeping global ignore files configured in git working. However, callinggit_global(false)on the walker had the unintended side effect of ignoringcore.excludesFile(and$XDG_CONFIG_HOME/git/ignore) entirely.This change enables
builder.git_global(true)so that ignore patterns configured in Git's global configuration are respected during source scanning.Fixes #20509
Test plan
respects_git_core_excludes_fileincrates/oxide/tests/scanner.rsverifying that files matched by Git'score.excludesFileare skipped.[ci-all]