Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't modify exact dependencies index #1593

Merged
merged 1 commit into from
Jan 21, 2021

Conversation

pkoenig10
Copy link
Member

@pkoenig10 pkoenig10 commented Dec 22, 2020

Before this PR

The values in the classToDependency index can be modified if the same class is produced by multiple artifacts and subprojects are not all using the same artifact to provide this class.

This can happen quite easily when using our internal jakarta renames plugin.

After this PR

The classToDependency index stores sets of artifacts that provide a given class. The checkUnusedDependencies and checkImplicitDependencies tasks have been updated to handle multiple artifacts as gracefully as possible.

This was pretty trivial for the checkUnusedDependencies task. It was a bit more involved for the checkImplicitDependencies task since we have to treat the potential artifacts as a conjunctive normal form.

@policy-bot policy-bot bot requested a review from iamdanfox December 22, 2020 10:10
@pkoenig10 pkoenig10 force-pushed the pkoenig/classToDependency branch from 23a9289 to 7b1bb14 Compare December 22, 2020 18:11
Copy link
Contributor

@fawind fawind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very reasonable, thanks for fixing!

@bulldozer-bot bulldozer-bot bot merged commit 37a3711 into develop Jan 21, 2021
@bulldozer-bot bulldozer-bot bot deleted the pkoenig/classToDependency branch January 21, 2021 15:31
@svc-autorelease
Copy link
Collaborator

Released 3.66.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants