Gradle deprecation warning of compileOnly not being directly resolved is removed

[breskeby]

While running e.g. ./gradlew precommit we see about 120 deprecation warnings of

The compileOnly configuration has been deprecated for resolution.

From investigating the build I found three places that cause a direct resolution of this configuration:

1. ThirdPartyAuditTask (see https://github.com/elastic/elasticsearch/blob/master/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java#L177-L180)
2. DependenciesInfoTask (see

   elasticsearch/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

   Line 302 in c0ee68b

   task.compileOnlyConfiguration = project.configurations.getByName(JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME)

   )
3. DependencyLicensesTask (see configuration at

   elasticsearch/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

   Lines 292 to 295 in c0ee68b

   	project.tasks.withType(DependencyLicensesTask).named('dependencyLicenses').configure {
   	it.dependencies = project.configurations.getByName(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME).fileCollection { Dependency dependency ->
   	dependency.group.startsWith('org.elasticsearch') == false
   	} - project.configurations.getByName(JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME)

   )

In all of those 3 places we subtract the compileOnly dependencies from the runtimeClasspath configuration. I wonder what is the scenario here that actually makes this happen or a concern? Shouldn't by design all compileOnly entries never end up on the runtimeClasspath?

Here's an overview of how gradle configurations are related by default: https://docs.gradle.org/6.4.1/userguide/java_library_plugin.html#sec:java_library_configurations_graph

[elasticmachine]

Pinging @elastic/es-core-infra (:Core/Infra/Build)

[mark-vieira]

@rjernst noticed this recently. We should just use runtime classpath here since it already excludes compile only dependencies.

[rjernst]

I think this is historical. We used the concept of compileOnly in the ES build before it existed in gradle. When we switched to using gradle's compileOnly, we simply swapped out the old configuration for gradle's, without auditing whether or not it was necessary anymore.

+1 to remove

[rjernst]

While working on #56926, I've realized there is another important side effect of compileOnly in the dependencyLicenses case. For modules/plugins, we put the server project (ie the core elasticsearch jar) in compileOnly, and use this as "provided" (in fact that was the old configuration we had before compileOnly). When we subtract it out of runtime for dependencyLicenses, this is to not have to repeat the licenses for server dependencies in every module/plugin. We need to maintain that functionality somehow.

[breskeby]

@rjernst do you know where in the build these compileOnly dependencies are wired into the runtimeClasspath?

[rjernst]

They are not wired into the compileOnly classpath. Let me give a more concrete example as to what I see happening.

plugins/repository-s3 has a dependency on aws, which transitively depends on jackson. Currently we explicitly add this jackson dep since we don't use transitive deps, but elasticsearch server depends on jackson as well. Anything that server depends on is automatically provided to all modules/plugins because it is loaded in the root classloader. For this reason the server project is added as compileOnly to all modules/plugins: we don't want them to include any jars provided by server in the plugin, because that would cause jarhell. By subtracting compileOnly, we not only remove ES server, but all of its deps. If we remove the subtraction, we would be including jackson in both the built plugin, as well as in dependency license checking. So we need a way to still remove all those common deps.

[rjernst]

Another note: I've found another issue related to this and extended plugins. Due to how we currently load SPI jars from extensible plugins, the plugins implementing SPI extensions get the extended plugin fully on their classpath. This means if they share any dependencies, we must remove those from the latter or we get jarhell. Currently removing compileOnly "fixes" this issue indirectly. One concrete example is eql, which extends painless. Both depend on antlr. I tried changing to only use the spi jar for painless in eql, as compileOnly. This caused antlr to get included in the eql plugin zip, which caused jarhell at runtime.

https://gradle-enterprise.elastic.co/s/s6xv366hjhiay/console-log?task=:docs:integTestRunner

[breskeby]

Thanks for the details explanation and those examples.

plugins/repository-s3 has a dependency on aws, which transitively depends on jackson. Currently we explicitly add this jackson dep since we don't use transitive deps, but elasticsearch server depends on jackson as well.

I see that in repository-s3 the jackson dependencies are added via the compile configuration. It seems to me that this should be added to the compileOnly configuration instead then shouldn't it?

[rjernst]

Yes and no. While that seems like it would solve the immediate issue, it still has the problem that when we switch back to transitive dependencies, we would have no mechanism to remove provided dependencies from the transitive dependencies.