KAFKA-17053: Restructure build.gradle to configure publishing last #16950
Conversation
KTKTK-HZ
changed the title
|
Hey @chia7712 @KTKTK-HZ, this PR contents does not match the ticket or the PR title. By restructuring the build.gradle, I meant that the shadowJar block should be moved above the publishing block. That is not happening in this PR, only changing the fork is. Please update the PR to match the ticket, and/or create a new ticket for upgrading/changing the fork. I did validate the published artifact and observed that the classifier is correct (i.e. not present). This is because the 8.3.0 release does not have the Goooler/shadow#80 patch that caused us to revert versions most recently, so this PR should not cause a regression. |
@KTKTK-HZ WDYT? could you fix it in this PR? |
|
Hey @gharris1727 @chia7712 , I will modify this PR and refactor build.gradle to move the shadowJar block before the publishing block. |
@KTKTK-HZ Thanks a bunch! BTW, could you please remove the "[Minor]"? The value of patch you will contribute is NOT MINOR I feel 😄 |
KTKTK-HZ
changed the title
|
Hey @gharris1727 @chia7712 , I have modified this pr, is there anything else that needs to be modified? thank you for your review. |
build.gradle
Outdated
| @@ -320,6 +320,62 @@ subprojects { | |||
|
|
|||
| if (shouldPublish) { | |||
|
|
|||
| if (shouldPublishWithShadow) { | |||
There was a problem hiding this comment.
This is part of the "subprojects" block, which applies to all subprojects, not just clients. Since the "shadowJar" is specific to the clients, I think it should stay where it is.
The "project.shadow.component(mavenJava)" stanza (which is also clients specific) should be moved down to the clients-specific block. Schematically:
subprojects {
if (clients) {
// clients jar + publishing
} else {
// normal publishing
}
}
clients {
// nothing
}
should be
subprojects {
if (!clients) {
// normal publishing
}
}
clients {
// clients jar + publishing
}
An alternative could be a second subprojects block and moving all of the publishing down there, but that also seems kinda confusing.
There was a problem hiding this comment.
Thanks for the suggestion, I've moved the clients-specific part to its corresponding block
build.gradle
Outdated
| @@ -1674,7 +1730,6 @@ project(':clients') { | |||
|
|
|||
| configurations { | |||
| generator | |||
| shadowed | |||
There was a problem hiding this comment.
I don't really know what this does. Can you explain?
There was a problem hiding this comment.
This is because moving shadowJar into the subprojects block required moving the dependencies forward, which is no longer needed in the new PR.
|
Hi @gharris1727 ,The new PR has been submitted, have you had time to review the new PR? Thanks. |
|
@KTKTK-HZ could you please fix the conflicts? |
|
Hi @chia7712 .I have resolved these conflicts, thanks for your reminder. |
| @@ -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 'io.github.goooler.shadow' version '8.1.8' apply false | |||
There was a problem hiding this comment.
Could you try the successor (https://github.com/GradleUp/shadow)? It may fix the issue like https://issues.apache.org/jira/browse/KAFKA-17052 and https://issues.apache.org/jira/browse/KAFKA-16359
There was a problem hiding this comment.
@chia7712 Yeah, I tried to upgrade Goooler/shadow to GradleUp/shadow, and after the upgrade, the KAFKA-17052 problem did not occur.But as @gharris1727 mentioned, This is because the GradleUp/shadow 8.3.0 release does not have the Goooler/shadow#80 patch that caused us to revert versions.Also, unfortunately, GradleUp/shadow does not fix the issue that caused KAFKA-16359.However, since Goooler/shadow is retired now.so I will create a new ticket for changing the fork.
|
GradleUp/shadow#945 is fixed already, so I guess we don't need this workaround now? |
|
@chia7712 @KTKTK-HZ We no longer need to "restructure to configure publishing last, but we do need to remove the ShadowExtension.component call: GradleUp/shadow#956 |
|
I'll close this PR later first and then re-raise a ticket to upgrade the shadow plugin and deal with GradleUp/shadow#956 |
Goooler/shadow 8.1.4-8.1.8 will produces incorrect results when publishing is configured before the actual archive is configured. so we Restructure build.gradle to configure publishing last to upgrade latest version of ShadowJavaPlugin. This can be seen in detail in GradleUp/shadow#945
BTW, because Goooler/shadow is retired now，so So we may need to upgrade to the latest version of GradleUp/shadow in order to keep track of subsequent updates to the shadow plugin. I will create a new ticket for changing the fork.
Committer Checklist (excluded from commit message)