Fix multi jar publishing #32195

alpar-t · 2018-07-19T13:19:07Z

Versions of Gradle before 4.9 don't support project that depend on other
projects which have multiple jar publications, see gradle/gradle#1061.

It just so happens that we upgraded master and 6.x to 4.9 recently, but
6.3 is still on Gradle 4.5 so the multi publications will break there.

This change removes the client jar publication, since now the artifactId
is the only one that differes, and adds the nebula configuration to the
plugin so we don't rely on it's presence in the build scipt to generate
a correct pom. Without the plugin applied, the generated dependencies
would be empty.

Relates #31946

Versions of Gradle before 4.9 don't support project that depend on other projects which have multiple jar publications, see gradle/gradle#1061. It just so happens that we upgraded master and 6.x to 4.9 recently, but 6.3 is still on Gradle 4.5 so the multi publications will break there. This change removes the client jar publication, since now the artifactId is the only one that differes, and adds the nebula configuration to the plugin so we don't rely on it's presence in the build scipt to generate a correct pom. Without the plugin applied, the generated dependencies would be empty.

elasticmachine · 2018-07-19T13:19:09Z

Pinging @elastic/es-core-infra

alpar-t · 2018-07-19T13:28:12Z

relates to #32180 #31946

nik9000

The generated POM looks good to me. I left some minor questions about the gradle changes.

nik9000 · 2018-07-19T13:35:15Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

