ADFA-4816: Prune excluded dirs during Spotless traversal - #1564
Conversation
spotlessCheck took 12+ minutes on machines with an activated flox environment. Spotless targets used root-relative **/ globs with targetExclude(), which filters after Gradle has already walked the whole tree -- including flox/*/.flox/run/* symlinks into /nix/store (millions of files), OOM-ing the daemon. Switch each **/-glob block (java, kotlin, kotlinGradle, xml, misc) to a fileTree(rootDir) whose excludes prune directories during traversal. Key details: - Excludes use bare directory names (flox, **/.flox, **/.git, **/.gradle), not dir/**: Gradle prunes a subtree only when the exclude matches the directory node itself; dir/** matches contents and still descends. - Build outputs are excluded via each project's buildDirectory (mirroring Spotless's own auto-exclude), not **/build -- source packages named build (e.g. .../actions/build) exist and **/build would drop them. - commonTargetExcludes moved onto the include-tree so those large vendored dirs prune during the walk too. shell is unchanged (literal-prefixed targets already prune). The two tracked flox/*/.flox/.gitignore files leave the misc target set; both already conform, so the violation set is unchanged.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough
WalkthroughSpotless configuration centralizes traversal and build-output exclusions. Java, Kotlin, Gradle Kotlin script, XML, and miscellaneous targets now use ChangesSpotless target selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)build.gradle.ktsTraceback (most recent call last): 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.gradle.kts`:
- Around line 298-303: Update the target fileTree configuration to preserve
formatting for .gradle files while still excluding .gradle directories from
traversal. Replace the broad traversalExcludes entry affecting .gradle with a
directory-only exclusion, keeping the existing include("**/.gitignore",
"**/.gradle") behavior and other exclusions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 95753d8c-a4b7-4d94-aa08-4ebb59adce0d
📒 Files selected for processing (1)
build.gradle.kts
.gradle is only ever a cache directory (already pruned via traversalExcludes), never a formattable file, so including it in the misc target did nothing. Removing it resolves the apparent include/exclude contradiction (flagged in review). Behavior-neutral -- spotlessCheck output unchanged.
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Problem
spotlessChecktakes 12+ minutes on machines with an activated flox environment, and seconds elsewhere. Spotless targets use root-relative**/globs withtargetExclude(...), which filters matches after Gradle has already walked the whole tree — includingflox/*/.flox/run/*symlinks into/nix/store(measured 8.8M files on Linux). The daemon OOMs.Fixes ADFA-4816.
Fix
Convert each
**/-glob block (java, kotlin, kotlinGradle, xml, misc) fromtarget("**/glob")+targetExclude(...)totarget(fileTree(rootDir) { include(...); exclude(...) }), so excludes prune directories during traversal instead of subtracting after the walk (FormatExtensiondoestarget.minus(targetExclude)post-walk).Two things the naive fix gets wrong
Research against Spotless 8.8.0 + Gradle's walker corrected the ticket's proposed exclude list:
dir/**does not prune — it descends and filters. Gradle'sPathVisitor.preVisitDirectoryreturnsSKIP_SUBTREEonly when the exclude matches the directory node itself.flox/**matches flox's contents, so Gradle still walks in. Spotless's own auto-excludes use bare names for this reason. → excludes here useflox,**/.flox,**/.git,**/.gradle.**/buildwould drop real source. Source packages namedbuildexist (e.g.app/src/main/java/.../actions/build/DebugAction.kt); bare**/buildprunes them. → build outputs are excluded via each project'sbuildDirectory(exactly what Spotless does today), which never collide withsrc/.../buildpackages.commonTargetExcludesmoved onto the include-tree (prunes those large vendored dirs during the walk too).shellis unchanged — its targets are literal-prefixed, so Gradle already prunes without a full walk.Behavior change
The two tracked
flox/{base,local}/.flox/.gitignorefiles leave themisctarget set. Both already conform (no leading spaces, no trailing whitespace, final newline), so the violation set is unchanged.Verification (local, macOS)
spotlessCheckpasses on a clean tree..ktunderapp/src/.buildis NOT dropped —app/src/main/java/.../actions/build/*.ktis still flagged when dirtied (provesbuildOutputExcludestargets only real output dirs)..github/workflows/debug.ymlrunsflox activate -d flox/base -- ./gradlew spotlessCheck --no-daemon.Out of scope (follow-ups from the ticket, to be filed separately)
Pre-push hook scoping to changed files;
origin/stageratchet-ref freshness; the dead Sentry Gradle plugin.