Skip to content

KAFKA-18142 Switch to com.gradleup.shadow #18018

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.

Sign up for GitHub

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

Merged
mumrah merged 5 commits into from
Mar 12, 2025
Merged

KAFKA-18142 Switch to com.gradleup.shadow #18018

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c66e1ce
KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleu…
dejan2609 Dec 3, 2024
47f9240
Fix shadow configs
Goooler Dec 24, 2024
647c07a
KAFKA-18142 shadow plugin version bumped to 8.3.6
dejan2609 Feb 6, 2025
62a8c6e
solved issues with dependency tree for external projects with 'org.ap…
dejan2609 Feb 7, 2025
5b263eb
Merge branch 'trunk' into KAFKA-18142
mumrah Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ plugins {

id "com.github.spotbugs" version '6.0.25' apply false
id 'org.scoverage' version '8.0.3' apply false
id 'io.github.goooler.shadow' version '8.1.3' apply false
id 'com.gradleup.shadow' version '8.3.6' apply false
id 'com.diffplug.spotless' version "6.25.0"
}

Expand Down Expand Up @@ -364,17 +364,14 @@ subprojects {
if (!shouldPublishWithShadow) {
from components.java
} else {
apply plugin: 'io.github.goooler.shadow'
project.shadow.component(mavenJava)
apply plugin: 'com.gradleup.shadow'
from components.shadow

// Fix for avoiding inclusion of runtime dependencies marked as 'shadow' in MANIFEST Class-Path.
// https://github.com/johnrengelman/shadow/issues/324
// https://github.com/GradleUp/shadow/issues/324
afterEvaluate {
pom.withXml { xml ->
if (xml.asNode().get('dependencies') == null) {
xml.asNode().appendNode('dependencies')
}
def dependenciesNode = xml.asNode().get('dependencies').get(0)
def dependenciesNode = xml.asNode().get('dependencies') ?: xml.asNode().appendNode('dependencies')
project.configurations.shadowed.allDependencies.each {
def dependencyNode = dependenciesNode.appendNode('dependency')
dependencyNode.appendNode('groupId', it.group)
Expand Down Expand Up @@ -1824,6 +1821,10 @@ project(':clients') {
generator project(':generator')
}

tasks.withType(GenerateModuleMetadata) {
enabled = false
Copy link
Contributor

@apoorvmittal10 apoorvmittal10 Feb 24, 2025

Choose a reason for hiding this comment

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

We should be publishing the module metadata, any reason to skip it?

Copy link
Contributor Author

@dejan2609 dejan2609 Feb 26, 2025

Choose a reason for hiding this comment

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

Sure, will update this asap.

Copy link
Contributor Author

@dejan2609 dejan2609 Mar 3, 2025

Choose a reason for hiding this comment

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

@apoorvmittal10
It started with this comment made by @poom-kitti here: #18018 (comment)

Let me digest what is going on here just by using kafka-clients as an external projects dependency.

Scenario:

  • create a new project someNewProject and use kafka-clients as dependency
  • from kafka project: create three kafka-clients versions (using three git commits, see below) and publish them into local maven repository
  • execute ./gradlew dependencies against someNewProject (while rotating kafka-clients versions created in a previous step)

Results for someNewProject ./gradlew dependencies command (and three different kafka-clients versions):

  1. Kafka trunk (with Gradle module metadata enabled by a default):
runtimeClasspath - Runtime classpath of source set 'main'.
\--- org.apache.kafka:kafka-clients:4.1.0-SNAPSHOT-test-no-gradle-module
     +--- com.github.luben:zstd-jni:1.5.6-6
     +--- org.lz4:lz4-java:1.8.0
     +--- org.xerial.snappy:snappy-java:1.1.10.5
     \--- org.slf4j:slf4j-api:1.7.36
  1. this PR HEAD~1 commit (with a new shadow plugin AND with Gradle module metadata enabled by default):
runtimeClasspath - Runtime classpath of source set 'main'.
\--- org.apache.kafka:kafka-clients:4.1.0-SNAPSHOT-test
  1. this PR HEAD comit (with new shadow plugin AND with Gradle module metadata explicitly disabled):
runtimeClasspath - Runtime classpath of source set 'main'.
\--- org.apache.kafka:kafka-clients:4.1.0-SNAPSHOT-test-no-gradle-module
     +--- com.github.luben:zstd-jni:1.5.6-6
     +--- org.lz4:lz4-java:1.8.0
     +--- org.xerial.snappy:snappy-java:1.1.10.5
     \--- org.slf4j:slf4j-api:1.7.36

(see my comment #18018 (comment) for more details about testing)

Copy link
Contributor Author

@dejan2609 dejan2609 Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Just to sum it up: @poom-kitti (and mine) intention is to have exact behavior for kafka-clients dependency (in external projects, that is) and hence I opted to honor his findings and to disable gradle module metadata publication for project(':clients') (i.e. for kafka-clients artifacts).

All in all: we ended up with a situation where we are trying to have all these three things at once:

  • switch shadow plugin (i.e. use GradleUp/shadow plugin)
  • have exactly the same dependency tree in external projects (with kafka-clients dependency)
  • publish gradle module metadata for project(':clients (kafka-clients artifacts)

I dare to assume that this could be addressed on https://github.com/GradleUp/shadow side, maybe @Goooler can provide at least some insights.

Copy link
Contributor

@apoorvmittal10 apoorvmittal10 Mar 3, 2025

Choose a reason for hiding this comment

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

Yeah, I agree it makes sense to do things step by step. This PR is an improvement over our current shadow plugin and fix for releasing signed libs.
We should merge it and address the module metadata concern defined here separately.

Copy link
Contributor Author

@dejan2609 dejan2609 Mar 3, 2025

Choose a reason for hiding this comment

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

Git commits are rebased onto trunk; will leave this conversation opened (just in case that someone wants to do an additional review).

Copy link
Member

@mumrah mumrah Mar 12, 2025

Choose a reason for hiding this comment

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

Do we have a KAFKA ticket for re-enabling the Gradle module metadata?

}

task createVersionFile() {
def receiptFile = file("${layout.buildDirectory.get().asFile.path}/kafka/$buildVersionFileName")
inputs.property "commitId", commitId
Expand Down Expand Up @@ -1858,7 +1859,7 @@ project(':clients') {
// dependencies excluded from the final jar, since they are declared as runtime dependencies
dependencies {
project.configurations.shadowed.allDependencies.each {
exclude(dependency(it.group + ':' + it.name))
exclude(dependency(it))
}
// exclude proto files from the jar
exclude "**/opentelemetry/proto/**/*.proto"
Expand Down Expand Up @@ -3217,7 +3218,7 @@ project(':streams:upgrade-system-tests-39') {

project(':jmh-benchmarks') {

apply plugin: 'io.github.goooler.shadow'
apply plugin: 'com.gradleup.shadow'

shadowJar {
archiveBaseName = 'kafka-jmh-benchmarks'
Expand Down