Skip files that fail JaCoCo instrumentation

    Instead of failing hard when JaCoCo encounters issues instrumenting a
    source file (e.g. because instrumentation pushes a large method over
    the bytecode limit of individual methods), print a warning and continue.

    It's more useful to have partial coverage information than none at all
    and users often cannot easily fix their code to appease the issues that
    instrumentation triggers.

    Fixes bazelbuild#21520.

Author: Julio Merino <julio.merino+oss@snowflake.com>
Date:   Thu Feb 29 17:57:14 2024 -0800

Description

Testing

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

@@ -117,28 +117,34 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
             if (!file.getFileName().toString().endsWith(".class")) {
               return FileVisitResult.CONTINUE;
             }
-            // TODO(bazel-team): filter with coverage_instrumentation_filter?
-            // 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) {
+              System.err.printf("WARNING: %s was not instrumented: %s%n\n", file, e.getMessage());
             }
             return FileVisitResult.CONTINUE;
           }