Skip to content

Commit c73329c

Browse files
abashevabashev
committed
Run formatDiff without the project when it executes
formatDiff called getProject() when it ran, for the project directory and for the root project's JavaFormatExtension, which loads the formatter. With the configuration cache on, Gradle refuses that, and the task failed on every run (#193). The task now gets the project directory and the formatter classpath as properties when it is configured. Loading the formatter moves from JavaFormatExtension to FormatterServiceLoader, which keeps it loaded for as long as its jars stay the same: a build that reuses its cache entry creates no extension to remember it in. One formatter at a time, so a daemon that sees a new formatter version drops the old one. serviceLoad() keeps working for build scripts that call it.
1 parent a44fd7d commit c73329c

3 files changed

Lines changed: 159 additions & 85 deletions

File tree

‎gradle-open-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatPlugin.java‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,22 @@
1717
package com.palantir.javaformat.gradle;
1818

1919
import com.palantir.javaformat.bootstrap.NativeImageFormatterService;
20-
import com.palantir.javaformat.java.FormatterService;
2120
import java.io.File;
2221
import java.io.IOException;
2322
import org.gradle.api.DefaultTask;
2423
import org.gradle.api.Plugin;
2524
import org.gradle.api.Project;
25+
import org.gradle.api.artifacts.Configuration;
26+
import org.gradle.api.file.ConfigurableFileCollection;
27+
import org.gradle.api.file.DirectoryProperty;
2628
import org.gradle.api.file.FileCollection;
2729
import org.gradle.api.file.RegularFileProperty;
2830
import org.gradle.api.logging.Logger;
2931
import org.gradle.api.logging.Logging;
3032
import org.gradle.api.provider.Provider;
33+
import org.gradle.api.tasks.Classpath;
3134
import org.gradle.api.tasks.InputFile;
35+
import org.gradle.api.tasks.Internal;
3236
import org.gradle.api.tasks.Nested;
3337
import org.gradle.api.tasks.PathSensitive;
3438
import org.gradle.api.tasks.PathSensitivity;
@@ -53,13 +57,20 @@ public void apply(Project project) {
5357
// TODO(dfox): in the future we may want to offer a simple 'format' task so people don't need to use
5458
// spotless to try out our formatter
5559
project.getTasks().register("formatDiff", FormatDiffTask.class, task -> {
60+
task.getProjectDirectory().set(project.getLayout().getProjectDirectory());
5661
if (getNativeImageSupport().isNativeImageConfigured()) {
5762
task.getNativeImage().fileProvider(getNativeImplConfiguration(project));
63+
} else {
64+
task.getFormatterClasspath().from(getImplConfiguration(project));
5865
}
5966
});
6067
});
6168
}
6269

