Gradle plugin may check the unresolved dependencies of a configuration's copy, resulting in a StackOverflowError when combined with the nebula.resolution-rules plugin

[danhyun]

Configuration#getResolvedConfiguration() set to be deprecated in favor of using DependencyResult

Future releases will have a direct method to find all unresolved deps in a more straightforward manner.

[philwebb]

@wilkinsona 2.1 or 2.x?

[wilkinsona]

Ideally neither in its current form as it introduces the use of an internal type. I need to take some time to understand the problem and consider any alternatives.

[wilkinsona]

I've pushed a branch that takes a slightly different approach and avoids the use of any internal API. However, it does still introduce the use of some incubating API:

• org.gradle.api.artifacts.component.ComponentSelector
• org.gradle.api.artifacts.component.ModuleComponentSelector
• org.gradle.api.artifacts.result.UnresolvedDependencyResult

By contrast, the current code doesn't use any internal or incubating API. So, while I think the code in my branch is slightly better than the code proposed in this pull request, it does still add some risk compared to what we currently have.

@danhyun Can you please help me to understand why you've proposed this change? Can you point me to Gradle's plans for deprecating Configuration#getResolvedConfiguration? If it's going away, I would hope that the alternatives are going to graduate from incubation.

[danhyun]

@wilkinsona thanks for the work!
I'm afraid all I can offer is the equivalent of a hallway conversation.

Another motivation for this PR is that plugin interactions causes Gradle to go into a tailspin, specifically interaction between Nebula's resolution/alignment functionality and the use of configuration.getResolvedConfiguration().

I spoke with some of the folks at Gradle about the usage of getResolvedConfiguration() and that's where they revealed the intention to take it out of incubating and even add getUnresolvedDependencies() to the DependencyResult in upcoming 5.X line.

This PR is an attempt to let users continue to consume Nebula plugin along with Spring Boot 2 plugin in a future forward looking manner.

I prefer your branch over mine for the same objection of using internals (yuck), I didn't even stop to check that that was what I was doing. (Would be a good time to add a lint rule to leave that as a warning although I've heard plugin maintainers argue that they can't get what they need otherwise :| ). I'm in favor of closing this PR if you'll include that other branch in the next release.

[wilkinsona]

@danhyun I'm still rather reluctant to move onto incubating API, certainly until we've eliminated other possibilities. I tried using your minimal sample to reproduce the problem and failed to do so. Could you please point me in the right direction? Looking at the StackOverflowError, I'm wondering if keeping track of the configurations that have already been seen would be sufficient to break the cycle.

[danhyun]

@wilkinsona we do keep track of configurations that we have seen, there's an issue with how a given config is created by the time it comes back to Nebula.

If you step through the debugger you'll find that the configuration name doesn't match up with what it is supposed to be, namely that it's being reported as the original config when in fact we're looking at a copy of that configuration.

I'll do a bit more digging to make sure that we've exhausted our options from Nebula's perspective.

Here's a reliable way to reproduce what I've been seeing.

boom.gradle

plugins {
  id "nebula.resolution-rules" version "5.1.1"
  id 'org.springframework.boot' version '2.0.1.RELEASE'
}

apply plugin: 'java'

repositories {
  jcenter()
}

dependencies {
  resolutionRules 'com.netflix.nebula:gradle-resolution-rules:latest.release'
  implementation 'javax.inject:javax.inject:1.0'
}

To reproduce:
gradle -s -b boom.gradle dependencies

[danhyun]

I've switched gears a bit.
New strategy here is to check that we're analyzing the unresolved modules for the proper config.
It's possible that Gradle will emit resolved events for copies of the original configuration that will trigger the action that we've registered for that original hook.

We ran into the same thing in Nebula and added a check such as:

https://github.com/nebula-plugins/nebula-gradle-interop/blob/master/src/main/kotlin/com/netflix/nebula/interop/configurations.kt

To ensure that we don't spiral out of control.

[wilkinsona]

Very nice! Thanks, @danhyun.

[danhyun]

Thanks! I'm guessing this will get out with the 2.0.2 release next month? https://github.com/spring-projects/spring-boot/milestone/109

[danhyun]

Are all items in 2.0.x milestone expected to ship with the next patch?
Just curious about when we can expect this to land.

[wilkinsona]

An issue assigned to 2.0.x will be fixed in a 2.0.something release, but we haven't yet committed to fixing it in the next patch release.