Skip to content

Commit 2dd9270

Browse files
committed
[Build] Rework internal build plugin plugin to work with Isolated Projects
This fixes a general flaw in our build logic where we reach out to configurations of other projects. This is not best practice and breaks future initiatives like IsolatedProjects that allow parallel configuration of subprojects.
1 parent 21b97c2 commit 2dd9270

File tree

74 files changed

+501
-121
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

74 files changed

+501
-121
lines changed

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/BaseInternalPluginBuildPlugin.java

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,14 @@
2020
import org.elasticsearch.gradle.testclusters.ElasticsearchCluster;
2121
import org.elasticsearch.gradle.testclusters.TestClustersPlugin;
2222
import org.elasticsearch.gradle.util.GradleUtils;
23+
import org.gradle.api.Action;
24+
import org.gradle.api.GradleException;
25+
import org.gradle.api.Named;
2326
import org.gradle.api.NamedDomainObjectContainer;
2427
import org.gradle.api.Plugin;
2528
import org.gradle.api.Project;
2629

30+
import java.util.HashMap;
2731
import java.util.Optional;
2832

2933
/**
@@ -47,6 +51,23 @@ public void apply(Project project) {
4751
project.getConfigurations().getByName("testImplementation").getDependencies().clear();
4852
var extension = project.getExtensions().getByType(PluginPropertiesExtension.class);
4953

54+
extension.getExtendedPluginsContainer().whenObjectAdded((Action<Named>) named -> {
55+
if (ExtendedPluginInternal.class.isAssignableFrom(named.getClass()) == false) {
56+
throw new GradleException(
57+
"Using `extendedPlugins` is not supported for internal plugins. Use `extendedPluginProjects` instead."
58+
);
59+
}
60+
});
61+
NamedDomainObjectContainer<ExtendedPluginInternal> container = project.container(
62+
ExtendedPluginInternal.class,
63+
name -> new ExtendedPluginInternal(name)
64+
);
65+
container.whenObjectAdded(
66+
internal -> extension.getExtendedPluginsContainer().add(internal)
67+
);
68+
69+
HashMap<String, String> value = new HashMap<>();
70+
extension.getExtensions().add("extendedPluginProjects", container);
5071
// We've ported this from multiple build scripts where we see this pattern into
5172
// an extension method as a first step of consolidation.
5273
// We might want to port this into a general pattern later on.
@@ -86,11 +107,12 @@ public void doCall() {
86107
NamedDomainObjectContainer<ElasticsearchCluster> testClusters = (NamedDomainObjectContainer<ElasticsearchCluster>) project
87108
.getExtensions()
88109
.getByName(TestClustersPlugin.EXTENSION_NAME);
89-
p.getExtensions().getByType(PluginPropertiesExtension.class).getExtendedPlugins().forEach(pluginName -> {
110+
extension.getExtendedPlugins().forEach(pluginName -> {
90111
// Auto add any dependent modules
91-
findModulePath(project, pluginName).ifPresent(
92-
path -> testClusters.configureEach(elasticsearchCluster -> elasticsearchCluster.module(path))
93-
);
112+
findModulePath(project, pluginName).ifPresent(path -> {
113+
System.out.println("module name:" + pluginName + " path: '" + path + "'");
114+
testClusters.configureEach(elasticsearchCluster -> elasticsearchCluster.module(path));
115+
});
94116
});
95117
});
96118
}
@@ -129,4 +151,20 @@ protected static void addNoticeGeneration(final Project project, PluginPropertie
129151
bundleSpec.from(generateNotice);
130152
}
131153
}
154+
155+
public static class ExtendedPluginInternal extends PluginPropertiesExtension.ExtendedPlugin {
156+
String projectPath;
157+
158+
public ExtendedPluginInternal(String name) {
159+
super(name);
160+
}
161+
162+
public String getProjectPath() {
163+
return projectPath;
164+
}
165+
166+
public void setProjectPath(String projectPath) {
167+
this.projectPath = projectPath;
168+
}
169+
}
132170
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.gradle.internal;
11+
12+
import org.elasticsearch.gradle.plugin.PluginPropertiesExtension;
13+
14+
public class InternalExtendedPluginProjectsExtension {
15+
16+
private final PluginPropertiesExtension pluginPropertiesExtension;
17+
18+
public InternalExtendedPluginProjectsExtension(PluginPropertiesExtension pluginPropertiesExtension) {
19+
this.pluginPropertiesExtension = pluginPropertiesExtension;
20+
}
21+
}

build-tools/src/main/java/org/elasticsearch/gradle/plugin/PluginPropertiesExtension.java

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@
99

1010
package org.elasticsearch.gradle.plugin;
1111

12+
import org.gradle.api.Named;
13+
import org.gradle.api.NamedDomainObjectContainer;
1214
import org.gradle.api.Project;
1315
import org.gradle.api.file.CopySpec;
1416
import org.gradle.api.file.RegularFileProperty;
1517
import org.gradle.api.plugins.BasePluginExtension;
18+
import org.gradle.api.plugins.ExtensionAware;
1619
import org.gradle.api.plugins.ExtraPropertiesExtension;
1720

1821
import java.io.File;
@@ -23,7 +26,7 @@
2326
* A container for plugin properties that will be written to the plugin descriptor, for easy
2427
* manipulation in the gradle DSL.
2528
*/
26-
public class PluginPropertiesExtension {
29+
abstract public class PluginPropertiesExtension implements ExtensionAware {
2730
private String name;
2831

2932
private String version;
@@ -33,7 +36,7 @@ public class PluginPropertiesExtension {
3336
private String classname;
3437

3538
/** Other plugins this plugin extends through SPI */
36-
private List<String> extendedPlugins = new ArrayList<>();
39+
private NamedDomainObjectContainer<ExtendedPlugin> extendedPlugins;
3740

3841
private boolean hasNativeController;
3942

@@ -57,6 +60,7 @@ public class PluginPropertiesExtension {
5760

5861
public PluginPropertiesExtension(Project project) {
5962
this.project = project;
63+
this.extendedPlugins = project.container(ExtendedPlugin.class);
6064
}
6165

6266
public String getName() {
@@ -94,6 +98,10 @@ public void setClassname(String classname) {
9498
}
9599

96100
public List<String> getExtendedPlugins() {
101+
return this.extendedPlugins.getNames().stream().toList();
102+
}
103+
104+
public NamedDomainObjectContainer<ExtendedPlugin> getExtendedPluginsContainer() {
97105
return this.extendedPlugins;
98106
}
99107

@@ -152,7 +160,8 @@ public Project getProject() {
152160
}
153161

154162
public void setExtendedPlugins(List<String> extendedPlugins) {
155-
this.extendedPlugins = extendedPlugins;
163+
List<ExtendedPlugin> list = extendedPlugins.stream().map((name) -> new ExtendedPlugin(name)).toList();
164+
this.extendedPlugins.addAll(list);
156165
}
157166

158167
public void setBundleSpec(CopySpec bundleSpec) {
@@ -162,4 +171,19 @@ public void setBundleSpec(CopySpec bundleSpec) {
162171
public CopySpec getBundleSpec() {
163172
return bundleSpec;
164173
}
174+
175+
176+
public static class ExtendedPlugin implements Named {
177+
178+
private final String name;
179+
public ExtendedPlugin(String name) {
180+
this.name = name;
181+
}
182+
183+
@Override
184+
public String getName() {
185+
return name;
186+
}
187+
}
188+
165189
}

build.gradle

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,8 @@ subprojects { proj ->
236236
}
237237

238238
allprojects {
239-
// We disable this plugin for now till we shaked out the issues we see
240-
// e.g. see https://github.com/elastic/elasticsearch/issues/72169
241-
// apply plugin:'elasticsearch.internal-test-rerun'
239+
apply plugin: 'elasticsearch.eclipse'
240+
apply plugin: 'elasticsearch.formatting'
242241

243242
plugins.withType(BaseInternalPluginBuildPlugin).whenPluginAdded {
244243
project.dependencies {
@@ -249,9 +248,6 @@ allprojects {
249248

250249
ext.bwc_tests_enabled = bwc_tests_enabled
251250

252-
// eclipse configuration
253-
apply plugin: 'elasticsearch.eclipse'
254-
255251
/*
256252
* Allow accessing com/sun/net/httpserver in projects that have
257253
* configured forbidden apis to allow it.
@@ -349,35 +345,6 @@ allprojects {
349345
it.enabled = false
350346
}
351347
}
352-
353-
project.afterEvaluate {
354-
// Ensure similar tasks in dependent projects run first. The projectsEvaluated here is
355-
// important because, while dependencies.all will pickup future dependencies,
356-
// it is not necessarily true that the task exists in both projects at the time
357-
// the dependency is added.
358-
if (project.path == ':test:framework') {
359-
// :test:framework:test cannot run before and after :server:test
360-
return
361-
}
362-
tasks.matching { it.name.equals('integTest') }.configureEach { integTestTask ->
363-
integTestTask.mustRunAfter tasks.matching { it.name.equals("test") }
364-
}
365-
366-
/* configurations.matching { it.canBeResolved }.all { Configuration configuration ->
367-
dependencies.matching { it instanceof ProjectDependency }.all { ProjectDependency dep ->
368-
Project upstreamProject = dep.dependencyProject
369-
if (project.path != upstreamProject?.path) {
370-
for (String taskName : ['test', 'integTest']) {
371-
project.tasks.matching { it.name == taskName }.configureEach { task ->
372-
task.shouldRunAfter(upstreamProject.tasks.matching { upStreamTask -> upStreamTask.name == taskName })
373-
}
374-
}
375-
}
376-
}
377-
}*/
378-
}
379-
380-
apply plugin: 'elasticsearch.formatting'
381348
}
382349

383350

libs/entitlement/qa/entitlement-test-plugin/build.gradle

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ esplugin {
1717
name = 'entitlement-test-plugin'
1818
description = 'A test plugin that invokes methods checked by entitlements'
1919
classname = 'org.elasticsearch.entitlement.qa.test.EntitlementTestPlugin'
20-
extendedPlugins = ['entitled']
20+
extendedPluginProjects {
21+
entitled {
22+
projectPath = ':libs:entitlement:qa:entitled-plugin'
23+
}
24+
}
2125
}
2226

2327
dependencies {

modules/aggregations/build.gradle

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ apply plugin: 'elasticsearch.internal-cluster-test'
1313
esplugin {
1414
description = 'Adds "built in" aggregations to Elasticsearch.'
1515
classname ='org.elasticsearch.aggregations.AggregationsPlugin'
16-
extendedPlugins = ['lang-painless']
16+
extendedPluginProjects {
17+
"lang-painless" {
18+
projectPath = ':modules:lang-painless'
19+
}
20+
}
1721
}
1822

1923
restResources {

modules/analysis-common/build.gradle

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@ apply plugin: 'elasticsearch.internal-cluster-test'
1515
esplugin {
1616
description = 'Adds "built in" analyzers to Elasticsearch.'
1717
classname = 'org.elasticsearch.analysis.common.CommonAnalysisPlugin'
18-
extendedPlugins = ['lang-painless']
18+
extendedPluginProjects {
19+
"lang-painless" {
20+
projectPath = ':modules:lang-painless'
21+
}
22+
}
1923
}
2024

2125
restResources {

modules/ingest-common/build.gradle

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@ apply plugin: 'elasticsearch.internal-cluster-test'
1515
esplugin {
1616
description = 'Module for ingest processors that do not require additional security permissions or have large dependencies and resources'
1717
classname ='org.elasticsearch.ingest.common.IngestCommonPlugin'
18-
extendedPlugins = ['lang-painless']
18+
extendedPluginProjects {
19+
"lang-painless" {
20+
projectPath = ':modules:lang-painless'
21+
}
22+
}
1923
}
2024

2125
dependencies {

modules/runtime-fields-common/build.gradle

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ apply plugin: 'elasticsearch.yaml-rest-compat-test'
1414
esplugin {
1515
description = 'Module for runtime fields features and extensions that have large dependencies'
1616
classname = 'org.elasticsearch.runtimefields.RuntimeFieldsCommonPlugin'
17-
extendedPlugins = ['lang-painless']
17+
extendedPluginProjects {
18+
"lang-painless" {
19+
projectPath = ':modules:lang-painless'
20+
}
21+
}
1822
}
1923

2024
dependencies {

plugins/examples/painless-whitelist/build.gradle

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ esplugin {
1313
name = 'painless-whitelist'
1414
description = 'An example whitelisting additional classes and methods in painless'
1515
classname ='org.elasticsearch.example.painlesswhitelist.MyWhitelistPlugin'
16-
extendedPlugins = ['lang-painless']
16+
extendedPluginProjects {
17+
"lang-painless" {
18+
projectPath = ':modules:lang-painless'
19+
}
20+
}
1721
licenseFile = rootProject.file('AGPL-3.0+SSPL-1.0+ELASTIC-LICENSE-2.0.txt')
1822
noticeFile = rootProject.file('NOTICE.txt')
1923
}

0 commit comments

Comments
 (0)