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: Move shadow customizations into common code #32014

Merged
merged 13 commits into from
Jul 17, 2018
15 changes: 0 additions & 15 deletions benchmarks/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,6 @@
* under the License.
*/

buildscript {
repositories {
maven {
url 'https://plugins.gradle.org/m2/'
}
}
dependencies {
classpath 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
}
}

apply plugin: 'elasticsearch.build'

// order of this section matters, see: https://github.com/johnrengelman/shadow/issues/336
Expand Down Expand Up @@ -81,10 +70,6 @@ thirdPartyAudit.excludes = [
'org.openjdk.jmh.util.Utils'
]

shadowJar {
classifier = 'benchmarks'
}

runShadow {
executable = new File(project.runtimeJavaHome, 'bin/java')
}
Expand Down
57 changes: 47 additions & 10 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/


import com.github.jengelman.gradle.plugins.shadow.ShadowPlugin
import org.apache.tools.ant.taskdefs.condition.Os
import org.apache.tools.ant.filters.ReplaceTokens
import org.elasticsearch.gradle.BuildPlugin
Expand Down Expand Up @@ -303,18 +303,55 @@ subprojects {
if (project.plugins.hasPlugin(BuildPlugin)) {
String artifactsHost = VersionProperties.elasticsearch.isSnapshot() ? "https://snapshots.elastic.co" : "https://artifacts.elastic.co"
Closure sortClosure = { a, b -> b.group <=> a.group }
Closure depJavadocClosure = { dep ->
if (dep.group != null && dep.group.startsWith('org.elasticsearch')) {
Project upstreamProject = dependencyToProject(dep)
if (upstreamProject != null) {
project.javadoc.dependsOn "${upstreamProject.path}:javadoc"
String artifactPath = dep.group.replaceAll('\\.', '/') + '/' + dep.name.replaceAll('\\.', '/') + '/' + dep.version
project.javadoc.options.linksOffline artifactsHost + "/javadoc/" + artifactPath, "${upstreamProject.buildDir}/docs/javadoc/"
Closure depJavadocClosure = { shadowed, dep ->
if (dep.group == null || false == dep.group.startsWith('org.elasticsearch')) {
return
}
Project upstreamProject = dependencyToProject(dep)
if (upstreamProject == null) {
return
}
if (shadowed) {
/*
* Include the source of shadowed upstream projects so we don't
* to publish their javadoc.
Copy link
Member

Choose a reason for hiding this comment

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

to publish -> publish?

Copy link
Member Author

Choose a reason for hiding this comment

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

"have to publish" is what I was going for. I'll fix.

*/
project.evaluationDependsOn(upstreamProject.path)
project.javadoc.source += upstreamProject.javadoc.source
/*
* Do not add those projects to the javadoc classpath because
* we are going to resolve them with their source instead.
*/
project.javadoc.classpath = project.javadoc.classpath.filter { f ->
false == upstreamProject.configurations.archives.artifacts.files.files.contains(f)
}
/*
* Instead we need the upstream project's javadoc classpath so
* we don't barf on the classes that it references.
*/
project.javadoc.classpath += upstreamProject.javadoc.classpath
} else {
// Link to non-shadowed dependant projects
project.javadoc.dependsOn "${upstreamProject.path}:javadoc"
Copy link
Member

Choose a reason for hiding this comment

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

Cross project task dependencies like this are highly frowned upon in gradle, as they limit the ability to detect when a project needs to be evaluated (which is a goal they are working towards with the new lazy task api). Normally one would only use dependencies to describe this relationship, and use a different configuration than the default/runtime one. Is that possible here?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case I'm just reindenting some code that we've had for a while but I'll look into it.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, sorry I missed that. A followup to look at that is fine then.

String artifactPath = dep.group.replaceAll('\\.', '/') + '/' + dep.name.replaceAll('\\.', '/') + '/' + dep.version
project.javadoc.options.linksOffline artifactsHost + "/javadoc/" + artifactPath, "${upstreamProject.buildDir}/docs/javadoc/"
}
}
project.configurations.compile.dependencies.findAll().toSorted(sortClosure).each(depJavadocClosure)
project.configurations.compileOnly.dependencies.findAll().toSorted(sortClosure).each(depJavadocClosure)
boolean hasShadow = project.plugins.hasPlugin(ShadowPlugin)
project.configurations.compile.dependencies
.findAll()
.toSorted(sortClosure)
.each({ c -> depJavadocClosure(hasShadow, c) })
project.configurations.compileOnly.dependencies
.findAll()
.toSorted(sortClosure)
.each({ c -> depJavadocClosure(hasShadow, c) })
if (hasShadow) {
project.configurations.shadow.dependencies
.findAll()
.toSorted(sortClosure)
.each({ c -> depJavadocClosure(false, c) })
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ dependencies {
compile 'de.thetaphi:forbiddenapis:2.5'
compile 'org.apache.rat:apache-rat:0.11'
compile "org.elasticsearch:jna:4.5.1"
compile 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
testCompile "junit:junit:${props.getProperty('junit')}"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.elasticsearch.gradle

import com.carrotsearch.gradle.junit4.RandomizedTestingTask
import com.github.jengelman.gradle.plugins.shadow.ShadowPlugin
import org.apache.tools.ant.taskdefs.condition.Os
import org.eclipse.jgit.lib.Constants
import org.eclipse.jgit.lib.RepositoryBuilder
Expand All @@ -36,12 +37,14 @@ import org.gradle.api.artifacts.ModuleDependency
import org.gradle.api.artifacts.ModuleVersionIdentifier
import org.gradle.api.artifacts.ProjectDependency
import org.gradle.api.artifacts.ResolvedArtifact
import org.gradle.api.artifacts.SelfResolvingDependency
import org.gradle.api.artifacts.dsl.RepositoryHandler
import org.gradle.api.execution.TaskExecutionGraph
import org.gradle.api.plugins.JavaPlugin
import org.gradle.api.publish.maven.MavenPublication
import org.gradle.api.publish.maven.plugins.MavenPublishPlugin
import org.gradle.api.publish.maven.tasks.GenerateMavenPom
import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.bundling.Jar
import org.gradle.api.tasks.compile.GroovyCompile
import org.gradle.api.tasks.compile.JavaCompile
Expand Down Expand Up @@ -498,7 +501,41 @@ class BuildPlugin implements Plugin<Project> {
}
}
}
project.plugins.withType(ShadowPlugin).whenPluginAdded {
project.publishing {
publications {
nebula(MavenPublication) {
artifact project.tasks.shadowJar
artifactId = project.archivesBaseName
/*
* Configure the pom to include the "shadow" as compile dependencies
* because that is how we're using them but remove all other dependencies
* because they've been shaded into the jar.
*/
pom.withXml { XmlProvider xml ->
Node root = xml.asNode()
root.remove(root.dependencies)
Node dependenciesNode = root.appendNode('dependencies')
project.configurations.shadow.allDependencies.each {
if (false == it instanceof SelfResolvingDependency) {
Node dependencyNode = dependenciesNode.appendNode('dependency')
dependencyNode.appendNode('groupId', it.group)
dependencyNode.appendNode('artifactId', it.name)
dependencyNode.appendNode('version', it.version)
dependencyNode.appendNode('scope', 'compile')
}
}
// Be tidy and remove the element if it is empty
if (dependenciesNode.children.empty) {
root.remove(dependenciesNode)
}
}
}
}
}
}
}

}

/** Adds compiler settings to the project */
Expand Down Expand Up @@ -660,6 +697,28 @@ class BuildPlugin implements Plugin<Project> {
}
}
}
project.plugins.withType(ShadowPlugin).whenPluginAdded {
/*
* When we use the shadow plugin we entirely replace the
* normal jar with the shadow jar so we no longer want to run
* the jar task.
*/
project.tasks.jar.enabled = false
project.tasks.shadowJar {
/*
* Replace the default "shadow" classifier with null
* which will leave the classifier off of the file name.
*/
classifier = null
/*
* Not all cases need service files merged but it is
* better to be safe
*/
mergeServiceFiles()
}
// Make sure we assemble the shadow jar
project.tasks.assemble.dependsOn project.tasks.shadowJar
}
}

