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
35 changes: 34 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` plugin is applied to the project then this code is
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to link to the shadow to be 100% clear on what plugin is meant.
There are often multiple Gradle plugins doing the same thing and its easy to get confused.

Copy link
Member Author

Choose a reason for hiding this comment

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

Linked!

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
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,24 @@ 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
/*
* We bundle the plugin's jar file and its dependencies. We have
* to wait until the project is evaluated before we know if the
* project *has* the shadow plugin. If it does we need the shadow
* jar and the deps in the shadow configuration because those are
* the jars that are not bundled into the plugin's jar. Otherwise
* we're fine with the jar an all of the non-provided runtime
* dependencies.
*/
project.afterEvaluate {
Copy link
Contributor

Choose a reason for hiding this comment

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

from accepts closures that run at execution time for this type of usage.
It does mean that the conditional has to move into the closure

   from { project.plugins.hasPlugin(ShadowPlugin) ?  project.shadowJar :  project.jar }
   from { project.plugins.hasPlugin(ShadowPlugin) ? project.configurations.shadow :  project.configurations.compileOnly }

Copy link
Member Author

Choose a reason for hiding this comment

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

That works! Awesome!

if (project.plugins.hasPlugin(ShadowPlugin)) {
from project.shadowJar
from project.configurations.shadow
} else {
from project.jar
from 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
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
4 changes: 2 additions & 2 deletions x-pack/qa/full-cluster-restart/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ apply plugin: 'elasticsearch.build'
test.enabled = false

dependencies {
testCompile project(path: xpackModule('core'), configuration: 'runtime')
testCompile project(path: xpackModule('core'), configuration: 'shadow')
testCompile (project(path: xpackModule('security'), configuration: 'runtime')) {
// Need to drop the guava dependency here or we get a conflict with watcher's guava dependency.
// This is total #$%, but the solution is to get the SAML realm (which uses guava) out of security proper
Expand Down Expand Up @@ -249,7 +249,7 @@ subprojects {
check.dependsOn(integTest)

dependencies {
testCompile project(path: xpackModule('core'), configuration: 'runtime')
testCompile project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('watcher'), configuration: 'runtime')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
testCompile project(path: xpackModule('security'), configuration: 'testArtifacts')
Expand Down
2 changes: 1 addition & 1 deletion x-pack/qa/ml-basic-multi-node/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.rest-test'

dependencies {
testCompile project(path: xpackModule('core'), configuration: 'runtime')
testCompile project(path: xpackModule('core'), configuration: 'shadow')
testCompile project(path: xpackModule('ml'), configuration: 'runtime')
}

Expand Down
Loading