Fix ArtifactsReportTask caching for included builds

seregamorph commented 2 months ago

Problem statement

We've observed an issue with our isolated (included) builds: the projectHealth was producing illegal advices to remove a dependency - and these advices were breaking the compilation. Worth to mention that problem only happened with cache enabled.

Root cause analysis

Finally, after debugging it was found that the root problem is in the ArtifactsReportTask and it's related to caching. With equal (from the Gradle perspective) task inputs it was 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):

coordinates.identifier which can be different depending on current root build project (prefix)
coordinates.resolvedProject.gradleVariantIdentification.capabilities - same - different prefix, depends on current root build project

Solution

All possible ways to address the problem are to extend the task inputs - the input should be reproducible, but different for different isolated builds. Options considered:

~~include root project name as a task input~~ (does not cover all cases)
~~include root project full path as a task input~~ (does not cover all cases)
~~include relative path from current project to root (like "..") (explained below)~~ (does not cover all cases)
include variant capabilities as task input as they reflect the difference of isolated builds (explained below)

In this PR you can see the root absolute project path as input and there are few comments why this solution seems to be valid:

the generated artifacts.json contains absolute file paths, so it seems to be reasonable to make inputs absolute-path sensitive e.g. to distinguish CI/CD and local builds where such paths are usually different

the ArtifactsReportTask already has an absolute-path sensitive input:

@PathSensitive(PathSensitivity.ABSOLUTE)
@InputFiles
fun getClasspathArtifactFiles(): FileCollection = artifacts.artifactFiles

Additional notes

This seems to be related to https://github.com/autonomousapps/dependency-analysis-gradle-plugin/pull/916 authored by @jjohannes where there was an effort to address the problem. Unfortunately, the task input parameter buildPath always has value ":", so it's equal input for Gradle - and this fix was not enough.

Testing

I'm not quite sure how to write a proper functional test scenario for this change, advices are welcome. This change is validated on our (pretty big) project.

Alternative solution: relative path to root

Using absolute root path as input is motivated, but still there is an opportunity to declare relative path from current project of task to root project (note: not vice versa). It means, that in most cases such Input parameter will have value like ".." or "../..". But for included builds it will look like "../../included/composite-project"

Alternative solution: Set Input

I'm not quite sure, but perhaps the capabilities as input parameter can be an alternative solution:

@Input
fun getClasspathCapabilities(): Set<Capability> = artifacts.asSequence()
    .filterNonGradle()
    .flatMap { it.variant.capabilities }
    .toSet()

Elements of this collection are different depending on current root project, the difference is in the field group (has specific prefix). This was an initial implementation that worked pretty well. But I'd like to not to complicate things too early.

seregamorph commented 2 months ago

Some updates from experience with this issue. The fix with adding new Input with absolute path was not enough. Eventually similar problem happened on moving one of the modules in one of our included builds. So the set of input artifacts were the same (hence cached task result was obtained), but the actual project name became different. In the generated artifacts.json it was represented also as different coordindates.identifier and different gradleVariantIdentification.capabilities.

To fix the problem the mentioned capabilities input was added to task inputs:

@Input
fun getClasspathCapabilities(): Set<Capability> = artifacts.asSequence()
    .filterNonGradle()
    .flatMap { it.variant.capabilities }
    .toSet()

So far works good for our projects. I hope you will have time soon to dig deeper this issue.

seregamorph commented 1 month ago

Sorry for the delay, I've created simple project reproducing the issue https://github.com/seregamorph/demo-gradle-included-issue.git

I'd like to focus attention:

the build has to be binary reproducible (files packed to jar have constant timestamp), it's done via ReproducibleBuildProjectPlugin
for the sake of own security you can run gradle command instead of ./gradlew - the result is the same
the remote cache is not required here, the problem is visible with local cache

Please follow the instruction. Run the build for isolated-build-1:

cd isolated-build-1
./gradlew clean artifactsReportMain -i

Check output:

> Task :module-impl:artifactsReportMain
Build cache key for task ':module-impl:artifactsReportMain' is 3c9a8a4ff40ea0212d6eefd2be101534
Task ':module-impl:artifactsReportMain' is not up-to-date because:
  No history is available.
Stored cache entry for task ':module-impl:artifactsReportMain' with cache key 3c9a8a4ff40ea0212d6eefd2be101534
^^^^ Pay attention to build cache key ^^^^

Then run for isolated-build-2

cd ../isolated-build-2
./gradlew clean artifactsReportMain -i

and check output:

> Task :module-impl:artifactsReportMain FROM-CACHE
Build cache key for task ':module-impl:artifactsReportMain' is 3c9a8a4ff40ea0212d6eefd2be101534
Task ':module-impl:artifactsReportMain' is not up-to-date because:
  No history is available.
Loaded cache entry for task ':module-impl:artifactsReportMain' with cache key 3c9a8a4ff40ea0212d6eefd2be101534

^^^^ Same build cache key ^^^^

Now we can check the task output in the console:

cat ../module-impl/build/reports/dependency-analysis/main/intermediates/artifacts.json | jq

[
  {
    "coordinates": {
      "type": "included_build",
      "identifier": "isolated-build-1:module-contracts",
      "resolvedProject": {
        "identifier": ":module-contracts",
        "gradleVariantIdentification": {
          "capabilities": [
            "isolated-build-1:module-contracts"
          ],
          "attributes": {}
        },
        "buildPath": ":"
      },
      "gradleVariantIdentification": {
        "capabilities": [
          "isolated-build-1:module-contracts"
        ],
        "attributes": {}
      }
    },
    "file": "/Users/username/Projects/zzz/tmp11/demo-gradle-included-issue/module-contracts/build/libs/module-contracts.jar"
  }
]

As you can see, the dependency coordinates is "identifier": "isolated-build-1:module-contracts" which is correct for first build, but not second.

autonomousapps / dependency-analysis-gradle-plugin