/** Returns a closure of common configuration shared by unit and integration tests. */
Expand Down Expand Up @@ -744,6 +803,18 @@ class BuildPlugin implements Plugin<Project> {
}

exclude '**/*$*.class'

project.plugins.withType(ShadowPlugin).whenPluginAdded {
/*
* If we make a shaded jar we test against it.
*/
classpath -= project.tasks.compileJava.outputs.files
classpath -= project.configurations.compile
classpath -= project.configurations.runtime
classpath += project.configurations.shadow
classpath += project.tasks.shadowJar.outputs.files
dependsOn project.tasks.shadowJar
}
}
}

Expand All @@ -766,7 +837,22 @@ class BuildPlugin implements Plugin<Project> {
additionalTest.dependsOn(project.tasks.testClasses)
test.dependsOn(additionalTest)
});
return test

project.plugins.withType(ShadowPlugin).whenPluginAdded {
/*
* We need somewhere to configure dependencies that we don't wish
* to shade into the jar. The shadow plugin creates a "shadow"
* configuration which is *almost* exactly that. It is never
* bundled into the shaded jar but is used for main source
* compilation. Unfortunately, by default it is not used for
* *test* source compilation and isn't used in tests at all. This
* change makes it available for test compilation.
*/
SourceSet testSourceSet = project.sourceSets.findByName('test')
if (testSourceSet != null) {
testSourceSet.compileClasspath += project.configurations.shadow
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment that this is ok for only test because we would not be using shading for rest integ tests or qa tests (since they have no artifacts)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add a comment about how wouldn't make sense for qa projects because they don't have artifacts. I think I should fail the build if you try to add the shadow plugin and the es plugin build plugin. That isn't likely to work and we may as well catch it and warn anyone that tries.

}
}
}

