Skip to content

Commit

Permalink
Remove --experimental_java_coverage/--incompatible_java_coverage flag.
Browse files Browse the repository at this point in the history
RELNOTES: --experimental_java_coverage/--incompatible_java_coverage flag was
removed. See #7425.
PiperOrigin-RevId: 241342271
  • Loading branch information
iirina authored and copybara-github committed Apr 1, 2019
1 parent 998b410 commit bfd13b9
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 236 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -584,25 +584,6 @@ public static class Options extends FragmentOptions implements Cloneable {
)
public boolean collectCodeCoverage;

@Option(
name = "incompatible_java_coverage",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
oldName = "experimental_java_coverage",
help =
"If true Bazel will use a new way of computing code coverage for java targets. "
+ "It allows collecting coverage for Starlark JVM rules and java_import. "
+ "Only includes JVM files in the coverage report (e.g. dismisses data files). "
+ "The report includes the actual path of the files relative to the workspace root "
+ "instead of the package path (e.g. src/com/google/Action.java instead of "
+ "com/google/Action.java.")
public boolean experimentalJavaCoverage;

@Option(
name = "incompatible_cc_coverage",
defaultValue = "true",
Expand Down Expand Up @@ -1739,10 +1720,6 @@ public boolean isCodeCoverageEnabled() {
return options.collectCodeCoverage;
}

public boolean isExperimentalJavaCoverage() {
return true;
}

public boolean useGcovCoverage() {
return options.useGcovCoverage;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,19 +558,9 @@ 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.
if (configuration.isExperimentalJavaCoverage()) {
// This value ("released") tells lcov_merger whether it should use the old or the new
// java coverage implementation. The meaning of "released" is that lcov_merger will receive
// this value only after blaze containing this change will be released.
env.put("NEW_JAVA_COVERAGE_IMPL", "released");
} else {
// This value ("True") should have told lcov_merger whether it should use the old or the new
// java coverage implementation. Due to several failed attempts at submitting the new
// implementation, this value will be treated still as the old implementation. This
// environment variable must be set to a value recognized by lcov_merger.
env.put("NEW_JAVA_COVERAGE_IMPL", "True");
}
// TODO(elenairina): Remove this and its usage in lcov_merger. Note it requires syncing
// between the blaze release and the lcov_merger release.
env.put("NEW_JAVA_COVERAGE_IMPL", "released");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,7 @@ public Artifact createStubAction(
+ javaArtifacts.getInstrumentedJar().getRootRelativePath().getPathString()
: "";

if (ruleContext.getConfiguration().isCodeCoverageEnabled()
&& ruleContext.getConfiguration().isExperimentalJavaCoverage()) {
if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
if (createCoverageMetadataJar) {
Artifact runtimeClassPathArtifact =
ruleContext.getUniqueDirectoryArtifact(
Expand Down Expand Up @@ -391,15 +390,11 @@ public Artifact createStubAction(
arguments.add(
Substitution.of("%java_start_class%", ShellEscaper.escapeString(javaStartClass)));
} else {
arguments.add(Substitution.of(JavaSemantics.JACOCO_METADATA_PLACEHOLDER,
ruleContext.getConfiguration().isCodeCoverageEnabled()
? "export JACOCO_METADATA_JAR=" + path : ""));
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, ""));
arguments.add(
Substitution.of(
JavaSemantics.JAVA_COVERAGE_NEW_IMPLEMENTATION_PLACEHOLDER,
"export JAVA_COVERAGE_NEW_IMPLEMENTATION=NO"));
Substitution.of(JavaSemantics.JAVA_COVERAGE_NEW_IMPLEMENTATION_PLACEHOLDER, ""));
}

arguments.add(Substitution.of("%java_start_class%",
Expand Down Expand Up @@ -703,55 +698,9 @@ public static boolean hasInstrumentationMetadata(JavaTargetAttributes.Builder at
}

@Override
public String addCoverageSupport(
JavaCompilationHelper helper,
JavaTargetAttributes.Builder attributes,
Artifact executable,
Artifact instrumentationMetadata,
JavaCompilationArtifacts.Builder javaArtifactsBuilder,
String mainClass) throws InterruptedException {
return addCoverageSupport(helper, attributes, executable, instrumentationMetadata,
javaArtifactsBuilder, mainClass, false);
}

// TODO(yueg): refactor this (only mainClass different for now)
@Override
public String addCoverageSupport(
JavaCompilationHelper helper,
JavaTargetAttributes.Builder attributes,
Artifact executable,
Artifact instrumentationMetadata,
JavaCompilationArtifacts.Builder javaArtifactsBuilder,
String mainClass,
boolean isExperimentalCoverage)
throws InterruptedException {
public String addCoverageSupport(JavaCompilationHelper helper, Artifact executable) {
// This method can be called only for *_binary/*_test targets.
Preconditions.checkNotNull(executable);
if (!isExperimentalCoverage) {
// 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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ public static RuleConfiguredTargetBuilder createAndroidBinary(
new ApkInfo(
zipAlignedApk,
unsignedApk,
getCoverageInstrumentationJarForApk(ruleContext, androidCommon),
getCoverageInstrumentationJarForApk(ruleContext),
resourceApk.getManifest(),
AndroidCommon.getApkDebugSigningKey(ruleContext)))
.addNativeDeclaredProvider(new AndroidPreDexJarProvider(jarToDex))
Expand All @@ -637,20 +637,17 @@ public static RuleConfiguredTargetBuilder createAndroidBinary(

/**
* For coverage builds, this returns a Jar containing <b>un</b>instrumented bytecode for the
* coverage reporter's consumption. This Jar is often confusingly called <i>instrumented</i>.
* This method simply returns the deploy Jar when "new" coverage is used, otherwise the
* traditional "instrumented" Jar. Note the deploy Jar is built anyway for Android binaries.
* coverage reporter's consumption. This method simply returns the deploy Jar. Note the deploy Jar
* is built anyway for Android binaries.
*
* @return A Jar containing uninstrumented bytecode or {@code null} for non-coverage builds
*/
@Nullable
private static Artifact getCoverageInstrumentationJarForApk(
RuleContext ruleContext, AndroidCommon androidCommon) throws InterruptedException {
if (ruleContext.getConfiguration().isCodeCoverageEnabled()
&& ruleContext.getConfiguration().isExperimentalJavaCoverage()) {
return ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_DEPLOY_JAR);
}
return androidCommon.getInstrumentedJar();
private static Artifact getCoverageInstrumentationJarForApk(RuleContext ruleContext)
throws InterruptedException {
return ruleContext.getConfiguration().isCodeCoverageEnabled()
? ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_DEPLOY_JAR)
: null;
}

public static NestedSet<Artifact> getLibraryResourceJars(RuleContext ruleContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,7 @@ private void initJava(
nativeHeaderOutput = helper.createNativeHeaderJar(classJar);

JavaCompileAction javaCompileAction =
helper.createCompileActionWithInstrumentation(
classJar, manifestProtoOutput, genSourceJar, javaArtifactsBuilder, nativeHeaderOutput);
helper.createCompileAction(classJar, manifestProtoOutput, genSourceJar, nativeHeaderOutput);
outputDepsProto = javaCompileAction.getOutputDepsProto();

if (generateExtensionRegistry) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
srcJar == null ? ImmutableList.<Artifact>of() : ImmutableList.of(srcJar));

JavaCompilationArtifacts.Builder javaArtifactsBuilder = new JavaCompilationArtifacts.Builder();
Artifact instrumentationMetadata =
helper.createInstrumentationMetadata(classJar, javaArtifactsBuilder);
Artifact executable; // the artifact for the rule itself
if (OS.getCurrent() == OS.WINDOWS) {
executable =
Expand All @@ -215,15 +213,16 @@ public ConfiguredTarget create(RuleContext ruleContext)
String mainClass = javaSemantics.getTestRunnerMainClass();
String originalMainClass = mainClass;
if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
mainClass = addCoverageSupport(
ruleContext,
javaSemantics,
helper,
executable,
instrumentationMetadata,
javaArtifactsBuilder,
attributesBuilder,
mainClass);
mainClass =
addCoverageSupport(
ruleContext,
javaSemantics,
helper,
executable,
/* instrumentationMetadata= */ null,
javaArtifactsBuilder,
attributesBuilder,
mainClass);
}

// JavaCompilationHelper.getAttributes() builds the JavaTargetAttributes, after which the
Expand All @@ -246,7 +245,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
classJar,
manifestProtoOutput,
genSourceJar,
instrumentationMetadata,
/* nativeHeaderOutput= */ null);
helper.createSourceJarAction(srcJar, genSourceJar);
javaRuleOutputJarsProviderBuilder.setJdeps(javaCompileAction.getOutputDepsProto());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
Artifact unstrippedLauncher = launcherAndUnstrippedLauncher.second;

JavaCompilationArtifacts.Builder javaArtifactsBuilder = new JavaCompilationArtifacts.Builder();
Artifact instrumentationMetadata =
helper.createInstrumentationMetadata(classJar, javaArtifactsBuilder);

NestedSetBuilder<Artifact> filesBuilder = NestedSetBuilder.stableOrder();
Artifact executableForRunfiles = null;
Expand All @@ -194,15 +192,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
filesBuilder.add(classJar).add(executableForRunfiles);

if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
mainClass =
semantics.addCoverageSupport(
helper,
attributesBuilder,
executableForRunfiles,
instrumentationMetadata,
javaArtifactsBuilder,
mainClass,
ruleContext.getConfiguration().isExperimentalJavaCoverage());
mainClass = semantics.addCoverageSupport(helper, executableForRunfiles);
}
} else {
filesBuilder.add(classJar);
Expand Down Expand Up @@ -254,7 +244,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
classJar,
manifestProtoOutput,
genSourceJar,
instrumentationMetadata,
/* nativeHeaderOutput= */ null);
helper.createSourceJarAction(srcJar, genSourceJar);
ruleOutputJarsProviderBuilder.setJdeps(javaCompileAction.getOutputDepsProto());
Expand Down Expand Up @@ -464,8 +453,7 @@ public ConfiguredTarget create(RuleContext ruleContext)

NestedSetBuilder<Pair<String, String>> coverageEnvironment = NestedSetBuilder.stableOrder();
NestedSetBuilder<Artifact> coverageSupportFiles = NestedSetBuilder.stableOrder();
if (ruleContext.getConfiguration().isCodeCoverageEnabled()
&& ruleContext.getConfiguration().isExperimentalJavaCoverage()) {
if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {

// Create an artifact that contains the root relative paths of the jars on the runtime
// classpath.
Expand Down Expand Up @@ -613,14 +601,6 @@ private void collectDefaultRunfiles(
builder.addTargets(runtimeDeps, JavaRunfilesProvider.TO_RUNFILES);
builder.addTargets(runtimeDeps, RunfilesProvider.DEFAULT_RUNFILES);

if (ruleContext.getConfiguration().isCodeCoverageEnabled()
&& !ruleContext.getConfiguration().isExperimentalJavaCoverage()) {
Artifact instrumentedJar = javaArtifacts.getInstrumentedJar();
if (instrumentedJar != null) {
builder.addArtifact(instrumentedJar);
}
}

builder.addArtifacts((Iterable<Artifact>) common.getRuntimeClasspath());

// Add the JDK files if it comes from the source repository (see java_stub_template.txt).
Expand Down
Loading

0 comments on commit bfd13b9

Please sign in to comment.