Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build: Shadow x-pack:protocol into x-pack:plugin:core #32240

Merged
merged 18 commits into from
Jul 24, 2018
36 changes: 35 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ If your changes affect only the documentation, run:
```sh
./gradlew -p docs check
```
For more information about testing code examples in the documentation, see
For more information about testing code examples in the documentation, see
https://github.com/elastic/elasticsearch/blob/master/docs/README.asciidoc

### Project layout
Expand Down Expand Up @@ -305,6 +305,39 @@ the `qa` subdirectory functions just like the top level `qa` subdirectory. The
Elasticsearch process. The `transport-client` subdirectory contains extensions
to Elasticsearch's standard transport client to work properly with x-pack.

### Gradle Build

We use Gradle to build Elasticsearch because it is flexible enough to not only
build and package Elasticsearch, but also orchestrate all of the ways that we
have to test Elasticsearch.

#### Configurations

Gradle organizes dependencies and build artifacts into "configurations" and
allows you to use these configurations arbitrarilly. Here are some of the most
common configurations in our build and how we use them:

<dl>
<dt>`compile`</dt><dd>Code that is on the classpath at both compile and
runtime. If the [`shadow`][shadow-plugin] plugin is applied to the project then
this code is bundled into the jar produced by the project.</dd>
<dt>`runtime`</dt><dd>Code that is not on the classpath at compile time but is
on the classpath at runtime. We mostly use this configuration to make sure that
we do not accidentally compile against dependencies of our dependencies also
known as "transitive" dependencies".</dd>
<dt>`compileOnly`</dt><dd>Code that is on the classpath at comile time but that
should not be shipped with the project because it is "provided" by the runtime
somehow. Elasticsearch plugins use this configuration to include dependencies
that are bundled with Elasticsearch's server.</dd>
<dt>`shadow`</dt><dd>Only available in projects with the shadow plugin. Code
that is on the classpath at both compile and runtime but it *not* bundled into
the jar produced by the project. If you depend on a project with the `shadow`
plugin then you need to depend on this configuration because it will bring
along all of the dependencies you need at runtime.</dd>
<dt>`testCompile`</dt><dd>Code that is on the classpath for compiling tests
that are part of this project but not production code. The canonical example
of this is `junit`.</dd>
</dl>

Contributing as part of a class
-------------------------------
Expand Down Expand Up @@ -337,3 +370,4 @@ repeating in this section because it has come up in this context.

[eclipse]: http://www.eclipse.org/community/eclipse_newsletter/2017/june/
[intellij]: https://blog.jetbrains.com/idea/2017/07/intellij-idea-2017-2-is-here-smart-sleek-and-snappy/
[shadow-plugin]: https://github.com/johnrengelman/shadow
25 changes: 25 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,31 @@ allprojects {
tasks.eclipse.dependsOn(cleanEclipse, copyEclipseSettings)
}

allprojects {
/*
* IntelliJ and Eclipse don't know about the shadow plugin so when we're
* in "IntelliJ mode" or "Eclipse mode" add "runtime" dependencies
* eveywhere where we see a "shadow" dependency which will cause them to
* reference shadowed projects directly rather than rely on the shadowing
* to include them. This is the correct thing for it to do because it
* doesn't run the jar shadowing at all. This isn't needed for the project
* itself because the IDE configuration is done by SourceSets but it is
* *is* needed for projects that depends on the project doing the shadowing.
* Without this they won't properly depend on the shadowed project.
*/
if (isEclipse || isIdea) {
configurations.all { Configuration configuration ->
dependencies.all { Dependency dep ->
if (dep instanceof ProjectDependency) {
if (dep.getTargetConfiguration() == 'shadow') {
configuration.dependencies.add(project.dependencies.project(path: dep.dependencyProject.path, configuration: 'runtime'))
}
}
}
}
}
}