70+
private static Provider<Configuration> getImplConfiguration(Project project) {
71+
return project.getRootProject().getConfigurations().named(PalantirJavaFormatProviderPlugin.CONFIGURATION_NAME);
72+
}
73+
6374
private static Provider<File> getNativeImplConfiguration(Project project) {
6475
return (project.getRootProject()
6576
.getConfigurations()
@@ -78,25 +89,36 @@ public abstract static class FormatDiffTask extends DefaultTask {
7889
@PathSensitive(PathSensitivity.NAME_ONLY)
7990
abstract RegularFileProperty getNativeImage();
8091

92+
/** The jars of the Java-based formatter, which runs when the native image is not configured. */
93+
@Classpath
94+
abstract ConfigurableFileCollection getFormatterClasspath();
95+
96+
/**
97+
* The project whose changed lines are formatted. The task reads it here rather than from {@code getProject()},
98+
* which a task must not call when it runs with the configuration cache.
99+
*/
100+
@Internal
101+
abstract DirectoryProperty getProjectDirectory();
102+
81103
public FormatDiffTask() {
82104
setDescription("Format only chunks of files that appear in git diff");
83105
setGroup("Formatting");
84106
}
85107

86108
@TaskAction
87109
public final void formatDiff() throws IOException, InterruptedException {
110+
var projectDir = getProjectDirectory().get().getAsFile().toPath();
88111
if (getNativeImage().isPresent()) {
89112
log.info("Using the native-image formatter");
90113
FormatDiff.formatDiff(
91-
getProject().getProjectDir().toPath(),
114+
projectDir,
92115
new NativeImageFormatterService(
93116
getNativeImage().get().getAsFile().toPath()));
94117
} else {
95118
log.info("Using the Java-based formatter");
96-
JavaFormatExtension extension =
97-
getProject().getRootProject().getExtensions().getByType(JavaFormatExtension.class);
98-
FormatterService formatterService = extension.serviceLoad();
99-
FormatDiff.formatDiff(getProject().getProjectDir().toPath(), formatterService);
119+
FormatDiff.formatDiff(
120+
projectDir,
121+
FormatterServiceLoader.load(getFormatterClasspath().getFiles()));
100122
}
101123
}
102124
}

‎gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/FormatterServiceLoader.java‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
package com.palantir.javaformat.gradle;
16+
17+
import com.google.common.collect.Iterables;
18+
import com.palantir.javaformat.java.FormatterService;
19+
import java.io.File;
20+
import java.io.UncheckedIOException;
21+
import java.net.MalformedURLException;
22+
import java.net.URL;
23+
import java.net.URLClassLoader;
24+
import java.util.Collection;
25+
import java.util.HashMap;
26+
import java.util.List;
27+
import java.util.Map;
28+
import java.util.ServiceLoader;
29+
import java.util.stream.Collectors;
30+
import org.gradle.api.GradleException;
31+
32+
/**
33+
* Loads the Java-based formatter from its jars, in a class loader of its own, so that the formatter's dependencies do
34+
* not meet the build's.
35+
*
36+
* <p>The formatter stays loaded for as long as its jars stay the same, across the tasks and builds of one daemon. The
37+
* tasks have nowhere else to keep it: a build that reuses its configuration cache entry creates no project and no
38+
* {@link JavaFormatExtension}, and the Spotless step comes back from the cache with nothing that does not serialize.
39+
*/
40+
public final class FormatterServiceLoader {
41+
/**
42+
* The javac packages the formatter reads. A plain Gradle JVM exports none of them, and the formatter then fails
43+
* on its first file with an IllegalAccessError that names a class and a module and nothing the user can change.
44+
* The check below fails before the first file instead, with the two settings that make it work.
45+
*/
46+
private static final List<String> JAVAC_PACKAGES = List.of(
47+
"com.sun.tools.javac.api",
48+
"com.sun.tools.javac.file",
49+
"com.sun.tools.javac.parser",
50+
"com.sun.tools.javac.tree",
51+
"com.sun.tools.javac.util");
52+
53+
private static final String DOCS = "https://openjavaformat.dev/get-started/gradle/#choose-how-the-formatter-runs";
54+
55+
/**
56+
* The formatter loaded last, by the jars it came from. One at a time: a build has one formatter classpath, and a
57+
* daemon that outlives a change of formatter version should not keep the old one.
58+
*/
59+
private static final Map<List<Jar>, FormatterService> LOADED = new HashMap<>();
60+
61+
private FormatterServiceLoader() {}
62+
63+
/** The formatter in these jars: the one loaded last if they have not changed since, a new one otherwise. */
64+
public static synchronized FormatterService load(Collection<File> jars) {
65+
var key = jars.stream().map(Jar::of).toList();
66+
if (!LOADED.containsKey(key)) {
67+
LOADED.clear();
68+
}
69+
return LOADED.computeIfAbsent(key, _key -> loadUncached(jars));
70+
}
71+
72+
@SuppressWarnings("for-rollout:NullAway")
73+
private static FormatterService loadUncached(Collection<File> jars) {
74+
URL[] jarUris = jars.stream()
75+
.map(file -> {
76+
try {
77+
return file.toURI().toURL();
78+
} catch (MalformedURLException e) {
79+
throw new UncheckedIOException("Unable to convert URI to URL: " + file, e);
80+
}
81+
})
82+
.toArray(URL[]::new);
83+
84+
ClassLoader classLoader = new URLClassLoader(jarUris, FormatterService.class.getClassLoader());
85+
checkJavacIsExportedTo(classLoader);
86+
return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader));
87+
}
88+
89+
/**
90+
* Fails with the fix spelt out when this JVM does not export javac's internals to the formatter's class loader:
91+
* either the native binary, which runs outside the JVM, or the {@code --add-exports} flags on the Gradle JVM.
92+
*/
93+
private static void checkJavacIsExportedTo(ClassLoader formatterClassLoader) {
94+
Module jdkCompiler = ModuleLayer.boot()
95+
.findModule("jdk.compiler")
96+
.orElseThrow(() -> new GradleException("open-java-format needs the module jdk.compiler, which this"
97+
+ " Gradle JVM does not have: run Gradle on a JDK, not a JRE. See " + DOCS));
98+
Module formatter = formatterClassLoader.getUnnamedModule();
99+
List<String> notExported = JAVAC_PACKAGES.stream()
100+
.filter(javacPackage -> !jdkCompiler.isExported(javacPackage, formatter))
101+
.toList();
102+
if (notExported.isEmpty()) {
103+
return;
104+
}
105+
String addExports = JAVAC_PACKAGES.stream()
106+
.map(javacPackage -> "--add-exports jdk.compiler/" + javacPackage + "=ALL-UNNAMED")
107+
.collect(Collectors.joining(" "));
108+
throw new GradleException("""
109+
open-java-format cannot run inside this Gradle JVM: module jdk.compiler does not export %s to it. \
110+
Set one of these in gradle.properties:
111+
112+
openjavaformat.native.formatter=true
113+
runs the formatter as a native binary, outside the Gradle JVM (Linux with glibc, macOS, \
114+
Windows on x86-64)
115+
116+
org.gradle.jvmargs=%s
117+
opens javac's packages to the Gradle JVM; if the file already sets org.gradle.jvmargs, add \
118+
the flags to that line
119+
120+
See %s\
121+
""".formatted(String.join(", ", notExported), addExports, DOCS));
122+
}
123+
124+
/** A jar as it was when the formatter was loaded from it: rebuilt in place, it changes size or time. */
125+
private record Jar(File file, long length, long lastModified) {
126+
static Jar of(File file) {
127+
return new Jar(file, file.length(), file.lastModified());
128+
}
129+
}
130+
}

