Make JavaBuilder use a unique coverage metadata directory

for each test instead of the same directory for all the tests. The previous implementation was using one directory for instrumenting the classes of a jar. For each each jar the metadata directory was deleted if it already existed. This is problematic for local execution when multiple tests are run in parallel because some threads will try to delete the directory and some will try to perform read/write operations on it.

This is an important fix for Bazel coverage users.

Fixes bazelbuild#4398.

RELNOTES: Java coverage works now with multiple jobs.
PiperOrigin-RevId: 199764483

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java

@@ -148,17 +148,21 @@ public BlazeJavacResult run(JavaLibraryBuildRequest build) throws Exception {
 
  public void buildJar(JavaLibraryBuildRequest build) throws IOException {
  JarCreator jar = new JarCreator(build.getOutputJar());
+ JacocoInstrumentationProcessor processor = null;
  try {
  jar.setNormalize(true);
  jar.setCompression(build.compressJar());
  jar.addDirectory(build.getClassDir());
  jar.setJarOwner(build.getTargetLabel(), build.getInjectingRuleKind());
- JacocoInstrumentationProcessor processor = build.getJacocoInstrumentationProcessor();
+ processor = build.getJacocoInstrumentationProcessor();
  if (processor != null) {
  processor.processRequest(build, processor.isNewCoverageImplementation() ? jar : null);
  }
  } finally {
  jar.execute();
+ if (processor != null) {
+ processor.cleanup();
+ }
  }
  }
 

src/java_tools/buildjar/java/com/google/devtools/build/buildjar/instrumentation/JacocoInstrumentationProcessor.java

@@ -37,24 +37,23 @@ public final class JacocoInstrumentationProcessor {
 
  public static JacocoInstrumentationProcessor create(List<String> args)
  throws InvalidCommandLineException {
- if (args.size() < 2) {
+ if (args.size() < 1) {
  throw new InvalidCommandLineException(
- "Number of arguments for Jacoco instrumentation should be 2+ (given "
+ "Number of arguments for Jacoco instrumentation should be 1+ (given "
  + args.size()
- + ": metadataOutput metadataDirectory [filters*].");
+ + ": metadataOutput [filters*].");
  }
 
  // ignoring filters, they weren't used in the previous implementation
  // TODO(bazel-team): filters should be correctly handled
- return new JacocoInstrumentationProcessor(args.get(1), args.get(0));
+ return new JacocoInstrumentationProcessor(args.get(0));
  }
 
- private final String metadataDir;
+ private Path instrumentedClassesDirectory;
  private final String coverageInformation;
  private final boolean isNewCoverageImplementation;
 
- private JacocoInstrumentationProcessor(String metadataDir, String coverageInfo) {
- this.metadataDir = metadataDir;
+ private JacocoInstrumentationProcessor(String coverageInfo) {
  this.coverageInformation = coverageInfo;
  // This is part of the new Java coverage implementation where JacocoInstrumentationProcessor
  // receives a file that includes the relative paths of the uninstrumented Java files, instead
@@ -71,30 +70,37 @@ public boolean isNewCoverageImplementation() {
  * jacocoMetadataDir, to be zipped up in the output file jacocoMetadataOutput.
  */
  public void processRequest(JavaLibraryBuildRequest build, JarCreator jar) throws IOException {
- // Clean up jacocoMetadataDir to be used by postprocessing steps. This is important when
- // running JavaBuilder locally, to remove stale entries from previous builds.
- if (metadataDir != null) {
- Path workDir = Paths.get(metadataDir);
- if (Files.exists(workDir)) {
- recursiveRemove(workDir);
- }
- Files.createDirectories(workDir);
- }
+ // Use a directory for coverage metadata that is unique to each built jar. Avoids
+ // multiple threads performing read/write/delete actions on the instrumented classes directory.
+ instrumentedClassesDirectory = getMetadataDirRelativeToJar(build.getOutputJar());
+ Files.createDirectories(instrumentedClassesDirectory);
  if (jar == null) {
  jar = new JarCreator(coverageInformation);
  }
  jar.setNormalize(true);
  jar.setCompression(build.compressJar());
  Instrumenter instr = new Instrumenter(new OfflineInstrumentationAccessGenerator());
  instrumentRecursively(instr, build.getClassDir());
- jar.addDirectory(metadataDir);
+ jar.addDirectory(instrumentedClassesDirectory);
  if (isNewCoverageImplementation) {
  jar.addEntry(coverageInformation, coverageInformation);
  } else {
  jar.execute();
+ cleanup();
+ }
+ }
+
+ public void cleanup() throws IOException {
+ if (Files.exists(instrumentedClassesDirectory)) {
+ recursiveRemove(instrumentedClassesDirectory);
  }
  }
 
+ // Return the path of the coverage metadata directory relative to the output jar path.
+ private static Path getMetadataDirRelativeToJar(Path outputJar) {
+ return outputJar.resolveSibling(outputJar + "-coverage-metadata");
+ }
+
  /**
  * Runs Jacoco instrumentation processor over all .class files recursively, starting with root.
  */
@@ -119,9 +125,9 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
  if (isNewCoverageImplementation) {
  Path absoluteUninstrumentedCopy = Paths.get(file + ".uninstrumented");
  uninstrumentedCopy =
- Paths.get(metadataDir).resolve(root.relativize(absoluteUninstrumentedCopy));
+ instrumentedClassesDirectory.resolve(root.relativize(absoluteUninstrumentedCopy));
  } else {
- uninstrumentedCopy = Paths.get(metadataDir).resolve(root.relativize(file));
+ uninstrumentedCopy = instrumentedClassesDirectory.resolve(root.relativize(file));
  }
  Files.createDirectories(uninstrumentedCopy.getParent());
  Files.move(file, uninstrumentedCopy);