@@ -69,16 +65,16 @@ public class PluginBuildPlugin extends BuildPlugin {
            String name = project.pluginProperties.extension.name
            project.archivesBaseName = name

+            // while the jar isn't normally published, we still at least build a pom of deps


I think we should mention x-pack-core here explicitly.

nik9000 · 2018-07-19T13:45:54Z

modules/transport-netty4/build.gradle

@@ -23,6 +23,7 @@
   * fix the hack in the build framework that copies transport-netty4 into the integ test cluster
   * maybe figure out a way to run all tests from core with netty4/network?
 */
+


Probably worth reverting this one.

Git annotate says these were added by @jasontedor, I'm happy to help with it.

I misread this to mean "revisiting" and taught it's about the TODO above ... will revert

nik9000 · 2018-07-19T14:10:50Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

+                    .parse(project.tasks.generatePomFileForNebulaPublication.outputs.files.singleFile)
+            pom.artifactId[0].value = project.name + "-client"
+            jarFile.resolveSibling(clientPomFileName).toFile().text = XmlUtil.serialize(pom)
+            // Groovy has an odd way of formatting the XML, fix it up3


nik9000 · 2018-07-19T14:13:53Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

    /** Adds a task to move jar and associated files to a "-client" name. */
    protected static void addClientJarTask(Project project) {
        Task clientJar = project.tasks.create('clientJar')
-        clientJar.dependsOn(project.jar, project.tasks.generatePomFileForClientJarPublication, project.javadocJar, project.sourcesJar)
+        clientJar.dependsOn(project.jar, project.tasks.generatePomFileForNebulaPublication, project.javadocJar, project.sourcesJar)
        clientJar.doFirst {
            Path jarFile = project.jar.outputs.files.singleFile.toPath()
            String clientFileName = jarFile.fileName.toString().replace(project.version, "client-${project.version}")


It'd be nice if this were declare an output. I know it didn't have it before though.

nik9000 · 2018-07-19T14:16:02Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

@@ -169,21 +165,42 @@ public class PluginBuildPlugin extends BuildPlugin {
        project.artifacts.add('zip', bundle)
    }

+    /** Find the reponame. */
+    static String urlFromOrigin(String origin) {


Is this still used?

yes, by a build script at distribution/archives/build.gradle we could probably have that use nebula and clean it up, but didn't want to get into that now.

This commit fixes the build for 6.3 after the backport of #32180. First, the meta plugin build no longer needed to call zip pom generation. Second, pom generation for the jar needs to only happen when not building a client jar, because gradle versions before 4.9 get confused with multiple versions of the same artifact (see #32195).

rjernst

I'm not sure we actually need this, given there is a workaround (only have client jar or normal jar+pom). Given we don't currently have a need for both, is there any reason not to just apply the same tweak (add back the else condition for configuring the jar pom)?

alpar-t · 2018-07-21T12:49:49Z

@rjernst I'm ok with moving jar generation so it doesn't happen at the same time with client jar.
If we don't it would make sense for me to depend on main jar and not client jar directly. Not sure how disruptive that would be. A client jar makes sense to me as a separate Gradle project that really only has the client part and is depended on from the server, but admittedly I am missing a lot of context here.
An argument for keeping it might be for plugin authors that build multiple inter dependent plugins, but that's probably an overstretch, as those users can easily do their own pom generation.

I do think we need to keep from this PR the part about applying nebula, as it is it's too easy to forget that and don't see a reason to rely on the build script to apply it rather than doing it in the plugin.
The pom without nebula base applied does not have dependencies, so I don't think it's useful in any way.
We could also do it without nebula (as done by client jar prior to this change) or include only base publishing from nebula and leave it up to the build script to decide if it wants scm info as well.

rjernst · 2018-07-24T20:37:46Z

the part about applying nebula, as it is it's too easy to forget that and don't see a reason to rely on the build script to apply it rather than doing it in the plugin

Publishing is a choice. Plugin authors could be publishing their plugins in many ways. This let's them choose if they want to use nebula. For all the stuff that build-tools pulls in for plugin development, all of it is optional (the tasks can be disabled). AFAIK, publishing is not something that can simply be disabled, because the tasks are generated late and not available when the plugin is applied.

alpar-t · 2018-07-24T21:24:31Z

We don't have to pull in Nebula, but we do already pull in Publishing, so I would expect that either we don't generate any pom, or generate a correct one out of the box. Without this change we will generate a pom that doesn't have dependencies. That doesn't feel right.

I very much like your statement: "publishing is a choice". I will check to see if we can avoid applying the publishing plugin. Maybe have the publishing just for the build scripts where we need it and avoid dealing with it in the plugin. Also in Gradle 4.9 with stable publishing ( which we enable ) the tasks might no longer be created that late.

rjernst · 2018-07-24T23:45:49Z

I suspect the fact we always pull in publishing and generate a pom is leftover from the very first iteration of gradle, when we were still always publishing plugin zips to maven. Since we don't do this anymore (and haven't since 5.0 beta), we should change this to only create a pom if configured.

alpar-t · 2018-07-25T05:52:58Z

Closing in favor of #32351

alpar-t added 2 commits July 19, 2018 15:55

Keep method, used elsewhere

b416c99

alpar-t added :Delivery/Build Build or test infrastructure v7.0.0 v6.4.0 v6.3.2 labels Jul 19, 2018

alpar-t requested review from rjernst and jasontedor July 19, 2018 13:19

remove commented code

25ea874

jasontedor requested a review from nik9000 July 19, 2018 13:29

jasontedor added the blocker label Jul 19, 2018

nik9000 reviewed Jul 19, 2018

View reviewed changes

rjernst reviewed Jul 20, 2018

View reviewed changes

alpar-t mentioned this pull request Jul 25, 2018

Only configure publishing if it's applied externally #32351

Merged

alpar-t closed this Jul 25, 2018

alpar-t deleted the fix-multi-jar-publishing branch July 25, 2018 05:53

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix multi jar publishing #32195

Fix multi jar publishing #32195

alpar-t commented Jul 19, 2018 •

edited by jasontedor

elasticmachine commented Jul 19, 2018

alpar-t commented Jul 19, 2018

nik9000 left a comment

nik9000 Jul 19, 2018

nik9000 Jul 19, 2018

alpar-t Jul 20, 2018

alpar-t Jul 23, 2018

nik9000 Jul 19, 2018

nik9000 Jul 19, 2018

nik9000 Jul 19, 2018

alpar-t Jul 20, 2018

rjernst left a comment

alpar-t commented Jul 21, 2018 •

edited

rjernst commented Jul 24, 2018

alpar-t commented Jul 24, 2018

rjernst commented Jul 24, 2018

alpar-t commented Jul 25, 2018

Fix multi jar publishing #32195

Fix multi jar publishing #32195

Conversation

alpar-t commented Jul 19, 2018 • edited by jasontedor

elasticmachine commented Jul 19, 2018

alpar-t commented Jul 19, 2018

nik9000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjernst left a comment

Choose a reason for hiding this comment

alpar-t commented Jul 21, 2018 • edited

rjernst commented Jul 24, 2018

alpar-t commented Jul 24, 2018

rjernst commented Jul 24, 2018

alpar-t commented Jul 25, 2018

alpar-t commented Jul 19, 2018 •

edited by jasontedor

alpar-t commented Jul 21, 2018 •

edited