Skip to content

Commit

Permalink
Create temp directories instead of making Bazel create them for us
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 335459680
  • Loading branch information
@cushon @copybara-github
cushon authored and copybara-github committed Oct 5, 2020
1 parent 360265b commit 2350239
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 102 deletions.
38 changes: 29 additions & 9 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@

package com.google.devtools.build.buildjar;

import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.MoreFiles;
import com.google.devtools.build.buildjar.instrumentation.JacocoInstrumentationProcessor;
import com.google.devtools.build.buildjar.javac.BlazeJavacArguments;
import com.google.devtools.build.buildjar.javac.JavacOptions;
Expand All @@ -29,6 +31,7 @@
import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule;
import com.google.devtools.build.buildjar.javac.plugins.processing.AnnotationProcessingModule;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
Expand Down Expand Up @@ -133,11 +136,11 @@ public JavaLibraryBuildRequest(
depsBuilder.setTargetLabel(optionsParser.getTargetLabel());
}
this.dependencyModule = depsBuilder.build();
this.sourceGenDir =
deriveDirectory(optionsParser.getTargetLabel(), optionsParser.getOutputJar(), "_sources");

AnnotationProcessingModule.Builder processingBuilder = AnnotationProcessingModule.builder();
if (optionsParser.getSourceGenDir() != null) {
processingBuilder.setSourceGenDir(Paths.get(optionsParser.getSourceGenDir()));
}
processingBuilder.setSourceGenDir(sourceGenDir);
if (optionsParser.getManifestProtoPath() != null) {
processingBuilder.setManifestProtoPath(Paths.get(optionsParser.getManifestProtoPath()));
}
Expand All @@ -162,10 +165,10 @@ public JavaLibraryBuildRequest(
this.processorPath = asPaths(optionsParser.getProcessorPath());
this.processorNames = optionsParser.getProcessorNames();
this.builtinProcessorNames = ImmutableSet.copyOf(optionsParser.getBuiltinProcessorNames());
// Since the default behavior of this tool with no arguments is "rm -fr <classDir>", let's not
// default to ".", shall we?
this.classDir = asPath(firstNonNull(optionsParser.getClassDir(), "classes"));
this.tempDir = asPath(firstNonNull(optionsParser.getTempDir(), "_tmp"));
this.classDir =
deriveDirectory(optionsParser.getTargetLabel(), optionsParser.getOutputJar(), "_classes");
this.tempDir =
deriveDirectory(optionsParser.getTargetLabel(), optionsParser.getOutputJar(), "_tmp");
this.outputJar = asPath(optionsParser.getOutputJar());
this.nativeHeaderOutput = asPath(optionsParser.getNativeHeaderOutput());
for (Map.Entry<String, List<String>> entry : optionsParser.getPostProcessors().entrySet()) {
Expand All @@ -179,13 +182,30 @@ public JavaLibraryBuildRequest(
}
}
this.javacOpts = ImmutableList.copyOf(optionsParser.getJavacOpts());
this.sourceGenDir = asPath(optionsParser.getSourceGenDir());
this.generatedSourcesOutputJar = asPath(optionsParser.getGeneratedSourcesOutputJar());
this.generatedClassOutputJar = asPath(optionsParser.getManifestProtoPath());
this.targetLabel = optionsParser.getTargetLabel();
this.injectingRuleKind = optionsParser.getInjectingRuleKind();
}

/**
* Derive a temporary directory path based on the path to the output jar, to avoid breaking
* fragile assumptions made by the implementation of javahotswap.
*/
// TODO(b/169793789): kill this with fire if javahotswap starts using jars instead of classes
@VisibleForTesting
static Path deriveDirectory(String label, String outputJar, String suffix) throws IOException {
if (outputJar == null || label == null) {
// TODO(b/169944970): require --output to be set and fix up affected tests
return Files.createTempDirectory(suffix);
}
checkArgument(label.contains(":"), label);
Path path = Paths.get(outputJar);
String name = MoreFiles.getNameWithoutExtension(path);
String base = label.substring(label.lastIndexOf(':') + 1);
return path.resolveSibling("_javac").resolve(base).resolve(name + suffix);
}

