Skip to content

Commit 3d5a392

Browse files
Break cycles when parsing Gradle projects. (#6666)
* Break cycles when parsing Gradle projects. 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. * Update rewrite-gradle-tooling-model/model/src/test/java/org/openrewrite/gradle/marker/GradleProjectBuilderTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Cleanup --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 86ba5ca commit 3d5a392

2 files changed

Lines changed: 361 additions & 7 deletions

File tree

rewrite-gradle-tooling-model/model/src/main/java/org/openrewrite/gradle/marker/GradleProjectBuilder.java

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ static List<MavenRepository> mapRepositories(List<ArtifactRepository> repositori
100100
.collect(toList());
101101
}
102102

103+
@SuppressWarnings("ResultOfMethodCallIgnored")
103104
private static MavenRepository.Builder withAuthentication(MavenArtifactRepository repo, MavenRepository.Builder builder) {
104105
if (!(repo instanceof AuthenticationSupportedInternal)) {
105106
return builder;
@@ -227,6 +228,7 @@ static Map<String, GradleDependencyConfiguration> dependencyConfigurations(Confi
227228
.collect(toMap(GradleProjectBuilder::groupArtifact, dep -> dep, (a, b) -> a));
228229
String exceptionType = null;
229230
String exceptionMessage = null;
231+
List<String> detectedCycles = new ArrayList<>();
230232
// Archives and default are redundant with other configurations
231233
// Newer versions of gradle display warnings with long stack traces when attempting to resolve them
232234
// Some Scala plugin we don't care about creates configurations that, for some unknown reason, are difficult to resolve
@@ -242,10 +244,24 @@ static Map<String, GradleDependencyConfiguration> dependencyConfigurations(Confi
242244
}
243245
Map<GroupArtifact, ResolvedDependency> gaToResolved = resolvedConf.getFirstLevelModuleDependencies().stream()
244246
.collect(toMap(GradleProjectBuilder::groupArtifact, dep -> dep, (a, b) -> a));
245-
resolved = resolved(gaToRequested, gaToResolved);
247+
resolved = resolved(gaToRequested, gaToResolved, detectedCycles);
246248
} else {
247249
resolved = emptyList();
248250
}
251+
252+
// If cycles were detected, record them in the exception fields
253+
if (!detectedCycles.isEmpty()) {
254+
if (exceptionType == null) {
255+
exceptionType = "org.openrewrite.gradle.marker.CyclicDependencyException";
256+
}
257+
String cycleMessage = "Cyclic dependency detected: " + String.join("; ", detectedCycles);
258+
if (exceptionMessage == null) {
259+
exceptionMessage = cycleMessage;
260+
} else {
261+
exceptionMessage = exceptionMessage + "; " + cycleMessage;
262+
}
263+
}
264+
249265
GradleDependencyConfiguration dc = new GradleDependencyConfiguration(conf.getName(), conf.getDescription(),
250266
conf.isTransitive(), conf.isCanBeResolved(), conf.isCanBeConsumed(), isCanBeDeclared(conf), emptyList(), requested, resolved, exceptionType, exceptionMessage, constraints(configurationContainer, conf), attributes(conf));
251267
results.put(conf.getName(), dc);
@@ -370,6 +386,7 @@ private static String projectPath(ProjectDependency pd) {
370386
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
371387
// ProjectDependency.getDependencyProject() scheduled for removal in Gradle 9.0
372388
try {
389+
//noinspection JavaReflectionMemberAccess
373390
Method getDependencyProject = ProjectDependency.class.getMethod("getDependencyProject");
374391
return ((Project) getDependencyProject.invoke(pd)).getPath();
375392
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException ex) {
@@ -379,9 +396,10 @@ private static String projectPath(ProjectDependency pd) {
379396
}
380397
}
381398

382-
private static List<org.openrewrite.maven.tree.ResolvedDependency> resolved(
399+
static List<org.openrewrite.maven.tree.ResolvedDependency> resolved(
383400
Map<GroupArtifact, org.openrewrite.maven.tree.Dependency> gaToRequested,
384-
Map<GroupArtifact, ResolvedDependency> gaToResolved) {
401+
Map<GroupArtifact, ResolvedDependency> gaToResolved,
402+
List<String> detectedCycles) {
385403
Map<org.openrewrite.maven.tree.ResolvedGroupArtifactVersion, org.openrewrite.maven.tree.ResolvedDependency>
386404
resolvedCache = new HashMap<>();
387405
return gaToResolved.entrySet().stream()
@@ -394,13 +412,16 @@ private static List<org.openrewrite.maven.tree.ResolvedDependency> resolved(
394412
org.openrewrite.maven.tree.ResolvedDependency resolvedDependency = resolvedCache.get(resolvedGav);
395413
if (resolvedDependency == null) {
396414
org.openrewrite.maven.tree.Dependency requested = gaToRequested.getOrDefault(ga, dependency(resolved));
415+
// Track the traversal path to detect cycles at any depth
416+
List<GroupArtifactVersion> ancestorPath = new ArrayList<>();
417+
ancestorPath.add(groupArtifactVersion(resolved));
397418
resolvedDependency = org.openrewrite.maven.tree.ResolvedDependency.builder()
398419
.gav(resolvedGav)
399420
// There may not be a requested entry if a dependency substitution rule took effect
400421
// the DependencyHandler has the substitution mapping buried inside it, but not exposed publicly
401422
.requested(requested)
402423
.dependencies(resolved.getChildren().stream()
403-
.map(child -> resolved(child, 1, resolvedCache))
424+
.map(child -> resolved(child, 1, resolvedCache, detectedCycles, ancestorPath))
404425
.collect(toList()))
405426
.licenses(emptyList())
406427
.type(requested.getType())
@@ -442,9 +463,11 @@ private static org.openrewrite.maven.tree.Dependency dependency(ResolvedDependen
442463
});
443464
}
444465

445-
private static org.openrewrite.maven.tree.ResolvedDependency resolved(
466+
static org.openrewrite.maven.tree.ResolvedDependency resolved(
446467
ResolvedDependency dep, int depth,
447-
Map<org.openrewrite.maven.tree.ResolvedGroupArtifactVersion, org.openrewrite.maven.tree.ResolvedDependency> resolvedCache
468+
Map<org.openrewrite.maven.tree.ResolvedGroupArtifactVersion, org.openrewrite.maven.tree.ResolvedDependency> resolvedCache,
469+
List<String> detectedCycles,
470+
List<GroupArtifactVersion> ancestorPath
448471
) {
449472
ResolvedGroupArtifactVersion resolvedGav = resolvedGroupArtifactVersion(dep);
450473
org.openrewrite.maven.tree.ResolvedDependency resolvedDependency = resolvedCache.get(resolvedGav);
@@ -461,7 +484,52 @@ private static org.openrewrite.maven.tree.ResolvedDependency resolved(
461484
.build();
462485
//we add a temporal resolved dependency in the cache to avoid stackoverflow with dependencies that have cycles
463486
resolvedCache.put(resolvedGav, resolvedDependency);
464-
dep.getChildren().forEach(child -> dependencies.add(resolved(child, depth + 1, resolvedCache)));
487+
488+
// Detect cycles at any depth by checking if a child dependency is already in our traversal path
489+
dep.getChildren().forEach(child -> {
490+
GroupArtifactVersion childGav = groupArtifactVersion(child);
491+
// Check if the child is already in the current traversal path (cycle detection)
492+
// Compare by GroupArtifact only since the same artifact may appear with different versions
493+
GroupArtifact childGa = new GroupArtifact(childGav.getGroupId(), childGav.getArtifactId());
494+
int cycleIndex = -1;
495+
for (int i = 0; i < ancestorPath.size(); i++) {
496+
GroupArtifactVersion ancestor = ancestorPath.get(i);
497+
if (childGa.equals(new GroupArtifact(ancestor.getGroupId(), ancestor.getArtifactId()))) {
498+
cycleIndex = i;
499+
break;
500+
}
501+
}
502+
503+
if (cycleIndex >= 0) {
504+
// Build the full cycle chain for the error message
505+
StringBuilder cycleChain = new StringBuilder();
506+
for (int i = cycleIndex; i < ancestorPath.size(); i++) {
507+
GroupArtifactVersion gav = ancestorPath.get(i);
508+
if (i > cycleIndex) {
509+
cycleChain.append(" -> ");
510+
}
511+
cycleChain.append(gav.getGroupId()).append(":").append(gav.getArtifactId());
512+
if (gav.getVersion() != null) {
513+
cycleChain.append(":").append(gav.getVersion());
514+
}
515+
}
516+
cycleChain.append(" -> ");
517+
cycleChain.append(childGav.getGroupId()).append(":").append(childGav.getArtifactId());
518+
if (childGav.getVersion() != null) {
519+
cycleChain.append(":").append(childGav.getVersion());
520+
}
521+
detectedCycles.add(cycleChain.toString());
522+
} else {
523+
// Add current child to the path before recursing
524+
ancestorPath.add(childGav);
525+
try {
526+
dependencies.add(resolved(child, depth + 1, resolvedCache, detectedCycles, ancestorPath));
527+
} finally {
528+
// Remove from path after processing (backtrack)
529+
ancestorPath.remove(ancestorPath.size() - 1);
530+
}
531+
}
532+
});
465533
}
466534
return resolvedDependency;
467535
}

0 commit comments

Comments
 (0)