From f2bafe28e47f3ffd2376547ec5abe4bbe9df3896 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 24 Aug 2026 18:07:01 -0700 Subject: [PATCH 1/7] ADFA-5263: Delete copyToTestDir, which copied a jar nothing reads The task copied tooling-api-model.jar to /tests/test-home/.cg/init/model.jar. Nothing reads that file, and nothing reads that directory. The only consumer of a "test home" is gradle-plugin's test helper, which resolves FileProvider.testHomeDir() -- testing/resources/test-home, a different directory -- and then *writes its own* init script there, with a classpath from Gradle's PluginUnderTestMetadataReading. It never asks for a model jar. A grep for model.jar across the repo returned only the task that produced it. The destination had drifted before: 2a841748b (Feb 2023) is "fix: invalid path specified in copyToTestDir", and #1161 renamed .androidide to .cg inside it. Removing it takes three problems with it: - into(rootProject.mkdir(...)) ran at configuration time, so merely realizing the task created directories in the source tree -- on --dry-run, and again after every clean. That is why tests/test-home kept reappearing. - outputs.upToDateWhen { false } on both the copy and jar meant any build touching this module re-jarred and re-copied unconditionally. - Its output being a directory inside the source tree is what tripped Gradle's implicit-dependency validation against Spotless (ADFA-5244). That was worked around at the consumer by excluding the directory from the Spotless walk. Verified: the jar still builds; tests/ is no longer created at configuration time; :app:assembleV8Debug succeeds with the task absent from the graph; and `:common:compileV8DebugKotlin spotlessCheck` -- the exact invocation ADFA-5244 was filed for -- now passes on this branch, which carries no Spotless exclude at all. The two .gitignore entries that existed only for this task's output go too. Co-Authored-By: Claude Opus 5 --- .gitignore | 2 -- subprojects/tooling-api-model/build.gradle.kts | 13 ------------- 2 files changed, 15 deletions(-) diff --git a/.gitignore b/.gitignore index af44d5bf1c..dec603ee97 100755 --- a/.gitignore +++ b/.gitignore @@ -104,8 +104,6 @@ sentry.properties .DS_Store # Generated files for tooling API -tests/test-home -/tests/**/.cg/init/model.jar /composite-builds/build-deps-common/constants/build/ /composite-builds/build-deps/build/ diff --git a/subprojects/tooling-api-model/build.gradle.kts b/subprojects/tooling-api-model/build.gradle.kts index 2c85462c59..49620778b5 100644 --- a/subprojects/tooling-api-model/build.gradle.kts +++ b/subprojects/tooling-api-model/build.gradle.kts @@ -31,16 +31,3 @@ dependencies { implementation(libs.common.jkotlin) } - -tasks.register("copyToTestDir") { - from(project.layout.buildDirectory.file("libs/tooling-api-model.jar")) - into(project.rootProject.mkdir("tests/test-home/.cg/init")) - rename { "model.jar" } - - outputs.upToDateWhen { false } -} - -project.tasks.jar { - finalizedBy("copyToTestDir") - outputs.upToDateWhen { false } -} From a5b0ade84a9ba23afc85c1b390d48333fb69c795 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 21 Aug 2026 22:34:14 -0700 Subject: [PATCH 2/7] ADFA-5244: Stop Spotless walking the tool-generated tests/test-home tooling-api-model:copyToTestDir writes a jar into tests/test-home, which lives in the source tree, so root spotlessJava's fileTree(rootDir) consumed another task's output. Gradle's validation then failed any invocation that both compiled and checked formatting -- the combination you run before pushing. spotlessCheck alone always passed, because copyToTestDir never entered the graph. That is what made this read as "my change broke Spotless". traversalExcludes already exists for exactly this, and its comment carries the rule this needs: a bare directory name so Gradle prunes the subtree instead of descending and filtering. Verified: the failing invocation (:common:compileV8DebugAndroidTestKotlin spotlessCheck) now succeeds; nothing is tracked under tests/ so no real source is hidden; and an injected 4-space indent still fails spotlessKotlinGradleCheck, so the check remains effective. Co-Authored-By: Claude Opus 5 --- build.gradle.kts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index b0f6438ace..f97cfaa4c3 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -188,6 +188,10 @@ spotless { "**/.flox", "**/.git", "**/.gradle", + // ADFA-5244: tooling-api-model:copyToTestDir writes a jar into this directory, which + // lives in the source tree. Walking it makes spotlessJava consume another task's + // output, so Gradle fails any build that both compiles and checks formatting. + "tests/test-home", ) // Gradle build-output dirs (root + subprojects), relative to rootDir. Not From 662c4ac149496997800e5c8c8b4632623a009d7e Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 24 Aug 2026 16:36:42 -0700 Subject: [PATCH 3/7] ADFA-5244: Exclude all of tests/, and make CI able to catch a regression Review found three things worth acting on here. The exclude was anchored to tests/test-home while .gitignore reserves tests/**/.cg/init/model.jar -- so a sibling generated directory would reintroduce the identical failure. Nothing under tests/ is tracked at all, so the whole subtree goes. The comment three lines above said "bare dir names required", which the new rooted entry appeared to violate. The real rule is that the pattern must match the directory node itself, rooted or bare; a bare name additionally matches that name at any depth, which is why "tests" is rooted here -- "test-home" would also prune testing/resources/test-home, the directory FileProvider.testHomeDir() actually points at. CI could not have caught a regression of this: it ran spotlessCheck standalone, the one invocation that always passed, because copyToTestDir never enters that graph. The step now runs a compile task in the same invocation, which is the combination that fails. What this PR still does not fix is the cause: copyToTestDir writes a jar nothing reads, into the source tree, from a configuration-time mkdir. That is ADFA-5263. Fixing it there would make this exclude unnecessary rather than permanent. Co-Authored-By: Claude Opus 5 --- .github/workflows/debug.yml | 8 ++++++-- build.gradle.kts | 17 ++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/.github/workflows/debug.yml b/.github/workflows/debug.yml index f10d2fe89d..15a6f10ba6 100644 --- a/.github/workflows/debug.yml +++ b/.github/workflows/debug.yml @@ -197,9 +197,13 @@ jobs: run: | flox activate -d flox/base -- ./gradlew :plugin-api:apiCheck --no-daemon - - name: Run spotlessCheck + # Combined with a compile task on purpose: spotlessCheck alone never pulls a + # source-tree-writing task into the graph, so the implicit-dependency failure ADFA-5244 fixed + # cannot reproduce in a standalone run. Without this step a regression of that exclude is + # invisible to CI and shows up only on a developer's machine at push time. + - name: Run spotlessCheck alongside a compile task run: | - flox activate -d flox/base -- ./gradlew spotlessCheck --no-daemon || { + flox activate -d flox/base -- ./gradlew :common:compileV8DebugKotlin spotlessCheck --no-daemon || { echo "::error::Spotless found formatting violations in this branch. Run './gradlew spotlessApply' locally, then commit and push the formatted files." exit 1 } diff --git a/build.gradle.kts b/build.gradle.kts index f97cfaa4c3..00ad93c77f 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -179,19 +179,22 @@ spotless { // Spotless's auto-prune of .git/.gradle/build, so restore them. flox is the // ADFA-4816 fix: its /nix/store symlinks (millions of files) made // spotlessCheck take 12+ minutes. - // Bare dir names required: Gradle prunes a subtree only when an exclude - // matches the dir node itself; `dir/**` matches contents and forces a - // descend-and-filter (no pruning). + // The pattern must match the directory node itself -- a bare name or a rooted path both do -- + // because Gradle prunes a subtree only then; `dir/**` matches the contents instead and forces a + // descend-and-filter. A bare name also matches that name at any depth, which is why the entries + // below say what they mean: "tests" is rooted, where "test-home" would also prune + // testing/resources/test-home. val traversalExcludes = arrayOf( "flox", "**/.flox", "**/.git", "**/.gradle", - // ADFA-5244: tooling-api-model:copyToTestDir writes a jar into this directory, which - // lives in the source tree. Walking it makes spotlessJava consume another task's - // output, so Gradle fails any build that both compiles and checks formatting. - "tests/test-home", + // ADFA-5244: generated, and inside the source tree, so walking it makes Spotless consume + // another task's output and Gradle fails any build that both compiles and checks + // formatting. The whole subtree, not just tests/test-home: .gitignore reserves + // tests/**/.cg/init/model.jar, and nothing under tests/ is tracked at all. + "tests", ) // Gradle build-output dirs (root + subprojects), relative to rootDir. Not From 3d62974ac66f904cfb211607d9df4c42e60d20b6 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 24 Aug 2026 18:36:00 -0700 Subject: [PATCH 4/7] ADFA-5244: Drop the exclude; ADFA-5263 removed what it was hiding The exclude existed because copyToTestDir wrote a jar into tests/test-home inside the source tree, which made Spotless consume another task's output. That task is deleted in ADFA-5263, so nothing creates the directory and there is nothing to prune -- keeping the exclude would leave a permanent workaround for a problem that no longer exists. What stays is the part that is worth keeping either way: the CI step that runs a compile task in the same invocation as spotlessCheck. CI ran spotlessCheck standalone, which is the one invocation that can never reproduce this class of failure, so any regression was invisible to it by construction. The comment above traversalExcludes also stays corrected: the rule is that a pattern must match the directory node itself, and a bare name matches that name at any depth -- "test-home" would have pruned testing/resources/test-home too. This branch is now based on ADFA-5263 rather than stage, because without that fix removing the exclude reintroduces the failure -- verified both ways. Co-Authored-By: Claude Opus 5 --- build.gradle.kts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 00ad93c77f..19eb07e5cc 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -190,11 +190,6 @@ spotless { "**/.flox", "**/.git", "**/.gradle", - // ADFA-5244: generated, and inside the source tree, so walking it makes Spotless consume - // another task's output and Gradle fails any build that both compiles and checks - // formatting. The whole subtree, not just tests/test-home: .gitignore reserves - // tests/**/.cg/init/model.jar, and nothing under tests/ is tracked at all. - "tests", ) // Gradle build-output dirs (root + subprojects), relative to rootDir. Not From 6761879a226fa096ff1676152165215f7eb4ce56 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Tue, 25 Aug 2026 15:20:53 -0700 Subject: [PATCH 5/7] ADFA-5244: Say what Gradle exclude patterns actually do The comment this PR added claimed a bare exclude name "also matches that name at any depth". It does not -- bare names are root-anchored, so exclude("flox") prunes rootDir/flox and nothing else. Verified with a probe project: with tests/ and testing/resources/test-home/ present, exclude("tests") leaves a/tests/x untouched and exclude("test-home") matches nothing at all. That made the array's own **/ prefixes look redundant, so the comment invited exactly the wrong simplification: reading "**/.gradle" as equivalent to ".gradle" stops pruning every per-project cache dir and brings back the ADFA-4816 12-minute spotlessCheck. The comment it replaced ("Bare dir names required") had it right. Also drops the reference to a "tests" entry, which commit 3d62974a removed, and the test-home example, which was inverted -- a bare "test-home" is not dangerously broad, it is silently narrow. Found in review of PR #1730. --- build.gradle.kts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 19eb07e5cc..cd5c3f7120 100755 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -179,11 +179,12 @@ spotless { // Spotless's auto-prune of .git/.gradle/build, so restore them. flox is the // ADFA-4816 fix: its /nix/store symlinks (millions of files) made // spotlessCheck take 12+ minutes. - // The pattern must match the directory node itself -- a bare name or a rooted path both do -- - // because Gradle prunes a subtree only then; `dir/**` matches the contents instead and forces a - // descend-and-filter. A bare name also matches that name at any depth, which is why the entries - // below say what they mean: "tests" is rooted, where "test-home" would also prune - // testing/resources/test-home. + // The pattern must match the directory node itself, because Gradle prunes a subtree only then; + // `dir/**` matches the contents instead and forces a descend-and-filter. Bare names are + // root-anchored: "flox" prunes only rootDir/flox, so anything that has to match at depth needs + // the `**/` prefix the three entries below carry. Dropping it -- reading "**/.gradle" as + // equivalent to ".gradle" -- would stop pruning every per-project cache dir and bring back the + // ADFA-4816 12-minute spotlessCheck. val traversalExcludes = arrayOf( "flox", From 6c7a73fb16d659e5801417c79163a3b9f04298ca Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 26 Aug 2026 17:49:27 -0700 Subject: [PATCH 6/7] ADFA-5244: Keep ignoring tests/test-home, which the tests still fill Dropping both entries went one too far. The model.jar line is dead -- ADFA-5263 deleted the task that wrote it -- but the directory is not. writeInitScript() in gradle-plugin/src/test/.../utils.kt resolves FileProvider.testHomeDir() and creates .cg/init/androidide.init.gradle on every run of :gradle-plugin:test. That is a test writing a file at execution time, not a Gradle task declaring an output, so it never triggers the implicit-dependency validation this branch is about -- it just leaves "?? tests/" in git status after a test run, in a tree where nothing under tests/ is tracked. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M4sTwYg47aK8VB9kRKZicU --- .gitignore | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index dec603ee97..bcf269c478 100755 --- a/.gitignore +++ b/.gitignore @@ -105,6 +105,12 @@ sentry.properties # Generated files for tooling API +# Written by the gradle-plugin tests, not by a Gradle task: writeInitScript() in +# gradle-plugin/src/test/.../utils.kt resolves FileProvider.testHomeDir() and creates +# .cg/init/androidide.init.gradle there on every run. ADFA-5263 deleted copyToTestDir, so the +# model.jar entry that used to sit beside this one is dead, but the directory still fills up. +tests/test-home + /composite-builds/build-deps-common/constants/build/ /composite-builds/build-deps/build/ From e7ae8a1244660c8d4e96d9ffaf49023ab34c39af Mon Sep 17 00:00:00 2001 From: David Schachter Date: Thu, 27 Aug 2026 15:04:36 -0700 Subject: [PATCH 7/7] ADFA-5244: Stop the combined step blaming Spotless for a compile error Adding the compile task to this step gave it two possible causes, but the failure message still named only one -- and told the developer to run spotlessApply, which cannot fix a compile error. That is precisely the misdiagnosis the step exists to catch, so it should not commit it itself. The message now names the invocation rather than a cause, and says which half spotlessApply can help with. The exit status propagates instead of a hard-coded 1, so a Gradle exit code survives into the job. Verified: the YAML parses, `bash -n` accepts the run block, and a simulated failure returning 3 now exits 3 rather than 1. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M4sTwYg47aK8VB9kRKZicU --- .github/workflows/debug.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/debug.yml b/.github/workflows/debug.yml index 15a6f10ba6..7bc53c7a28 100644 --- a/.github/workflows/debug.yml +++ b/.github/workflows/debug.yml @@ -203,10 +203,16 @@ jobs: # invisible to CI and shows up only on a developer's machine at push time. - name: Run spotlessCheck alongside a compile task run: | - flox activate -d flox/base -- ./gradlew :common:compileV8DebugKotlin spotlessCheck --no-daemon || { - echo "::error::Spotless found formatting violations in this branch. Run './gradlew spotlessApply' locally, then commit and push the formatted files." - exit 1 - } + # Two tasks, so two possible causes: the message must not name one of them. Blaming + # Spotless for a compile error is the exact misdiagnosis this step exists to catch. + set +e + flox activate -d flox/base -- ./gradlew :common:compileV8DebugKotlin spotlessCheck --no-daemon + status=$? + set -e + if [ "$status" -ne 0 ]; then + echo "::error::Gradle failed on ':common:compileV8DebugKotlin spotlessCheck'. If the output above shows formatting violations, run './gradlew spotlessApply' locally and commit the result; if it shows a compile error, spotlessApply will not help. The Gradle output says which." + exit "$status" + fi - name: Install advancecomp (advzip, used by recompressApk) run: |