Automated rollback of commit 4110393.

*** Reason for rollback ***

Breaks coverage for android_test (N/A).

Can be reproduced with unknown commit.

*** Original change description ***

Rollforward change of Java coverage logic.

RELNOTES: None.

*** Original change description ***

Automated rollback of commit 8d6fc64.

PiperOrigin-RevId: 170322801

src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java

@@ -461,8 +461,8 @@ public void setupEnvVariables(Map<String, String> env, Duration timeout) {
       env.put("COVERAGE_MANIFEST", getCoverageManifest().getExecPathString());
       env.put("COVERAGE_DIR", getCoverageDirectory().getPathString());
       env.put("COVERAGE_OUTPUT_FILE", getCoverageData().getExecPathString());
-      // TODO(elenairina): Remove this after it reaches a blaze release.
-      env.put("NEW_JAVA_COVERAGE_IMPL", "released");
+      // TODO(elenairina): Remove this after the next blaze release (after 2017.07.30).
+      env.put("NEW_JAVA_COVERAGE_IMPL", "True");
     }
   }
 

src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java

@@ -28,7 +28,6 @@
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
-import com.google.devtools.build.lib.analysis.actions.LazyWritePathsFileAction;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.ComputedSubstitution;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Substitution;
@@ -262,8 +261,6 @@ public Artifact createStubAction(
       List<String> jvmFlags,
       Artifact executable,
       String javaStartClass,
-      String coverageStartClass,
-      NestedSetBuilder<Artifact> filesBuilder,
       String javaExecutable) {
     Preconditions.checkState(ruleContext.getConfiguration().hasFragment(Jvm.class));
 
@@ -317,35 +314,22 @@ public Artifact createStubAction(
     }
     arguments.add(new ComputedClasspathSubstitution(classpath, workspacePrefix, isRunfilesEnabled));
 
-    if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
-      Artifact runtimeClassPathArtifact = ruleContext.getUniqueDirectoryArtifact(
-          "coverage_runtime_classpath",
-          "runtime-classpath.txt",
-          ruleContext.getBinOrGenfilesDirectory());
-      ruleContext.registerAction(new LazyWritePathsFileAction(
-          ruleContext.getActionOwner(),
-          runtimeClassPathArtifact,
-          javaCommon.getRuntimeClasspath(),
-          false));
-      filesBuilder.add(runtimeClassPathArtifact);
-      arguments.add(Substitution.of(
-          JACOCO_METADATA_PLACEHOLDER,
-          "export JACOCO_METADATA_JAR=${JAVA_RUNFILES}/" + workspacePrefix + "/"
-              + runtimeClassPathArtifact.getRootRelativePathString()
-      ));
-      arguments.add(Substitution.of(
-          JACOCO_MAIN_CLASS_PLACEHOLDER, "export JACOCO_MAIN_CLASS=" + coverageStartClass));
-      arguments.add(Substitution.of(
-          JACOCO_JAVA_RUNFILES_ROOT_PLACEHOLDER,
-          "export JACOCO_JAVA_RUNFILES_ROOT=${JAVA_RUNFILES}/" + workspacePrefix)
-      );
-    } else {
-      arguments.add(Substitution.of(JavaSemantics.JACOCO_METADATA_PLACEHOLDER, ""));
-      arguments.add(Substitution.of(JavaSemantics.JACOCO_MAIN_CLASS_PLACEHOLDER, ""));
-      arguments.add(Substitution.of(JavaSemantics.JACOCO_JAVA_RUNFILES_ROOT_PLACEHOLDER, ""));
-    }
+    JavaCompilationArtifacts javaArtifacts = javaCommon.getJavaCompilationArtifacts();
+    String path =
+        javaArtifacts.getInstrumentedJar() != null
+            ? "${JAVA_RUNFILES}/"
+                + workspacePrefix
+                + javaArtifacts.getInstrumentedJar().getRootRelativePath().getPathString()
+            : "";
+    arguments.add(
+        Substitution.of(
+            "%set_jacoco_metadata%",
+            ruleContext.getConfiguration().isCodeCoverageEnabled()
+                ? "export JACOCO_METADATA_JAR=" + path
+                : ""));
 
-    arguments.add(Substitution.of("%java_start_class%", ShellEscaper.escapeString(javaStartClass)));
+    arguments.add(Substitution.of("%java_start_class%",
+        ShellEscaper.escapeString(javaStartClass)));
 
     ImmutableList<String> jvmFlagsList = ImmutableList.copyOf(jvmFlags);
     arguments.add(Substitution.ofSpaceSeparatedList("%jvm_flags%", jvmFlagsList));
