autonomousapps / dependency-analysis-gradle-plugin

Gradle plugin for JVM projects written in Java, Kotlin, Groovy, or Scala; and Android projects written in Java or Kotlin. Provides advice for managing dependencies and other applied plugins
Apache License 2.0
1.74k stars 117 forks source link

Bug: caching issues for included builds #1220

Open seregamorph opened 2 months ago

seregamorph commented 2 months ago

Problem statement

We've observed severe issues with our isolated (included) builds: the projectHealth producing invalid advices to remove a dependency (if applied as suggested - it just breaks the compilation). Worth to mention that problem only happened with build cache enabled (different behaviour with --no-build-cache executions).

Root cause analysis

Finally, after debugging and digging into build scans it was found that the problem is with these two tasks:

and it's related to build caching. With equal (from the Gradle perspective) task inputs it is reusing inappropriate cached task output from another isolated builds.

The detailed analysis highlighted key differences in generated build/reports/dependency-analysis/{sourceSet}/intermediates/artifacts.json file (taken from cache vs no build cache):

Similar situation with GraphViewTask outputs: the graph-[compile/runtime].json output files contain incorrect included_build.identifier from another included build cached result. Or incorrect node.identifier.

Additionally, the behavior of projectHealth (more precise-computeAdvice) task logic should be adjusted - instead of providing invalid advice it should fail with diagnostics like inconsistent input state or smth like this. As currently it just gives misleading outputs.

Reproducing locally

There are several known scenarios when this happens

Solutions

Several possible solution strategies are suggested.

Rearrange the implementation

In most cases the projectHealth is producing wrong advice, because it (reuses wrong cached output of mentioned tasks and) cannot find matching module by their fully qualified coordinates. Is it possible not to rely on fully qualified coordinates and use internal coordinates instead? According to https://github.com/autonomousapps/dependency-analysis-gradle-plugin/pull/916 we now use IncludedBuildCoordinates intentionally (cc @jjohannes).

Disable caching for artifactsReportX and graphViewX

This solution has obvious disadvantage, but as pros:

This approach can be optional. E.g. if it's known that there are included builds in the project, the caching of task may be disabled.

Related change: https://github.com/autonomousapps/dependency-analysis-gradle-plugin/pull/1202 reproducible outputs of tasks to enhance caching of other tasks in build graph

Add new task inputs

Root project name as a task input

Does not cover all cases

Root project full/relative path as a task input

Does not cover all cases

Include variant capabilities as task input as they reflect the difference of isolated builds

PoC: https://github.com/autonomousapps/dependency-analysis-gradle-plugin/pull/1168 Does not cover the scenario with moving module from one included build to another, but looks like a nice first step

Use configuration.incoming.resolutionResult.rootComponent as task input

Slows down initialization phase (a bit). Looks most promising and most correct

Related issues:

autonomousapps commented 1 month ago

Thanks for the issue.