You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dejan2609 (via GitHub)" <gi...@apache.org> on 2024/12/03 13:29:44 UTC
[PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
dejan2609 opened a new pull request, #18018:
URL: https://github.com/apache/kafka/pull/18018
**Prologue (related PR's):**
- https://github.com/apache/kafka/pull/16295
- https://github.com/apache/kafka/pull/17218
- https://github.com/apache/kafka/pull/16489
**Action points:**
- switch shadow plugin from `io.github.goooler.shadow` to `com.gradleup.shadow`
- upgrade plugin version from **8.1.3** to **8.3.5** (release notes: https://gradleup.com/shadow/changes/#v8-3-5-2024-11-03)
**Rationale:** both previous one and currently used shadow plugins are now deprecated (in favor of `com.gradleup.shadow`):
- `com.github.johnrengelman.shadow` maintenance was transferred to `com.gradleup.shadow`: https://github.com/GradleUp/shadow/tree/8.3.5?tab=readme-ov-file#gradle-shadow
- `io.github.goooler.shadow`: changes are ported to `com.gradleup.shadow`: https://github.com/Goooler/shadow?tab=readme-ov-file#gradle-shadow
**Pitfall (to keep in mind):**
- https://github.com/apache/kafka/pull/15532
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2561291671
@apoorvmittal10 feel free to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1971506613
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
Sure, will update this asap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2551056526
Hmmm, so I re-verified and seems [this](https://github.com/johnrengelman/shadow/issues/324) issue is still open, hence we might need to keep the [workaround](https://github.com/apache/kafka/pull/15532). I verified with 8.3.5 version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977114543
##########
build.gradle:
##########
@@ -353,17 +353,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 {
Review Comment:
@Goooler I have this on my mind and will (probably) address in a separate PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2693906211
@dejan2609 Can you please merge the upstream changes and re-run the build to see if tests pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2551044386
@apoorvmittal10 let me check this, will update you here asap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2514604618
Update: I just double-checked one of the PR's mentioned above and there is a chance that issue (incorrect classifier for the published archive) is still occurs:
- https://github.com/GradleUp/shadow/pull/904 Explicitly add classifier to maven publication
- https://github.com/apache/kafka/pull/16489
Will convert this to PR a draft and have a second look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2640147208
@poom-kitti thank you for investigation, much appreciated. I just checked your scenario and was able to recreate same results.
Will return to this next week for sure !
@apoorvmittal10 moving this PR back to a draft.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2645884106
@apoorvmittal10, @Goooler and @poom-kitti: feel free to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2515014757
Update 2: there is a JIRA ticket (and corresponding PR) that already covers this subject: https://issues.apache.org/jira/browse/KAFKA-17053
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2561291264
Kudos for @Goooler for jumping in and saving a day :medal_sports: :+1:
It seems that PR is good to go now, herewith some comparison (trunk vs. this PR):
1. **pom.xml files are equivalent (PR version just adds some comments):**
```
dejan@dejan-HP-ProBook-450-G7:~$ diff ~/.m2/repository/org/apache/kafka-8.3.5/kafka-clients/4.1.0-SNAPSHOT/kafka-clients-4.1.0-SNAPSHOT.pom ~/.m2/repository/org/apache/kafka-trunk/kafka-clients/4.1.0-SNAPSHOT/kafka-clients-4.1.0-SNAPSHOT.pom
3,7d2
< <!-- This module was also published with a richer model, Gradle metadata, -->
< <!-- which should be used instead. Do not delete the following line which -->
< <!-- is to indicate to Gradle or any Gradle module metadata file consumer -->
< <!-- that they should prefer consuming it instead. -->
< <!-- do_not_remove: published-with-gradle-metadata -->
dejan@dejan-HP-ProBook-450-G7:~$
```
2. **MANIFEST.MF files are identical:**
```
dejan@dejan-HP-ProBook-450-G7:~$ diff ~/.m2/repository/org/apache/kafka-trunk/kafka-clients/4.1.0-SNAPSHOT/kafka-clients-4.1.0-SNAPSHOT.jar.folder/META-INF/MANIFEST.MF ~/.m2/repository/org/apache/kafka-8.3.5/kafka-clients/4.1.0-SNAPSHOT/kafka-clients-4.1.0-SNAPSHOT.jar.folder/META-INF/MANIFEST.MF
dejan@dejan-HP-ProBook-450-G7:~$ date
Tue 24 Dec 17:32:48 CET 2024
```
3. **there is a one difference when it comes to publications; I assume this is an expected behavior (and by the looks of it it won't hurt in any way):**
- _trunk publications:_
```
-rw-rw-r-- 1 dejan dejan 9.2M Dec 24 16:57 kafka-clients-4.1.0-SNAPSHOT.jar
-rw-rw-r-- 1 dejan dejan 2.2M Dec 24 16:57 kafka-clients-4.1.0-SNAPSHOT-javadoc.jar
-rw-rw-r-- 1 dejan dejan 1.4K Dec 24 16:57 kafka-clients-4.1.0-SNAPSHOT.pom
-rw-rw-r-- 1 dejan dejan 3.7M Dec 24 16:57 kafka-clients-4.1.0-SNAPSHOT-sources.jar
-rw-rw-r-- 1 dejan dejan 2.6M Dec 24 16:57 kafka-clients-4.1.0-SNAPSHOT-test.jar
-rw-rw-r-- 1 dejan dejan 1.3M Dec 24 16:57 kafka-clients-4.1.0-SNAPSHOT-test-sources.jar
-rw-rw-r-- 1 dejan dejan 1.6K Dec 24 16:57 maven-metadata-local.xml
```
- _this PR is publishing one additional file (kafka-clients-4.1.0-SNAPSHOT.module):_
```
-rw-rw-r-- 1 dejan dejan 9.2M Dec 24 17:05 kafka-clients-4.1.0-SNAPSHOT.jar
-rw-rw-r-- 1 dejan dejan 2.2M Dec 24 17:05 kafka-clients-4.1.0-SNAPSHOT-javadoc.jar
-rw-rw-r-- 1 dejan dejan 1.1K Dec 24 17:05 kafka-clients-4.1.0-SNAPSHOT.module
-rw-rw-r-- 1 dejan dejan 1.8K Dec 24 17:05 kafka-clients-4.1.0-SNAPSHOT.pom
-rw-rw-r-- 1 dejan dejan 3.7M Dec 24 17:05 kafka-clients-4.1.0-SNAPSHOT-sources.jar
-rw-rw-r-- 1 dejan dejan 2.6M Dec 24 17:05 kafka-clients-4.1.0-SNAPSHOT-test.jar
-rw-rw-r-- 1 dejan dejan 1.3M Dec 24 17:05 kafka-clients-4.1.0-SNAPSHOT-test-sources.jar
```
- _kafka-clients-4.1.0-SNAPSHOT.module content:_
```
dejan@dejan-HP-ProBook-450-G7:~/.m2/repository/org/apache/kafka-8.3.5/kafka-clients/4.1.0-SNAPSHOT$ cat kafka-clients-4.1.0-SNAPSHOT.module
{
"formatVersion": "1.1",
"component": {
"group": "org.apache.kafka",
"module": "kafka-clients",
"version": "4.1.0-SNAPSHOT",
"attributes": {
"org.gradle.status": "integration"
}
},
"createdBy": {
"gradle": {
"version": "8.10.2"
}
},
"variants": [
{
"name": "shadowRuntimeElements",
"attributes": {
"org.gradle.category": "library",
"org.gradle.dependency.bundling": "shadowed",
"org.gradle.libraryelements": "jar",
"org.gradle.usage": "java-runtime"
},
"files": [
{
"name": "kafka-clients-4.1.0-SNAPSHOT.jar",
"url": "kafka-clients-4.1.0-SNAPSHOT.jar",
"size": 9607290,
"sha512": "1b2165fe9a93bbc437f61b4144c406e2677d33a62ce1c20be75bf90fca4ca1364e597365301bd6a0cf74fa58fe454bf09a0cef81727fb2d3a253926968874a15",
"sha256": "d76560f10d577b554c1aca1a98bfc72144c8fafd90cb787e6c0bb819af254918",
"sha1": "20efda5807de75530682f567ef8622cd23ca283f",
"md5": "0661f254d99c92749d06c464170cfc16"
}
]
}
]
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2639043829
> @dejan2609 Hello, could you please help check whether if you create a new Gradle project and declaring the published snapshot version of Kafka clients in Maven Local as dependencies, would Gradle knows about the runtime dependencies (i.e., zstd, lz4, snappy, slf4jApi)?
@poom-kitti could you please open a separate issue with more details (and explain your rationale and/or your corner case) ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 Switch to `com.gradleup.shadow` [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah merged PR #18018:
URL: https://github.com/apache/kafka/pull/18018
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977235728
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
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](https://docs.gradle.org/8.10.2/userguide/publishing_gradle_module_metadata.html#sub:disabling-gmm-publication)
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. GradleUp shadow plugin)
- have exactly the same dependency tree in external projects (with `kafka-clients` dependency)
- publish gradle module metadata
I dare to assume that this could be addressed on https://github.com/GradleUp/shadow side, maybe @Goooler can provide at least some insights.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2700506846
@mumrah Can you please review it as well and help merging then changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977801099
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
Git commits are rebased onto trunk; will leave this conversation opened (just in case that someone wants to do an additional review).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "Goooler (via GitHub)" <gi...@apache.org>.
Goooler commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2561236036
A fix for this migration has been addressed to https://github.com/dejan2609/kafka/pull/1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2558416954
I just created Gradle Shadow issue here https://github.com/GradleUp/shadow/issues/1115 (but chances are this is not a bug; hopefully we will get some guidelines how to upgrade to `com.gradleup.shadow` version **_above_** 8.3.1).
This PR is updated and now contains:
- switch from `io.github.goooler.shadow` to `com.gradleup.shadow` (version 8.3.1 that works with current Kafka's build setup).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2672100187
@poom-kitti interesting takes, I believe @Goooler is a person that can look into GradleUp/shadow subject.
@apoorvmittal10 just a reminder for you to review this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 Switch to `com.gradleup.shadow` [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1993571512
##########
build.gradle:
##########
@@ -353,17 +353,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 {
Review Comment:
Offshoot PR is created here: https://github.com/apache/kafka/pull/19200
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2565381681
Thanks @dejan2609, mostly looks good to me. Can you also please confirm about the signing of release jar, if that's working fine as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2684827328
@apoorvmittal10 will check this asap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977242641
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
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](https://github.com/apache/kafka/pull/18018#pullrequestreview-2638667585) separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2675579957
@dejan2609 Sorry for the delay, let's aim to close on it next week. We encountered a publication issue with `io.github.goooler.shadow' version '8.1.3'`, I ll verify if current PR fixes publication issue permanently for Kafka releases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2550982501
Hi @dejan2609, thanks for the PR. I would prefer to get this [workaround avoided](https://github.com/apache/kafka/pull/15532) as it seems to be fixed as per comment [here](https://github.com/GradleUp/shadow/issues/901#issuecomment-2326151381). Is it possible for you to take it up with this PR or let me know I can drive the change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "Goooler (via GitHub)" <gi...@apache.org>.
Goooler commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1897024859
##########
build.gradle:
##########
@@ -353,17 +353,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')
Review Comment:
The real change is here, picked it from https://github.com/GradleUp/shadow/blob/19ed88908a8390d6ca9ada50f4067f653bbae6b6/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowExtension.groovy#L39-L50.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "Goooler (via GitHub)" <gi...@apache.org>.
Goooler commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1897025851
##########
build.gradle:
##########
@@ -353,17 +353,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 {
Review Comment:
And I guess this `afterEvaluate` is redundant, could be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2514580990
@chia7712 feel free to review.
Note: Github Actions bot classified this change as small and it's kind of true (if we only count LOC) and hence CI checks are not triggered, I assume.
Can you please force it, somehow ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "poom-kitti (via GitHub)" <gi...@apache.org>.
poom-kitti commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2637629283
@dejan2609 Hello, could you please help check whether if you create a new Gradle project and declaring the published snapshot version of Kafka clients as dependencies, would Gradle knows about the runtime dependencies (i.e., zstd, lz4, snappy, slf4jApi)?
E.g.,
```
repositories {
mavenLocal()
}
dependencies {
implementation 'org.apache.kafka:kafka-clients:4.1.0-SNAPSHOT'
}
```
If I understand correctly, Gradle will check the module metadata `kafka-clients-4.1.0-SNAPSHOT.module` instead of POM file, and since there is no [`dependencies`](https://github.com/gradle/gradle/blob/v8.8.0/platforms/documentation/docs/src/docs/design/gradle-module-metadata-latest-specification.md#dependencies-value), it would not know about the runtime dependencies and will not be included.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2639047102
@apoorvmittal10 let me know do you prefer to squash commits on your side (using Github) or I have to do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "poom-kitti (via GitHub)" <gi...@apache.org>.
poom-kitti commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2639311771
@dejan2609 I would like to think this is directly related to your MR because the runtime dependencies are not found, but just to be sure, I will explain how I tested it out.
## Testing out runtime dependencies
So these are the steps I took after forking your repo
1. Build to Maven local and specify version as `4.1.0-SNAPSHOT-test`
```sh
gradle :clients:publishToMavenLocal -PskipSigning=true
```
2. Ensure the files are created like you have posted before
```
❯ ls -l ~/.m2/repository/org/apache/kafka/kafka-clients/4.1.0-SNAPSHOT-test
-rw-r--r-- 1 pkittisrisaw staff 2675485 Feb 6 16:11 kafka-clients-4.1.0-SNAPSHOT-test-javadoc.jar
-rw-r--r-- 1 pkittisrisaw staff 3777356 Feb 6 16:11 kafka-clients-4.1.0-SNAPSHOT-test-sources.jar
-rw-r--r-- 1 pkittisrisaw staff 1344312 Feb 6 16:11 kafka-clients-4.1.0-SNAPSHOT-test-test-sources.jar
-rw-r--r-- 1 pkittisrisaw staff 2651152 Feb 6 16:11 kafka-clients-4.1.0-SNAPSHOT-test-test.jar
-rw-r--r-- 1 pkittisrisaw staff 9605170 Feb 6 16:11 kafka-clients-4.1.0-SNAPSHOT-test.jar
-rw-r--r--@ 1 pkittisrisaw staff 1116 Feb 6 16:11 kafka-clients-4.1.0-SNAPSHOT-test.module
-rw-r--r-- 1 pkittisrisaw staff 1785 Feb 6 16:11 kafka-clients-4.1.0-SNAPSHOT-test.pom
```
This is the content of the `kafka-clients-4.1.0-SNAPSHOT-test.module`
```
❯ cat ~/.m2/repository/org/apache/kafka/kafka-clients/4.1.0-SNAPSHOT-test/kafka-clients-4.1.0-SNAPSHOT-test.module
{
"formatVersion": "1.1",
"component": {
"group": "org.apache.kafka",
"module": "kafka-clients",
"version": "4.1.0-SNAPSHOT-test",
"attributes": {
"org.gradle.status": "release"
}
},
"createdBy": {
"gradle": {
"version": "8.10.2"
}
},
"variants": [
{
"name": "shadowRuntimeElements",
"attributes": {
"org.gradle.category": "library",
"org.gradle.dependency.bundling": "shadowed",
"org.gradle.libraryelements": "jar",
"org.gradle.usage": "java-runtime"
},
"files": [
{
"name": "kafka-clients-4.1.0-SNAPSHOT-test.jar",
"url": "kafka-clients-4.1.0-SNAPSHOT-test.jar",
"size": 9605170,
"sha512": "92212b8f530381d7f39edcef5cc1c8e8c92ef8e166bb52af5b0148ebe56233ae3aba9f3b5a24cd1ac893bdcd33331764bf691813e95de387a98821910cbf5a73",
"sha256": "d7687bb2ad6f7231c4c792bace955d4a7b0420e8d78cf4176b5471b2568636b6",
"sha1": "34567fc759dd2142067bd3f9d535ef554127534c",
"md5": "33efa7f6713026cad004bfada772a7ac"
}
]
}
]
}
```
3. Create a brand new Gradle Java project (I just use template from Intellij) where my Gradle version is 8.10 (irrelevant). This is the content of my `build.gradle.kts`
```
plugins {
id("java")
}
group = "com.example"
version = "1.0-SNAPSHOT"
repositories {
mavenCentral()
mavenLocal()
}
dependencies {
implementation("org.apache.kafka:kafka-clients:4.1.0-SNAPSHOT-test")
testImplementation(platform("org.junit:junit-bom:5.10.0"))
testImplementation("org.junit.jupiter:junit-jupiter")
}
tasks.test {
useJUnitPlatform()
}
```
4. We can check dependencies by running `gradle dependencies` in the `runtimeClasspath` section I cannot see the runtime dependencies needed
```
runtimeClasspath - Runtime classpath of source set 'main'.
\--- org.apache.kafka:kafka-clients:4.1.0-SNAPSHOT-test
```
## Testing removing Gradle module metadata
1. I have follow the [Gradle guide](https://docs.gradle.org/8.10.2/userguide/publishing_gradle_module_metadata.html#sub:disabling-gmm-publication) and disable by using
```
tasks.withType(GenerateModuleMetadata) {
enabled = false
}
```
2. I build to Maven Local again with version `4.1.0-SNAPSHOT-test-no-gradle-module`. The content are shown below, please notice we do not have `.module` file anymore
```
❯ ls -l ~/.m2/repository/org/apache/kafka/kafka-clients/4.1.0-SNAPSHOT-test-no-gradle-module
-rw-r--r-- 1 pkittisrisaw staff 2682848 Feb 6 16:15 kafka-clients-4.1.0-SNAPSHOT-test-no-gradle-module-javadoc.jar
-rw-r--r-- 1 pkittisrisaw staff 3777356 Feb 6 16:15 kafka-clients-4.1.0-SNAPSHOT-test-no-gradle-module-sources.jar
-rw-r--r-- 1 pkittisrisaw staff 1344312 Feb 6 16:15 kafka-clients-4.1.0-SNAPSHOT-test-no-gradle-module-test-sources.jar
-rw-r--r-- 1 pkittisrisaw staff 2651152 Feb 6 16:15 kafka-clients-4.1.0-SNAPSHOT-test-no-gradle-module-test.jar
-rw-r--r-- 1 pkittisrisaw staff 9605187 Feb 6 16:15 kafka-clients-4.1.0-SNAPSHOT-test-no-gradle-module.jar
-rw-r--r-- 1 pkittisrisaw staff 1445 Feb 6 16:15 kafka-clients-4.1.0-SNAPSHOT-test-no-gradle-module.pom
```
Also there is no longer the following comment in the POM file
```
<!-- This module was also published with a richer model, Gradle metadata, -->
<!-- which should be used instead. Do not delete the following line which -->
<!-- is to indicate to Gradle or any Gradle module metadata file consumer -->
<!-- that they should prefer consuming it instead. -->
<!-- do_not_remove: published-with-gradle-metadata -->
```
3. Change to my created Gradle project then modify the dependency to
```
implementation("org.apache.kafka:kafka-clients:4.1.0-SNAPSHOT-test-no-gradle-module")
```
4. This is the content of `runtimeClasspath` after running `gradle dependencies`
```
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
```
## Conclusion
Due to the module metadata existing, I would guess Gradle will check that instead of POM, so the runtime dependencies are not discoverable as they are not declared there.
Please correct me if I am mistaken anywhere though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2694058225
> @dejan2609 Can you please merge the upstream changes and re-run the build to see if tests pass.
@apoorvmittal10 Done, PR is rebased onto trunk. I opted to keep all commits separated just for the sake of clarity if someone else wants to do a review (but will easily squash them into one when the time comes).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2555665196
> Hmmm, so I re-verified and seems [this](https://github.com/johnrengelman/shadow/issues/324) issue is still open, hence we might need to keep the [workaround](https://github.com/apache/kafka/pull/15532). I verified with 8.3.5 version.
Can you please doublecheck (and/or share with me Gradle build that you used) ? It seems that Github Actions labeled this as a trivial/minor change and hence no checks have been executed.
I discovered that publishing fails for `com.gradleup.shadow` version above 8.3.1 (I used this command: `./gradlew clean releaseTarGz publishToMavenLocal`). And I may have a root cause, also (plugin changed some things in version 8.3.2).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "Goooler (via GitHub)" <gi...@apache.org>.
Goooler commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1942276633
##########
build.gradle:
##########
@@ -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.5' apply false
Review Comment:
```suggestion
id 'com.gradleup.shadow' version '8.3.6' apply false
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2633227002
> Thanks @dejan2609, mostly looks good to me. Can you also please confirm about the signing of release jar, if that's working fine as well?
@apoorvmittal10 this one almost slipped through the cracks, but I finally rembembered about.
I did some checking and signing works fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "poom-kitti (via GitHub)" <gi...@apache.org>.
poom-kitti commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2660844921
@dejan2609 Hello, I would include some observations I found, but unsure it would be best practice.
It seems to be possible that we can use `shadow` declarable configuration that the [workaround](https://github.com/apache/kafka/pull/15532) tries to solve by creating our own ShadowJar task. A custom ShadowJar task does not add `Class-Path` entry to `MANIFEST.MF`. But we would need to make `shadow` software component uses our custom ShadowJar instead when publishing. I have raised and shown an example here https://github.com/GradleUp/shadow/issues/1252.
In our context, if we do go with custom ShadowJar approach, we can remove following:
- No longer needed to loop through removing dependencies that are currently declared as `shadowed` in ShadowJar task as we will declare them as `shadow` instead
- No longer needed to prevent generation of Gradle module metadata nor modify POM file generation ourselves as `shadow` software component already configured the metadata correctly to include dependencies declared as `shadow` to be runtime scope
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977235728
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
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](https://docs.gradle.org/8.10.2/userguide/publishing_gradle_module_metadata.html#sub:disabling-gmm-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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977235728
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
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](https://docs.gradle.org/8.10.2/userguide/publishing_gradle_module_metadata.html#sub:disabling-gmm-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. us 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977235728
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
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](https://docs.gradle.org/8.10.2/userguide/publishing_gradle_module_metadata.html#sub:disabling-gmm-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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1968540211
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
We should be publishing the module metadata, any reason to skip it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2561315618
> @apoorvmittal10 feel free to review.
It's on my list, I ll do it next week.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2568232654
@apoorvmittal10 sure, let me check that during the weekend.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1897305837
##########
build.gradle:
##########
@@ -353,17 +353,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 {
Review Comment:
thanks @Goooler, will check this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2645879873
I just pushed another commit that solves issues described by @poom-kitti above.
Herewith my environment and some test I performed :arrow_down:
## Environment:
Java version:
```
dejan@dejan-HP-ProBook-450-G7:~/kafka$ java -version
openjdk version "17.0.14" 2025-01-21
OpenJDK Runtime Environment (build 17.0.14+7-Ubuntu-124.04)
OpenJDK 64-Bit Server VM (build 17.0.14+7-Ubuntu-124.04, mixed mode, sharing)
```
Related PR git commits (four of them) plus trunk baseline commit:
```
dejan@dejan-HP-ProBook-450-G7:~/kafka$ git log -5 --oneline
9a2043041d (HEAD -> KAFKA-18142, origin/KAFKA-18142) solved issues with dependency tree for external projects with 'org.apache.kafka:kafka-clients' dependency
1e37b4e77b KAFKA-18142 shadow plugin version bumped to 8.3.6
7ddc6a519b (KAFKA-18142-shadow-8.3.5) Fix shadow configs
e386d10229 KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow`
649cd6b42c MINOR Fix conditional in Update Test Catalog workflow (#18289)
dejan@dejan-HP-ProBook-450-G7:~/kafka$
```
## Testing:
### Create three `"kafka-clients"` versions:
1. create `kafka-clients` artifacts without PR changes/commits (i.e. use trunk baseline):
```
git checkout 649cd6b42c
change 'gradle.properties' version to `4.1.0.1-SNAPSHOT`
execute ./gradlew :clients:publishToMavenLocal -PskipSigning=true
```
2. create `kafka-clients` artifacts with shadow plugin upgrade related commits (note: these artifacts will reproduce error described by @poom-kitti)
```
git checkout 1e37b4e77b
change 'gradle.properties' version to `4.1.0.2-SNAPSHOT`
execute ./gradlew :clients:publishToMavenLocal -PskipSigning=true
```
3. create `kafka-clients` artifacts with a latest commit (that solves issues mentioned above, in step number 2.):
```
git checkout 9a2043041d
change 'gradle.properties' version to `4.1.0.3-SNAPSHOT`
execute ./gradlew :clients:publishToMavenLocal -PskipSigning=true
```
### Generate dependency tree for a simple project (that uses `kafka-clients` versions from above):
Use https://github.com/dejan2609/poc-kafka-18142.git in order to test dependency tree for project (and test all three kafka-clients versions created above).
Note: `txt` files with dependency trees are created using `./gradlew dependencies > deps-X.txt`
:bulb: _**You will find that these two files are equivalent:**_
- https://github.com/dejan2609/poc-kafka-18142/blob/trunk/deps-1.txt
- https://github.com/dejan2609/poc-kafka-18142/blob/trunk/deps-3.txt
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977235728
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
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](https://docs.gradle.org/8.10.2/userguide/publishing_gradle_module_metadata.html#sub:disabling-gmm-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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1977193479
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
@apoorvmittal10
It started with this comment made by @poom-kitti here: https://github.com/apache/kafka/pull/18018#issuecomment-2637629283
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
```
2. 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
```
3. 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 https://github.com/apache/kafka/pull/18018#issuecomment-2645879873 for more details about testing)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2706733670
Thank you for a merge @mumrah, it seems that tests are fine.
Could you spare some time to perform a review ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 switch Kafka's Gradle build shadow plugin to `com.gradleup.shadow` (and upgrade plugin version) [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18018:
URL: https://github.com/apache/kafka/pull/18018#discussion_r1991943081
##########
build.gradle:
##########
@@ -1916,6 +1913,10 @@ project(':clients') {
generator project(':generator')
}
+ tasks.withType(GenerateModuleMetadata) {
+ enabled = false
Review Comment:
Do we have a KAFKA ticket for re-enabling the Gradle module metadata?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18142 Switch to `com.gradleup.shadow` [kafka]
Posted by "dejan2609 (via GitHub)" <gi...@apache.org>.
dejan2609 commented on PR #18018:
URL: https://github.com/apache/kafka/pull/18018#issuecomment-2718833869
@mumrah related JIRA ticket is created here:
https://issues.apache.org/jira/browse/KAFKA-18970 **_[kafka-clients] Gradle module metadata publication should be enabled_**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org