‎gradle-open-java-format/src/main/java/com/palantir/javaformat/gradle/JavaFormatExtension.java‎

Lines changed: 1 addition & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -17,96 +17,18 @@
1717
package com.palantir.javaformat.gradle;
1818

1919
import com.google.common.base.Suppliers;
20-
import com.google.common.collect.ImmutableList;
21-
import com.google.common.collect.Iterables;
2220
import com.palantir.javaformat.java.FormatterService;
23-
import java.io.UncheckedIOException;
24-
import java.net.MalformedURLException;
25-
import java.net.URL;
26-
import java.net.URLClassLoader;
27-
import java.util.List;
28-
import java.util.ServiceLoader;
2921
import java.util.function.Supplier;
30-
import java.util.stream.Collectors;
31-
import org.gradle.api.GradleException;
3222
import org.gradle.api.artifacts.Configuration;
3323

3424
public class JavaFormatExtension {
35-
/**
36-
* The javac packages the formatter reads. A plain Gradle JVM exports none of them, and the formatter then fails
37-
* on its first file with an IllegalAccessError that names a class and a module and nothing the user can change.
38-
* The check below fails before the first file instead, with the two settings that make it work.
39-
*/
40-
private static final ImmutableList<String> JAVAC_PACKAGES = ImmutableList.of(
41-
"com.sun.tools.javac.api",
42-
"com.sun.tools.javac.file",
43-
"com.sun.tools.javac.parser",
44-
"com.sun.tools.javac.tree",
45-
"com.sun.tools.javac.util");
46-
47-
private static final String DOCS = "https://openjavaformat.dev/get-started/gradle/#choose-how-the-formatter-runs";
48-
49-
private final Configuration configuration;
5025
private final Supplier<FormatterService> memoizedService;
5126

5227
public JavaFormatExtension(Configuration configuration) {
53-
this.configuration = configuration;
54-
this.memoizedService = Suppliers.memoize(this::serviceLoadInternal);
28+
this.memoizedService = Suppliers.memoize(() -> FormatterServiceLoader.load(configuration.getFiles()));
5529
}
5630

5731
public FormatterService serviceLoad() {
5832
return memoizedService.get();
5933
}
60-
61-
@SuppressWarnings("for-rollout:NullAway")
62-
private FormatterService serviceLoadInternal() {
63-
URL[] jarUris = configuration.getFiles().stream()
64-
.map(file -> {
65-
try {
66-
return file.toURI().toURL();
67-
} catch (MalformedURLException e) {
68-
throw new UncheckedIOException("Unable to convert URI to URL: " + file, e);
69-
}
70-
})
71-
.toArray(URL[]::new);
72-
73-
ClassLoader classLoader = new URLClassLoader(jarUris, FormatterService.class.getClassLoader());
74-
checkJavacIsExportedTo(classLoader);
75-
return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader));
76-
}
77-
78-
/**
79-
* Fails with the fix spelt out when this JVM does not export javac's internals to the formatter's class loader:
80-
* either the native binary, which runs outside the JVM, or the {@code --add-exports} flags on the Gradle JVM.
81-
*/
82-
private static void checkJavacIsExportedTo(ClassLoader formatterClassLoader) {
83-
Module jdkCompiler = ModuleLayer.boot()
84-
.findModule("jdk.compiler")
85-
.orElseThrow(() -> new GradleException("open-java-format needs the module jdk.compiler, which this"
86-
+ " Gradle JVM does not have: run Gradle on a JDK, not a JRE. See " + DOCS));
87-
Module formatter = formatterClassLoader.getUnnamedModule();
88-
List<String> notExported = JAVAC_PACKAGES.stream()
89-
.filter(javacPackage -> !jdkCompiler.isExported(javacPackage, formatter))
90-
.collect(Collectors.toList());
91-
if (notExported.isEmpty()) {
92-
return;
93-
}
94-
String addExports = JAVAC_PACKAGES.stream()
95-
.map(javacPackage -> "--add-exports jdk.compiler/" + javacPackage + "=ALL-UNNAMED")
96-
.collect(Collectors.joining(" "));
97-
throw new GradleException("""
98-
open-java-format cannot run inside this Gradle JVM: module jdk.compiler does not export %s to it. \
99-
Set one of these in gradle.properties:
100-
101-
openjavaformat.native.formatter=true
102-
runs the formatter as a native binary, outside the Gradle JVM (Linux with glibc, macOS, \
103-
Windows on x86-64)
104-
105-
org.gradle.jvmargs=%s
106-
opens javac's packages to the Gradle JVM; if the file already sets org.gradle.jvmargs, add \
107-
the flags to that line
108-
109-
See %s\
110-
""".formatted(String.join(", ", notExported), addExports, DOCS));
111-
}
11234
}

0 commit comments

Comments
 (0)