Move license checks back to BuildPlugin #56975

jakelandis · 2020-05-20T00:11:20Z

Some standalone projects such as :distribution:archives:integ-test-zip have Java code
via tests, but are not need license and notice files. Applying the new ElasticsearchJavaPlugin
in favor over the elder Standalone*Plugin can result in these non-jar
projects test projects throwing an error due to the lack of the license or notice file.

This commit moves the license check out of the new ElasticsearchJavaPlugin and applies
only via the elder BuildPlugin

related: #56841

======

~~I'm not 100% this is the right direction, but seems pretty harmless to allow projects to opt out of the default jar creation.~~ This PR is related to #56841 (comment) ...trying to move away from the conditional checks to that ensure specific plugins are applied.

Some standalone projects such as :distribution:archives:integ-test-zip have tests but are not intended to ever create a jar. Applying the new ElasticsearchJavaPlugin in favor over the elder "Standalone*Plugin" or BuildPlugin can result in these non-jar projects erroring due to the lack of the license or notice file. To avoid creating more permutations of ElasticsearchJavaPlugin (i.e. ElasticsearchJavaPluginWithoutJar), the conuming project should be able disable the jar task (e.g. `jar.enabled = false`) and the license checks should be skipped as well as the javadoc and sources jars. related: elastic#56841

elasticmachine · 2020-05-20T00:11:22Z

Pinging @elastic/es-core-infra (:Core/Infra/Build)

rjernst · 2020-05-20T00:32:35Z

To me this seems like something that should go in the publish plugin, since that is where we need the jars to contain the license files. However, we don't apply the publish plugin to all projects that go into our distributions (eg under libs). I think there is an argument for another plugin, one for any project that will end up in the distribution but is not published. I can't think of what to call it right now though..

jakelandis · 2020-05-20T12:55:56Z

@rjernst I don't disagree, however would you mind if I logged a follow up issue to address that and proceed with this shorter term solution here ? The change here helps to unblock another PR.

mark-vieira · 2020-05-20T18:50:45Z

However, we don't apply the publish plugin to all projects that go into our distributions (eg under libs).

Actually we do since we apply the BuildPlugin plugin to those, which in turn applies the publish plugin.

I'd move the jar license/notice check into BuildPlugin plugin. That plugin might change evolve but it's the best place now as that's applied to stuff we "assemble". I anticipate going forward we'll remove the publish plugin from that, and explicitly apply publishing to specific modules. That way we can apply elasticsearch.build to :libs projects, w/o all the publishing stuff (generate POM) but the build plugin still applies conventions, precommit checks, etc that aren't neccessarily applicable to standalone qa projects (i.e. elasticsearch.java).

mark-vieira · 2020-05-20T21:13:22Z

@jakelandis discussed with @rjernst and we've agreed to just move this into BuildPlugin instead of ElasticsearchJavaPlugin. We can ditch all the enabled stuff and this still accomplishes what you want, which is the ability to use ElasticsearchJavaPlugin w/o dealing with these strict enforcements.

This reverts commit b1f5a23.

This reverts commit 235e3e8.

jakelandis · 2020-05-21T14:07:48Z

thanks @rjernst @mark-vieira - I reverted the original commit(s) and moved the method call out of ElasticsearchJavaPlugin and into BuildPlugin.

rjernst · 2020-05-21T17:01:19Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -82,6 +82,7 @@ class BuildPlugin implements Plugin<Project> {
            )
        }
        project.pluginManager.apply('elasticsearch.java')
+        ElasticsearchJavaPlugin.configureJars(project)


We shouldn't be calling these methods statically; that is what we are trying to move away from. Can you please move the notice/license configuration to a new method in BuildPlugin, and leave the other stuff intact?

I can, however, aren't we also moving away from adding code to the Groovy parts of the build ? (I can move the code to Groovy and keep in the code in idiomatic Java, but isn't it still is going the wrong way ?)

We shouldn't be calling these methods statically

In that case, should these methods not be public static ?

I will be finishing moving this code to java soon. I'll take care of re-javaizing it.

OK ... I pulled the groovy version from git history and removed the Java version. 908ee17

Sorry that isn't what I meant. Only the notice/license code should be moved. That is the problem you are running into right?

pulled only the license and notice parts back to the groovy plugin

mark-vieira

Changes LGTM 👍

…c#57169)

