KAFKA-12417: streams copyDependentLibs should not copy testRuntime configuration jars

[dejan2609]

This also fixes a Gradle deprecation that unblocks the upgrade to Gradle 7.0.

[ijuma]

I filed a JIRA for this issue, the build failed with this change before. Did it work for you?

[ijuma]

The change I had attempted was slightly different: https://issues.apache.org/jira/browse/KAFKA-12417

[dejan2609]

I just pushed configurations.testRuntimeOnly.setCanBeResolved(true) (I forgot to include that directive into my first commit).

And yes, I do receive same circular dependency error with Gradle 7 (as you mentioned in a JIRA ticket above):

./gradlew -PscalaVersion=2.12 jar
......
* What went wrong:
Circular dependency between the following tasks:
:streams:copyDependantLibs
+--- :streams:jar
|    \--- :streams:copyDependantLibs (*)
\--- :streams:test-utils:jar
     \--- :streams:test-utils:copyDependantLibs
          \--- :streams:jar (*)
......

This Gradle issue seems to be related : gradle/gradle#16604 (see step 4. under Steps to Reproduce).

@ijuma I can take that JIRA ticket and hopefully provide some solution. Is that ok with you ?

[ijuma]

@dejan2609 yes, please take the ticket. It would be great to solve this so that we can build with Java 16.

[ijuma]

Gradle 7.0 has been released. Any luck figuring out the issue?

[dejan2609]

@ijuma I narrowed the gap and will post my findings here in a few days.... some more digging will be required in order to solve this (I will try to squeeze it asap).

[dejan2609]

I am going to split my findings to a separate comments... here it goes.

I decided to work with Gradle 6.8.3 for the moment (that is, until circular dependency issue is resolved). After that we can try with Gradle 7.0.

So, I am going to change title and align it with a corresponding JIRA ticket that was mentioned above: https://issues.apache.org/jira/browse/KAFKA-12417

[dejan2609]

Also, after some digging I stumbled upon these GitHub resources (I will intentionally leave them formatted as code):

• https://github.com/gradle/gradle/issues/6353
• https://github.com/gradle/gradle/issues/847 (especially comment: https://github.com/gradle/gradle/issues/847#issuecomment-287515195):

Came across this and was looking for a workaround...so in case anyone else is trying to find a way out: it seems sufficient to change group for the subprojects such that they end up with different module coordinates

I tested this quick and dirty solution (experimented with changing group for subprojects) but that action yield no results... 😐

Also, I played around with dependencies and tasks:

• ./gradlew streams:dependencies > DEPS.txt
• ./gradlew tiTree :streams:copyDependantLibs > TasksGraph.txt
• ./gradlew tiOrder :streams:copyDependantLibs > TasksOrder.txt (appropriate plugin is added in order to create Gradle taks graph and list).

So, after some time, I moved to a phase two.

[dejan2609]

Digging through project history I managed to find some commits that are seems to be related (see the comment):

project(':streams') {
  dependencies {
    api project(':clients')
....
    // testCompileOnly prevents streams from exporting a dependency on test-utils, which would cause a dependency cycle
    testCompileOnly project(':streams:test-utils')
....

    testRuntimeOnly project(':streams:test-utils')
  }
....
}

