Skip to content

Commit 12fcdbb

Browse files
authored
Remove project usages in internal tasks for better configuration cache support (#63701) (#64859)
- This brings us closer to be compliant with the gradle configuration cache feature - All internal tasks used during precommit should now be free of invalid project references - configurations are not serializable so we ported it to FileCollection which is enough for the task properties
1 parent 463b16d commit 12fcdbb

13 files changed

+207
-68
lines changed

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LicenseHeadersPrecommitPlugin.groovy

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,33 @@
1919

2020
package org.elasticsearch.gradle.precommit
2121

22+
import org.elasticsearch.gradle.util.GradleUtils
2223
import org.gradle.api.Project
2324
import org.gradle.api.Task
25+
import org.gradle.api.provider.ProviderFactory
26+
import org.gradle.api.tasks.SourceSetContainer
2427
import org.gradle.api.tasks.TaskProvider
2528

29+
import javax.inject.Inject
30+
2631
class LicenseHeadersPrecommitPlugin extends PrecommitPlugin {
32+
33+
private ProviderFactory providerFactory
34+
35+
@Inject
36+
LicenseHeadersPrecommitPlugin(ProviderFactory providerFactory) {
37+
this.providerFactory = providerFactory
38+
}
39+
2740
@Override
2841
TaskProvider<? extends Task> createTask(Project project) {
29-
return project.getTasks().register("licenseHeaders", LicenseHeadersTask.class);
42+
return project.getTasks().register("licenseHeaders", LicenseHeadersTask.class) {
43+
SourceSetContainer sourceSets = GradleUtils.getJavaSourceSets(getProject());
44+
it.getSourceFolders().addAll(
45+
providerFactory.provider() {
46+
return sourceSets.collect { it.allJava }.flatten()
47+
}
48+
)
49+
}
3050
}
3151
}

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LicenseHeadersTask.groovy

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import org.apache.rat.anttasks.SubstringLicenseMatcher
2323
import org.apache.rat.license.SimpleLicenseFamily
2424
import org.elasticsearch.gradle.AntTask
2525
import org.gradle.api.file.FileCollection
26+
import org.gradle.api.provider.ListProperty
2627
import org.gradle.api.tasks.Input
2728
import org.gradle.api.tasks.InputFiles
29+
import org.gradle.api.tasks.Internal
2830
import org.gradle.api.tasks.OutputFile
2931
import org.gradle.api.tasks.SkipWhenEmpty
3032

@@ -35,7 +37,7 @@ import java.nio.file.Files
3537
* <p>
3638
* This is a port of the apache lucene check
3739
*/
38-
class LicenseHeadersTask extends AntTask {
40+
abstract class LicenseHeadersTask extends AntTask {
3941

4042
@OutputFile
4143
File reportFile = new File(project.buildDir, 'reports/licenseHeaders/rat.log')
@@ -69,7 +71,7 @@ class LicenseHeadersTask extends AntTask {
6971
@InputFiles
7072
@SkipWhenEmpty
7173
List<FileCollection> getJavaFiles() {
72-
return project.sourceSets.collect({it.allJava})
74+
return sourceFolders.get()
7375
}
7476

7577
/**
@@ -99,38 +101,38 @@ class LicenseHeadersTask extends AntTask {
99101
// run rat, going to the file
100102
ant.ratReport(reportFile: reportFile.absolutePath, addDefaultLicenseMatchers: true) {
101103
for (FileCollection dirSet : javaFiles) {
102-
for (File dir: dirSet.srcDirs) {
103-
// sometimes these dirs don't exist, e.g. site-plugin has no actual java src/main...
104-
if (dir.exists()) {
105-
ant.fileset(dir: dir, excludes: excludes.join(' '))
106-
}
107-
}
104+
for (File dir : dirSet.srcDirs) {
105+
// sometimes these dirs don't exist, e.g. site-plugin has no actual java src/main...
106+
if (dir.exists()) {
107+
ant.fileset(dir: dir, excludes: excludes.join(' '))
108+
}
109+
}
108110
}
109111

110112
// BSD 4-clause stuff (is disallowed below)
111113
// we keep this here, in case someone adds BSD code for some reason, it should never be allowed.
112114
substringMatcher(licenseFamilyCategory: "BSD4 ",
113-
licenseFamilyName: "Original BSD License (with advertising clause)") {
114-
pattern(substring: "All advertising materials")
115+
licenseFamilyName: "Original BSD License (with advertising clause)") {
116+
pattern(substring: "All advertising materials")
115117
}
116118

117119
// Apache
118120
substringMatcher(licenseFamilyCategory: "AL ",
119-
licenseFamilyName: "Apache") {
120-
// Apache license (ES)
121-
pattern(substring: "Licensed to Elasticsearch under one or more contributor")
121+
licenseFamilyName: "Apache") {
122+
// Apache license (ES)
123+
pattern(substring: "Licensed to Elasticsearch under one or more contributor")
122124
}
123125

124126
// Generated resources
125127
substringMatcher(licenseFamilyCategory: "GEN ",
126-
licenseFamilyName: "Generated") {
127-
// parsers generated by antlr
128-
pattern(substring: "ANTLR GENERATED CODE")
128+
licenseFamilyName: "Generated") {
129+
// parsers generated by antlr
130+
pattern(substring: "ANTLR GENERATED CODE")
129131
}
130132

131133
// Vendored Code
132134
substringMatcher(licenseFamilyCategory: "VEN ",
133-
licenseFamilyName: "Vendored") {
135+
licenseFamilyName: "Vendored") {
134136
pattern(substring: "@notice")
135137
}
136138

@@ -139,7 +141,7 @@ class LicenseHeadersTask extends AntTask {
139141
String category = additional.getKey().substring(0, 5)
140142
String family = additional.getKey().substring(5)
141143
substringMatcher(licenseFamilyCategory: category,
142-
licenseFamilyName: family) {
144+
licenseFamilyName: family) {
143145
pattern(substring: additional.getValue())
144146
}
145147
}
@@ -178,4 +180,7 @@ class LicenseHeadersTask extends AntTask {
178180
throw new IllegalStateException("License header problems were found! Full details: " + reportFile.absolutePath)
179181
}
180182
}
183+
184+
@Internal
185+
abstract ListProperty<FileCollection> getSourceFolders();
181186
}

buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsPrecommitPlugin.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,36 @@
1919

2020
package org.elasticsearch.gradle.precommit;
2121

22+
import org.elasticsearch.gradle.util.GradleUtils;
2223
import org.gradle.api.Project;
2324
import org.gradle.api.Task;
25+
import org.gradle.api.provider.ProviderFactory;
2426
import org.gradle.api.tasks.TaskProvider;
2527

28+
import javax.inject.Inject;
29+
import java.util.stream.Collectors;
30+
2631
public class FilePermissionsPrecommitPlugin extends PrecommitPlugin {
32+
33+
private ProviderFactory providerFactory;
34+
35+
@Inject
36+
public FilePermissionsPrecommitPlugin(ProviderFactory providerFactory) {
37+
this.providerFactory = providerFactory;
38+
}
39+
2740
@Override
2841
public TaskProvider<? extends Task> createTask(Project project) {
29-
return project.getTasks().register("filepermissions", FilePermissionsTask.class);
42+
return project.getTasks()
43+
.register(
44+
"filepermissions",
45+
FilePermissionsTask.class,
46+
filePermissionsTask -> filePermissionsTask.getSources()
47+
.addAll(
48+
providerFactory.provider(
49+
() -> GradleUtils.getJavaSourceSets(project).stream().map(s -> s.getAllSource()).collect(Collectors.toList())
50+
)
51+
)
52+
);
3053
}
3154
}

buildSrc/src/main/java/org/elasticsearch/gradle/precommit/FilePermissionsTask.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,29 @@
2828
import java.util.stream.Collectors;
2929

3030
import org.apache.tools.ant.taskdefs.condition.Os;
31-
import org.elasticsearch.gradle.util.GradleUtils;
3231
import org.gradle.api.DefaultTask;
3332
import org.gradle.api.GradleException;
3433
import org.gradle.api.file.FileCollection;
3534
import org.gradle.api.file.FileTree;
35+
import org.gradle.api.file.ProjectLayout;
36+
import org.gradle.api.provider.ListProperty;
3637
import org.gradle.api.tasks.InputFiles;
38+
import org.gradle.api.tasks.Internal;
3739
import org.gradle.api.tasks.OutputFile;
3840
import org.gradle.api.tasks.SkipWhenEmpty;
3941
import org.gradle.api.tasks.StopExecutionException;
4042
import org.gradle.api.tasks.TaskAction;
4143
import org.gradle.api.tasks.util.PatternFilterable;
4244
import org.gradle.api.tasks.util.PatternSet;
4345

46+
import javax.inject.Inject;
47+
4448
/**
4549
* Checks source files for correct file permissions.
4650
*/
47-
public class FilePermissionsTask extends DefaultTask {
51+
public abstract class FilePermissionsTask extends DefaultTask {
52+
53+
private final ProjectLayout projectLayout;
4854

4955
/**
5056
* A pattern set of which files should be checked.
@@ -57,7 +63,9 @@ public class FilePermissionsTask extends DefaultTask {
5763

5864
private File outputMarker = new File(getProject().getBuildDir(), "markers/filePermissions");
5965

60-
public FilePermissionsTask() {
66+
@Inject
67+
public FilePermissionsTask(ProjectLayout projectLayout) {
68+
this.projectLayout = projectLayout;
6169
setDescription("Checks java source files for correct file permissions");
6270
}
6371

@@ -80,11 +88,11 @@ private static boolean isExecutableFile(File file) {
8088
@InputFiles
8189
@SkipWhenEmpty
8290
public FileCollection getFiles() {
83-
return GradleUtils.getJavaSourceSets(getProject())
91+
return getSources().get()
8492
.stream()
85-
.map(sourceSet -> sourceSet.getAllSource().matching(filesFilter))
93+
.map(sourceTree -> sourceTree.matching(filesFilter))
8694
.reduce(FileTree::plus)
87-
.orElse(getProject().files().getAsFileTree());
95+
.orElse(projectLayout.files().getAsFileTree());
8896
}
8997

9098
@TaskAction
@@ -111,4 +119,6 @@ public File getOutputMarker() {
111119
return outputMarker;
112120
}
113121

122+
@Internal
123+
public abstract ListProperty<FileTree> getSources();
114124
}

buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenPatternsPrecommitPlugin.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,34 @@
1919

2020
package org.elasticsearch.gradle.precommit;
2121

22+
import org.elasticsearch.gradle.util.GradleUtils;
2223
import org.gradle.api.Project;
2324
import org.gradle.api.Task;
25+
import org.gradle.api.provider.ProviderFactory;
2426
import org.gradle.api.tasks.TaskProvider;
2527

28+
import javax.inject.Inject;
29+
import java.util.stream.Collectors;
30+
2631
public class ForbiddenPatternsPrecommitPlugin extends PrecommitPlugin {
32+
33+
private final ProviderFactory providerFactory;
34+
35+
@Inject
36+
public ForbiddenPatternsPrecommitPlugin(ProviderFactory providerFactory) {
37+
this.providerFactory = providerFactory;
38+
}
39+
2740
@Override
2841
public TaskProvider<? extends Task> createTask(Project project) {
29-
return project.getTasks().register("forbiddenPatterns", ForbiddenPatternsTask.class);
42+
return project.getTasks().register("forbiddenPatterns", ForbiddenPatternsTask.class, forbiddenPatternsTask -> {
43+
forbiddenPatternsTask.getSourceFolders()
44+
.addAll(
45+
providerFactory.provider(
46+
() -> GradleUtils.getJavaSourceSets(project).stream().map(s -> s.getAllSource()).collect(Collectors.toList())
47+
)
48+
);
49+
forbiddenPatternsTask.getRootDir().set(project.getRootDir());
50+
});
3051
}
3152
}

buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenPatternsTask.java

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,27 @@
2323
import org.gradle.api.InvalidUserDataException;
2424
import org.gradle.api.file.FileCollection;
2525
import org.gradle.api.file.FileTree;
26-
import org.gradle.api.plugins.JavaPluginConvention;
26+
import org.gradle.api.file.ProjectLayout;
27+
import org.gradle.api.provider.ListProperty;
28+
import org.gradle.api.provider.Property;
29+
import org.gradle.api.provider.Provider;
2730
import org.gradle.api.tasks.Input;
2831
import org.gradle.api.tasks.InputFiles;
32+
import org.gradle.api.tasks.Internal;
33+
import org.gradle.api.tasks.Optional;
2934
import org.gradle.api.tasks.OutputFile;
35+
import org.gradle.api.tasks.PathSensitive;
36+
import org.gradle.api.tasks.PathSensitivity;
3037
import org.gradle.api.tasks.SkipWhenEmpty;
3138
import org.gradle.api.tasks.TaskAction;
3239
import org.gradle.api.tasks.util.PatternFilterable;
3340
import org.gradle.api.tasks.util.PatternSet;
3441

42+
import javax.inject.Inject;
3543
import java.io.File;
3644
import java.io.IOException;
3745
import java.io.UncheckedIOException;
46+
import java.net.URI;
3847
import java.nio.charset.StandardCharsets;
3948
import java.nio.file.Files;
4049
import java.util.AbstractMap;
@@ -51,7 +60,7 @@
5160
/**
5261
* Checks for patterns in source files for the project which are forbidden.
5362
*/
54-
public class ForbiddenPatternsTask extends DefaultTask {
63+
public abstract class ForbiddenPatternsTask extends DefaultTask {
5564

5665
/*
5766
* A pattern set of which files should be checked.
@@ -73,8 +82,11 @@ public class ForbiddenPatternsTask extends DefaultTask {
7382
* The rules: a map from the rule name, to a rule regex pattern.
7483
*/
7584
private final Map<String, String> patterns = new HashMap<>();
85+
private final ProjectLayout projectLayout;
7686

77-
public ForbiddenPatternsTask() {
87+
@Inject
88+
public ForbiddenPatternsTask(ProjectLayout projectLayout) {
89+
this.projectLayout = projectLayout;
7890
setDescription("Checks source files for invalid patterns like nocommits or tabs");
7991
getInputs().property("excludes", filesFilter.getExcludes());
8092
getInputs().property("rules", patterns);
@@ -86,15 +98,14 @@ public ForbiddenPatternsTask() {
8698
}
8799

88100
@InputFiles
101+
@PathSensitive(PathSensitivity.RELATIVE)
89102
@SkipWhenEmpty
90103
public FileCollection getFiles() {
91-
return getProject().getConvention()
92-
.getPlugin(JavaPluginConvention.class)
93-
.getSourceSets()
104+
return getSourceFolders().get()
94105
.stream()
95-
.map(sourceSet -> sourceSet.getAllSource().matching(filesFilter))
106+
.map(sourceFolder -> sourceFolder.matching(filesFilter))
96107
.reduce(FileTree::plus)
97-
.orElse(getProject().files().getAsFileTree());
108+
.orElse(projectLayout.files().getAsFileTree());
98109
}
99110

100111
@TaskAction
@@ -113,7 +124,8 @@ public void checkInvalidPatterns() throws IOException {
113124
.boxed()
114125
.collect(Collectors.toList());
115126

116-
String path = getProject().getRootProject().getProjectDir().toURI().relativize(f.toURI()).toString();
127+
URI baseUri = getRootDir().orElse(projectLayout.getProjectDirectory().getAsFile()).get().toURI();
128+
String path = baseUri.relativize(f.toURI()).toString();
117129
failures.addAll(
118130
invalidLines.stream()
119131
.map(l -> new AbstractMap.SimpleEntry<>(l + 1, lines.get(l)))
@@ -137,7 +149,7 @@ public void checkInvalidPatterns() throws IOException {
137149

138150
@OutputFile
139151
public File getOutputMarker() {
140-
return new File(getProject().getBuildDir(), "markers/" + getName());
152+
return new File(projectLayout.getBuildDirectory().getAsFile().get(), "markers/" + getName());
141153
}
142154

143155
@Input
@@ -164,4 +176,17 @@ public void rule(Map<String, String> props) {
164176
// TODO: fail if pattern contains a newline, it won't work (currently)
165177
patterns.put(name, pattern);
166178
}
179+
180+
@Internal
181+
abstract ListProperty<FileTree> getSourceFolders();
182+
183+
@Internal
184+
abstract Property<File> getRootDir();
185+
186+
@Input
187+
@Optional
188+
Provider<String> getRootDirPath() {
189+
return getRootDir().map(t -> t.toPath().relativize(projectLayout.getProjectDirectory().getAsFile().toPath()).toString());
190+
}
191+
167192
}

0 commit comments

Comments
 (0)