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

fix: when building 'dependencyUsages', merge usages of the same dependency at different versions. #1230

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

autonomousapps
Copy link
Owner

This resolves an issue when two versions of a dependency are in the various dependency graphs
(e.g., main and test). Without this, we were getting erroneous advice to remove used dependencies,
and then correct advice to re-add used-transitive dependencies. Additionally, the 'reason' results
were confusing and incorrect for a similar reason. As the simplest solution, require full GAV when
requesting the 'reason for' some advice in this circumstance.

There is still one minor issue in the reason output, which is it'll print the shortest path to all
versions of a given dep, even when a specific version is requested.

@autonomousapps
Copy link
Owner Author

@jjohannes I think this resolves #947.

Comment on lines +95 to +111
val other = keys.find { it.identifier == trace.coordinates.identifier }
if (other != null) {
val otherVersion = (other as? ModuleCoordinates)?.resolvedVersion
val thisVersion = (trace.coordinates as? ModuleCoordinates)?.resolvedVersion
if (otherVersion != null && thisVersion != null && otherVersion != thisVersion) {
// This mutates the existing set in place
val usages = get(other)!!.apply { add(usage) }

// If the new coordinates are "greater than" the other coordinates, add the new and remove the old
if (thisVersion > otherVersion) {
put(trace.coordinates, usages)
remove(other)
}

// We're done
return
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

This change here is the fix.

@autonomousapps autonomousapps changed the title fix: compare full GAV when looking for declarations that match coordinates. fix: when building 'dependencyUsages', merge usages of the same dependency at different versions. Jul 26, 2024
This resolves an issue when a dependency is available at more than one version,
and different usages at each version.
@autonomousapps autonomousapps force-pushed the trobalik.transitive-test-dep branch from 08725fb to eda9cfa Compare July 27, 2024 05:08
@autonomousapps autonomousapps merged commit d03e72b into main Jul 27, 2024
1 check passed
@autonomousapps autonomousapps deleted the trobalik.transitive-test-dep branch July 27, 2024 05:09
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.

1 participant