@@ -676,11 +660,49 @@ public Iterable<String> getJvmFlags(
     return jvmFlags.build();
   }
 
+  /**
+   * Returns whether coverage has instrumented artifacts.
+   */
+  public static boolean hasInstrumentationMetadata(JavaTargetAttributes.Builder attributes) {
+    return !attributes.getInstrumentationMetadata().isEmpty();
+  }
+
+  // TODO(yueg): refactor this (only mainClass different for now)
   @Override
-  public String addCoverageSupport(JavaCompilationHelper helper, Artifact executable)
+  public String addCoverageSupport(
+      JavaCompilationHelper helper,
+      JavaTargetAttributes.Builder attributes,
+      Artifact executable,
+      Artifact instrumentationMetadata,
+      JavaCompilationArtifacts.Builder javaArtifactsBuilder,
+      String mainClass)
       throws InterruptedException {
     // This method can be called only for *_binary/*_test targets.
     Preconditions.checkNotNull(executable);
+    // Add our own metadata artifact (if any).
+    if (instrumentationMetadata != null) {
+      attributes.addInstrumentationMetadataEntries(ImmutableList.of(instrumentationMetadata));
+    }
+
+    if (!hasInstrumentationMetadata(attributes)) {
+      return mainClass;
+    }
+
+    Artifact instrumentedJar =
+        helper
+            .getRuleContext()
+            .getBinArtifact(helper.getRuleContext().getLabel().getName() + "_instrumented.jar");
+
+    // Create an instrumented Jar. This will be referenced on the runtime classpath prior
+    // to all other Jars.
+    JavaCommon.createInstrumentedJarAction(
+        helper.getRuleContext(),
+        this,
+        attributes.getInstrumentationMetadata(),
+        instrumentedJar,
+        mainClass);
+    javaArtifactsBuilder.setInstrumentedJar(instrumentedJar);
+
     // Add the coverage runner to the list of dependencies when compiling in coverage mode.
     TransitiveInfoCollection runnerTarget =
         helper.getRuleContext().getPrerequisite("$jacocorunner", Mode.TARGET);

src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt

@@ -254,8 +254,6 @@ export CLASSLOADER_PREFIX_PATH="${RUNPATH}"
 # We need to make the metadata jar with uninstrumented classes available for generating
 # the lcov-compatible coverage report, and we don't want it on the classpath.
 %set_jacoco_metadata%
-%set_jacoco_main_class%
-%set_jacoco_java_runfiles_root%
 
 if [[ -n "$JVM_DEBUG_PORT" ]]; then
   JVM_DEBUG_SUSPEND=${DEFAULT_JVM_DEBUG_SUSPEND:-"y"}

src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java

@@ -670,6 +670,7 @@ public static RuleConfiguredTargetBuilder createAndroidBinary(
             ApkProvider.create(
                 zipAlignedApk,
                 unsignedApk,
+                androidCommon.getInstrumentedJar(),
                 applicationManifest.getManifest(),
                 debugKeystore))
         .addProvider(AndroidPreDexJarProvider.class, AndroidPreDexJarProvider.create(jarToDex))

src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java

@@ -452,11 +452,12 @@ private void compileResourceJar(
       } else {
         Artifact outputDepsProto =
             javacHelper.createOutputDepsProtoArtifact(resourceClassJar, javaArtifactsBuilder);
-        javacHelper.createCompileAction(
+        javacHelper.createCompileActionWithInstrumentation(
             resourceClassJar,
             null /* manifestProtoOutput */,
             null /* genSourceJar */,
-            outputDepsProto);
+            outputDepsProto,
+            javaArtifactsBuilder);
       }
     } else {
       // Otherwise, it should have been the AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR.
@@ -704,7 +705,8 @@ private void initJava(
     helper.createSourceJarAction(srcJar, genSourceJar);
 
     outputDepsProto = helper.createOutputDepsProtoArtifact(classJar, javaArtifactsBuilder);
-    helper.createCompileAction(classJar, manifestProtoOutput, genSourceJar, outputDepsProto);
+    helper.createCompileActionWithInstrumentation(classJar, manifestProtoOutput, genSourceJar,
+        outputDepsProto, javaArtifactsBuilder);
 
     if (isBinary) {
       generatedExtensionRegistryProvider =
@@ -885,10 +887,6 @@ public ImmutableList<String> getJavacOpts() {
     return javaCommon.getJavacOpts();
   }
 
-  public Artifact getClassJar() {
-    return classJar;
-  }
-
   public Artifact getGenClassJar() {
     return genClassJar;
   }
@@ -914,6 +912,10 @@ public NestedSet<Artifact> getJarsProducedForRuntime() {
     return jarsProducedForRuntime;
   }
 
+  public Artifact getInstrumentedJar() {
+    return javaCommon.getJavaCompilationArtifacts().getInstrumentedJar();
+  }
+
   public NestedSet<Artifact> getTransitiveNeverLinkLibraries() {
     return transitiveNeverlinkLibraries;
   }

src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java

@@ -126,6 +126,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
     JavaCompilationHelper helper =
         getJavaCompilationHelperWithDependencies(ruleContext, javaSemantics, javaCommon,
             attributesBuilder);
+    Artifact instrumentationMetadata =
+        helper.createInstrumentationMetadata(classJar, javaArtifactsBuilder);
     Artifact executable; // the artifact for the rule itself
     if (OS.getCurrent() == OS.WINDOWS
         && ruleContext.getConfiguration().enableWindowsExeLauncher()) {
@@ -152,7 +154,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
             javaSemantics,
             helper,
             executable,
-            null,
+            instrumentationMetadata,
             javaArtifactsBuilder,
             attributesBuilder);
 
@@ -176,7 +178,11 @@ public ConfiguredTarget create(RuleContext ruleContext)
         helper.createOutputDepsProtoArtifact(classJar, javaArtifactsBuilder);
     javaRuleOutputJarsProviderBuilder.setJdeps(outputDepsProtoArtifact);
     helper.createCompileAction(
-        classJar, manifestProtoOutput, genSourceJar, outputDepsProtoArtifact);
+        classJar,
+        manifestProtoOutput,
+        genSourceJar,
+        outputDepsProtoArtifact,
+        instrumentationMetadata);
     helper.createSourceJarAction(srcJar, genSourceJar);
 
     setUpJavaCommon(javaCommon, helper, javaArtifactsBuilder.build());
@@ -196,8 +202,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
         getJvmFlags(ruleContext, testClass),
         executable,
         mainClass,
-        "com.google.testing.junit.runner.GoogleTestRunner",
-        filesToBuildBuilder,
         javaExecutable);
 
     Artifact deployJar =
@@ -431,6 +435,13 @@ private Runfiles collectDefaultRunfiles(
     builder.addTargets(depsForRunfiles, RunfilesProvider.DEFAULT_RUNFILES);
     builder.addTransitiveArtifacts(transitiveAarArtifacts);
 
+    if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
+      Artifact instrumentedJar = javaCommon.getJavaCompilationArtifacts().getInstrumentedJar();
+      if (instrumentedJar != null) {
+        builder.addArtifact(instrumentedJar);
+      }
+    }
+
     // We assume that the runtime jars will not have conflicting artifacts
     // with the same root relative path
     builder.addTransitiveArtifactsWrappedInStableOrder(javaCommon.getRuntimeClasspath());
@@ -467,4 +478,3 @@ private static String getAndCheckTestClass(
     return testClass;
   }
 }
-

src/main/java/com/google/devtools/build/lib/rules/android/ApkProvider.java

@@ -17,6 +17,7 @@
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import javax.annotation.Nullable;
 
 /** A provider for targets that produce an apk file. */
 @AutoValue
@@ -26,9 +27,10 @@ public abstract class ApkProvider implements TransitiveInfoProvider {
   public static ApkProvider create(
       Artifact apk,
       Artifact unsignedApk,
+      @Nullable Artifact coverageMetdata,
       Artifact mergedManifest,
       Artifact keystore) {
-    return new AutoValue_ApkProvider(apk, unsignedApk, mergedManifest, keystore);
+    return new AutoValue_ApkProvider(apk, unsignedApk, coverageMetdata, mergedManifest, keystore);
   }
 
   /** Returns the APK file built in the transitive closure. */
@@ -37,6 +39,10 @@ public static ApkProvider create(
   /** Returns the unsigned APK file built in the transitive closure. */
   public abstract Artifact getUnsignedApk();
 
+  /** Returns the coverage metadata artifacts generated in the transitive closure. */
+  @Nullable
+  public abstract Artifact getCoverageMetadata();
+
   /** Returns the merged manifest. */
   public abstract Artifact getMergedManifest();