configuration cache #728

jaredsburrows · 2025-12-12T17:10:31Z

Closes #217
Closes #368

Copilot

Pull request overview

This pull request refactors the Gradle license plugin to support Gradle's configuration cache feature. The changes move POM file resolution and dependency analysis from task execution time to configuration time, making task inputs explicit and serializable.

Key changes include:

Refactored LicenseReportTask to use declarative inputs (pomFiles, rootCoordinates, pomCoordinatesToFile) instead of accessing project state at execution time
Moved POM resolution logic to configuration time in Project.kt with buildPomInput() function
Updated test version matrix to focus on Java 21 compatible versions (Gradle 8.5+ and 9.2.1, AGP 8.4.2+)

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`LicenseReportTask.kt`	Changed from open class to abstract class with injection; replaced buildFile input with pomFiles, rootCoordinates, and pomCoordinatesToFile inputs; refactored POM resolution to work with pre-resolved data
`Project.kt`	Added buildPomInput() function to resolve POMs at configuration time; implemented recursive parent POM resolution; updated configureCommon() to accept configuration names
`ProjectJava.kt`	Updated to pass Java-specific configuration names (compileClasspath, runtimeClasspath) to configureCommon()
`ProjectAndroid.kt`	Updated to pass variant-specific configuration names to configureCommon()
`LicensePluginVersionSpec.groovy`	Reduced test matrix to focus on recent Gradle/AGP versions compatible with Java 21; removed older version combinations
`build.gradle`	Refactored createClasspathManifest task as abstract class with proper input annotations for configuration cache compatibility

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-12T17:16:36Z

gradle-license-plugin/src/main/kotlin/com/jaredsburrows/license/LicenseReportTask.kt

+          "project.artifactId" to effectiveArtifactId,
+          "pom.artifactId" to effectiveArtifactId,
+          "artifactId" to effectiveArtifactId,
+          "project.name" to effectiveArtifactId,


The placeholder "project.name" is mapped to effectiveArtifactId rather than the actual project name from the POM. While this may be intentional behavior to avoid circular references, it could be confusing since it doesn't match the typical Maven interpolation semantics where project.name would refer to the name field. Consider adding a comment explaining this design choice, or using a more appropriate value if the actual name should be resolved.

Suggested change

"project.name" to effectiveArtifactId,

"project.name" to rawName,

Copilot · 2025-12-12T17:16:37Z

gradle-license-plugin/build.gradle

+    def entries = classpath.files.collect { it.path }.toSet().toList()
+    entries.sort()


The entries are converted to a set and then back to a list. The toSet() call removes duplicates, but then toList() creates an unnecessary intermediate list before sorting. Consider simplifying to: classpath.files.collect { it.path }.unique().sort() or classpath.files.collect { it.path }.toSet().toSorted() for better readability and efficiency.

Suggested change

def entries = classpath.files.collect { it.path }.toSet().toList()

entries.sort()

def entries = classpath.files.collect { it.path }.unique().sort()

Copilot · 2025-12-12T17:16:37Z

gradle-license-plugin/src/main/kotlin/com/jaredsburrows/license/Project.kt

+    pomFiles.from(pomInput.files.files)
+    pomFiles.disallowChanges()


The pomFiles collection is configured with disallowChanges() immediately after being populated. While this is good for configuration cache compatibility, it means the collection is finalized during configuration time. If any lazy resolution of POM files is expected later, this will fail. Consider documenting this behavior or ensuring all POM resolution happens eagerly during the buildPomInput call.

Copilot · 2025-12-12T17:16:37Z

gradle-license-plugin/src/main/kotlin/com/jaredsburrows/license/Project.kt

+    val parentPomFile = resolvePomFile(parentGroupId, parentArtifactId, parentVersion) ?: continue
+    coordinateToFile[parentCoordinate] = parentPomFile.absolutePath
+    fileCollection.from(parentPomFile)
+    pomFilesToInspect.addLast(parentPomFile)
+  }


The parent POM resolution uses resolvePomFile which calls createArtifactResolutionQuery during configuration time (when buildPomInput is called). This query executes eagerly and may fail or produce unexpected results if repositories are not fully configured yet. While this is necessary for configuration cache support, it could cause issues in some build setups. Consider adding error handling to gracefully handle resolution failures and log appropriate warnings.

Copilot · 2025-12-12T17:16:37Z

gradle-license-plugin/src/test/groovy/com/jaredsburrows/license/LicensePluginVersionSpec.groovy

+      ['8.4.2', ['8.14.3',]],
+      ['8.5.2', ['8.14.3',]],
+      ['8.6.1', ['8.14.3',]],
+      ['8.7.2', ['8.14.3',]],


All AGP versions are tested with only a single Gradle version (8.14.3). This significantly reduces test coverage compared to the previous implementation which tested multiple Gradle versions per AGP version. While this may be intentional to reduce test execution time, it means compatibility issues with other Gradle versions may go undetected. Consider at least testing the minimum and maximum supported Gradle version for each AGP version.

