Consolidate dev-tools folder into elasticsearch-parent.

[rmuir]

This makes dev-tools a little module that other modules pull with maven-remote-resources plugin. This way we can include PMD configuration, whatever we need. Entirety of dev-tools soon...

I added the forbidden API configuration from es-core. Plugins don't need to do the path based exceptions in POM files, these days just use the @SuppressForbidden(reason = ...) mechanism via annotations for exceptions.

I also added 'dev' profile, since it kinda goes along with forbidden-apis, its a way to skip the check.

I tested this with kuromoji plugin, everything works easily with just these additions to its POM:

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-remote-resources-plugin</artifactId>
</plugin>
<plugin>
  <groupId>de.thetaphi</groupId>
  <artifactId>forbiddenapis</artifactId>
</plugin>

Closes #42

[rmuir]

This needs some review from people that have maven knowledge. @dadoonet and @s1monw in case you guys have a free moment to take a look, I'd appreciate it.

I evaluated a couple alternatives:

• forbidden-apis could instead use a "url" for the config, e.g. public http endpoint to source repo, but this is messy and wouldnt work well when you are actually testing changes to it. No atomicity of builds either. Also doesnt solve the general problem that we have all kinds of duplicated "resources" today I'd like to eliminate.
• looks like we could use other plugins like maven-assembly-plugin but these looked like more XML. So i used the solution with less XML.

[dadoonet]

I like this solution a lot. Just wondering if you could define it as a module so it will be published at the same time than its parent.

[rmuir]

@dadoonet I think that is correct. let me give it a try, if it works, I will update the PR!

[rmuir]

@dadoonet I switched it to a module. works nice:

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Elasticsearch Build Resources ..................... SUCCESS [  0.660 s]
[INFO] Elasticsearch Parent POM .......................... SUCCESS [  0.089 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS

[dadoonet]

\o/ LGTM

[rmuir]

I defined property elasticsearch.tools.directory (${project.build.directory}/dev-tools) which was already referred 5 times in the config. This makes it easier to reuse for other purposes. Like maybe release scripts, PMD or whatever duplication we want to attack next.

[uschindler]

forbidden-apis could instead use a "url" for the config, e.g. public http endpoint to source repo, but this is messy and wouldnt work well when you are actually testing changes to it. No atomicity of builds either. Also doesnt solve the general problem that we have all kinds of duplicated "resources" today I'd like to eliminate.

These URL references are bad for off-line builds. This would make it impossible to run the maven build without internet connection in an airplane :-) So the solution that I would prefer is to publish signatures as maven artifacts (there is an issue open: https://github.com/policeman-tools/forbidden-apis/issues/13), which will be downloaded to the local Maven repository.

The solution presented here is more universally usable and I like it more (because you can share the whole dev-tools folders with scripts, PMD/CheckStyle stuff,...)

[dadoonet]

Yeah we came to your issue yesterday with @rmuir. Once there we could use it!

[uschindler]

I tested this with kuromoji plugin, everything works easily with just these additions to its POM: [...]

This is a current limitation of the structure of the parent POM. Basically there are 2 ways to define plugins in the parent POM:

• inside build/pluginManagement/plugins: This just defines setting used by plugins, but don't enable them. This is nice to define default versions of plugins or settings, but each submodule has to enable the plugin. This is the reason, why it has to be enabled in kuromoji
• if you define the plugins in build/plugins directly (outside pluginManagement), the configuration effect is the same, but it automatically enables the plugin, also for all submodules. This also executes the plugin on the parent POM (yes!), but this is generally handled correctly by the plugins. E.g. compiler plugin or forbidden-apis does not execute in <packaging>pom</packaging> (you see a message about skipped execution in build log).

See for example Apache TIKA's parent POM: https://github.com/apache/tika/blob/fd8484792260fe9412b2756623c3f1afd58465f5/tika-parent/pom.xml#L307

It enables several plugins for all submodules. If one of the submodules wants to disable a plugin inherited from the parent pom, you can set its execution to "none" in the submodule, e.g., https://github.com/apache/tika/blob/fd8484792260fe9412b2756623c3f1afd58465f5/tika-bundle/pom.xml#L357 (which disables forbiddenapis in the bundle module of TIKA),

[rmuir]

@uschindler thanks very much for explaining that. I prefer this approach. I will update the PR, and can disable forbidden temporarily or fix any violations in our plugins before we pushing here.

We really have to minimize the elements we have in plugin POMs and that will help a lot... Kuromoji POM (https://github.com/elastic/elasticsearch-analysis-kuromoji/blob/master/pom.xml) is already far too large for my tastes. I don't understand things like the test-dependencies having to be declared in each child and so on, we need to get to the bottom of this stuff :)

[uschindler]

Just keep in mind that not all plugins should be moved out of pluginManagement. For example, the "lifecycle-mapping" Eclipse plugin that tells m2e to not execute some of the plugins, needs to stay in pluginManagement (it is not able to execute at all, its just a "placeholder" to configure Eclipse's m2e).

in general +1! I would also prefer management like this: Split between configuration only definitions in POMs and inherited execution.

[uschindler]

We really have to minimize the elements we have in plugin POMs and that will help a lot... Kuromoji POM (https://github.com/elastic/elasticsearch-analysis-kuromoji/blob/master/pom.xml) is already far too large for my tastes. I don't understand things like the test-dependencies having to be declared in each child and so on, we need to get to the bottom of this stuff :)

I would define a parent for all "ES plugins" with all the plugins for the ES-plugins correctly configured (like assembly plugin, maybe test-resources. Some structure like this:

elasticsearch-parent -> elasticsearch elasticsearch-parent -> elasticsearch-plugin-parent -> kuromoji, foobar

This would also make it easier for 3rd party plugins. They could use the "official" plugin parent POM and it will build correctly out of box.

This would also allow to handle the "shaded" resources shit :-) The plugin-parent would use the shaded resources.

In addition the plugin-parent module would also already configure dependencies needed: it could add the elasticsearch dependency already (as Robert mentioned) - because there can be no plugin that does not depend on elasticsearch?

[dadoonet]

Totally agreed. It will help to share sames settings for maven plugins across Elasticsearch plugins. That said, we can introduce that later on if we don't want to go too far for the first iteration

[uschindler]

Yeah, 2nd iteration, because this involves more cleanup work and is not really related to the stuff here.

[rmuir]

OK, thanks again @uschindler . Now plugins are getting much simpler, with less duplication in each child POM. I will fix forbidden API violations in all plugins now, so that we can move forward.