-
Notifications
You must be signed in to change notification settings - Fork 502
Break cycles when parsing Gradle projects. #6666
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
Conversation
Gradle doesn't allow cyclic dependencies... usually. But we've found at least one case where, with some creative usage of composite builds, it is possible. This breaks cycles in the dependency representation, preserving the evidence in the GradleConfiguration's exception fields.
|
|
||
| import java.util.*; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import static org.assertj.core.api.Assertions.assertThat; | |
| import static java.util.Collections.emptySet; | |
| import static org.assertj.core.api.Assertions.assertThat; |
...ooling-model/model/src/test/java/org/openrewrite/gradle/marker/GradleProjectBuilderTest.java
Outdated
Show resolved
Hide resolved
| if (resolvedDependency == null) { | ||
| org.openrewrite.maven.tree.Dependency requested = gaToRequested.getOrDefault(ga, dependency(resolved)); | ||
| // Track the traversal path to detect cycles at any depth | ||
| List<GroupArtifactVersion> ancestorPath = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A HashSet would probably be a bit quicker and gives the same uniqueness constraint while also having the helpful boolean add(T) method.
I'm also not sure if we actually need to track version, but it won't hurt. Gradle's dependency tree puts the max version everywhere by the time we get to traverse it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HashSet loses order so we couldn't reproduce the A -> B -> C -> A as easily for meaningful error message. Could use LinkedHashSet, though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, I see. It probably isn't worth it then in that case.
…te/gradle/marker/GradleProjectBuilderTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Gradle doesn't allow cyclic dependencies... usually. But we've found at least one case where, with some creative usage of composite builds, it is possible.
This breaks cycles in the dependency representation, preserving the evidence in the GradleConfiguration's exception fields.