fix(gradle): support builds that configure subprojects from an ancestor build file - #36693
Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit 0936756
☁️ Nx Cloud last updated this comment at |
…g allprojects block (#36697) Split out of #36693 so it can be reviewed on its own — it is unrelated to the plugin changes there. ## Current Behavior When a **Groovy** build file already contains an `allprojects {}` block, `@nx/gradle:init` writes: ```groovy allprojects { apply plugin "dev.nx.gradle.project-graph" … } ``` The named-argument colon is missing. `apply plugin: "x"` is sugar for `apply(plugin: "x")` — a `Map` passed to `Project.apply(Map)`. Without the colon, `plugin` is no longer a map key but an identifier, so Groovy resolves it as a property and the build fails during configuration, before anything else can run: ``` * What went wrong: A problem occurred evaluating project ':x'. > Could not get unknown property 'plugin' for project ':x' of type org.gradle.api.Project. ``` Only this one branch is affected. When there is no existing `allprojects {}` block the generator appends `allprojects { apply { plugin("…") } }`, which is valid in both DSLs, and the Kotlin branch emits `apply plugin("…")`, also valid. Hit in practice running `@nx/gradle:init` against [apache/kafka](https://github.com/apache/kafka), whose root build file already has an `allprojects {}` block. The result is unusable until the colon is added by hand. ## Expected Behavior The Groovy branch emits `apply plugin: "dev.nx.gradle.project-graph"`. The already-applied check needed widening too: it only recognised the Kotlin `plugin("x")` form, so once the Groovy output gained its colon, a second `init` run would no longer detect the existing apply and would append a duplicate. It now matches both forms. The plugin name is also escaped before interpolation into the regex — previously its dots were live regex metacharacters. ## Why this was not caught Worth flagging, because the gap is structural rather than a missing assertion. **The unit test asserted the broken output.** `gradle-project-graph-plugin-utils.spec.ts` covered this exact branch with: ```js /allprojects\s*{\s*apply\s*plugin\s*['"]dev\.nx\.gradle\.project-graph['"]/ ``` Note `\s*` between `plugin` and the quote: that matches the colon-less output and would **fail** on the correct form. So the branch had coverage, but the expectation pinned the bug in place — anyone fixing the generator would have been met with a red test. It is corrected here, and a regression test covers the idempotent re-run, whose failure mode (duplicate `apply` lines) is otherwise silent. **No e2e fixture can reach the branch.** Every Gradle e2e fixture is generated by `gradle init --type <dsl>-application --split-project` (`e2e/gradle/src/utils/create-gradle-project.ts`), and that template has no `allprojects {}` block — so the generator always takes the path that appends a fresh one, in both the Kotlin and Groovy variants. The only e2e that writes an `allprojects` block is `gradle-plugin-v1.test.ts`, which is the deprecated v1 path: it writes the block itself as setup and applies `project-report`, never going through this generator. Had a fixture had that shape, e2e would have caught this immediately and loudly — the build fails at configuration time. I have not added such a fixture here because I could not run the Gradle e2e suite locally and would rather not land a test I have not seen pass; a follow-up adding a fixture whose build file already contains an `allprojects {}` block would close the gap properly. ## Testing `gradle-project-graph-plugin-utils.spec.ts`: 35 tests pass, including the corrected assertion and the new idempotency test. <!-- polygraph-session-start --> --- <p><a href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/Fix-nx-gradle-project-graph-failure-on-Kafka-6be88540">View Polygraph session ↗</a></p> <!-- polygraph-session-end -->
There was a problem hiding this comment.
✅ The fix from Nx Cloud was applied automatically
We ran nx format to fix the failing format:check task caused by two unformatted JSON mock files (gradle_composite.json and gradle_tutorial.json) in packages/gradle/src/plugin/utils/__mocks__/. These files were modified as part of the PR's gradle plugin changes but were not passed through prettier before commit, which our format:check CI gate caught.
Tip
✅ We verified this fix by re-running nx-cloud record -- nx format:check.
Warning
The suggested diff is too large to display here, but you can view it on Nx Cloud ↗
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
View interactive diff ↗🎓 Learn more about Self-Healing CI on nx.dev
baf8286 to
03f6e79
Compare
…g allprojects block (#36697) Split out of #36693 so it can be reviewed on its own — it is unrelated to the plugin changes there. ## Current Behavior When a **Groovy** build file already contains an `allprojects {}` block, `@nx/gradle:init` writes: ```groovy allprojects { apply plugin "dev.nx.gradle.project-graph" … } ``` The named-argument colon is missing. `apply plugin: "x"` is sugar for `apply(plugin: "x")` — a `Map` passed to `Project.apply(Map)`. Without the colon, `plugin` is no longer a map key but an identifier, so Groovy resolves it as a property and the build fails during configuration, before anything else can run: ``` * What went wrong: A problem occurred evaluating project ':x'. > Could not get unknown property 'plugin' for project ':x' of type org.gradle.api.Project. ``` Only this one branch is affected. When there is no existing `allprojects {}` block the generator appends `allprojects { apply { plugin("…") } }`, which is valid in both DSLs, and the Kotlin branch emits `apply plugin("…")`, also valid. Hit in practice running `@nx/gradle:init` against [apache/kafka](https://github.com/apache/kafka), whose root build file already has an `allprojects {}` block. The result is unusable until the colon is added by hand. ## Expected Behavior The Groovy branch emits `apply plugin: "dev.nx.gradle.project-graph"`. The already-applied check needed widening too: it only recognised the Kotlin `plugin("x")` form, so once the Groovy output gained its colon, a second `init` run would no longer detect the existing apply and would append a duplicate. It now matches both forms. The plugin name is also escaped before interpolation into the regex — previously its dots were live regex metacharacters. ## Why this was not caught Worth flagging, because the gap is structural rather than a missing assertion. **The unit test asserted the broken output.** `gradle-project-graph-plugin-utils.spec.ts` covered this exact branch with: ```js /allprojects\s*{\s*apply\s*plugin\s*['"]dev\.nx\.gradle\.project-graph['"]/ ``` Note `\s*` between `plugin` and the quote: that matches the colon-less output and would **fail** on the correct form. So the branch had coverage, but the expectation pinned the bug in place — anyone fixing the generator would have been met with a red test. It is corrected here, and a regression test covers the idempotent re-run, whose failure mode (duplicate `apply` lines) is otherwise silent. **No e2e fixture can reach the branch.** Every Gradle e2e fixture is generated by `gradle init --type <dsl>-application --split-project` (`e2e/gradle/src/utils/create-gradle-project.ts`), and that template has no `allprojects {}` block — so the generator always takes the path that appends a fresh one, in both the Kotlin and Groovy variants. The only e2e that writes an `allprojects` block is `gradle-plugin-v1.test.ts`, which is the deprecated v1 path: it writes the block itself as setup and applies `project-report`, never going through this generator. Had a fixture had that shape, e2e would have caught this immediately and loudly — the build fails at configuration time. I have not added such a fixture here because I could not run the Gradle e2e suite locally and would rather not land a test I have not seen pass; a follow-up adding a fixture whose build file already contains an `allprojects {}` block would close the gap properly. ## Testing `gradle-project-graph-plugin-utils.spec.ts`: 35 tests pass, including the corrected assertion and the new idempotency test. <!-- polygraph-session-start --> --- <p><a href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/Fix-nx-gradle-project-graph-failure-on-Kafka-6be88540">View Polygraph session ↗</a></p> <!-- polygraph-session-end --> (cherry picked from commit 15a8861)
aa4a5d5 to
e0b9dbe
Compare
…or build file
Three independent bugs, all surfaced by running @nx/gradle against apache/kafka.
1. The init generator emitted invalid Groovy. When a build file already had an
`allprojects {}` block, it wrote `apply plugin "dev.nx.gradle.project-graph"`
without the named-argument colon, so Groovy read `plugin` as a property and
the build failed with "Could not get unknown property 'plugin'". The existing
unit test asserted the broken output, so it went unnoticed. The idempotency
check is updated to recognise both DSL forms, otherwise a second run appended
a duplicate apply.
2. nxProjectReport deadlocked. It calls TaskDependency.getDependencies() from a
task action; resolving a dependsOn declared as a task *path* sends Gradle
through ensureProjectsConfigured, re-entering the configuration phase during
execution and blocking forever on the build-lifecycle state lock. Builds that
declare cross-project dependsOn by path hang until the plugin's timeout.
Such tasks now skip that call and recover the edges by parsing the path —
findProject() returns the project without configuring it. Both absolute
(`:a:b:test`) and relative (`connect:api:jar`) forms are handled, and nested
collections are flattened first because `dependsOn: [a, b]` stores the whole
list as a single element. Bare names still go through TaskDependency, which
keeps effectiveDependencyPatterns able to see through lifecycle tasks.
3. Projects without their own build file were dropped entirely. A project can be
configured from an ancestor via `project(':core') { }`; Kafka configures ~40
subprojects this way and has only one build file in the whole repo. Four
places assumed otherwise, so those projects reached the graph with no node,
no dependsOn and no dependency edges. They are now attributed to the build
file that configures them, and nodes are built from the report's project
roots rather than from dirname(buildFile) — which would collapse every
subproject onto the root. Nx rejects a static dependency whose sourceFile
lives outside the source project, so edges attributed to an ancestor file are
recorded as implicit.
Also memoizes the task-dependency and output-pattern lookups, which
effectiveDependencyPatterns otherwise recomputes once per task.
On apache/kafka, `nx show projects` goes from 5 projects and 72 graph edges to
71 projects and 547 edges.
Co-authored-by: FrozenPandaz <FrozenPandaz@users.noreply.github.com>
…d it The dependencies array was still typed Array<StaticDependency> while the implicit branch pushes an ImplicitDependency, which does not compile. Comment fixes from review: - effectiveBuildFile was inserted between getNxProjectName's KDoc and its declaration, leaving that doc describing the wrong function. - pathStringDeps' doc claimed it returns absolute paths; it returns every string in dependsOn, and qualifiedPathDeps is what filters them. - Two comments said "absolute path" where the code tests for a qualified one.
A qualified-path dependsOn is deliberately never resolved to a Task, so the dependency walk behind dependentTasksOutputFiles finds nothing and the task's inputs came out narrower than they should be — silently, and in the direction that yields a stale cache hit rather than a loud failure. Fail open to the catch-all instead, matching what declaresDirectoryOutput already does when it cannot read a task's output model. Over-declaring costs a rebuild; under-declaring costs a wrong answer. On apache/kafka, :streams:testAll goes from no dependentTasksOutputFiles entry at all to the catch-all.
…ncy bypass
Critical: pluginCache collided across project roots. calculateHashesForCreateNodes
hashes the files under a root, not the root itself, so two roots that own no files
of their own hashed identically — a shape this PR introduced by driving off the
report's project roots. The second root then read the first's config, which the
checked-in plugin.spec.ts snapshot had pinned as expected: it recorded
proj/application under the root project's name and targets. The cache key now
includes the root, and the snapshot records proj/application as "application".
Also:
- Roots outside the workspace (an includeBuild("../x"), which the reporter leaves
absolute by design) were attributed to the workspace-root build file instead of
being rejected.
- buildFileFor matched a hardcoded ['build.gradle', 'build.gradle.kts'], so a
renamed build file reached allBuildFiles through the report but never matched.
It is indexed by directory now.
- One qualified path discarded every TaskDependency edge for that task, including
same-project ones: `dependsOn("classes", ":other:jar")` lost `classes`, and every
dependsOn(taskProvider) went with it. Those resolve inside the already-configured
declaring project, so they are recovered locally.
- NX_GRADLE_SKIP_TASK_DEPS suppressed the dependency walk without triggering the
compensating input widening — the stale-cache hazard the widening exists to
prevent. Both sites now share one predicate.
- taskDependencyCache still keyed on task.path, which collides across included
builds. Barely reachable at 5 projects; reachable at 71.
- Documented NX_GRADLE_SKIP_TASK_DEPS, and covered resolvePathDeps' path forms and
the nested-list dependsOn shape.
Co-authored-by: FrozenPandaz <FrozenPandaz@users.noreply.github.com>
Each per-project report already names one project and the one build file that
configures it, but processNxProjectGraph flattened those into a Set and threw the
pairing away, so nodes.ts rebuilt it by walking directories upward and guessing.
The pairing is now carried through and used directly.
That removes the guessing rather than refining it: the walk could not be right for
a project whose directory ancestry and Gradle project ancestry differ (settings.gradle
remapping a projectDir), and it matched a hardcoded pair of build file names, so a
renamed build file never resolved. A project the report gives no build file for is
skipped with a verbose log naming the likely cause, instead of vanishing silently —
it is unreachable once `@nx/gradle:init` has run, since that writes a build file next
to every settings.gradle.
Also, from the same review pass:
- A `dependsOn { … }` Callable is now expanded rather than dropped when the bypass
fires. Gradle resolves it by calling it and so do we; the result is classified by
the same rules, so nothing reaches Gradle's task resolver.
- A task whose dependsOn holds a FileCollection or a raw TaskDependency is reported
uncacheable when the bypass fires. Those only yield their tasks by resolving, which
is what deadlocks, so the dependency set is knowably short — and a cache key built
from a short dependency set is a stale hit waiting to happen. Not caching is the
honest answer; recovering them properly needs a configuration-time phase, which is
a larger change than this fix.
- flattenDependsOn keeps an identity-based visited set. Gradle's own
DefaultTaskDependency drains an ArrayDeque with no cycle guard, so a cyclic
structure spins there; it cannot reach dependsOn literally (Gradle hashes values
into a Set first, which overflows), but it can now arrive via a Callable's return.
Adds a dependencies.spec.ts for the static/implicit predicate, which had no coverage,
and Kotlin tests for the path forms, the cache gating, the Callable, and the cycle.
Co-authored-by: FrozenPandaz <FrozenPandaz@users.noreply.github.com>
…splitting
Two defects found reviewing the branch.
A Groovy `dependsOn { … }` stores a `groovy.lang.Closure`, which implements
`GroovyCallable` and so is a `Callable`. It matched the Callable arm and was
invoked with no argument, while Gradle calls it with the task — so any closure
using its parameter threw, was swallowed, and lost its edge. Closures are now
handled ahead of Callable and receive the task.
Expanding `dependsOn` also invokes user closures, and several call sites per task
need that expansion, so it is memoized rather than recomputed — previously a
closure could run once per caller.
`resolvePathDeps` had no effective coverage: both tests passed against an
implementation returning nothing, one via a short-circuiting `||` and the other
vacuously. Replacing its body with `emptyList()` left the whole suite green. The
tests now build real child projects and assert the resolved pairs, and both new
tests fail when their fix is reverted.
…hatch row The doc on `buildFileByProjectRoot` said the field is absent for reports written by a Gradle plugin older than 0.1.25. No plugin version writes it at all: it is derived on the Nx side from each per-project report, and the persisted copy is version-gated anyway. It also hid the reachable failure, which is that an individual project whose report names no build file gets no entry. `nodes.ts` repeated the same premise on the branch that handles exactly that case. The NX_GRADLE_SKIP_TASK_DEPS row named two consequences but not the one that costs the most: with the flag set every task takes the bypass, so a task carrying a FileCollection or a raw TaskDependency in dependsOn is reported uncacheable. `dependsOn(configurations.x)` is ordinary Gradle, so a reader following that row out of a hang loses caching with nothing explaining why. Rewrote the row to the style guide while touching it: no semicolon, contractions, active voice.
…solver The recovery no longer reimplements dependency resolution. For bypassed tasks it calls Gradle's own DefaultTaskDependency.getDependencies — the engine that handles every declaration shape — with its one lock-touching collaborator, the build-scoped TaskResolver, replaced by a pure findProject + findByName lookup. At execution time every project is already configured, so the lookup is exactly what the real resolver's ceremony reduces to; the ceremony is what deadlocks. Values whose internals may hold the real resolver are screened by allowlist (Task, string, Provider) — a FileCollection's builtBy can carry a path string, and resolving one from a worker thread is the same deadlock. An unforeseen type is counted, never resolved. Strings are pre-probed so a miss is counted rather than surfacing as a null inside the engine. TaskResolver's signature changed across Gradle majors (String in 8, Path in 9) and the JVM dispatches by descriptor, so the resolver implements both. Engine failure on a future Gradle falls back to the hand-rolled recovery and fails open. A fully resolved dependsOn now feeds real Tasks to the input derivation, so the **/* fail-open applies only when screening or resolution actually lost a value. On apache/kafka, testAll/jarConnect/testConnect/jmh drop from the catch-all to per-extension patterns; the graph is unchanged at 71 projects / 547 edges; the issue #36668 repro passes with precise inputs on the affected target. Mutation-checked: nulling the lookup fails the resolved-path test; dropping the Provider arm fails the provider test.
resolvePathDeps, DepRef and sameProjectDeps duplicated what the engine now does with real Tasks — resolvePathDeps also duplicated lookupTask's path arithmetic line for line, and its edges were only surviving dedup in mapTasksToObjects. One resolution path remains: the engine with the safe resolver. The engine- failure fallback becomes minimal insurance — keep the Task instances in hand, count everything else unresolved, fail open — instead of a shadow implementation maintained in lockstep. The name-only resolveTargetName overload existed for DepRef and goes with it. The resolvePathDeps tests retarget to lookupTask, which now holds the only copy of the path arithmetic. Verified end-to-end: the #36668 repro passes with the edge intact, and Kafka is unchanged at 71 projects / 547 edges with testAll keeping its 30 cross-project refs and per-extension patterns.
… state Comment-analyzer pass over the branch. Five comments had gone false — most from this branch's own refactors outrunning its prose: - The NX_GRADLE_SKIP_TASK_DEPS doc said the flag falls back to raw Task instances; under the engine it resolves strings and providers too. - The engine-failure comment referenced "the hand-rolled recovery", deleted in the previous commit. - A comment claimed mapTasksToObjects recovers path strings; that moved into resolveDependsOn. - Two test comments described their fixtures doing things they do not: one claimed a path "is fully resolved" where the project does not exist, the other that a qualified path "is never resolved to a Task" while a sibling test asserts exactly that resolution. The rest is volume: rationale and design history moved out of the source, each repeated fact kept in one place (the deadlock invariant on the gate, the cross-major dispatch on the resolver, the hashing gotchas at their sites). TaskUtils.kt's added comment lines drop from 93 to 35.
…creened Whether the plugin could resolve a task's dependsOn is a fact about the plugin, not the task, so it must not decide cacheability. If Gradle caches the task, Nx caches it; a screened value keeps the `**/*` dependentTasksOutputFiles fail-open, which is the mitigation that already existed for the same case.
Gradle's getTaskDependencies() is the union of dependsOn and the task's inputs. The bypass reconstructed only dependsOn, so a producer wired in by `inputs.files(producer.outputs.files)` was dropped, and since dependentTasksOutputFiles hashes only tasks reachable through graph edges, the `**/*` fail-open could not cover it either. The bypass now runs Gradle's own CachingTaskDependencyResolveContext over dependsOn plus task.inputs, with add() overridden so every DefaultTaskDependency it meets (a dependsOn, a FileCollection's builtBy, an artifact's build dependencies) is swapped for a copy that resolves path strings through the deadlock-free lookup. That is the only class that resolves a path, so the swap is the one interception point; screening a FileCollection out wholesale is no longer needed. A nested instance's immutable values are read reflectively and counted lost if unreadable, so the walk is always a superset of what the previous screening kept. A name relative to a producer in another project resolves against the consumer and counts lost when that misses. All APIs used exist on Gradle 8.14.3 as well as 9.x.
The unit fixtures use ProjectBuilder, which accepts a path to a project that does not exist. Only a real Gradle build settles whether an edge resolved, and the e2e harness publishes the locally built plugin to mavenLocal, so it exercises the Kotlin changes directly. Every form declares the same edge on :list:jar by a different route: a qualified path, a nested list, a Callable, a Provider, a path string inside a FileCollection's builtBy, and a producer wired in through inputs.files rather than dependsOn at all. The suite warms the cache, proves the tasks do cache, then makes one change to the dependency's source and requires every dependent to rebuild -- a form whose edge was lost replays a stale output. An unrelated-file test keeps that from holding vacuously if inputs over-declared to a catch-all, and a task with a path to no task must stay cacheable while carrying the catch-all. `gradle init --split-project` writes no root build file, so adding one that configures a build-file-less subproject reproduces Kafka's shape; core takes the same edge and is held to the same rules.
The gate that sent only qualified-path tasks through the safe resolution had no reason left to exist: the safe context is Gradle's own walker, and the one place it fell short of getDependencies() -- a bare name inside a container from another project -- is fixed by leaving such a container untouched. A bare name never deadlocks (ProjectScopedTaskResolver answers it from the project's own container without the lifecycle lock), so only a container holding a qualified path is copied; the rest keep their own resolver, immutable values included. A bare name beside a qualified path in a foreign container is counted lost rather than resolved against the consumer, which would be the wrong task. Two more holes closed. Gradle's walker visits a bare Buildable's dependencies directly, skipping the add() hook, so add() unwraps it to its buildDependencies first. And a qualified path under task.inputs (a builtBy) deadlocks the same way as one in dependsOn but never triggered the gate; with the gate gone every task takes the safe path. LostCounter now ignores the transform nodes Gradle's own resolver ignores instead of over-declaring for them. NX_GRADLE_SKIP_TASK_DEPS keeps only the Task objects in dependsOn and fails every target's inputs open.
…n project A container copied for its qualified path lost the one thing that knew where its bare names live: the resolver Gradle built it with, bound to the container's own project. Resolving "assemble" against the consumer instead would be the wrong task, so it was counted lost and the target failed open. The original resolver is read reflectively and used for bare names only. That branch is lock-free on both majors -- ProjectScopedTaskResolver and DefaultTaskContainer.findByPath both end in the project's own getByName -- while a qualified path still goes through the pure lookup. resolveTask took a String in Gradle 8 and a Path in 9, so the call is reflective too. A container whose resolver cannot be read keeps counting bare names lost.
Re-measuring Kafka (Gradle 9.6.1) with the safe context universal lost one
edge: the root report task's dependency on the api-checker included build.
On Gradle 9, includedBuild.task(":nxProjectReport") is a DefaultTaskReference
wrapping a DefaultTaskDependency whose value is the path string and whose
resolver belongs to the included build. The copy resolved that absolute
path in the consuming build and found the consumer's own task -- a self
edge in place of the cross-build one. An absolute path is only unambiguous
within a build.
The container's build is read from its resolver (Gradle 9 keeps the
BuildState on the resolver chain; Gradle 8's resolver is the task container,
which knows its project) and, when it is not the owner's build, every path
resolves with the pure lookup in that build's root project. Reading the
root project goes through getGradle(), which Gradle already makes safe for
execution-time callers. Unknown build falls back to the owner's, as before.
Kafka: 71 projects / 547 edges, edge-for-edge identical to the commit before
this series; 1596 catch-all and 138 uncacheable targets, both unchanged.
Drop the "resolvable dependency set stays cacheable" test: cacheability is name-only now, so its assertion could not fail, and its fixture had no `other` project, so the set it called resolvable was not. Its sibling's comment said the second path did not exist when neither did. A bare name inside a container from another build resolved against that build's root project -- a guess, where every other unknown is counted lost. Now counted lost; a test pins it. The nodes.ts skip comment named only a plugin that predates effectiveBuildFile, and the log told that user to upgrade. A build with no build file anywhere up the ancestry reaches the same skip on the newest plugin; both now name both remedies.
It was added in this branch as the way out of a hanging project graph. Every task now resolves through the safe context and no path string can reach the build-scoped resolver from the report task, so the hang it guarded against has no path left. What the flag still did -- Task objects only, catch-all inputs on every target -- is what the walk already falls back to on any failure. Never released, so nothing depends on it.
SafeTaskResolver, SafeResolveContext, the lock-free lookup and the reflection readers behind them are one mechanism with one consumer, resolveDependsOn. They now live in SafeTaskResolution.kt; TaskUtils.kt keeps the target-building code and calls in. flattenDependsOn and flattenValues become internal since both files use them.
flattenDependsOn/flattenValues and getDependsOnTask move to SafeTaskResolution.kt, which now owns the mechanism end to end; TaskUtils.kt only consumes it. The union of resolved tasks with the raw Task objects in dependsOn is dropped: the safe resolution already yields every one of them, and its failure path keeps exactly those, so the union and its second cache duplicated what resolveDependsOn's cache holds.
The cache stored nodes[root] under a hash of the files beneath that root and read it straight back, on a report populateProjectGraph had already put in memory. It never gated the Gradle run -- the report cache does that, keyed on every Gradle config and test file -- so its only effects were a file-map hash per project root on every createNodes call (71 on Kafka, up from 5 build files at base) and two wrong answers: a stale node when the report changed for a reason outside the root, and a ghost project whose root still hashed the same after it left settings.gradle. The deprecated createNodesV1 path keeps its own cache untouched.
c19277f to
0936756
Compare
…6808) ## Current Behavior `@nx/gradle` still pins `dev.nx.gradle.project-graph` at `0.1.24`. The Kotlin half of #36693 — the `nxProjectReport` deadlock fix and ancestor-configured project support — is merged but reaches no workspace until the plugin is published and applied. ## Expected Behavior `gradleProjectGraphVersion` and the plugin's own `build.gradle.kts` are `0.1.25`, and a `change-plugin-version-0-1-25` migration (nx `23.2.0-beta.11`) rewrites the plugin version in build files and version catalogs on `nx migrate`. Same five-file shape as the previous bumps (#36100): `versions.ts`, `project-graph/build.gradle.kts`, the migration `.ts` + `.md` under `src/migrations/23-2-0/`, and the `migrations.json` entry. ## Related Issue(s) Follow-up to #36693 (NXC-4831). Ships the fix for #36668 to users. <!-- polygraph-session-start --> --- <p><a href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/steady-condor-63e4e974">View Polygraph session ↗</a></p> <!-- polygraph-session-end -->
Two defects in
@nx/gradle, both surfaced by running it against apache/kafka, one independently reported with a minimal reproduction in #36668.Current Behavior
1.
nxProjectGraphdeadlocks when a task depends on another project's task by path.getDependsOnTaskcallstask.taskDependencies.getDependencies(task)from inside the report task's action. Resolving a dependency declared as a task path (dependsOn(":lib-two:prepare")) routes throughensureProjectsConfigured→DefaultSynchronizer.takeOwnership, which blocks forever on the build-lifecycle state lock — a worker thread can never acquire it during execution. The build sits at ~0% CPU until nx's timeout kills it, and every nx command in the workspace fails. The same edge declared as aTaskProvidercarries the task reference and is unaffected, which is why only some builds hang (#36668 demonstrates both forms).2. Projects configured from an ancestor build file are dropped. A Gradle project can be configured via
project(':core') { }from an ancestor; Kafka configures ~64 subprojects this way with a single build file. Four places assumed one project per build file, so those projects reached the graph with no node, nodependsOn, and no edges. Net result on Kafka: 5 projects / 72 edges where there should be 71 / 547.Expected Behavior
Deadlock: dependency resolution goes through Gradle's own engine, made safe for the execution phase. For tasks that declare a qualified path — the only shape that hangs — the plugin calls the real
DefaultTaskDependency.getDependencies(), with its one lock-touching collaborator replaced: strings resolve through a purefindProject+findByNamelookup instead of the build-scopedTaskResolver. At execution time every project is already configured, so the lookup is exactly what the real resolver's ceremony reduces to; the ceremony is what deadlocks. Gradle'sgetTaskDependencies()is the union ofdependsOnand the task's inputs, so the bypass runs Gradle's ownCachingTaskDependencyResolveContextover both halves (aninputs.files(producer.outputs.files)producer is kept), withadd()overridden so everyDefaultTaskDependencyit meets — adependsOn, aFileCollection'sbuiltBy(a path string there reproduces the deadlock from a worker thread — probed), an artifact's build dependencies — is swapped for a copy that resolves through the safe lookup. That is the only class that resolves a path, so it is the one interception point. A path the lookup cannot find is dropped and counted, so inputs fail open.TaskResolver's signature changed across Gradle majors (Stringin 8,Pathin 9) and the JVM dispatches by descriptor, so the safe resolver implements both. Engine failure on a future Gradle falls back to a hand-rolled recovery and fails open.Because the engine hands back real
Tasks, a fully resolved dependency set feeds the normal input derivation: the**/*fail-open applies only when resolution actually lost a value. Cacheability is left to Gradle either way — whether the plugin could resolve a dependency is a fact about the plugin, not the task. On Kafka,testAll/jarConnect/testConnect/jmhcarry per-extension patterns (**/*.class,**/*.jar) instead of the catch-all. AdependsOn { … }Groovy closure is called with the task, matching Gradle's dispatch, and expansion is memoized and cycle-guarded (Gradle's own flattening has no cycle guard).Ancestor-configured projects: attribution comes from the report's own pairing. Kotlin-side,
effectiveBuildFilewalks project ancestry to the nearest existing build file. TS-side, each per-project report pairs one project with one build file;processNxProjectGraphpreserves that pairing instead of flattening it, andcreateNodesiterates the report's project roots. Out-of-workspace roots (includeBuild("../x"), deliberately left absolute by the reporter) are rejected, and the node-cache key includes the root —calculateHashesForCreateNodeshashes the files under a root, so file-less roots would otherwise collide. Edges attributed to an ancestor's file are recorded as implicit, sincevalidateDependencyrejects a staticsourceFileoutside the source project.Verified:
0.1.24Providerarm, guttingresolvePathDeps, removing theClosurearm, and reverting the fail-open each fail their test)findByNameunder full parallelismgradle-cache+gradlesuitesbuiltBytask + path string, and a lost path each covered by unit tests)Notes for reviewers:
nodes.spec.tssnapshot churn affects all Gradle workspaces, not just Kafka-shaped ones — the change is additive (projects previously dropped now appear).**/*fail-open.NX_GRADLE_SKIP_TASK_DEPSis documented as an escape hatch for unforeseen variants.gradleProjectGraphVersionis deliberately still0.1.24; the0.1.25bump plus itschange-plugin-versionmigration is a deliberate follow-up, so workspaces see the fix once that publishes.apply plugin:generator fix originally in this PR was split out and merged separately as fix(gradle): emit valid Groovy when applying the plugin to an existing allprojects block #36697.Related Issue(s)
Fixes #36668
Tracked in NXC-4831. The fix reaches users when
dev.nx.gradle.project-graph0.1.25 publishes (follow-up bump + migration).View Polygraph session ↗