private static configurePrecommit(Project project) {
Expand Down
12 changes: 0 additions & 12 deletions client/benchmark/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@
* under the License.
*/

buildscript {
repositories {
maven {
url 'https://plugins.gradle.org/m2/'
}
}
dependencies {
classpath 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
}
}


apply plugin: 'elasticsearch.build'
// build an uberjar with all benchmarks
apply plugin: 'com.github.johnrengelman.shadow'
Expand Down
99 changes: 0 additions & 99 deletions client/rest-high-level/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,6 @@ import org.elasticsearch.gradle.precommit.PrecommitTasks
import org.gradle.api.XmlProvider
import org.gradle.api.publish.maven.MavenPublication

buildscript {
repositories {
maven {
url 'https://plugins.gradle.org/m2/'
}
}
dependencies {
classpath 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
}
}

apply plugin: 'elasticsearch.build'
apply plugin: 'elasticsearch.rest-test'
apply plugin: 'nebula.maven-base-publish'
Expand All @@ -41,49 +30,6 @@ apply plugin: 'com.github.johnrengelman.shadow'
group = 'org.elasticsearch.client'
archivesBaseName = 'elasticsearch-rest-high-level-client'

publishing {
publications {
nebula(MavenPublication) {
artifact shadowJar
artifactId = archivesBaseName
/*
* Configure the pom to include the "shadow" as compile dependencies
* because that is how we're using them but remove all other dependencies
* because they've been shaded into the jar.
*/
pom.withXml { XmlProvider xml ->
Node root = xml.asNode()
root.remove(root.dependencies)
Node dependenciesNode = root.appendNode('dependencies')
project.configurations.shadow.allDependencies.each {
if (false == it instanceof SelfResolvingDependency) {
Node dependencyNode = dependenciesNode.appendNode('dependency')
dependencyNode.appendNode('groupId', it.group)
dependencyNode.appendNode('artifactId', it.name)
dependencyNode.appendNode('version', it.version)
dependencyNode.appendNode('scope', 'compile')
}
}
}
}
}
}

/*
* We need somewhere to configure dependencies that we don't wish to shade
* into the high level REST client. The shadow plugin creates a "shadow"
* configuration which is *almost* exactly that. It is never bundled into
* the shaded jar but is used for main source compilation. Unfortunately,
* by default it is not used for *test* source compilation and isn't used
* in tests at all. This change makes it available for test compilation.
* A change below makes it available for testing.
*/
sourceSets {
test {
compileClasspath += configurations.shadow
}
}

dependencies {
/*
* Everything in the "shadow" configuration is *not* copied into the
Expand Down Expand Up @@ -118,48 +64,3 @@ forbiddenApisMain {
signaturesURLs += [PrecommitTasks.getResource('/forbidden/http-signatures.txt')]
signaturesURLs += [file('src/main/resources/forbidden/rest-high-level-signatures.txt').toURI().toURL()]
}

shadowJar {
classifier = null
mergeServiceFiles()
}

// We don't need normal jar, we use shadow jar instead
jar.enabled = false
assemble.dependsOn shadowJar

javadoc {
/*
* Bundle all of the javadoc from all of the shaded projects into this one
* so we don't *have* to publish javadoc for all of the "client" jars.
*/
configurations.compile.dependencies.all { Dependency dep ->
Project p = dependencyToProject(dep)
if (p != null) {
evaluationDependsOn(p.path)
source += p.sourceSets.main.allJava
}
}
}

/*
* Use the jar for testing so we have tests of the bundled jar.
* Use the "shadow" configuration for testing because we need things
* in it.
*/
test {
classpath -= compileJava.outputs.files
classpath -= configurations.compile
classpath -= configurations.runtime
classpath += configurations.shadow
classpath += shadowJar.outputs.files
dependsOn shadowJar
}
integTestRunner {
classpath -= compileJava.outputs.files
classpath -= configurations.compile
classpath -= configurations.runtime
classpath += configurations.shadow
classpath += shadowJar.outputs.files
dependsOn shadowJar
}
Loading