Copilot · 2025-12-12T17:16:37Z

gradle-license-plugin/src/main/kotlin/com/jaredsburrows/license/LicenseReportTask.kt

+    private fun parseCoordinate(coordinate: String): Triple<String, String, String> {
+      val parts = coordinate.split(":")
+      if (parts.size != 3) {
+        return Triple("", coordinate, "")


The parseCoordinate function doesn't handle malformed coordinates gracefully. When a coordinate doesn't have exactly 3 parts, it returns an empty groupId and version, but keeps the full coordinate as the artifactId. This could lead to misleading error messages and logs where the artifactId appears to be a full coordinate string. Consider logging a warning for malformed coordinates or throwing an exception to fail fast.

Suggested change

return Triple("", coordinate, "")

throw IllegalArgumentException("Malformed coordinate: '$coordinate'. Expected format: 'groupId:artifactId:version'")

Copilot · 2025-12-12T17:16:38Z

gradle-license-plugin/src/test/groovy/com/jaredsburrows/license/LicensePluginVersionSpec.groovy

    gradleVersion << [
-      '7.3.3',
-      '7.4.2',
-      '7.5.1',
-      '7.6.3',
-      '8.0.2',
-      '8.1.1',
-      '8.2.1',
-      '8.3',
-      '8.4',
      '8.5',
      '8.6',
      '8.7',
      '8.8',
      '8.9',
      '8.10.2',
+      '8.14.3',
+      '9.2.1',
    ]


The PR title indicates this change is for "configuration cache" support, but there are no tests verifying that the configuration cache actually works. Consider adding tests that verify the plugin works correctly when the configuration cache is enabled (e.g., by adding '--configuration-cache' to the Gradle arguments and verifying the task is marked as compatible and executes correctly on subsequent runs).

Copilot · 2025-12-12T17:16:38Z

gradle-license-plugin/src/main/kotlin/com/jaredsburrows/license/LicenseReportTask.kt

+    private fun resolveEffectiveGroupId(
+      mavenReader: MavenXpp3Reader,
+      pomFile: File?,
+      loggedMissingParentPomCoordinates: MutableSet<String>,
+    ): String {
+      val model = pomFile?.let { readModel(mavenReader, it) } ?: return ""
+      val groupId = model.groupId.orEmpty().trim()
+      if (groupId.isNotEmpty()) {
+        return groupId
+      }
+
+      val parentFile = getParentPomFile(model, loggedMissingParentPomCoordinates) ?: return ""
+      return resolveEffectiveGroupId(mavenReader, parentFile, loggedMissingParentPomCoordinates)
+    }

-  private fun Exception.shortMessage(): String =
-    (message ?: "<no message>").let {
-      if (it.length > MAX_EXCEPTION_MESSAGE_LENGTH) {
-        "${it.take(MAX_EXCEPTION_MESSAGE_LENGTH)}... (see --debug for complete message)"
-      } else {
-        it
+    private fun resolveEffectiveVersion(
+      mavenReader: MavenXpp3Reader,
+      pomFile: File?,
+      loggedMissingParentPomCoordinates: MutableSet<String>,
+    ): String {
+      val model = pomFile?.let { readModel(mavenReader, it) } ?: return ""
+      val version = model.version.orEmpty().trim()
+      if (version.isNotEmpty()) {
+        return version
      }
+
+      val parentFile = getParentPomFile(model, loggedMissingParentPomCoordinates) ?: return ""
+      return resolveEffectiveVersion(mavenReader, parentFile, loggedMissingParentPomCoordinates)
    }


The resolveEffectiveGroupId and resolveEffectiveVersion methods use recursion without explicit depth limits. While circular dependencies are prevented by the parent resolution logic, deeply nested POM hierarchies could cause performance issues or stack overflow. Consider adding a depth limit parameter (e.g., max depth of 50) to prevent excessive recursion, or refactor to use an iterative approach.

configuration cache

2812f99

jaredsburrows requested a review from Copilot December 12, 2025 17:10

Copilot started reviewing on behalf of jaredsburrows December 12, 2025 17:10 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

configuration cache #728

configuration cache #728

Uh oh!

jaredsburrows commented Dec 12, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	"project.name" to effectiveArtifactId,
	"project.name" to rawName,

		def entries = classpath.files.collect { it.path }.toSet().toList()
		entries.sort()

		pomFiles.from(pomInput.files.files)
		pomFiles.disallowChanges()

	return Triple("", coordinate, "")
	throw IllegalArgumentException("Malformed coordinate: '$coordinate'. Expected format: 'groupId:artifactId:version'")

Uh oh!

configuration cache #728

Are you sure you want to change the base?

configuration cache #728

Uh oh!

Conversation

jaredsburrows commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jaredsburrows commented Dec 12, 2025 •

edited

Loading