refactor(oss-licenses): replace eager dependency resolution with lazy ArtifactView providers#381
Conversation
dff5638 to
d84b4a3
Compare
66c7d5d to
64a11e5
Compare
64a11e5 to
b9839ad
Compare
|
@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 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. |
|
I'm out of office for a week. I'll review this when I get back |
b1c72e0 to
c0f4d58
Compare
7d0d00b to
8e2123d
Compare
c0f4d58 to
c007df8
Compare
c007df8 to
f51d90c
Compare
…shot hashing
## Summary
This PR refactors the `oss-licenses-plugin` to fully support the **Gradle Configuration Cache** by replacing eager configuration-phase dependency resolution with lazy `ArtifactView` providers. It also introduces SHA-256 hashing for `-SNAPSHOT` dependencies to ensure license data remains fresh during active development.
### Key Changes
- **Lazy ArtifactView Resolution**: Moved dependency resolution from the configuration phase to the execution phase using lazy `ArtifactView` and `resolvedArtifacts` providers.
- **Configuration Cache Support**:
- Removed `DependencyUtil` and `ArtifactFiles`.
- Rewrote property mapping to avoid capturing the `Project` object in closures.
- **Snapshot Integrity**: Added SHA-256 hashing for dependencies with `-SNAPSHOT` versions in `DependencyTask` to trigger downstream task re-runs.
- **Ratchet pins**: Bumped `abcxyz/actions/lint-github-actions` and `sethvargo/ratchet` pins on `main.yml` (both track `@main` and had drifted).
388fb9c to
21d0bd5
Compare
| // We look up the POM file using the pre-resolved map provided during configuration. | ||
| ArtifactFiles files = getArtifactFiles().get().get(artifactInfo.toString()) | ||
| addLicensesFromPom(files?.pomFile, artifactInfo.group, artifactInfo.name) | ||
| protected void addLicensesFromPom(Map<String, File> pomMap, ArtifactInfo artifactInfo) { |
There was a problem hiding this comment.
Does this need to be protected? As far as I can tell it is only used within the same file.
There was a problem hiding this comment.
I've had issues with Gradle's Groovy handling managing to get at the plugin from some code path that wasn't in the right namespace to access private members. Would love to convert to Kotlin, but I don't have enough testing coverage yet to feel comfortable doing so.
| protected void addLicensesFromPom(File pomFile, String group, String name) { | ||
| if (pomFile == null || !pomFile.exists()) { | ||
| logger.error("POM file $pomFile for $group:$name does not exist.") | ||
| logger.info("POM file $pomFile for $group:$name does not exist. This is expected for some libraries from androidx and org.jetbrains") |
There was a problem hiding this comment.
Why would we be ok missing pom files? That sounds like a bug. Which AndroidX libraries are missing pom files?
There was a problem hiding this comment.
It is a preexisting issue and I'm tracking it in #387. Will want to work on it soon when I have time. I believe the new test app shows missing licenses, but I'm out of the office so can't grab logs atm.
Summary
This PR refactors the
oss-licenses-pluginto fully support the Gradle Configuration Cache by replacing eager configuration-phase dependency resolution with lazyArtifactViewproviders. It also introduces SHA-256 hashing for-SNAPSHOTdependencies to ensure license data remains fresh during active development.Key Changes
ArtifactViewandresolvedArtifactsproviders.DependencyUtilandArtifactFiles.Projectobject in closures.-SNAPSHOTversions inDependencyTaskto trigger downstream task re-runs.abcxyz/actions/lint-github-actionsandsethvargo/ratchetpins onmain.yml(both track@mainand had drifted).