@vvcephei contributed two PRs (#4821 and #4812) 3 years ago and these are related commits:

• 05. Apr 2018. d5db4e9
• 23. Apr 2018. ed51b2c

Out of curiosity I tried to consolidate these dependency into one testImplementation project(':streams:test-utils') and here are the results:

• on top of trunk: build works fine ✔️
• on top of this PR: build fails ❌ (with identical Circular dependency between the following tasks error as above)

[ijuma]

Could we reintroduce a configuration that behaves the same as testRuntime did before it was removed?

[dejan2609]

So, I started to abandon quick and dirty solution and to lean towards something heavier (i.e. to a some form of refactoring):

• https://discuss.gradle.org/t/gradle-support-for-cyclic-dependencies/14355/2
• https://stackoverflow.com/questions/38062841/how-to-resolve-circular-dependency-in-gradle/38063421#38063421
• https://stackoverflow.com/questions/58178381/resolve-circular-dependency-in-gradle/58186686#58186686

Gradle introduced new thing (among many others): java-test-fixtures plugin (that goes hand in hand with `java-library') that is able to fine-grain test related dependency configurations:

• https://docs.gradle.org/6.8.3/userguide/java_testing.html#declaring_dependencies_of_test_fixtures (some browsers show page bottom... in that case scroll up to **"Declaring dependencies of test fixtures"*)
• https://stackoverflow.com/questions/5644011/multi-project-test-dependencies-with-gradle/60138176#60138176
• https://stackoverflow.com/questions/64133013/gradle-test-fixtures-plugin-and-core-module-dependencies
• https://stackoverflow.com/questions/5144325/gradle-test-dependency/66138658#66138658

[dejan2609]

Ok, I finished my writings above (I just had to be as detailed as possible, these comments serve me a lot).

Could we reintroduce a configuration that behaves the same as testRuntime did before it was removed?

Well, I can try to revert to a previous solution, see how that goes and build it from there.

[dejan2609]

Could we reintroduce a configuration that behaves the same as testRuntime did before it was removed?

@ijuma Can you please expand on this ? Are you referring to a testRuntime mentioned in a copyDependantLibs task ?

https://github.com/apache/kafka/blob/2.8.0-rc2/build.gradle#L1472

project(':streams') {
...
  tasks.create(name: "copyDependantLibs", type: Copy) {
    from (configurations.testRuntime) {
...
  }
...
}

Or we now just brainstorm about Gradle behavior in-general ?

Thing is that this code was introduced back in the year 2015. (as a fresh/newly added code).
Also testRuntime configuration was added to an initial Gradle version: https://docs.gradle.org/1.0/userguide/artifact_dependencies_tutorial.html#configurations

I see that tasks.create(name: "copyDependantLibs", type: Copy) logic is applied to a dozen of submodules (15 in total, to be more precise). I didn't check, but it is safe to say that this is an prerequisite for releaseTarGz tasks (i.e. full distribution tasks).

I will try to compare other 14 (that work as expected) with this one (that fails).

[dejan2609]

@ijuma quick update: I did made some progress developing a quick and dirty fast solution for this... will update this ticket today or tomorrow.

[dejan2609]

@ijuma So, now I have something to show: I managed perform some reverse-engineering and received exactly the same content in /streams/build/dependent-libs-${version.scala} folder (that folder is being filled with dependencies by a gradle task streams:copyDependantLibs). Mental note: it would be interesting to fully discover this task background and usage.

Also: Gradle version is bumped (from 6.8.3 to 7.0).

Pitfall: ./gradlew clean streams:copyDependantLibs execution times are significantly higher when comparing to a trunk... will try to sort that out, if possible.

git branch	Gradle 6.8.3	Gradle 7.0
trunk	16 seconds	not applicable
this PR	1 minute 55 seconds	2 minutes 10 seconds

[ijuma]

@dejan2609 can you please rebase on trunk?

[dejan2609]

@ijuma branch is rebased onto trunk (I can also squash all commits into one).

[dejan2609]

@ijuma previous commits are rebased/squashed and PR branch is force-pushed.

Overall: Gradle build looks ok (on my loptop, that is). Some warnings do appear (related to spotless plugin / scala compilation) but I guess those things can be solved elsewhere (I volunteer to create JIRA tickets and see what can be done).

I opted to change commit message in order to emphasize Gradle version upgrade (if that is ok with you we can also change summaries for JIRA ticket and this PR).

One more note: Gradle patch 7.0.1 is around the corner (with lots of issues being solved: https://github.com/gradle/gradle/milestone/173).

[ijuma]

Can you remove the Gradle version bump from this PR then? We can merge the other change and see if there's any impact. We can then go straight to Gradle 7.0.1 once it's released. It may take 1-3 weeks from the link you shared.

[dejan2609]

Sure, let go step-by-step then.

[dejan2609]

Done, changes are trimmed down.

[ijuma]

LGTM. @vvcephei @guozhangwang @mjsax Any of you would like to review this too?

[ijuma]

Unrelated failures, merging to trunk.