jakelandis added :Delivery/Build Build or test infrastructure v8.0.0 v7.7.1 v7.8.1 v7.9.0 labels May 20, 2020

jakelandis requested review from rjernst and mark-vieira May 20, 2020 00:11

elasticmachine added the Team:Core/Infra Meta label for core/infra team label May 20, 2020

remove unecessary enabled check

b1f5a23

jakelandis added 4 commits May 21, 2020 09:00

Revert "remove unecessary enabled check"

fa86e24

This reverts commit b1f5a23.

Revert "Allow ElasticsearchJavaPlugin for non-jar based projects"

2e38b55

This reverts commit 235e3e8.

Merge remote-tracking branch 'upstream/master' into allow_disable_jar

8cff148

remove license check from ElasticsearchJavaPlugin

845a2a8

rjernst reviewed May 21, 2020

View reviewed changes

jakelandis added 3 commits May 21, 2020 12:55

resurect Groovy configureJars and remove from Java

908ee17

only move license stuff back to groovy

5812825

call java configureJars

e504625

mark-vieira approved these changes May 21, 2020

View reviewed changes

jakelandis changed the title ~~Allow ElasticsearchJavaPlugin for non-jar based projects~~ Move license checks back to BuildPlugin May 26, 2020

jakelandis merged commit 2e1e33d into elastic:master May 26, 2020

jakelandis deleted the allow_disable_jar branch May 26, 2020 17:42

jakelandis added the backport pending label May 26, 2020

jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request May 26, 2020

Move license checks back to BuildPlugin (elastic#56975)

4df6b2b

jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request May 26, 2020

Move license checks back to BuildPlugin (elastic#56975)

22aeded

jakelandis added a commit that referenced this pull request May 26, 2020

[7.x] Move license checks back to BuildPlugin (#56975) (#57169)

41629b7

jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request May 26, 2020

[7.x] Move license checks back to BuildPlugin (elastic#56975) (elasti…

f99a229

…c#57169)

jakelandis mentioned this pull request May 26, 2020

[7.8] Move license checks back to BuildPlugin (#56975) #57177

Merged

jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request May 26, 2020

[7.x] Move license checks back to BuildPlugin (elastic#56975) (elasti…

6008e65

…c#57169)

jakelandis mentioned this pull request May 26, 2020

[7.7] Move license checks back to BuildPlugin (#56975) #57178

Merged

jakelandis added a commit that referenced this pull request May 27, 2020

[7.8] Move license checks back to BuildPlugin (#56975)

0f774c9

jakelandis added a commit that referenced this pull request May 27, 2020

[7.7] Move license checks back to BuildPlugin (#56975)

f98ef20

jakelandis removed the backport pending label May 27, 2020

mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move license checks back to BuildPlugin #56975

Move license checks back to BuildPlugin #56975

jakelandis commented May 20, 2020 •

edited

Loading

elasticmachine commented May 20, 2020

rjernst commented May 20, 2020

jakelandis commented May 20, 2020

mark-vieira commented May 20, 2020 •

edited

Loading

mark-vieira commented May 20, 2020

jakelandis commented May 21, 2020

rjernst May 21, 2020

jakelandis May 21, 2020

rjernst May 21, 2020 •

edited

Loading

jakelandis May 21, 2020

rjernst May 21, 2020

jakelandis May 21, 2020

mark-vieira left a comment

Move license checks back to BuildPlugin #56975

Move license checks back to BuildPlugin #56975

Conversation

jakelandis commented May 20, 2020 • edited Loading

elasticmachine commented May 20, 2020

rjernst commented May 20, 2020

jakelandis commented May 20, 2020

mark-vieira commented May 20, 2020 • edited Loading

mark-vieira commented May 20, 2020

jakelandis commented May 21, 2020

rjernst May 21, 2020

Choose a reason for hiding this comment

jakelandis May 21, 2020

Choose a reason for hiding this comment

rjernst May 21, 2020 • edited Loading

Choose a reason for hiding this comment

jakelandis May 21, 2020

Choose a reason for hiding this comment

rjernst May 21, 2020

Choose a reason for hiding this comment

jakelandis May 21, 2020

Choose a reason for hiding this comment

mark-vieira left a comment

Choose a reason for hiding this comment

jakelandis commented May 20, 2020 •

edited

Loading

mark-vieira commented May 20, 2020 •

edited

Loading

rjernst May 21, 2020 •

edited

Loading