Create temp directories instead of making Bazel create them for us
PiperOrigin-RevId: 335459680
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
index 6dd1255..9cf409c 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java
@@ -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;
@@ -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;
@@ -133,11 +136,11 @@
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()));
}
@@ -162,10 +165,10 @@
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()) {
@@ -179,13 +182,30 @@
}
}
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());
}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java
index 802fa6e..a3f2bed 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java
@@ -70,7 +70,6 @@
private int fullClasspathLength = -1;
private int reducedClasspathLength = -1;
- private String sourceGenDir;
private String generatedSourcesOutputJar;
private String manifestProtoPath;
@@ -89,9 +88,6 @@
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;
@@ -166,9 +162,6 @@
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;
@@ -215,13 +208,10 @@
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":
@@ -408,10 +398,6 @@
return reducedClasspathLength;
}
- public String getSourceGenDir() {
- return sourceGenDir;
- }
-
public String getGeneratedSourcesOutputJar() {
return generatedSourcesOutputJar;
}
@@ -465,14 +451,6 @@
return nativeHeaderOutput;
}
- public String getClassDir() {
- return classDir;
- }
-
- public String getTempDir() {
- return tempDir;
- }
-
public Map<String, List<String>> getPostProcessors() {
return postProcessors;
}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java
index 694330b..b2a4da2 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java
@@ -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;
@@ -158,13 +157,17 @@
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;
@@ -182,10 +185,10 @@
}
}
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) {
@@ -254,7 +257,11 @@
/** 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());
@@ -264,15 +271,11 @@
}
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(
@@ -300,14 +303,15 @@
}
/** 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();
}
@@ -327,11 +331,11 @@
}
/** 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();
}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClass.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClass.java
index 16d4f10..7a38b43 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClass.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClass.java
@@ -67,10 +67,11 @@
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. */
@@ -161,11 +162,11 @@
}
/** 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();
}
}
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptions.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptions.java
index 9e46d96..708d010 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptions.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptions.java
@@ -18,7 +18,7 @@
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}. */
@@ -26,7 +26,6 @@
private Path manifest;
private Path classJar;
private Path outputJar;
- private Path tempDir;
public Builder() {}
@@ -42,25 +41,19 @@
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. */
@@ -78,11 +71,6 @@
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();
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptionsParser.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptionsParser.java
index 606185b..795eac0 100644
--- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptionsParser.java
+++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/genclass/GenClassOptionsParser.java
@@ -37,7 +37,8 @@
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(
diff --git a/src/java_tools/buildjar/javatests/com/google/devtools/build/buildjar/VanillaJavaBuilderTest.java b/src/java_tools/buildjar/javatests/com/google/devtools/build/buildjar/VanillaJavaBuilderTest.java
index bfcc547..36fdccd 100644
--- a/src/java_tools/buildjar/javatests/com/google/devtools/build/buildjar/VanillaJavaBuilderTest.java
+++ b/src/java_tools/buildjar/javatests/com/google/devtools/build/buildjar/VanillaJavaBuilderTest.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.buildjar;
+import static com.google.common.base.StandardSystemProperty.JAVA_HOME;
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -91,11 +92,7 @@
"--output",
output.toString(),
"--bootclasspath",
- Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString(),
- "--tempdir",
- temporaryFolder.newFolder().toString(),
- "--classdir",
- temporaryFolder.newFolder().toString()));
+ Paths.get(JAVA_HOME.value()).resolve("lib/rt.jar").toString()));
assertThat(result.output()).isEmpty();
assertThat(result.ok()).isTrue();
@@ -136,11 +133,7 @@
"--output",
output.toString(),
"--bootclasspath",
- Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString(),
- "--tempdir",
- temporaryFolder.newFolder().toString(),
- "--classdir",
- temporaryFolder.newFolder().toString()));
+ Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString()));
assertThat(result.output()).contains("possible fall-through");
assertThat(result.ok()).isFalse();
@@ -175,11 +168,7 @@
"--sources",
source.toString(),
"--output",
- output.toString(),
- "--tempdir",
- temporaryFolder.newFolder().toString(),
- "--classdir",
- temporaryFolder.newFolder().toString()));
+ output.toString()));
assertThat(result.output()).contains("note: Some messages have been simplified");
assertThat(result.ok()).isFalse();
@@ -221,11 +210,7 @@
"--output",
output.toString(),
"--bootclasspath",
- Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString(),
- "--tempdir",
- temporaryFolder.newFolder().toString(),
- "--classdir",
- classDir.toString()));
+ Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString()));
assertThat(result.output()).isEmpty();
assertThat(result.ok()).isTrue();
@@ -267,11 +252,7 @@
"--output",
output.toString(),
"--bootclasspath",
- Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString(),
- "--tempdir",
- temporaryFolder.newFolder().toString(),
- "--classdir",
- temporaryFolder.newFolder().toString()));
+ Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString()));
assertThat(result.output()).isEmpty();
assertThat(result.ok()).isTrue();
@@ -314,11 +295,7 @@
"--native_header_output",
nativeHeaderOutput.toString(),
"--bootclasspath",
- Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString(),
- "--tempdir",
- temporaryFolder.newFolder().toString(),
- "--classdir",
- temporaryFolder.newFolder().toString()));
+ Paths.get(System.getProperty("java.home")).resolve("lib/rt.jar").toString()));
assertThat(result.output()).isEmpty();
assertThat(result.ok()).isTrue();
