Elasticsearch: Replace groovy build-tools (buildSrc) with java

Created on 15 Oct 2018  路  27Comments  路  Source: elastic/elasticsearch

We want to have build code implemented in Java to make it easier to maintain.
We can easily mix java and groovy implementations and everything is already set up to do so, so it's already easy to create new classes in Java.
It would also be great to take this opportunity to add tests for the newly written Java code.

The ingredients of a great rewrite:

  • the groovy code is removed, the java implementation is actually used by the build
  • the java implementation does not use the Groovy SDK ( no groovy imports in Java).

    • note that the "convert to java" option in IDEA does add calls to it

  • Existing Groovy tests are also ported to Java

    • when there are no existing tests, new unit and/or integration tests are created. There is a precedent for both. Integration tests use Gradle Test Kit. We should have tests for basic functionality, making sure that the test is up-to-date when the inputs/outputs don't change and in some cases for error conditions.

  • follow conventions showcased in Gradle example plugin and detailed in the Gradle user guide: writing custom tasks and writing plugins. Make sure to follow the instructions on making tasks incremental.
  • Don't use, and replace the use of project.afterEvaluate in custom task implementations as well as any other configuration logic from the task constructor. The custom task constructor should not reference project. Gradle calls methods annotated with @TaskAction as well as @Input, @Output and the like at execution time rather than configuration time as defined by the
    Gradle build lifecycle.

Here's a list of what is still in Groovy. Note that it goes from simple to complex and there are some inner dependencies :

  • stand alone tasks and plugins

    • [x] ConcatFilesTask (#34476, #37497)

    • [x] NoticeTask (#34769)

    • [x] PluginPropertiesTask #39605

    • [x] PluginPropertiesExtension #39605

    • [ ] MavenFilteringHack

    • [x] FileContentsTask (#34539)

    • [ ] DependenciesInfoTask

    • [x] VersionCollection (#34050)

    • [x] MessyTestPlugin ( possibly dead code elastic/x-plugins/issues/724 )

    • [x] EmptyDirTask (#34672)

  • precommit

    • [x] FilePermissionsTask (#34674)

    • [x] ForbiddenPatternsTask (#36194)

    • [x] UpdateShasTask ( #35231 , #41921 )

    • [x] DependencyLicensesTask (#35231 , #41921)

    • [ ] PrecommitTasks

  • [ ] doc

    • RestTestFromSnippetsTaskTests

    • RestTestsFromSnippetsTask

    • DocsTestPlugin

    • SnippetsTask

    • RestTestsFromSnippetsTask

  • [ ] ant and and fixtures

    • LicenseHeadersTask

    • AntTask

  • [ ] dependent on the new testclusters implementation

    • [x] AntFixture ( used in multipe build scripts )

    • RestTestPlugin

    • StandaloneRestTestPlugin

    • [X] Fixture

    • RestIntegTestTask

    • [x ]RunTask

    • [x] ClusterConfiguration

    • [x ]NodeInfo

    • [x] ClusterFormationTasks

  • [x] vagrant

    • VagrantFixture

    • VagrantLoggerOutputStream

    • VagrantCommandTask

    • VagrantTestPlugin

    • BatsOverVagrantTask

    • TapLoggerOutputStream

    • VagrantPropertiesExtension

  • plugins

    • [ ] BuildPlugin

    • [ ] StandaloneTestPlugin

    • [ ] PluginBuildPlugin

    • [ ] RestTestPlugin

    • [ ] StandaloneRestTestPlugin

    • [X] VagrantSupportPlugin

    • [ ] TestWithDependenciesPlugin

  • [x] carrotsearch : either push changes upstream or replace with Gradle runner ( #31496 )
:DeliverBuild >refactoring Delivery good first issue

All 27 comments

Pinging @elastic/es-core-infra

Hello, I would like to work on this.

Hi @pt429sy that's great ! Feel free to pick a class and open a PR.

just for info, I am working on NoticeTask

Doing UpdateShasTask, probably will make a PR tonight.

EDIT: Or tomorrow.

I am interested in giving a try to PluginPropertiesExtension. From my understanding it should be quite straightforward.

Going to give ForbiddenPatternsTask a shot, FYI

I am working on DependenciesInfoTask, will raise a PR later

I tried porting vagrant.VagrantPropertiesExtension file to Java but it seems all modules inside vagrant package have to be ported in one go to make it work; Please let me know if this work on VagrantPropertiesExtension can anyhow be used/merged separately, otherwise anybody who aims to port all the other files can feel free to use this patch.

@atorok From what I understand, https://github.com/elastic/elasticsearch/pull/34769 can be added against NoticeTask in the first comment.

Going to give MavenFilteringHack a shot, FYI

@akki Java code can't reference Groovy classes, but groovy can reference Java, so one has to start with leafs in the dependency tree when porting classes.

Due to long inactivity on #34476 I am picking up ConcatFilesTask.

@atorok, are there any updates on merges / pieces that have merged?

I can take the PrecommitTasks

@bsamartins you will need the other PRs merge first as you won't be able to reference Groovy classes in Java code.

Is this in the PR description PluginBuildPlugin #39605 correct? Shouldn't the PR #39605 be added besides PluginPropertiesExtension?

I'd like to give DependenciesInfoTask a try if nobody's working on it

Hi @atorok, I would like to contribute but I can see that tasks above are not updated. Which task can be taken next?

Thanks for your interest @michal-rozak . I Updated the task to mark the ones that were completed.

@atorok I think the correct PR about the DependencyLicensesTask and UpdateShasTask is the #41921 one. The one listed is the old one

@atorok, I want to pick PrecommitTasks, ok?

@atorok Oh, it seems that PrecommitTasks is taken/in progress. Maybe DocsTestPlugin then?

I don't think @bsamartins did any work on PrecommitTasks so I think you could take either one @michal-rozak

Hi @atorok, I'd like to work on MavenFilteringHack if no one's working on it

hello @alpar-t would like to work on AntTask if no one's working on it

@mark-vieira where do we stand on the AntTask ?

My preference would be to remove it, and refactor the two remaining subclasses to not rely on Ant. So I'm not sure it's a good candidate for migration right now.

@mark-vieira @alpar-t I just pulled the latest code and I see PrecommitTasks is still pending migration. I would like to take it on if its not hanging in the PR state. Thanks

Was this page helpful?
0 / 5 - 0 ratings