From 320e5ba05f2916cb14839a829f73151776d1755b Mon Sep 17 00:00:00 2001 From: Dmitrii Tikhomirov Date: Wed, 31 Jan 2024 18:21:41 -0800 Subject: [PATCH] Do not proc apt if possible (#232) We can determine that an external dependency is an annotation processor if there's "META-INF/services/javax.annotation.processing.Processor" in the jar, allowing us to avoid unpacking and transpiling it. Processors declared in the reactor continue to build and run as normal, and need not be packaged in a jar. Co-authored-by: Colin Alworth --- .../com/vertispan/j2cl/build/Project.java | 46 ++++++++++++++ .../vertispan/j2cl/build/task/Project.java | 16 +++++ .../j2cl/mojo/AbstractBuildMojo.java | 4 +- .../j2cl/build/provided/BytecodeTask.java | 62 ++++++++++++++++++- .../j2cl/build/provided/J2clTask.java | 4 +- .../j2cl/build/provided/JavacTask.java | 3 +- .../j2cl/build/provided/TurbineTask.java | 5 +- .../java/com/vertispan/j2cl/tools/Javac.java | 8 ++- 8 files changed, 140 insertions(+), 8 deletions(-) diff --git a/build-caching/src/main/java/com/vertispan/j2cl/build/Project.java b/build-caching/src/main/java/com/vertispan/j2cl/build/Project.java index a3bc29b7..d23ca20d 100644 --- a/build-caching/src/main/java/com/vertispan/j2cl/build/Project.java +++ b/build-caching/src/main/java/com/vertispan/j2cl/build/Project.java @@ -1,6 +1,16 @@ package com.vertispan.j2cl.build; +import java.io.BufferedReader; +import java.io.File; +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; /** * Represents a set of sources and the dependencies used to build them. The sourceRoots property @@ -15,6 +25,10 @@ public class Project implements com.vertispan.j2cl.build.task.Project { private boolean isJsZip = false; + private File jar; + + private Set processors; + public Project(String key) { this.key = key; } @@ -82,4 +96,36 @@ public void markJsZip() { public boolean isJsZip() { return isJsZip; } + + @Override + public File getJar() { + return jar; + } + + public void setJar(File jar) { + this.jar = jar; + } + + @Override + public Set getProcessors() { + if (processors == null) { + if (jar == null || isJsZip() || hasSourcesMapped()) { + processors = Collections.emptySet(); + } else if (jar.exists()) { + processors = new HashSet<>(); + try (ZipFile zipFile = new ZipFile(jar)) { + ZipEntry entry = zipFile.getEntry("META-INF/services/javax.annotation.processing.Processor"); + if (entry != null) { + try (InputStreamReader inputStreamReader = new InputStreamReader(zipFile.getInputStream(entry), StandardCharsets.UTF_8); + BufferedReader bufferedReader = new BufferedReader(inputStreamReader)) { + bufferedReader.lines().forEach(line -> processors.add(line.trim())); + } + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + } + return processors; + } } diff --git a/build-caching/src/main/java/com/vertispan/j2cl/build/task/Project.java b/build-caching/src/main/java/com/vertispan/j2cl/build/task/Project.java index cd753744..39c27c28 100644 --- a/build-caching/src/main/java/com/vertispan/j2cl/build/task/Project.java +++ b/build-caching/src/main/java/com/vertispan/j2cl/build/task/Project.java @@ -1,6 +1,8 @@ package com.vertispan.j2cl.build.task; +import java.io.File; import java.util.List; +import java.util.Set; /** * @@ -19,4 +21,18 @@ public interface Project { * @return true if this project should only be used for its JS content, false otherwise */ boolean isJsZip(); + + /** + * If this dependency is a maven external dependency, return the jar file that represents it. + * @return File pointing at the jar, or null if this is a maven reactor project. + */ + File getJar(); + + /** + * If this project is a maven external dependency and has an annotation processors in it, + * return the set of declared processors. If this is a maven reactor project (with annotataion processor or not), + * return an empty set. + */ + Set getProcessors(); + } diff --git a/j2cl-maven-plugin/src/main/java/com/vertispan/j2cl/mojo/AbstractBuildMojo.java b/j2cl-maven-plugin/src/main/java/com/vertispan/j2cl/mojo/AbstractBuildMojo.java index a0c5ead1..47b01495 100644 --- a/j2cl-maven-plugin/src/main/java/com/vertispan/j2cl/mojo/AbstractBuildMojo.java +++ b/j2cl-maven-plugin/src/main/java/com/vertispan/j2cl/mojo/AbstractBuildMojo.java @@ -324,6 +324,8 @@ private Project buildProjectHelper(MavenProject mavenProject, Artifact artifact, } } + child.setJar(mavenDependency.getFile()); + // construct a dependency node for this, and attach it to the new project Dependency dep = new Dependency(); dep.setProject(child); @@ -406,7 +408,7 @@ protected TaskRegistry createTaskRegistry() { protected Predicate withSourceRootFilter() { return path -> new File(path).exists() && !(annotationProcessorMode.pluginShouldExcludeGeneratedAnnotationsDir() - && (path.endsWith("generated-test-sources" + File.separator + "test-annotations") || + && (path.endsWith("generated-test-sources" + File.separator + "test-annotations") || path.endsWith("generated-sources" + File.separator + "annotations"))); } diff --git a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/BytecodeTask.java b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/BytecodeTask.java index adb9030a..14d46cd8 100644 --- a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/BytecodeTask.java +++ b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/BytecodeTask.java @@ -6,12 +6,21 @@ import com.vertispan.j2cl.tools.Javac; import javax.annotation.Nullable; +import java.io.BufferedReader; import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStreamReader; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.PathMatcher; +import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -29,6 +38,9 @@ public class BytecodeTask extends TaskFactory { public static final PathMatcher JAVA_BYTECODE = withSuffix(".class"); public static final PathMatcher NOT_BYTECODE = p -> !JAVA_BYTECODE.matches(p); + public static final PathMatcher APT_PROCESSOR = p -> + p.equals(Paths.get("META-INF", "services", "javax.annotation.processing.Processor")); + @Override public String getOutputType() { return OutputTypes.BYTECODE; @@ -67,14 +79,43 @@ public Task resolve(Project project, Config config) { // be if we had built a jar Input resources = input(project, OutputTypes.INPUT_SOURCES).filter(NOT_BYTECODE); - List bytecodeClasspath = scope(project.getDependencies(), com.vertispan.j2cl.build.task.Dependency.Scope.COMPILE) + List bytecodeClasspath = scope(project.getDependencies() + .stream() + .filter(dependency -> dependency.getProject().getProcessors().isEmpty()).collect(Collectors.toSet()), + com.vertispan.j2cl.build.task.Dependency.Scope.COMPILE) .stream() .map(inputs(OutputTypes.BYTECODE)) .collect(Collectors.toUnmodifiableList()); + List inReactorProcessors = scope(project.getDependencies().stream().filter(dependency -> dependency.getProject().hasSourcesMapped() + && !dependency.getProject().isJsZip()).collect(Collectors.toSet()), + com.vertispan.j2cl.build.task.Dependency.Scope.COMPILE) + .stream() + .map(inputs(OutputTypes.BYTECODE)) + .map(input -> input.filter(APT_PROCESSOR)) + .collect(Collectors.toUnmodifiableList()); + File bootstrapClasspath = config.getBootstrapClasspath(); - List extraClasspath = config.getExtraClasspath(); + List extraClasspath = new ArrayList<>(config.getExtraClasspath()); + Set processors = new HashSet<>(); + project.getDependencies() + .stream() + .map(d -> d.getProject()) + .filter(p -> !p.getProcessors().isEmpty()) + .forEach(p -> { + processors.addAll(p.getProcessors()); + extraClasspath.add(p.getJar()); + }); + return context -> { + /* we don't know if reactor dependency project is apt, before it's compiled, so we need to check it on the fly + 1) there are no processors in the project, so we pass empty set to javac, nothing happens + 2) there are only reactor processors, so we pass empty set to javac, they will be triggered by javac + 3) thee are both reactor and non-reactor processors, so we pass both to javac via set + 4) there are only non-reactor processors, we pass them to javac via set + */ + Set aptProcessors = maybeAddInReactorAptProcessor(inReactorProcessors, processors); + if (!inputSources.getFilesAndHashes().isEmpty()) { // At least one .java file in sources, compile it (otherwise skip this and just copy resource) @@ -86,7 +127,7 @@ public Task resolve(Project project, Config config) { List sourcePaths = inputDirs.getParentPaths().stream().map(Path::toFile).collect(Collectors.toUnmodifiableList()); File generatedClassesDir = getGeneratedClassesDir(context); File classOutputDir = context.outputPath().toFile(); - Javac javac = new Javac(context, generatedClassesDir, sourcePaths, classpathDirs, classOutputDir, bootstrapClasspath); + Javac javac = new Javac(context, generatedClassesDir, sourcePaths, classpathDirs, classOutputDir, bootstrapClasspath, aptProcessors); // TODO convention for mapping to original file paths, provide FileInfo out of Inputs instead of Paths, // automatically relativized? @@ -119,4 +160,19 @@ public Task resolve(Project project, Config config) { protected File getGeneratedClassesDir(TaskContext context) { return context.outputPath().toFile(); } + + private Set maybeAddInReactorAptProcessor(List reactorProcessors, Set processors) { + if (processors.isEmpty()) { + return Collections.emptySet(); + } + Set existingProcessors = new HashSet<>(processors); + reactorProcessors.forEach(input -> input.getFilesAndHashes().forEach(file -> { + try (Stream lines = Files.lines(file.getAbsolutePath())) { + lines.forEach(line -> existingProcessors.add(line.trim())); + } catch (IOException e) { + throw new RuntimeException(e); + } + })); + return existingProcessors; + } } diff --git a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/J2clTask.java b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/J2clTask.java index bd3b87ec..0018e5af 100644 --- a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/J2clTask.java +++ b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/J2clTask.java @@ -42,7 +42,9 @@ public Task resolve(Project project, Config config) { List ownNativeJsSources = Collections.singletonList(input(project, OutputTypes.BYTECODE).filter(NATIVE_JS_SOURCES)); // From our classpath, j2cl is only interested in our compile classpath's bytecode - List classpathHeaders = scope(project.getDependencies(), com.vertispan.j2cl.build.task.Dependency.Scope.COMPILE) + List classpathHeaders = scope(project.getDependencies().stream() + .filter(dep -> dep.getProject().getProcessors().isEmpty()) + .collect(Collectors.toSet()), com.vertispan.j2cl.build.task.Dependency.Scope.COMPILE) .stream() .map(inputs(OutputTypes.STRIPPED_BYTECODE_HEADERS)) // we only want bytecode _changes_, but we'll use the whole dir diff --git a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/JavacTask.java b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/JavacTask.java index 2a36278b..bc7a7d71 100644 --- a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/JavacTask.java +++ b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/JavacTask.java @@ -9,6 +9,7 @@ import java.nio.file.Path; import java.nio.file.PathMatcher; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -59,7 +60,7 @@ public Task resolve(Project project, Config config) { ).collect(Collectors.toUnmodifiableList()); List sourcePaths = ownSources.getParentPaths().stream().map(Path::toFile).collect(Collectors.toUnmodifiableList()); - Javac javac = new Javac(context, null, sourcePaths, classpathDirs, context.outputPath().toFile(), bootstrapClasspath); + Javac javac = new Javac(context, null, sourcePaths, classpathDirs, context.outputPath().toFile(), bootstrapClasspath, Collections.emptySet()); // TODO convention for mapping to original file paths, provide FileInfo out of Inputs instead of Paths, // automatically relativized? diff --git a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java index 691bc444..5c8c0ae5 100644 --- a/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java +++ b/j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java @@ -52,7 +52,10 @@ public Task resolve(Project project, Config config) { List extraClasspath = config.getExtraClasspath(); - List compileClasspath = scope(project.getDependencies(), Dependency.Scope.COMPILE).stream() + List compileClasspath = scope(project.getDependencies().stream() + .filter(dep -> dep.getProject().getProcessors().isEmpty()) + .collect(Collectors.toSet()), Dependency.Scope.COMPILE) + .stream() .map(p -> input(p, OutputTypes.STRIPPED_BYTECODE_HEADERS)) .map(input -> input.filter(JAVA_BYTECODE)) .collect(Collectors.toUnmodifiableList()); diff --git a/j2cl-tasks/src/main/java/com/vertispan/j2cl/tools/Javac.java b/j2cl-tasks/src/main/java/com/vertispan/j2cl/tools/Javac.java index 68b3243c..7b711af1 100644 --- a/j2cl-tasks/src/main/java/com/vertispan/j2cl/tools/Javac.java +++ b/j2cl-tasks/src/main/java/com/vertispan/j2cl/tools/Javac.java @@ -13,6 +13,7 @@ import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Set; import java.util.stream.Collectors; /** @@ -33,7 +34,7 @@ public class Javac { StandardJavaFileManager fileManager; private DiagnosticCollector listener; - public Javac(BuildLog log, File generatedClassesPath, List sourcePaths, List classpath, File classesDirFile, File bootstrap) throws IOException { + public Javac(BuildLog log, File generatedClassesPath, List sourcePaths, List classpath, File classesDirFile, File bootstrap, Set processors) throws IOException { this.log = log; // for (File file : classpath) { // System.out.println(file.getAbsolutePath() + " " + file.exists() + " " + file.isDirectory()); @@ -46,6 +47,11 @@ public Javac(BuildLog log, File generatedClassesPath, List sourcePaths, Li //java 9+ javacOptions.add("--release=8"); } + if (!processors.isEmpty()) { + javacOptions.add("-processor"); + javacOptions.add(String.join(",", processors)); + } + compiler = ToolProvider.getSystemJavaCompiler(); listener = new DiagnosticCollector<>(); fileManager = compiler.getStandardFileManager(listener, null, null);