Skip to content

Commit

Permalink
Build: Shadow x-pack:protocol into x-pack:plugin:core (#32240)
Browse files Browse the repository at this point in the history
This bundles the x-pack:protocol project into the x-pack:plugin:core
project because we'd like folks to consider it an implementation detail
of our build rather than a separate artifact to be managed and depended
on. It is now bundled into both x-pack:plugin:core and
client:rest-high-level. To make this work I had to fix a few things.

Firstly, I had to make PluginBuildPlugin work with the shadow plugin.
In that case we have to bundle only the `shadow` dependencies and the
shadow jar.

Secondly, every reference to x-pack:plugin:core has to use the `shadow`
configuration. Without that the reference is missing all of the
un-shadowed dependencies. I tried to make it so that applying the shadow
plugin automatically redefines the `default` configuration to mirror the
`shadow` configuration which would allow us to use bare project references
to the x-pack:plugin:core project but I couldn't make it work. It'd *look*
like it works but then fail for transitive dependencies anyway. I think
it is still a good thing to do but I don't have the willpower to do it
now.

Finally, I had to fix an issue where Eclipse and IntelliJ didn't properly
reference shadowed transitive dependencies. Neither IDE supports shadowing
natively so they have to reference the shadowed projects. We fix this by
detecting `shadow` dependencies when in "Intellij mode" or "Eclipse mode"
and adding `runtime` dependencies to the same target. This convinces
IntelliJ and Eclipse to play nice.
  • Loading branch information
nik9000 committed Jul 25, 2018
1 parent 1e6fa9a commit 07e855b
Show file tree
Hide file tree
Showing 50 changed files with 148 additions and 85 deletions.
37 changes: 36 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,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 @@ -271,6 +271,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 @@ -300,3 +333,5 @@ especially when they are unlikely to become long time contributors.
Finally, we require that you run `./gradlew check` before submitting a
non-documentation contribution. This is mentioned above, but it is worth
repeating in this section because it has come up in this context.

[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 @@ -510,6 +510,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 @@ -391,6 +391,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 @@ -883,11 +886,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 {
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 @@ -151,8 +139,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 @@ -265,7 +265,7 @@ setups['farequote_index'] = '''
airline:
type: keyword
doc_count:
type: integer
type: integer
'''
setups['farequote_data'] = setups['farequote_index'] + '''
- do:
Expand All @@ -278,7 +278,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 @@ -310,7 +310,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
24 changes: 13 additions & 11 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 @@ -28,19 +29,19 @@ 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 'org.bouncycastle:bcprov-jdk15on:1.59'
compile 'org.bouncycastle:bcpkix-jdk15on:1.59'
compile project(path: ':modules:transport-netty4', configuration: 'runtime')
shadow 'com.unboundid:unboundid-ldapsdk:3.2.0'
shadow 'org.bouncycastle:bcprov-jdk15on:1.59'
shadow 'org.bouncycastle:bcpkix-jdk15on:1.59'
shadow project(path: ':modules:transport-netty4', configuration: 'runtime')

testCompile 'org.elasticsearch:securemock:1.2'
testCompile "org.elasticsearch:mocksocket:${versions.mocksocket}"
Expand Down Expand Up @@ -110,7 +111,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 @@ -12,7 +12,7 @@ archivesBaseName = 'x-pack-deprecation'
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')
}

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
2 changes: 1 addition & 1 deletion 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')

testCompile project(path: xpackModule('monitoring'))
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')
testCompile project(path: xpackModule('watcher'), configuration: 'runtime')
}
Expand All @@ -40,7 +40,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')

testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
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 @@ -261,7 +261,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
2 changes: 1 addition & 1 deletion x-pack/qa/ml-disabled/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

0 comments on commit 07e855b

Please sign in to comment.