Skip to content

Commit

Permalink
Use java.nio.file.Paths instead of strings in JavaBuilder
Browse files Browse the repository at this point in the history
This avoids #4108.

PiperOrigin-RevId: 177096864
  • Loading branch information
cushon authored and Copybara-Service committed Nov 28, 2017
1 parent e5a37f9 commit 0b2352d
Show file tree
Hide file tree
Showing 13 changed files with 272 additions and 200 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ java_library(
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins:processing",
"//src/main/protobuf:worker_protocol_java_proto",
"//third_party:guava",
"//third_party:jsr305",
"//third_party/java/jacoco:core",
"//third_party/java/jdk/langtools:javac",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@

package com.google.devtools.build.buildjar;

import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;

import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.devtools.build.buildjar.instrumentation.JacocoInstrumentationProcessor;
import com.google.devtools.build.buildjar.javac.BlazeJavacArguments;
import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin;
Expand All @@ -30,37 +30,39 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map.Entry;
import javax.annotation.Nullable;

/** All the information needed to perform a single Java library build operation. */
public final class JavaLibraryBuildRequest {
private ImmutableList<String> javacOpts;

/** Where to store source files generated by annotation processors. */
private final String sourceGenDir;
private final Path sourceGenDir;

/** The path to an output jar for source files generated by annotation processors. */
private final String generatedSourcesOutputJar;
private final Path generatedSourcesOutputJar;

/** The path to an output jar for classfiles generated by annotation processors. */
private final String generatedClassOutputJar;
private final Path generatedClassOutputJar;

private final ArrayList<Path> sourceFiles;
private final ImmutableList<String> sourceJars;
private final ImmutableList<Path> sourceJars;

private final ImmutableList<String> sourcePath;
private final ImmutableList<String> classPath;
private final ImmutableList<String> bootClassPath;
private final ImmutableList<String> extClassPath;
private final ImmutableList<Path> sourcePath;
private final ImmutableList<Path> classPath;
private final ImmutableList<Path> bootClassPath;
private final ImmutableList<Path> extClassPath;

private final ImmutableList<String> processorPath;
private final ImmutableList<Path> processorPath;
private final List<String> processorNames;

private final String outputJar;
private final Path outputJar;

private final String classDir;
private final String tempDir;
private final Path classDir;
private final Path tempDir;

private JacocoInstrumentationProcessor jacocoInstrumentationProcessor;

Expand Down Expand Up @@ -103,18 +105,30 @@ public JavaLibraryBuildRequest(
List<BlazeJavaCompilerPlugin> extraPlugins,
DependencyModule.Builder depsBuilder)
throws InvalidCommandLineException, IOException {
depsBuilder.addDirectMappings(optionsParser.getDirectMappings());
depsBuilder.addIndirectMappings(optionsParser.getIndirectMappings());
depsBuilder.addDirectMappings(
optionsParser
.getDirectMappings()
.entrySet()
.stream()
.collect(toImmutableMap(e -> Paths.get(e.getKey()), e -> e.getValue())));
depsBuilder.addIndirectMappings(
optionsParser
.getIndirectMappings()
.entrySet()
.stream()
.collect(toImmutableMap(e -> Paths.get(e.getKey()), e -> e.getValue())));
if (optionsParser.getStrictJavaDeps() != null) {
depsBuilder.setStrictJavaDeps(optionsParser.getStrictJavaDeps());
}
if (optionsParser.getOutputDepsProtoFile() != null) {
depsBuilder.setOutputDepsProtoFile(optionsParser.getOutputDepsProtoFile());
depsBuilder.setOutputDepsProtoFile(Paths.get(optionsParser.getOutputDepsProtoFile()));
}
depsBuilder.addDepsArtifacts(optionsParser.getDepsArtifacts());
depsBuilder.addDepsArtifacts(asPaths(optionsParser.getDepsArtifacts()));
depsBuilder.setPlatformJars(
ImmutableSet.copyOf(
Iterables.concat(optionsParser.getBootClassPath(), optionsParser.getExtClassPath())));
ImmutableSet.<Path>builder()
.addAll(asPaths(optionsParser.getBootClassPath()))
.addAll(asPaths(optionsParser.getExtClassPath()))
.build());
if (optionsParser.reduceClasspath()) {
depsBuilder.setReduceClasspath();
}
Expand Down Expand Up @@ -143,27 +157,19 @@ public JavaLibraryBuildRequest(
this.plugins = pluginsBuilder.build();

this.compressJar = optionsParser.compressJar();
this.sourceFiles = new ArrayList<>(toPaths(optionsParser.getSourceFiles()));
this.sourceJars = ImmutableList.copyOf(optionsParser.getSourceJars());
this.classPath = ImmutableList.copyOf(optionsParser.getClassPath());
this.sourcePath = ImmutableList.copyOf(optionsParser.getSourcePath());
this.bootClassPath = ImmutableList.copyOf(optionsParser.getBootClassPath());
this.extClassPath = ImmutableList.copyOf(optionsParser.getExtClassPath());
this.processorPath = ImmutableList.copyOf(optionsParser.getProcessorPath());
this.sourceFiles = new ArrayList<>(asPaths(optionsParser.getSourceFiles()));
this.sourceJars = asPaths(optionsParser.getSourceJars());
this.classPath = asPaths(optionsParser.getClassPath());
this.sourcePath = asPaths(optionsParser.getSourcePath());
this.bootClassPath = asPaths(optionsParser.getBootClassPath());
this.extClassPath = asPaths(optionsParser.getExtClassPath());
this.processorPath = asPaths(optionsParser.getProcessorPath());
this.processorNames = optionsParser.getProcessorNames();
// Since the default behavior of this tool with no arguments is "rm -fr <classDir>", let's not
// default to ".", shall we?
if (optionsParser.getClassDir() != null) {
this.classDir = optionsParser.getClassDir();
} else {
this.classDir = "classes";
}
if (optionsParser.getTempDir() != null) {
this.tempDir = optionsParser.getTempDir();
} else {
this.tempDir = "_tmp";
}
this.outputJar = optionsParser.getOutputJar();
this.classDir = asPath(firstNonNull(optionsParser.getClassDir(), "classes"));
this.tempDir = asPath(firstNonNull(optionsParser.getTempDir(), "_tmp"));
this.outputJar = asPath(optionsParser.getOutputJar());
for (Entry<String, List<String>> entry : optionsParser.getPostProcessors().entrySet()) {
switch (entry.getKey()) {
case "jacoco":
Expand All @@ -175,9 +181,17 @@ public JavaLibraryBuildRequest(
}
}
this.javacOpts = ImmutableList.copyOf(optionsParser.getJavacOpts());
this.sourceGenDir = optionsParser.getSourceGenDir();
this.generatedSourcesOutputJar = optionsParser.getGeneratedSourcesOutputJar();
this.generatedClassOutputJar = optionsParser.getManifestProtoPath();
this.sourceGenDir = asPath(optionsParser.getSourceGenDir());
this.generatedSourcesOutputJar = asPath(optionsParser.getGeneratedSourcesOutputJar());
this.generatedClassOutputJar = asPath(optionsParser.getManifestProtoPath());
}

private static ImmutableList<Path> asPaths(Collection<String> paths) {
return paths.stream().map(Paths::get).collect(toImmutableList());
}

private static @Nullable Path asPath(@Nullable String path) {
return path != null ? Paths.get(path) : null;
}

public ImmutableList<String> getJavacOpts() {
Expand All @@ -188,19 +202,19 @@ public void setJavacOpts(List<String> javacOpts) {
this.javacOpts = ImmutableList.copyOf(javacOpts);
}

public String getSourceGenDir() {
public Path getSourceGenDir() {
return sourceGenDir;
}

public ImmutableList<String> getSourcePath() {
public ImmutableList<Path> getSourcePath() {
return sourcePath;
}

public String getGeneratedSourcesOutputJar() {
public Path getGeneratedSourcesOutputJar() {
return generatedSourcesOutputJar;
}

public String getGeneratedClassOutputJar() {
public Path getGeneratedClassOutputJar() {
return generatedClassOutputJar;
}

Expand All @@ -209,23 +223,23 @@ public ArrayList<Path> getSourceFiles() {
return sourceFiles;
}

public ImmutableList<String> getSourceJars() {
public ImmutableList<Path> getSourceJars() {
return sourceJars;
}

public ImmutableList<String> getClassPath() {
public ImmutableList<Path> getClassPath() {
return classPath;
}

public ImmutableList<String> getBootClassPath() {
public ImmutableList<Path> getBootClassPath() {
return bootClassPath;
}

public ImmutableList<String> getExtClassPath() {
public ImmutableList<Path> getExtClassPath() {
return extClassPath;
}

public ImmutableList<String> getProcessorPath() {
public ImmutableList<Path> getProcessorPath() {
return processorPath;
}

Expand All @@ -235,15 +249,15 @@ public List<String> getProcessors() {
return processorNames;
}

public String getOutputJar() {
public Path getOutputJar() {
return outputJar;
}

public String getClassDir() {
public Path getClassDir() {
return classDir;
}

public String getTempDir() {
public Path getTempDir() {
return tempDir;
}

Expand All @@ -267,25 +281,25 @@ public ImmutableList<BlazeJavaCompilerPlugin> getPlugins() {
return plugins;
}

public BlazeJavacArguments toBlazeJavacArguments(ImmutableList<String> classPath) {
public BlazeJavacArguments toBlazeJavacArguments(ImmutableList<Path> classPath) {
return BlazeJavacArguments.builder()
.classPath(toPaths(classPath))
.classOutput(Paths.get(getClassDir()))
.bootClassPath(toPaths(Iterables.concat(getBootClassPath(), getExtClassPath())))
.classPath(classPath)
.classOutput(getClassDir())
.bootClassPath(
ImmutableList.<Path>builder()
.addAll(getBootClassPath())
.addAll(getExtClassPath())
.build())
.javacOptions(makeJavacArguments())
.sourceFiles(ImmutableList.copyOf(getSourceFiles()))
.processors(null)
.sourcePath(toPaths(getSourcePath()))
.sourceOutput(getSourceGenDir() != null ? Paths.get(getSourceGenDir()) : null)
.processorPath(toPaths(getProcessorPath()))
.sourcePath(getSourcePath())
.sourceOutput(getSourceGenDir())
.processorPath(getProcessorPath())
.plugins(getPlugins())
.build();
}

static ImmutableList<Path> toPaths(Iterable<String> files) {
return Streams.stream(files).map(Paths::get).collect(toImmutableList());
}

/** Constructs a command line that can be used for a javac invocation. */
ImmutableList<String> makeJavacArguments() {
ImmutableList.Builder<String> javacArguments = ImmutableList.builder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
case "--classpath":
collectClassPathArguments(classPath, argQueue);
break;
// TODO(#970): Consider wether we want to use --sourcepath for resolving of #970.
// TODO(#970): Consider whether we want to use --sourcepath for resolving of #970.
case "--sourcepath":
collectClassPathArguments(sourcePath, argQueue);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.devtools.build.buildjar.javac.FormattedDiagnostic;
import com.google.devtools.build.buildjar.javac.JavacRunner;
import java.io.IOException;
import java.nio.file.Path;

/**
* A variant of SimpleJavaLibraryBuilder that attempts to reduce the compile-time classpath right
Expand All @@ -42,7 +43,7 @@ BlazeJavacResult compileSources(JavaLibraryBuildRequest build, JavacRunner javac
throws IOException {
// Minimize classpath, but only if we're actually compiling some sources (some invocations of
// JavaBuilder are only building resource jars).
ImmutableList<String> compressedClasspath = build.getClassPath();
ImmutableList<Path> compressedClasspath = build.getClassPath();
if (!build.getSourceFiles().isEmpty()) {
compressedClasspath =
build.getDependencyModule().computeStrictClasspath(build.getClassPath());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.HashMap;
Expand All @@ -50,7 +49,7 @@ BlazeJavacResult compileSources(JavaLibraryBuildRequest build, JavacRunner javac
}

protected void prepareSourceCompilation(JavaLibraryBuildRequest build) throws IOException {
Path classDirectory = Paths.get(build.getClassDir());
Path classDirectory = build.getClassDir();
if (Files.exists(classDirectory)) {
try {
// Necessary for local builds in order to discard previous outputs
Expand All @@ -65,7 +64,7 @@ protected void prepareSourceCompilation(JavaLibraryBuildRequest build) throws IO

// Create sourceGenDir if necessary.
if (build.getSourceGenDir() != null) {
Path sourceGenDir = Paths.get(build.getSourceGenDir());
Path sourceGenDir = build.getSourceGenDir();
if (Files.exists(sourceGenDir)) {
try {
cleanupDirectory(sourceGenDir);
Expand Down Expand Up @@ -150,20 +149,19 @@ public void buildJar(JavaLibraryBuildRequest build) throws IOException {
* the build request. Empties the temporary directory, if it exists.
*/
private void setUpSourceJars(JavaLibraryBuildRequest build) throws IOException {
String sourcesDir = build.getTempDir();
Path sourcesDir = build.getTempDir();

Path sourceDirFile = Paths.get(sourcesDir);
if (Files.exists(sourceDirFile)) {
cleanupDirectory(sourceDirFile);
if (Files.exists(sourcesDir)) {
cleanupDirectory(sourcesDir);
}

if (build.getSourceJars().isEmpty()) {
return;
}

final ByteArrayOutputStream protobufMetadataBuffer = new ByteArrayOutputStream();
for (String sourceJar : build.getSourceJars()) {
for (Path root : getJarFileSystem(Paths.get(sourceJar)).getRootDirectories()) {
for (Path sourceJar : build.getSourceJars()) {
for (Path root : getJarFileSystem(sourceJar).getRootDirectories()) {
Files.walkFileTree(
root,
new SimpleFileVisitor<Path>() {
Expand All @@ -181,7 +179,7 @@ public FileVisitResult visitFile(Path path, BasicFileAttributes attrs)
});
}
}
Path output = Paths.get(build.getClassDir(), PROTOBUF_META_NAME);
Path output = build.getClassDir().resolve(PROTOBUF_META_NAME);
if (protobufMetadataBuffer.size() > 0) {
try (OutputStream outputStream = Files.newOutputStream(output)) {
protobufMetadataBuffer.writeTo(outputStream);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void processRequest(JavaLibraryBuildRequest build, JarCreator jar) throws
jar.setNormalize(true);
jar.setCompression(build.compressJar());
Instrumenter instr = new Instrumenter(new OfflineInstrumentationAccessGenerator());
instrumentRecursively(instr, Paths.get(build.getClassDir()));
instrumentRecursively(instr, build.getClassDir());
jar.addDirectory(metadataDir);
if (isNewCoverageImplementation) {
jar.addEntry(coverageInformation, coverageInformation);
Expand Down
Loading

0 comments on commit 0b2352d

Please sign in to comment.