Conversation
- Move dependency resolution from configuration to execution phase using providers - Replace lenientConfiguration with ArtifactView API for lazy resolution - Store configuration names during configuration, resolve during execution - Add debug logging to track resolution timing
- Add test dependencies (JUnit 5, Gradle TestKit) - Configure test task with JUnit Platform - Add comprehensive unit tests for core functionality
There was a problem hiding this comment.
Pull request overview
This PR optimizes the AboutLibraries Gradle plugin by deferring dependency resolution from the configuration phase to the execution phase, achieving 46-51% faster configuration times. The changes replace deprecated lenientConfiguration API with modern ArtifactView API and use lazy providers for dependency resolution.
Changes:
- Replaced
lenientConfigurationwithArtifactViewAPI for configuration-cache compatibility - Deferred dependency resolution using Gradle providers in task configuration
- Added comprehensive test suite for configuration cache, performance, and functional correctness
Reviewed changes
Copilot reviewed 9 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| DependencyCollector.kt | Switched from deprecated lenientConfiguration to ArtifactView API for lazy resolution |
| BaseAboutLibrariesTask.kt | Deferred dependency collection using providers evaluated at execution time |
| ConfigurationCacheTest.kt | Tests verifying configuration cache compatibility |
| ConfigurationTimeResolutionTest.kt | Tests ensuring no resolution during configuration phase |
| FunctionalTest.kt | Tests verifying functional behavior remains correct |
| PerformanceTest.kt | Tests measuring configuration performance improvements |
| build.gradle.kts | Added test dependencies for JUnit 5 and Gradle TestKit |
| .github/workflows/ci.yml | Added test execution step to CI workflow |
| aboutlibraries.json (3 files) | Updated sample output files from test runs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...-build/plugin/src/main/kotlin/com/mikepenz/aboutlibraries/plugin/util/DependencyCollector.kt
Outdated
Show resolved
Hide resolved
… aboutLibraries output
…el before logging
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 15 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Should not see POM downloads during configuration | ||
| // The string "Download " indicates Gradle is downloading artifacts | ||
| val downloadCount = result.output.lines().count { it.contains("Download ") && it.contains("pom") } | ||
| assertEquals(0, downloadCount, "POM files should not be downloaded during configuration phase") |
There was a problem hiding this comment.
The test assertion on line 55 expects exactly 0 POM downloads, but this may be too strict. If Gradle has already cached the POMs from previous builds or other tests, the count will be 0. However, if the local cache is empty, Gradle might still download POMs even during the tasks command if it needs to resolve plugin classpaths or other metadata. Consider making this test more robust by either clearing the cache first or using a more lenient assertion that focuses on whether the AboutLibraries plugin itself triggers downloads, rather than all POM downloads in general.
| // Should not see POM downloads during configuration | |
| // The string "Download " indicates Gradle is downloading artifacts | |
| val downloadCount = result.output.lines().count { it.contains("Download ") && it.contains("pom") } | |
| assertEquals(0, downloadCount, "POM files should not be downloaded during configuration phase") | |
| // Should not see POM downloads for the AboutLibraries plugin during configuration | |
| // The string "Download " indicates Gradle is downloading artifacts | |
| val downloadCount = result.output.lines() | |
| .count { it.contains("Download ") && it.contains("pom") && it.contains("aboutlibraries", ignoreCase = true) } | |
| assertEquals(0, downloadCount, "AboutLibraries POM files should not be downloaded during configuration phase") |
| // Use ArtifactView API which is configuration-cache compatible and lazy | ||
| fun Configuration.artifactsViaView() = incoming.artifactView { config -> | ||
| config.lenient(true) | ||
| }.artifacts.map { it.file to it.id.componentIdentifier } |
There was a problem hiding this comment.
The call to artifactsViaView() on line 273 triggers artifact resolution immediately, which happens during the provider evaluation. While this is better than configuration-time resolution, the function name and comment suggest lazy resolution, but incoming.artifactView().artifacts is not completely lazy - it resolves when accessed.
The artifacts property returns a ArtifactCollection that resolves when iterated. This means the resolution still occurs, just at a different time (during task execution when the provider is evaluated, rather than during configuration). The improvement is real but the implementation could be clearer about when resolution actually happens.
Generated by 🚫 Danger |
mikepenz
changed the title
Summary
This PR optimizes the AboutLibraries Gradle plugin
Changes
Technical Fix
resolvedConfiguration.lenientConfigurationwith modernArtifactViewAPIincoming.artifactView { config.lenient(true) }Files Modified
plugin-build/plugin/src/main/kotlin/com/mikepenz/aboutlibraries/plugin/util/DependencyCollector.ktplugin-build/plugin/src/main/kotlin/com/mikepenz/aboutlibraries/plugin/BaseAboutLibrariesTask.ktplugin-build/plugin/build.gradle.kts(test dependencies)Testing
Baseline: commit 7157c76 (develop)
Optimized: commit 6186515 (this branch)