Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
showuon merged PR #15532: URL: https://github.com/apache/kafka/pull/15532 -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
apoorvmittal10 commented on code in PR #15532:
URL: https://github.com/apache/kafka/pull/15532#discussion_r1550228079
##
build.gradle:
##
@@ -1435,10 +1454,10 @@ project(':clients') {
implementation libs.opentelemetryProto
// libraries which should be added as runtime dependencies in generated
pom.xml should be defined here:
-shadow libs.zstd
-shadow libs.lz4
-shadow libs.snappy
-shadow libs.slf4jApi
+shadowed libs.zstd
+shadowed libs.lz4
+shadowed libs.snappy
+shadowed libs.slf4jApi
Review Comment:
Hi @mimaison @showuon I have made the suggested change, just added version
as well with the group and name. I have verified the pom, jar and manifest file
they are correct.
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
showuon commented on code in PR #15532:
URL: https://github.com/apache/kafka/pull/15532#discussion_r1549312174
##
build.gradle:
##
@@ -1435,10 +1454,10 @@ project(':clients') {
implementation libs.opentelemetryProto
// libraries which should be added as runtime dependencies in generated
pom.xml should be defined here:
-shadow libs.zstd
-shadow libs.lz4
-shadow libs.snappy
-shadow libs.slf4jApi
+shadowed libs.zstd
+shadowed libs.lz4
+shadowed libs.snappy
+shadowed libs.slf4jApi
Review Comment:
@apoorvmittal10 , is there any update about the suggestion from @mimaison ?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
apoorvmittal10 commented on code in PR #15532:
URL: https://github.com/apache/kafka/pull/15532#discussion_r1542173327
##
build.gradle:
##
@@ -1435,10 +1454,10 @@ project(':clients') {
implementation libs.opentelemetryProto
// libraries which should be added as runtime dependencies in generated
pom.xml should be defined here:
-shadow libs.zstd
-shadow libs.lz4
-shadow libs.snappy
-shadow libs.slf4jApi
+shadowed libs.zstd
+shadowed libs.lz4
+shadowed libs.snappy
+shadowed libs.slf4jApi
Review Comment:
Thanks for the suggestion, I ll give it a try 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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
mimaison commented on code in PR #15532:
URL: https://github.com/apache/kafka/pull/15532#discussion_r1541600741
##
build.gradle:
##
@@ -1435,10 +1454,10 @@ project(':clients') {
implementation libs.opentelemetryProto
// libraries which should be added as runtime dependencies in generated
pom.xml should be defined here:
-shadow libs.zstd
-shadow libs.lz4
-shadow libs.snappy
-shadow libs.slf4jApi
+shadowed libs.zstd
+shadowed libs.lz4
+shadowed libs.snappy
+shadowed libs.slf4jApi
Review Comment:
I wonder if below in `shadowJar`, we should do something like:
```
// 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 proto files from the jar
exclude "**/opentelemetry/proto/**/*.proto"
exclude "**/google/protobuf/*.proto"
}
```
Instead of what we currently have where we again list these dependencies:
```
exclude(dependency(libs.snappy))
exclude(dependency(libs.zstd))
exclude(dependency(libs.lz4))
exclude(dependency(libs.slf4jApi))
```
This would avoid having these diverge in the future. WDYT?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
apoorvmittal10 commented on PR #15532: URL: https://github.com/apache/kafka/pull/15532#issuecomment-2010700271 @ijuma Build passes with unrelated test failures. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
apoorvmittal10 commented on PR #15532: URL: https://github.com/apache/kafka/pull/15532#issuecomment-1999421797 Re-triggering the build by re-opening the 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
apoorvmittal10 closed pull request #15532: KAFKA-16359: Corrected manifest file for kafka-clients URL: https://github.com/apache/kafka/pull/15532 -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] KAFKA-16359: Corrected manifest file for kafka-clients [kafka]
apoorvmittal10 opened a new pull request, #15532: URL: https://github.com/apache/kafka/pull/15532 The issue [KAFKA-16359](https://issues.apache.org/jira/browse/KAFKA-16359) reported inclusion of `kafka-clients` runtime dependencies in MANIFEST.MF `Class-Path`. The root cause is the issue defined [here](https://github.com/johnrengelman/shadow/issues/324) with the usage of `shadow` plugin. Looking into the specifics of plugin and [documentation](https://imperceptiblethoughts.com/shadow/configuration/#configuring-the-runtime-classpath), specifies that any dependency marked as `shadow` will be treated as following by the shadow plugin: 1. Adds the dependency as runtime dependency in resultant pom.xml - [code here](https://github.com/johnrengelman/shadow/blob/1d647471b9130d35b17771a50be382f6cb442d71/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowExtension.groovy#L32) 2. Adds the dependency as `Class-Path` in `MANIFEST.MF` as well - [code here](https://github.com/johnrengelman/shadow/blob/1d647471b9130d35b17771a50be382f6cb442d71/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowJavaPlugin.groovy#L71) ### Resolution We do need the runtime dependencies available in the pom (1 above) but not on manifest (2 above). Also there is no clear way to separate the behaviour as both above tasks relies on `shadow` configuration. To fix, I have defined another custom configuration named `shadowed` which is later used to populate the correct pom (the [code is similar to what shadow plugin does](https://github.com/johnrengelman/shadow/blob/1d647471b9130d35b17771a50be382f6cb442d71/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowExtension.groovy#L32) to populate pom for runtime dependencies). Though this might seem like a workaround, but I think that's the only way to fix the issue. I have checked other SDKs which suffered with same issue and went with similar route to populate pom. ### Verification 1. Exploded jar by `jar -xvf kafka-clientsjar` and compared pre and post manifest contents.  2. Verified pre and post resultant pom.  3. Verified pre and post jar tf output `jar -tf kafka-clients-3.8.0-SNAPSHOT.jar > jar_tf_output`  ### 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
