Skip to content

refactor(oss-licenses): replace eager dependency resolution with lazy ArtifactView providers#381

Draft
timothyfroehlich wants to merge 4 commits intotestappfrom
refactor-lazy-provider
Draft

refactor(oss-licenses): replace eager dependency resolution with lazy ArtifactView providers#381
timothyfroehlich wants to merge 4 commits intotestappfrom
refactor-lazy-provider

Conversation

@timothyfroehlich
Copy link
Member

@timothyfroehlich timothyfroehlich commented Mar 10, 2026

Summary

Replaces the DependencyUtil helper and ArtifactFiles wrapper with lazy Provider-based artifact resolution directly in OssLicensesPlugin. This ensures dependency resolution happens at task execution time rather than during configuration, which is required for Configuration Cache compatibility and avoids slowing down Android Studio sync.

Key changes

  • Use ArtifactView.resolvedArtifacts (returns Provider) instead of eagerly iterating configuration.incoming.artifacts
  • Split the single @Nested MapProperty<String, ArtifactFiles> into two @Internal MapProperty<String, File> properties (libraryFilesByGav, pomFilesByGav) to avoid expensive redundant hashing of immutable Maven artifacts
  • Extract DependencyHandler into a local variable before entering lazy closures to avoid capturing the non-serializable Project object
  • Move ABSENT_ARTIFACT constant from deleted DependencyUtil to DependencyTask where it is used
  • Delete ArtifactFiles, DependencyUtil, and DependencyResolutionTest (coverage provided by existing EndToEnd integration tests)
  • Add assertion to testConfigurationCache that fails if dependencies are resolved during configuration time, catching any regression

Result

  • Net -167 lines
  • All 52 tests pass (6 AGP versions × 5 test methods + unit tests)
  • Configuration cache, build cache relocatability, and complex dependency graph tests all green

@timothyfroehlich timothyfroehlich self-assigned this Mar 10, 2026
@timothyfroehlich timothyfroehlich force-pushed the refactor-lazy-provider branch 3 times, most recently from dff5638 to d84b4a3 Compare March 11, 2026 19:27
@timothyfroehlich timothyfroehlich force-pushed the refactor-lazy-provider branch 2 times, most recently from 66c7d5d to 64a11e5 Compare March 16, 2026 00:26
@timothyfroehlich timothyfroehlich changed the title refactor: return lazy Provider from DependencyUtil.resolveArtifacts refactor(oss-licenses): replace eager dependency resolution with lazy ArtifactView providers Mar 16, 2026
@timothyfroehlich
Copy link
Member Author

@liutikas I did a full rewrite to get things right and managed to significantly simplify things in the process. The Licenses task now separate arguments consisting of the runtimeConfiguration's resolvedArtifacts provider, with artifact resolution happening within the .map lambda. I also added a check in the configuration cache test to make sure we aren't resolving anything during configuration time.

I did use Gemini to help me get it straight, and had it add comments to help me (or other maintainers) remember for next time. I'm running an extra --scan now to double check things, but it should be pretty clean.

@liutikas
Copy link
Collaborator

I'm out of office for a week. I'll review this when I get back

timothyfroehlich and others added 4 commits March 17, 2026 16:52
… ArtifactView providers

Replace the DependencyUtil helper and ArtifactFiles wrapper with lazy
Provider-based artifact resolution directly in OssLicensesPlugin. This
ensures dependency resolution happens at task execution time rather than
during configuration, which is required for Configuration Cache
compatibility and avoids slowing down Android Studio sync.

Key changes:
- Use ArtifactView.resolvedArtifacts (returns Provider) instead of
  eagerly iterating configuration.incoming.artifacts
- Split the single @nested MapProperty<String, ArtifactFiles> into two
  @internal MapProperty<String, File> properties (libraryFilesByGav,
  pomFilesByGav) to avoid expensive redundant hashing of immutable
  Maven artifacts
- Extract DependencyHandler into a local variable before entering lazy
  closures to avoid capturing the non-serializable Project object
- Move ABSENT_ARTIFACT constant from deleted DependencyUtil to
  DependencyTask where it is used
- Delete ArtifactFiles, DependencyUtil, and DependencyResolutionTest
  (coverage provided by existing EndToEnd integration tests)
Add assertion to testConfigurationCache that fails if Gradle logs
"resolved during configuration time", catching any regression where
artifact resolution leaks back into the configuration phase.
This was surfaced by the test app being introduced in #380
@timothyfroehlich timothyfroehlich changed the base branch from main to testapp March 17, 2026 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants