Skip to content

Commit

Permalink
Replace compile configuration usage with api (7.x backport) (#58721)
Browse files Browse the repository at this point in the history
* Replace compile configuration usage with api (#58451)

- Use java-library instead of plugin to allow api configuration usage
- Remove explicit references to runtime configurations in dependency declarations
- Make test runtime classpath input for testing convention
  - required as java library will by default not have build jar file
  - jar file is now explicit input of the task and gradle will ensure its properly build

* Fix compile usages in 7.x branch
  • Loading branch information
breskeby authored Jun 30, 2020
1 parent 19190c5 commit d952b10
Show file tree
Hide file tree
Showing 125 changed files with 637 additions and 622 deletions.
4 changes: 2 additions & 2 deletions benchmarks/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ archivesBaseName = 'elasticsearch-benchmarks'
test.enabled = false

dependencies {
compile(project(":server")) {
api( project(":server")) {
// JMH ships with the conflicting version 4.6. This prevents us from using jopt-simple in benchmarks (which should be ok) but allows
// us to invoke the JMH uberjar as usual.
exclude group: 'net.sf.jopt-simple', module: 'jopt-simple'
}
compile "org.openjdk.jmh:jmh-core:$versions.jmh"
api "org.openjdk.jmh:jmh-core:$versions.jmh"
annotationProcessor "org.openjdk.jmh:jmh-generator-annprocess:$versions.jmh"
// Dependencies of JMH
runtime 'net.sf.jopt-simple:jopt-simple:4.6'
Expand Down
6 changes: 5 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,11 @@ allprojects {
dependsOn tasks.withType(ComposePull)
}
doLast {
configurations.findAll { it.isCanBeResolved() && !it.name.equals("testCompile") }.each { it.resolve() }
configurations.findAll {
it.isCanBeResolved() &&
it.name.equals("testCompile") == false &&
it.name.equals("compile") == false
}.each { it.resolve() }
}
}

Expand Down
33 changes: 17 additions & 16 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -108,24 +108,25 @@ repositories {
dependencies {
if (project.ext.has("isEclipse") == false || project.ext.isEclipse == false) {
// eclipse is confused if this is set explicitly
compile sourceSets.minimumRuntime.output
api sourceSets.minimumRuntime.output
}

compile localGroovy()

compile 'commons-codec:commons-codec:1.12'
compile 'org.apache.commons:commons-compress:1.19'

compile 'com.netflix.nebula:gradle-extra-configurations-plugin:3.0.3'
compile 'com.netflix.nebula:nebula-publishing-plugin:4.4.4'
compile 'com.netflix.nebula:gradle-info-plugin:7.1.3'
compile 'org.apache.rat:apache-rat:0.11'
compile "org.elasticsearch:jna:5.5.0"
compile 'com.github.jengelman.gradle.plugins:shadow:5.1.0'
compile 'de.thetaphi:forbiddenapis:3.0'
compile 'com.avast.gradle:gradle-docker-compose-plugin:0.8.12'
compile 'org.apache.maven:maven-model:3.6.2'
compile 'com.networknt:json-schema-validator:1.0.36'
api localGroovy()

api 'commons-codec:commons-codec:1.12'
api 'org.apache.commons:commons-compress:1.19'

api 'com.netflix.nebula:gradle-extra-configurations-plugin:3.0.3'
api 'com.netflix.nebula:nebula-publishing-plugin:4.4.4'
api 'com.netflix.nebula:gradle-info-plugin:7.1.3'
api 'org.apache.rat:apache-rat:0.11'
api "org.elasticsearch:jna:5.5.0"
api 'com.github.jengelman.gradle.plugins:shadow:5.1.0'
api 'de.thetaphi:forbiddenapis:3.0'
api 'com.avast.gradle:gradle-docker-compose-plugin:0.8.12'
api 'org.apache.maven:maven-model:3.6.2'
api 'com.networknt:json-schema-validator:1.0.36'

compileOnly "com.puppycrawl.tools:checkstyle:${props.getProperty('checkstyle')}"
testImplementation "com.puppycrawl.tools:checkstyle:${props.getProperty('checkstyle')}"
testFixturesApi "junit:junit:${props.getProperty('junit')}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import org.gradle.testkit.runner.GradleRunner
import org.junit.Rule
import org.junit.rules.TemporaryFolder
import spock.lang.Specification
import spock.lang.Unroll

import java.lang.management.ManagementFactory

Expand All @@ -44,17 +45,14 @@ class EnforceDeprecationFailuresPluginFuncTest extends Specification {
"""
}

def "fails on testCompile resolution"() {
@Unroll
def "fails on #compileConfigName resolution"() {
given:
buildFile << """
apply plugin:'java'
dependencies {
compile "org.acme:some-lib:1.0"
}
task resolve {
doLast {
configurations.testCompile.resolve()
configurations.${compileConfigName}.resolve()
}
}
"""
Expand All @@ -64,25 +62,27 @@ class EnforceDeprecationFailuresPluginFuncTest extends Specification {
assertOutputContains(result.output, """
* What went wrong:
Execution failed for task ':resolve'.
> Resolving configuration testCompile is no longer supported. Use testImplementation instead.
> Resolving configuration $compileConfigName is no longer supported. Use $implementationConfigName instead.
""")
where:
compileConfigName | implementationConfigName
"compile" | "implementation"
"testCompile" | "testImplementation"
}

def "fails on testCompile dependency declaration"() {
@Unroll
def "fails on #compileConfigName dependency declaration"() {
given:
buildFile << """
apply plugin:'java-base'
sourceSets {
test
}
apply plugin:'java'
dependencies {
testCompile "org.acme:some-lib:1.0"
$compileConfigName "org.acme:some-lib:1.0"
}
task resolve {
tasks.register("resolve") {
doLast {
configurations.testCompile.resolve()
configurations.${compileConfigName}.resolve()
}
}
"""
Expand All @@ -92,8 +92,12 @@ Execution failed for task ':resolve'.
assertOutputContains(result.output, """
* What went wrong:
Execution failed for task ':resolve'.
> Declaring dependencies for configuration testCompile is no longer supported. Use testImplementation instead.
> Declaring dependencies for configuration ${compileConfigName} is no longer supported. Use ${implementationConfigName} instead.
""")
where:
compileConfigName | implementationConfigName
"compile" | "implementation"
"testCompile" | "testImplementation"
}

private GradleRunner gradleRunner(String... arguments) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.gradle.api.execution.TaskActionListener;
import org.gradle.api.file.FileCollection;
import org.gradle.api.plugins.BasePlugin;
import org.gradle.api.plugins.JavaLibraryPlugin;
import org.gradle.api.plugins.JavaPlugin;
import org.gradle.api.plugins.JavaPluginExtension;
import org.gradle.api.tasks.SourceSet;
Expand Down Expand Up @@ -83,7 +84,7 @@ public void apply(Project project) {
// apply global test task failure listener
project.getRootProject().getPluginManager().apply(TestFailureReportingPlugin.class);

project.getPluginManager().apply(JavaPlugin.class);
project.getPluginManager().apply(JavaLibraryPlugin.class);

configureConfigurations(project);
configureRepositories(project);
Expand Down Expand Up @@ -141,7 +142,8 @@ public static void configureConfigurations(Project project) {
}
});
};
disableTransitiveDeps.accept(JavaPlugin.COMPILE_CONFIGURATION_NAME);
disableTransitiveDeps.accept(JavaPlugin.API_CONFIGURATION_NAME);
disableTransitiveDeps.accept(JavaPlugin.IMPLEMENTATION_CONFIGURATION_NAME);
disableTransitiveDeps.accept(JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME);
disableTransitiveDeps.accept(JavaPlugin.RUNTIME_ONLY_CONFIGURATION_NAME);
disableTransitiveDeps.accept(JavaPlugin.TEST_IMPLEMENTATION_CONFIGURATION_NAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private void handleDeprecatedConfigurations() {
sourceSetContainer.all(
sourceSet -> {
// TODO: remove that guard once we removed general compile usage from es build
if (sourceSet.getName().equals("test")) {
if (sourceSet.getName().equals("test") || sourceSet.getName().equals("main")) {
failOnCompileConfigurationResolution(sourceSet);
failOnCompileConfigurationDependencyDeclaration(sourceSet);
}
Expand Down Expand Up @@ -78,15 +78,13 @@ private void failOnCompileConfigurationResolution(SourceSet sourceSet) {
.getByName(sourceSet.getCompileConfigurationName())
.getIncoming()
.beforeResolve(resolvableDependencies -> {
if (resolvableDependencies.getDependencies().size() > 0) {
throw new GradleException(
"Resolving configuration "
+ sourceSet.getCompileConfigurationName()
+ " is no longer supported. Use "
+ sourceSet.getImplementationConfigurationName()
+ " instead."
);
}
throw new GradleException(
"Resolving configuration "
+ sourceSet.getCompileConfigurationName()
+ " is no longer supported. Use "
+ sourceSet.getImplementationConfigurationName()
+ " instead."
);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.gradle.api.Task;
import org.gradle.api.file.FileCollection;
import org.gradle.api.file.FileTree;
import org.gradle.api.tasks.Classpath;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.tasks.SourceSet;
Expand Down Expand Up @@ -348,12 +349,8 @@ private boolean isAnnotated(Method method, Class<?> annotation) {
return false;
}

private FileCollection getTestsClassPath() {
// Loading the classes depends on the classpath, so we could make this an input annotated with @Classpath.
// The reason we don't is that test classes are already inputs and while the dependencies are needed to load
// the classes these don't influence the checks done by this task.
// A side effect is that we could mark as up-to-date with missing dependencies, but these will be found when
// running the tests.
@Classpath
public FileCollection getTestsClassPath() {
return Util.getJavaTestSourceSet(getProject()).get().getRuntimeClasspath();
}

Expand Down
4 changes: 2 additions & 2 deletions buildSrc/src/testKit/elasticsearch.build/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ ext.licenseFile = file("LICENSE")
ext.noticeFile = file("NOTICE")

dependencies {
compile "junit:junit:${versions.junit}"
api "junit:junit:${versions.junit}"
// missing classes in thirdparty audit
compile 'org.hamcrest:hamcrest-core:1.3'
api 'org.hamcrest:hamcrest-core:1.3'
}

repositories {
Expand Down
18 changes: 9 additions & 9 deletions client/benchmark/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ mainClassName = 'org.elasticsearch.client.benchmark.BenchmarkMain'
test.enabled = false

dependencies {
compile 'org.apache.commons:commons-math3:3.2'
api 'org.apache.commons:commons-math3:3.2'

compile project(":client:rest")
api project(":client:rest")
// bottleneck should be the client, not Elasticsearch
compile project(path: ':client:client-benchmark-noop-api-plugin')
api project(path: ':client:client-benchmark-noop-api-plugin')
// for transport client
compile project(":server")
compile project(":client:transport")
compile project(path: ':modules:transport-netty4', configuration: 'runtime')
compile project(path: ':modules:reindex', configuration: 'runtime')
compile project(path: ':modules:lang-mustache', configuration: 'runtime')
compile project(path: ':modules:percolator', configuration: 'runtime')
api project(":server")
api project(":client:transport")
api project(':modules:transport-netty4')
api project(':modules:reindex')
api project(':modules:lang-mustache')
api project(':modules:percolator')
}

// No licenses for our benchmark deps (we don't ship benchmarks)
Expand Down
14 changes: 7 additions & 7 deletions client/rest-high-level/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ restResources {
}

dependencies {
compile project(':server')
compile project(':client:rest')
compile project(':modules:mapper-extras')
compile project(':modules:parent-join')
compile project(':modules:aggs-matrix-stats')
compile project(':modules:rank-eval')
compile project(':modules:lang-mustache')
api project(':server')
api project(':client:rest')
api project(':modules:mapper-extras')
api project(':modules:parent-join')
api project(':modules:aggs-matrix-stats')
api project(':modules:rank-eval')
api project(':modules:lang-mustache')

testImplementation project(':client:test')
testImplementation project(':test:framework')
Expand Down
12 changes: 6 additions & 6 deletions client/rest/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ group = 'org.elasticsearch.client'
archivesBaseName = 'elasticsearch-rest-client'

dependencies {
compile "org.apache.httpcomponents:httpclient:${versions.httpclient}"
compile "org.apache.httpcomponents:httpcore:${versions.httpcore}"
compile "org.apache.httpcomponents:httpasyncclient:${versions.httpasyncclient}"
compile "org.apache.httpcomponents:httpcore-nio:${versions.httpcore}"
compile "commons-codec:commons-codec:${versions.commonscodec}"
compile "commons-logging:commons-logging:${versions.commonslogging}"
api "org.apache.httpcomponents:httpclient:${versions.httpclient}"
api "org.apache.httpcomponents:httpcore:${versions.httpcore}"
api "org.apache.httpcomponents:httpasyncclient:${versions.httpasyncclient}"
api "org.apache.httpcomponents:httpcore-nio:${versions.httpcore}"
api "commons-codec:commons-codec:${versions.commonscodec}"
api "commons-logging:commons-logging:${versions.commonslogging}"

testImplementation project(":client:test")
testImplementation "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
Expand Down
12 changes: 6 additions & 6 deletions client/sniffer/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ group = 'org.elasticsearch.client'
archivesBaseName = 'elasticsearch-rest-client-sniffer'

dependencies {
compile project(":client:rest")
compile "org.apache.httpcomponents:httpclient:${versions.httpclient}"
compile "org.apache.httpcomponents:httpcore:${versions.httpcore}"
compile "commons-codec:commons-codec:${versions.commonscodec}"
compile "commons-logging:commons-logging:${versions.commonslogging}"
compile "com.fasterxml.jackson.core:jackson-core:${versions.jackson}"
api project(":client:rest")
api "org.apache.httpcomponents:httpclient:${versions.httpclient}"
api "org.apache.httpcomponents:httpcore:${versions.httpcore}"
api "commons-codec:commons-codec:${versions.commonscodec}"
api "commons-logging:commons-logging:${versions.commonslogging}"
api "com.fasterxml.jackson.core:jackson-core:${versions.jackson}"

testImplementation project(":client:test")
testImplementation "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
Expand Down
8 changes: 4 additions & 4 deletions client/test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ sourceCompatibility = JavaVersion.VERSION_1_8
group = "${group}.client.test"

dependencies {
compile "org.apache.httpcomponents:httpcore:${versions.httpcore}"
compile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
compile "junit:junit:${versions.junit}"
compile "org.hamcrest:hamcrest:${versions.hamcrest}"
api "org.apache.httpcomponents:httpcore:${versions.httpcore}"
api "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
api "junit:junit:${versions.junit}"
api "org.hamcrest:hamcrest:${versions.hamcrest}"
}

tasks.named('forbiddenApisMain').configure {
Expand Down
14 changes: 7 additions & 7 deletions client/transport/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ apply plugin: 'nebula.maven-base-publish'
group = 'org.elasticsearch.client'

dependencies {
compile project(":server")
compile project(":modules:transport-netty4")
compile project(":modules:reindex")
compile project(":modules:lang-mustache")
compile project(":modules:percolator")
compile project(":modules:parent-join")
compile project(":modules:rank-eval")
api project(":server")
api project(":modules:transport-netty4")
api project(":modules:reindex")
api project(":modules:lang-mustache")
api project(":modules:percolator")
api project(":modules:parent-join")
api project(":modules:rank-eval")
testImplementation "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
testImplementation "junit:junit:${versions.junit}"
testImplementation "org.hamcrest:hamcrest:${versions.hamcrest}"
Expand Down
6 changes: 3 additions & 3 deletions distribution/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -287,21 +287,21 @@ configure(subprojects.findAll { ['archives', 'packages'].contains(it.name) }) {
copySpec {
// delay by using closures, since they have not yet been configured, so no jar task exists yet
from { project(':server').jar }
from { project(':server').configurations.runtime }
from { project(':server').configurations.runtimeClasspath }
from { project(':libs:elasticsearch-plugin-classloader').jar }
from { project(':distribution:tools:java-version-checker').jar }
from { project(':distribution:tools:launchers').jar }
into('tools/plugin-cli') {
from { project(':distribution:tools:plugin-cli').jar }
from { project(':distribution:tools:plugin-cli').configurations.runtime }
from { project(':distribution:tools:plugin-cli').configurations.runtimeClasspath }
}
into('tools/keystore-cli') {
from { project(':distribution:tools:keystore-cli').jar }
}
if (oss == false) {
into('tools/security-cli') {
from { project(':x-pack:plugin:security:cli').jar }
from { project(':x-pack:plugin:security:cli').configurations.compile }
from { project(':x-pack:plugin:security:cli').configurations.compileClasspath }
}
}
}
Expand Down
Loading

0 comments on commit d952b10

Please sign in to comment.