search

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.77k stars 116 forks source link

Make graph view task cc compatible #1039

Closed cobexer closed 10 months ago

cobexer commented 10 months ago

Hey @autonomousapps, @ljacomet and I made projectHealth CC compatible and tested it with gradle/gradle and we confirmed that we get CC hits with the changes from this PR.

cobexer commented 10 months ago

Hey @autonomousapps is there anything holding this back from getting merged and shipped?

autonomousapps commented 10 months ago

Hey @autonomousapps is there anything holding this back from getting merged and shipped?

I think we also need to bump the minimum version of Gradle required to use this plugin to 7.5, right? It's currently at 7.4 (to account for a big user that has that as their current version), and there's also at least one check in ProjectPlugin for Gradle version. In addition to changing those things, we'd need to ensure that the functional tests all run against 7.5+ (and not 7.4+).

I'm not against this bump, btw. I just think we should land that change along with this one to avoid weird errors from users on Gradle 7.4.

ljacomet commented 10 months ago

Hey @autonomousapps,

Given the scope of the changes, I don't think a minimum version bump is needed. We ended up re-using code and not having to call any new Gradle API AFAICT.

autonomousapps commented 10 months ago

I thought that one of the APIs for CC-safe dependency graph input required Gradle 7.5? if I'm wrong, that's great.

autonomousapps commented 10 months ago

Note that there is still at least one issue with using the configuration cache. In the GenerateBuildHealthTask, there's this task input:

  @Transient
  @get:PathSensitive(PathSensitivity.RELATIVE)
  @get:InputFiles
  lateinit var projectHealthReports: Configuration

That task is marked with notCompatibleWithConfigurationCache("Cannot serialize Configurations"), as is ComputeDuplicateDependenciesTask (although this latter is not actually part of buildhealth).

autonomousapps commented 10 months ago

I'm enabling CC in all functional tests here. It will at least show if there are issues with enabling it, even if the cache entries can't currently be used.

ljacomet commented 10 months ago

Hey Tony,

Indeed, this work was targeted at unblocking projectHealth. It did not cover all the cases to make this plugin fully CC compatible.

That might come in a future PR though.

ljacomet commented 10 months ago

And mostly: thanks for the merge! 🎉