diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java index 50c936ac43360e..483b871d518c29 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java +++ b/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(); + } } } diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/instrumentation/JacocoInstrumentationProcessor.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/instrumentation/JacocoInstrumentationProcessor.java index d3b37d257abf57..995c95d408d02f 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/instrumentation/JacocoInstrumentationProcessor.java +++ b/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 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,15 +70,10 @@ 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); } @@ -87,14 +81,26 @@ public void processRequest(JavaLibraryBuildRequest build, JarCreator jar) throws 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);