Repository navigation
fix(gradle): identify modules by canonical Gradle path, not leaf directory name - #1621
Conversation
Derive GradleModule identity from settings.gradle include paths
(":feature:symptoms") instead of the leaf directory name, so sibling
modules that share a leaf name (e.g. two "impl" modules under
different feature parents) no longer collide on the {name: ...}
MERGE key. Falls back to the path relative to the root project when
a module has no matching include, and preserves the original
leaf-name/"root" behaviour for the root module itself so single-
module projects with no settings.gradle keep working unchanged.
Also fixes the include()/project() regexes, which never matched
colon-delimited multi-segment paths at all (character class lacked
":"), and removes the tgt_name leaf-downgrade so both endpoints of
MODULE_DEPENDS_ON agree by construction. writer.py is untouched:
GradleModule nodes and edge lookups both key off the same
canonicalized module_name local, so the MERGE/MATCH queries already
see consistent identities.
Co-Authored-By: Claude Opus 5 <[email protected]>
Tighten the single-module/no-includes characterization test from a truthiness check to an exact value assertion, so a future change that accidentally routes the root module through the canonical-path branch of _resolve_module_name would be caught. Co-Authored-By: Claude Opus 5 <[email protected]>
_parse_settings_includes stored the raw include(...) token verbatim while the inter-module dependency target was always slash-normalized to colon form. The two coincided for colon-only notation but diverged for a settings.gradle using "/" as the segment separator (e.g. one generated by a module-discovery script): the GradleModule node kept the verbatim name while MODULE_DEPENDS_ON's tgt_name was normalized, so the writer's MATCH found nothing and the edge dropped silently - the exact failure this slice targets, for a narrower input shape. Normalize at storage time so both constructions can't diverge. Also corrects a false docstring claim that multi-line include statements aren't parsed - verified the pattern's \s* already matches newlines, so multi-line lists parse correctly today. Co-Authored-By: Claude Opus 5 <[email protected]>
|
@rrodriguesNutrium is attempting to deploy a commit to the shashankss1205's projects Team on Vercel. A member of the Team first needs to authorize it. |
Shashankss1205
left a comment
Shashankss1205
left a comment
There was a problem hiding this comment.
Approving — the identity fix is solid and I verified it on a real multi-module tree. But I found that #1603 is only half-closed by this, and the remaining half is the headline symptom. Detail below so it doesn't get lost.
What this fixes — confirmed
Fixture with settings.gradle.kts declaring :app, :feature:symptoms, :feature:other:impl, :core:network:
modules: [':app', ':core:network', ':feature:other:impl', ':feature:symptoms']
Canonical paths throughout, settings.gradle(.kts) actually parsed, and the :feature:a:impl / :feature:b:impl collision on the GradleModule MERGE key is gone. That's the stated scope and it works.
Normalizing both sides through ":" + token.replace("/", ":") rather than trusting the two paths to coincide is the right instinct — that's exactly the class of bug where endpoints silently disagree.
What's still broken — MODULE_DEPENDS_ON is empty on Kotlin DSL
gradle.py:168:
re.finditer(r"""(\w+)\s+project\(['"]:([\w.\-/:]+)['"]\)""", source)The \s+ requires whitespace between the configuration keyword and project(. Groovy writes implementation project(":core:network") — matches. Kotlin DSL writes implementation(project(":core:network")) — no space, no match.
Measured on two otherwise-identical fixtures:
Groovy 'implementation project(...)' modules=2 inter_module_deps=1 :app -> :core:network
KotlinDSL 'implementation(project(...))' modules=4 inter_module_deps=0
So on a build.gradle.kts project — the modern Android default, and the exact case #1603 describes — the module graph still has zero dependency edges. The nodes are now correct; the edges are still missing.
This is the same gap you already flagged for _DEP_PATTERN and external libraries. It applies to inter-module deps too, and \s+ → \s*\(?\s* (or similar) should cover both forms.
I'm merging this because the identity work stands on its own and shouldn't wait. Leaving #1603 open for the Kotlin-DSL matching — worth a small follow-up PR, and easy to add a fixture pair like the one above so it can't regress.
Unit suite green: 1209 passed, gradle parser tests 5 passed.
github-project-automation
Bot
moved this from Backlog tasks
to Done
in CGC Progress Board
Shashankss1205
added
gssoc:approved
2 participants
GradleModuleidentity is derived from the leaf directory name, so in any multi-module build two modules that share a leaf name collide onto a single node.This is not a rare shape.
:feature:login:impland:feature:profile:implboth becomeimpland merge into one node — and becauseMODULE_DEPENDS_ONis keyed off the same name, their dependency edges merge too. The module graph of a large Android project is quietly wrong rather than incomplete.The change
Identity now comes from the canonical Gradle path in
settings.gradle(:feature:login:impl), falling back to the path relative to the root project when a module has no matchinginclude. The root module keeps its existing leaf-name/rootbehaviour, so single-module projects with nosettings.gradleare unaffected.Two supporting fixes fell out of it:
The
include()/project()regexes never matched multi-segment paths at all — the character class omitted:, soinclude(":feature:login")didn't match whileinclude(":app")did. Any project using nested modules was silently getting the fallback path for every one of them. This is arguably the more consequential half of the PR.Removed the
tgt_nameleaf-downgrade, so both endpoints ofMODULE_DEPENDS_ONare canonical by construction rather than one canonical and one truncated.Settings-derived names are also normalized at storage time, so the two producers of a module name agree instead of matching by luck —
include("feature/login")andinclude(":feature:login")are both legal and now converge.writer.pyis untouched:GradleModulenodes and edge lookups both key off the same canonicalized local, so theMERGE/MATCHqueries already see consistent identities.Verification
pytest tests/unit -q→12 failed, 1181 passed, 19 skipped.Same 12 failures on unmodified
origin/main(12 failed, 1176 passed, 19 skipped) — pre-existing and unrelated (MCP SSE disconnect, Kotlin overload resolution, SCIP pipeline).Tests cover the collision case directly — two
implmodules under different parents must produce two nodes — plus canonical identity on both ends of a dependency edge, the slash-separatedincludeform, and a characterization test pinning the exact root-module name so the single-module fallback can't drift silently.Scope
gradle.py+120, one test file. No schema change, no new node or edge type.Fifth of five in an Android/Kotlin series, and the only one that stands alone — it shares no code with the other four and can merge in any order.