// we need to add the same --debug-jvm option as
// the real RunTask has, so we can pass it through
class Run extends DefaultTask {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,9 @@ class BuildPlugin implements Plugin<Project> {
project.configurations.compile.dependencies.all(disableTransitiveDeps)
project.configurations.testCompile.dependencies.all(disableTransitiveDeps)
project.configurations.compileOnly.dependencies.all(disableTransitiveDeps)
project.plugins.withType(ShadowPlugin).whenPluginAdded {
project.configurations.shadow.dependencies.all(disableTransitiveDeps)
}
}

/** Adds repositories used by ES dependencies */
Expand Down Expand Up @@ -873,11 +876,20 @@ class BuildPlugin implements Plugin<Project> {
project.dependencyLicenses.dependencies = project.configurations.runtime.fileCollection {
it.group.startsWith('org.elasticsearch') == false
} - project.configurations.compileOnly
project.plugins.withType(ShadowPlugin).whenPluginAdded {
project.dependencyLicenses.dependencies += project.configurations.shadow.fileCollection {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is new, but I think this will resolve the configuration at configuration time which is undesirable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It mimics what we do above for the runtime configuration. I'm not arguing that it is right, just that it is in good company.

it.group.startsWith('org.elasticsearch') == false
}
}
}

private static configureDependenciesInfo(Project project) {
Task deps = project.tasks.create("dependenciesInfo", DependenciesInfoTask.class)
deps.runtimeConfiguration = project.configurations.runtime
project.plugins.withType(ShadowPlugin).whenPluginAdded {
deps.runtimeConfiguration = project.configurations.create('infoDeps')
deps.runtimeConfiguration.extendsFrom(project.configurations.runtime, project.configurations.shadow)
}
deps.compileOnlyConfiguration = project.configurations.compileOnly
project.afterEvaluate {
deps.mappings = project.dependencyLicenses.mappings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,6 @@ public class PluginBuildPlugin extends BuildPlugin {
@Override
public void apply(Project project) {
super.apply(project)
project.plugins.withType(ShadowPlugin).whenPluginAdded {
/*
* We've not tested these plugins together and we're fairly sure
* they aren't going to work properly as is *and* we're not really
* sure *why* you'd want to shade stuff in plugins. So we throw an
* exception here to make you come and read this comment. If you
* have a need for shadow while building plugins then know that you
* are probably going to have to fight with gradle for a while....
*/
throw new InvalidUserDataException('elasticsearch.esplugin is not '
+ 'compatible with com.github.johnrengelman.shadow');
}
configureDependencies(project)
// this afterEvaluate must happen before the afterEvaluate added by integTest creation,
// so that the file name resolution for installing the plugin will be setup
Expand Down Expand Up @@ -153,8 +141,13 @@ public class PluginBuildPlugin extends BuildPlugin {
include(buildProperties.descriptorOutput.name)
}
from pluginMetadata // metadata (eg custom security policy)
from project.jar // this plugin's jar
from project.configurations.runtime - project.configurations.compileOnly // the dep jars
/*
* If the plugin is using the shadow plugin then we need to bundle
* "shadow" things rather than the default jar and dependencies so
* we don't hit jar hell.
*/
from { project.plugins.hasPlugin(ShadowPlugin) ? project.shadowJar : project.jar }
from { project.plugins.hasPlugin(ShadowPlugin) ? project.configurations.shadow : project.configurations.runtime - project.configurations.compileOnly }
// extra files for the plugin to go into the zip
from('src/main/packaging') // TODO: move all config/bin/_size/etc into packaging
from('src/main') {
Expand Down
8 changes: 4 additions & 4 deletions x-pack/docs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ buildRestTests.expectedUnconvertedCandidates = [
]

dependencies {
testCompile project(path: xpackModule('core'), configuration: 'runtime')
testCompile project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
testCompile project(path: xpackProject('plugin').path, configuration: 'testArtifacts')
}
Expand Down Expand Up @@ -264,7 +264,7 @@ setups['farequote_index'] = '''
airline:
type: keyword
doc_count:
type: integer
type: integer
'''
setups['farequote_data'] = setups['farequote_index'] + '''
- do:
Expand All @@ -277,7 +277,7 @@ setups['farequote_data'] = setups['farequote_index'] + '''
{"airline":"JZA","responsetime":990.4628,"time":"2016-02-07T00:00:00+0000", "doc_count": 5}
{"index": {"_id":"2"}}
{"airline":"JBU","responsetime":877.5927,"time":"2016-02-07T00:00:00+0000", "doc_count": 23}
{"index": {"_id":"3"}}
{"index": {"_id":"3"}}
{"airline":"KLM","responsetime":1355.4812,"time":"2016-02-07T00:00:00+0000", "doc_count": 42}
'''
setups['farequote_job'] = setups['farequote_data'] + '''
Expand Down Expand Up @@ -309,7 +309,7 @@ setups['farequote_datafeed'] = setups['farequote_job'] + '''
"job_id":"farequote",
"indexes":"farequote"
}
'''
'''
setups['server_metrics_index'] = '''
- do:
indices.create:
Expand Down
4 changes: 2 additions & 2 deletions x-pack/license-tools/build.gradle
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apply plugin: 'elasticsearch.build'

dependencies {
compile project(xpackModule('core'))
compile project(path: xpackModule('core'), configuration: 'shadow')
compile "org.elasticsearch:elasticsearch:${version}"
testCompile "org.elasticsearch.test:framework:${version}"
}
Expand All @@ -17,7 +17,7 @@ task buildZip(type: Zip, dependsOn: jar) {
into(parentDir + '/lib') {
from jar
from configurations.runtime
}
}
into(parentDir + '/bin') {
from 'bin'
}
Expand Down
20 changes: 11 additions & 9 deletions x-pack/plugin/core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import java.nio.file.StandardCopyOption
apply plugin: 'elasticsearch.esplugin'
apply plugin: 'nebula.maven-base-publish'
apply plugin: 'nebula.maven-scm'
apply plugin: 'com.github.johnrengelman.shadow'

archivesBaseName = 'x-pack-core'

Expand All @@ -27,17 +28,17 @@ dependencyLicenses {
dependencies {
compileOnly "org.elasticsearch:elasticsearch:${version}"
compile project(':x-pack:protocol')
compile "org.apache.httpcomponents:httpclient:${versions.httpclient}"
compile "org.apache.httpcomponents:httpcore:${versions.httpcore}"
compile "org.apache.httpcomponents:httpcore-nio:${versions.httpcore}"
compile "org.apache.httpcomponents:httpasyncclient:${versions.httpasyncclient}"
shadow "org.apache.httpcomponents:httpclient:${versions.httpclient}"
shadow "org.apache.httpcomponents:httpcore:${versions.httpcore}"
shadow "org.apache.httpcomponents:httpcore-nio:${versions.httpcore}"
shadow "org.apache.httpcomponents:httpasyncclient:${versions.httpasyncclient}"

compile "commons-logging:commons-logging:${versions.commonslogging}"
compile "commons-codec:commons-codec:${versions.commonscodec}"
shadow "commons-logging:commons-logging:${versions.commonslogging}"
shadow "commons-codec:commons-codec:${versions.commonscodec}"

// security deps
compile 'com.unboundid:unboundid-ldapsdk:3.2.0'
compile project(path: ':modules:transport-netty4', configuration: 'runtime')
shadow 'com.unboundid:unboundid-ldapsdk:3.2.0'
shadow project(path: ':modules:transport-netty4', configuration: 'runtime')

testCompile 'org.elasticsearch:securemock:1.2'
testCompile "org.elasticsearch:mocksocket:${versions.mocksocket}"
Expand Down Expand Up @@ -107,7 +108,8 @@ test {
// TODO: don't publish test artifacts just to run messy tests, fix the tests!
// https://github.com/elastic/x-plugins/issues/724
configurations {
testArtifacts.extendsFrom testRuntime
testArtifacts.extendsFrom(testRuntime, shadow)
testArtifacts.exclude(group: project(':x-pack:protocol').group, module: project(':x-pack:protocol').name)
}
task testJar(type: Jar) {
appendix 'test'
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/deprecation/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ esplugin {
archivesBaseName = 'x-pack-deprecation'

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
}

run {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/graph/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ esplugin {
archivesBaseName = 'x-pack-graph'

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
}

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/logstash/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ esplugin {
archivesBaseName = 'x-pack-logstash'

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')

}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/ml/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ compileJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes,-serial,-try,
compileTestJava.options.compilerArgs << "-Xlint:-deprecation,-rawtypes,-serial,-try,-unchecked"

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
// This should not be here
testCompile project(path: xpackModule('security'), configuration: 'testArtifacts')
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/monitoring/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ esplugin {
archivesBaseName = 'x-pack-monitoring'

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')

// monitoring deps
Expand Down Expand Up @@ -66,7 +66,7 @@ task internalClusterTest(type: RandomizedTestingTask,
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}
check.dependsOn internalClusterTest
check.dependsOn internalClusterTest
internalClusterTest.mustRunAfter test

// also add an "alias" task to make typing on the command line easier task icTest {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/rollup/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ compileTestJava.options.compilerArgs << "-Xlint:-rawtypes"
dependencies {
compileOnly "org.elasticsearch:elasticsearch:${version}"

compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
}

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ esplugin {
archivesBaseName = 'x-pack-security'

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
compileOnly project(path: ':modules:transport-netty4', configuration: 'runtime')
compileOnly project(path: ':plugins:transport-nio', configuration: 'runtime')

Expand Down Expand Up @@ -52,7 +52,7 @@ dependencies {
compile "org.apache.httpcomponents:httpasyncclient:${versions.httpasyncclient}"
compile "org.apache.httpcomponents:httpcore-nio:${versions.httpcore}"
compile "org.apache.httpcomponents:httpclient-cache:${versions.httpclient}"
compile 'com.google.guava:guava:19.0'
compile 'com.google.guava:guava:19.0'

testCompile 'org.elasticsearch:securemock:1.2'
testCompile "org.elasticsearch:mocksocket:${versions.mocksocket}"
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/security/cli/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ archivesBaseName = 'elasticsearch-security-cli'

dependencies {
compileOnly "org.elasticsearch:elasticsearch:${version}"
compileOnly xpackProject('plugin:core')
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
compile 'org.bouncycastle:bcprov-jdk15on:1.59'
compile 'org.bouncycastle:bcpkix-jdk15on:1.59'
testImplementation 'com.google.jimfs:jimfs:1.1'
Expand All @@ -17,4 +17,4 @@ dependencies {

dependencyLicenses {
mapping from: /bc.*/, to: 'bouncycastle'
}
}
2 changes: 1 addition & 1 deletion x-pack/plugin/sql/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ archivesBaseName = 'x-pack-sql'
integTest.enabled = false

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
compileOnly(project(':modules:lang-painless')) {
// exclude ASM to not affect featureAware task on Java 10+
exclude group: "org.ow2.asm"
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/upgrade/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ esplugin {
archivesBaseName = 'x-pack-upgrade'

dependencies {
compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
}

Expand All @@ -39,7 +39,7 @@ task internalClusterTest(type: RandomizedTestingTask,
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}
check.dependsOn internalClusterTest
check.dependsOn internalClusterTest
internalClusterTest.mustRunAfter test

// also add an "alias" task to make typing on the command line easier
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/watcher/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ dependencyLicenses {
dependencies {
compileOnly "org.elasticsearch:elasticsearch:${version}"

compileOnly "org.elasticsearch.plugin:x-pack-core:${version}"
compileOnly project(path: xpackModule('core'), configuration: 'shadow')
compileOnly project(path: ':modules:transport-netty4', configuration: 'runtime')
compileOnly project(path: ':plugins:transport-nio', configuration: 'runtime')

Expand Down
Loading