private static ImmutableList<Path> asPaths(Collection<String> paths) {
return paths.stream().map(Paths::get).collect(toImmutableList());
}
Expand Down
28 changes: 3 additions & 25 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ public enum ReduceClasspathMode {
private int fullClasspathLength = -1;
private int reducedClasspathLength = -1;

private String sourceGenDir;
private String generatedSourcesOutputJar;
private String manifestProtoPath;

Expand All @@ -89,9 +88,6 @@ public enum ReduceClasspathMode {
private String outputJar;
@Nullable private String nativeHeaderOutput;

private String classDir;
private String tempDir;

private final Map<String, List<String>> postProcessors = new LinkedHashMap<>();

private boolean compressJar;
Expand Down Expand Up @@ -166,9 +162,6 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
case "--reduced_classpath_length":
reducedClasspathLength = Integer.parseInt(getArgument(argQueue, arg));
break;
case "--sourcegendir":
sourceGenDir = getArgument(argQueue, arg);
break;
case "--generated_sources_output":
generatedSourcesOutputJar = getArgument(argQueue, arg);
break;
Expand Down Expand Up @@ -215,13 +208,10 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
nativeHeaderOutput = getArgument(argQueue, arg);
break;
case "--classdir":
classDir = getArgument(argQueue, arg);
break;
case "--tempdir":
tempDir = getArgument(argQueue, arg);
break;
case "--gendir":
// TODO(bazel-team) - remove when Bazel no longer passes this flag to buildjar.
case "--sourcegendir":
case "--tempdir":
// TODO(bazel-team) - remove when Bazel no longer passes these flags to buildjar.
getArgument(argQueue, arg);
break;
case "--post_processor":
Expand Down Expand Up @@ -408,10 +398,6 @@ public int reducedClasspathLength() {
return reducedClasspathLength;
}

public String getSourceGenDir() {
return sourceGenDir;
}

public String getGeneratedSourcesOutputJar() {
return generatedSourcesOutputJar;
}
Expand Down Expand Up @@ -465,14 +451,6 @@ public String getNativeHeaderOutput() {
return nativeHeaderOutput;
}

public String getClassDir() {
return classDir;
}

public String getTempDir() {
return tempDir;
}

public Map<String, List<String>> getPostProcessors() {
return postProcessors;
}
Expand Down
36 changes: 20 additions & 16 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.google.devtools.build.buildjar;

import static com.google.common.base.MoreObjects.firstNonNull;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Locale.ENGLISH;

Expand Down Expand Up @@ -158,13 +157,17 @@ public VanillaJavaBuilderResult run(List<String> args) throws IOException {
DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
StringWriter output = new StringWriter();
JavaCompiler javaCompiler = ToolProvider.getSystemJavaCompiler();
Path tempDir = Paths.get(firstNonNull(optionsParser.getTempDir(), "_tmp"));
Path tempDir = Files.createTempDirectory("_tmp");
Path nativeHeaderDir = tempDir.resolve("native_headers");
Files.createDirectories(nativeHeaderDir);
Path sourceGenDir = tempDir.resolve("sources");
Files.createDirectories(sourceGenDir);
Path classDir = tempDir.resolve("classes");
Files.createDirectories(classDir);
boolean ok;
try (StandardJavaFileManager fileManager =
javaCompiler.getStandardFileManager(diagnosticCollector, ENGLISH, UTF_8)) {
setLocations(optionsParser, fileManager, nativeHeaderDir);
setLocations(optionsParser, fileManager, nativeHeaderDir, sourceGenDir, classDir);
ImmutableList<JavaFileObject> sources = getSources(optionsParser, fileManager);
if (sources.isEmpty()) {
ok = true;
Expand All @@ -182,10 +185,10 @@ public VanillaJavaBuilderResult run(List<String> args) throws IOException {
}
}
if (ok) {
writeOutput(optionsParser);
writeOutput(classDir, optionsParser);
writeNativeHeaderOutput(optionsParser, nativeHeaderDir);
}
writeGeneratedSourceOutput(optionsParser);
writeGeneratedSourceOutput(sourceGenDir, optionsParser);
// the jdeps output doesn't include any information about dependencies, but Bazel still expects
// the file to be created
if (optionsParser.getOutputDepsProtoFile() != null) {
Expand Down Expand Up @@ -254,7 +257,11 @@ public FileVisitResult visitFile(Path path, BasicFileAttributes attrs)

/** Sets the compilation search paths and output directories. */
private static void setLocations(
OptionsParser optionsParser, StandardJavaFileManager fileManager, Path nativeHeaderDir)
OptionsParser optionsParser,
StandardJavaFileManager fileManager,
Path nativeHeaderDir,
Path sourceGenDir,
Path classDir)
throws IOException {
fileManager.setLocation(StandardLocation.CLASS_PATH, toFiles(optionsParser.getClassPath()));
Iterable<File> bootClassPath = toFiles(optionsParser.getBootClassPath());
Expand All @@ -264,15 +271,11 @@ private static void setLocations(
}
fileManager.setLocation(
StandardLocation.ANNOTATION_PROCESSOR_PATH, toFiles(optionsParser.getProcessorPath()));
if (optionsParser.getSourceGenDir() != null) {
setOutputLocation(
fileManager, StandardLocation.SOURCE_OUTPUT, Paths.get(optionsParser.getSourceGenDir()));
}
setOutputLocation(fileManager, StandardLocation.SOURCE_OUTPUT, sourceGenDir);
if (optionsParser.getNativeHeaderOutput() != null) {
setOutputLocation(fileManager, StandardLocation.NATIVE_HEADER_OUTPUT, nativeHeaderDir);
}
setOutputLocation(
fileManager, StandardLocation.CLASS_OUTPUT, Paths.get(optionsParser.getClassDir()));
setOutputLocation(fileManager, StandardLocation.CLASS_OUTPUT, classDir);
}

private static void setOutputLocation(
Expand Down Expand Up @@ -300,14 +303,15 @@ private static void setProcessors(
}

/** Writes a jar containing any sources generated by annotation processors. */
private static void writeGeneratedSourceOutput(OptionsParser optionsParser) throws IOException {
private static void writeGeneratedSourceOutput(Path sourceGenDir, OptionsParser optionsParser)
throws IOException {
if (optionsParser.getGeneratedSourcesOutputJar() == null) {
return;
}
JarCreator jar = new JarCreator(optionsParser.getGeneratedSourcesOutputJar());
jar.setNormalize(true);
jar.setCompression(optionsParser.compressJar());
jar.addDirectory(optionsParser.getSourceGenDir());
jar.addDirectory(sourceGenDir);
jar.execute();
}

Expand All @@ -327,11 +331,11 @@ private static void writeNativeHeaderOutput(OptionsParser optionsParser, Path na
}

/** Writes the class output jar, including any resource entries. */
private static void writeOutput(OptionsParser optionsParser) throws IOException {
private static void writeOutput(Path classDir, OptionsParser optionsParser) throws IOException {
JarCreator jar = new JarCreator(optionsParser.getOutputJar());
jar.setNormalize(true);
jar.setCompression(optionsParser.compressJar());
jar.addDirectory(optionsParser.getClassDir());
jar.addDirectory(classDir);
jar.execute();
}

Expand Down
13 changes: 7 additions & 6 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClass.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc)
public static void main(String[] args) throws IOException {
GenClassOptions options = GenClassOptionsParser.parse(Arrays.asList(args));
Manifest manifest = readManifest(options.manifest());
deleteTree(options.tempDir());
Files.createDirectories(options.tempDir());
extractGeneratedClasses(options.classJar(), manifest, options.tempDir());
writeOutputJar(options);
Path tempDir = Files.createTempDirectory("tmp");
Files.createDirectories(tempDir);
extractGeneratedClasses(options.classJar(), manifest, tempDir);
writeOutputJar(tempDir, options);
deleteTree(tempDir);
}

/** Reads the compilation manifest. */
Expand Down Expand Up @@ -161,11 +162,11 @@ private static boolean prefixesContains(ImmutableSet<String> prefixes, String cl
}

/** Writes the generated class files to the output jar. */
private static void writeOutputJar(GenClassOptions options) throws IOException {
private static void writeOutputJar(Path tempDir, GenClassOptions options) throws IOException {
JarCreator output = new JarCreator(options.outputJar().toString());
output.setCompression(true);
output.setNormalize(true);
output.addDirectory(options.tempDir().toString());
output.addDirectory(tempDir);
output.execute();
}
}
18 changes: 3 additions & 15 deletions ...java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,14 @@

import java.nio.file.Path;

/** The options for a {@GenClass} action. */
/** The options for a {@link GenClass} action. */
public final class GenClassOptions {

/** A builder for {@link GenClassOptions}. */
public static final class Builder {
private Path manifest;
private Path classJar;
private Path outputJar;
private Path tempDir;

public Builder() {}

Expand All @@ -42,25 +41,19 @@ public void setOutputJar(Path outputJar) {
this.outputJar = outputJar;
}

public void setTempDir(Path tempDir) {
this.tempDir = tempDir;
}

GenClassOptions build() {
return new GenClassOptions(manifest, classJar, outputJar, tempDir);
return new GenClassOptions(manifest, classJar, outputJar);
}
}

private final Path manifest;
private final Path classJar;
private final Path outputJar;
private final Path tempDir;

private GenClassOptions(Path manifest, Path classJar, Path outputJar, Path tempDir) {
private GenClassOptions(Path manifest, Path classJar, Path outputJar) {
this.manifest = checkNotNull(manifest);
this.classJar = checkNotNull(classJar);
this.outputJar = checkNotNull(outputJar);
this.tempDir = checkNotNull(tempDir);
}

/** The path to the compilation manifest proto. */
Expand All @@ -78,11 +71,6 @@ public Path outputJar() {
return outputJar;
}

/** The path to the temp directory. */
public Path tempDir() {
return tempDir;
}

/** Returns a builder for {@link GenClassOptions}. */
public static Builder builder() {
return new Builder();
Expand Down
3 changes: 2 additions & 1 deletion ...ools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptionsParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public static GenClassOptions parse(Iterable<String> args) {
builder.setOutputJar(readPath(it));
break;
case "--temp_dir":
builder.setTempDir(readPath(it));
// TODO(b/169793789): remove once Blaze no longer passes the flag
readPath(it);
break;
default:
throw new IllegalArgumentException(
Expand Down
Loading

0 comments on commit 2350239

Please sign in to comment.