Skip to content

Commit

Permalink
Do not proc apt if possible (#232)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
treblereel and niloc132 authored Feb 1, 2024
1 parent 62aac38 commit 320e5ba
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 8 deletions.
46 changes: 46 additions & 0 deletions build-caching/src/main/java/com/vertispan/j2cl/build/Project.java
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -15,6 +25,10 @@ public class Project implements com.vertispan.j2cl.build.task.Project {

private boolean isJsZip = false;

private File jar;

private Set<String> processors;

public Project(String key) {
this.key = key;
}
Expand Down Expand Up @@ -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<String> 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;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.vertispan.j2cl.build.task;

import java.io.File;
import java.util.List;
import java.util.Set;

/**
*
Expand All @@ -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<String> getProcessors();

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -406,7 +408,7 @@ protected TaskRegistry createTaskRegistry() {
protected Predicate<String> 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")));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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<Input> bytecodeClasspath = scope(project.getDependencies(), com.vertispan.j2cl.build.task.Dependency.Scope.COMPILE)
List<Input> 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<Input> 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<File> extraClasspath = config.getExtraClasspath();
List<File> extraClasspath = new ArrayList<>(config.getExtraClasspath());
Set<String> 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<String> aptProcessors = maybeAddInReactorAptProcessor(inReactorProcessors, processors);

if (!inputSources.getFilesAndHashes().isEmpty()) {
// At least one .java file in sources, compile it (otherwise skip this and just copy resource)

Expand All @@ -86,7 +127,7 @@ public Task resolve(Project project, Config config) {
List<File> 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?
Expand Down Expand Up @@ -119,4 +160,19 @@ public Task resolve(Project project, Config config) {
protected File getGeneratedClassesDir(TaskContext context) {
return context.outputPath().toFile();
}

private Set<String> maybeAddInReactorAptProcessor(List<Input> reactorProcessors, Set<String> processors) {
if (processors.isEmpty()) {
return Collections.emptySet();
}
Set<String> existingProcessors = new HashSet<>(processors);
reactorProcessors.forEach(input -> input.getFilesAndHashes().forEach(file -> {
try (Stream<String> lines = Files.lines(file.getAbsolutePath())) {
lines.forEach(line -> existingProcessors.add(line.trim()));
} catch (IOException e) {
throw new RuntimeException(e);
}
}));
return existingProcessors;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ public Task resolve(Project project, Config config) {
List<Input> 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<Input> classpathHeaders = scope(project.getDependencies(), com.vertispan.j2cl.build.task.Dependency.Scope.COMPILE)
List<Input> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -59,7 +60,7 @@ public Task resolve(Project project, Config config) {
).collect(Collectors.toUnmodifiableList());

List<File> 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?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ public Task resolve(Project project, Config config) {

List<File> extraClasspath = config.getExtraClasspath();

List<Input> compileClasspath = scope(project.getDependencies(), Dependency.Scope.COMPILE).stream()
List<Input> 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());
Expand Down
8 changes: 7 additions & 1 deletion j2cl-tasks/src/main/java/com/vertispan/j2cl/tools/Javac.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -33,7 +34,7 @@ public class Javac {
StandardJavaFileManager fileManager;
private DiagnosticCollector<JavaFileObject> listener;

public Javac(BuildLog log, File generatedClassesPath, List<File> sourcePaths, List<File> classpath, File classesDirFile, File bootstrap) throws IOException {
public Javac(BuildLog log, File generatedClassesPath, List<File> sourcePaths, List<File> classpath, File classesDirFile, File bootstrap, Set<String> processors) throws IOException {
this.log = log;
// for (File file : classpath) {
// System.out.println(file.getAbsolutePath() + " " + file.exists() + " " + file.isDirectory());
Expand All @@ -46,6 +47,11 @@ public Javac(BuildLog log, File generatedClassesPath, List<File> 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);
Expand Down

0 comments on commit 320e5ba

Please sign in to comment.