Skip to content

Commit

Permalink
Merge pull request #31 from salesforce/rsalvador/jacoco-skip-if-fail
Browse files Browse the repository at this point in the history 
Apply: Skip files that fail JaCoCo instrumentation PR
  • Loading branch information
@rsalvador
rsalvador authored Aug 2, 2024
2 parents d6b2d21 + fd6ee8b commit 159f7fd
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 24 deletions.
14 changes: 11 additions & 3 deletions compiler/src/main/buildjar/com/google/devtools/build/buildjar/SimpleJavaLibraryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.io.Closeable;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand Down Expand Up @@ -125,7 +127,13 @@ public BlazeJavacResult run(JavaLibraryBuildRequest build) throws Exception {
try {
result = compileJavaLibrary(build);
if (result.isOk()) {
buildJar(build);
StringWriter errOutput = new StringWriter();
PrintWriter errWriter = new PrintWriter(errOutput);
buildJar(build, errWriter);
String output = errOutput.toString().trim();
if (output.length() > 0) {
result = BlazeJavacResult.createFullResult(result.status(), result.diagnostics(), output, result.statistics());
}
nativeHeaderOutput(build);
}
if (!build.getProcessors().isEmpty()) {
Expand All @@ -145,7 +153,7 @@ public BlazeJavacResult run(JavaLibraryBuildRequest build) throws Exception {
return result;
}

public void buildJar(JavaLibraryBuildRequest build) throws IOException {
public void buildJar(JavaLibraryBuildRequest build, PrintWriter errWriter) throws IOException {
Files.createDirectories(build.getOutputJar().getParent());
JarCreator jar = new JarCreator(build.getOutputJar());
JacocoInstrumentationProcessor processor = null;
Expand All @@ -156,7 +164,7 @@ public void buildJar(JavaLibraryBuildRequest build) throws IOException {
jar.setJarOwner(build.getTargetLabel(), build.getInjectingRuleKind());
processor = build.getJacocoInstrumentationProcessor();
if (processor != null) {
processor.processRequest(build, processor.isNewCoverageImplementation() ? jar : null);
processor.processRequest(build, processor.isNewCoverageImplementation() ? jar : null, errWriter);
}
} finally {
jar.execute();
Expand Down
54 changes: 33 additions & 21 deletions ...ar/com/google/devtools/build/buildjar/instrumentation/JacocoInstrumentationProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -30,6 +31,7 @@
import org.jacoco.core.instr.Instrumenter;
import org.jacoco.core.runtime.OfflineInstrumentationAccessGenerator;

import com.google.common.base.Throwables;
import com.google.common.io.MoreFiles;
import com.google.common.io.RecursiveDeleteOption;
import com.google.devtools.build.buildjar.InvalidCommandLineException;
Expand Down Expand Up @@ -73,7 +75,7 @@ public boolean isNewCoverageImplementation() {
* Instruments classes using Jacoco and keeps copies of uninstrumented class files in
* jacocoMetadataDir, to be zipped up in the output file jacocoMetadataOutput.
*/
public void processRequest(JavaLibraryBuildRequest build, JarCreator jar) throws IOException {
public void processRequest(JavaLibraryBuildRequest build, JarCreator jar, PrintWriter errWriter) throws IOException {
// 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());
Expand All @@ -84,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, build.getClassDir());
instrumentRecursively(instr, build.getClassDir(), errWriter);
jar.addDirectory(instrumentedClassesDirectory);
if (isNewCoverageImplementation) {
jar.addEntry(coverageInformation, coverageInformation);
Expand All @@ -109,7 +111,7 @@ private static Path getMetadataDirRelativeToJar(Path outputJar) {
/**
* Runs Jacoco instrumentation processor over all .class files recursively, starting with root.
*/
private void instrumentRecursively(Instrumenter instr, Path root) throws IOException {
private void instrumentRecursively(Instrumenter instr, Path root, PrintWriter errWriter) throws IOException {
Files.walkFileTree(
root,
new SimpleFileVisitor<Path>() {
Expand All @@ -123,24 +125,34 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
// It's not clear whether there is any advantage in not instrumenting *Test classes,
// apart from lowering the covered percentage in the aggregate statistics.

// We first move the original .class file to our metadata directory, then instrument it
// and output the instrumented version in the regular classes output directory.
Path instrumentedCopy = file;
Path uninstrumentedCopy;
if (isNewCoverageImplementation) {
Path absoluteUninstrumentedCopy = Paths.get(file + ".uninstrumented");
uninstrumentedCopy =
instrumentedClassesDirectory.resolve(root.relativize(absoluteUninstrumentedCopy));
} else {
uninstrumentedCopy = instrumentedClassesDirectory.resolve(root.relativize(file));
}
Files.createDirectories(uninstrumentedCopy.getParent());
Files.move(file, uninstrumentedCopy);
try (InputStream input =
new BufferedInputStream(Files.newInputStream(uninstrumentedCopy));
OutputStream output =
new BufferedOutputStream(Files.newOutputStream(instrumentedCopy))) {
instr.instrument(input, output, file.toString());
try {
// We first move the original .class file to our metadata directory, then instrument
// it and output the instrumented version in the regular classes output directory.
Path instrumentedCopy = file;
Path uninstrumentedCopy;
if (isNewCoverageImplementation) {
Path absoluteUninstrumentedCopy = Paths.get(file + ".uninstrumented");
uninstrumentedCopy =
instrumentedClassesDirectory.resolve(
root.relativize(absoluteUninstrumentedCopy));
} else {
uninstrumentedCopy = instrumentedClassesDirectory.resolve(root.relativize(file));
}
Files.createDirectories(uninstrumentedCopy.getParent());
Files.move(file, uninstrumentedCopy);
try (InputStream input =
new BufferedInputStream(Files.newInputStream(uninstrumentedCopy));
OutputStream output =
new BufferedOutputStream(Files.newOutputStream(instrumentedCopy))) {
instr.instrument(input, output, file.toString());
} catch (Exception e) {
Files.delete(instrumentedCopy);
Files.copy(uninstrumentedCopy, instrumentedCopy);
throw e; // Bubble up to the outer broader safety catch block for logging.
}
} catch (Exception e) {
errWriter.printf("WARNING: %s was not instrumented: %s\n", file, Throwables.getRootCause(e).toString());
e.printStackTrace();
}
return FileVisitResult.CONTINUE;
}
Expand Down

0 comments on commit 159f7fd

Please sign in to comment.