Skip to content

Commit

Permalink
Rollforward change of Java coverage logic.
Browse files Browse the repository at this point in the history
RELNOTES: None.

*** Original change description ***

Automated rollback of commit 8d6fc64.

PiperOrigin-RevId: 170038845
  • Loading branch information
iirina authored and katre committed Sep 26, 2017
1 parent 3b0bcd9 commit 4110393
Show file tree
Hide file tree
Showing 21 changed files with 315 additions and 288 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2017 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.build.lib.analysis.actions;

import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.util.Fingerprint;
import java.io.IOException;
import java.io.OutputStream;

/**
* Lazily writes the exec path of the given files separated by newline into a specified output file.
*/
public final class LazyWritePathsFileAction extends AbstractFileWriteAction {
private static final String GUID = "6be94d90-96f3-4bec-8104-1fb08abc2546";

private final NestedSet<Artifact> files;
private final boolean includeOnlyIfSource;

public LazyWritePathsFileAction(
ActionOwner owner, Artifact output, NestedSet<Artifact> files, boolean includeOnlyIfSource) {
super(owner, files, output, false);
this.files = NestedSetBuilder.fromNestedSet(files).build();
this.includeOnlyIfSource = includeOnlyIfSource;
}

public LazyWritePathsFileAction(
ActionOwner owner, Artifact output,
ImmutableSet<Artifact> files,
boolean includeOnlyIfSource) {
super(owner, Artifact.NO_ARTIFACTS, output, false);
this.files = NestedSetBuilder.<Artifact>stableOrder().addAll(files).build();
this.includeOnlyIfSource = includeOnlyIfSource;
}

@Override
public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
return new DeterministicWriter() {
@Override
public void writeOutputFile(OutputStream out) throws IOException {
out.write(getContents().toString().getBytes(UTF_8));
}
};
}

/**
* Computes the Action key for this action by computing the fingerprint for the file contents.
*/
@Override
protected String computeKey() {
Fingerprint f = new Fingerprint();
f.addString(GUID);
f.addBoolean(includeOnlyIfSource);
f.addString(getContents());
return f.hexDigestAndReset();
}

private String getContents() {
StringBuilder stringBuilder = new StringBuilder();
for (Artifact file : files) {
if (includeOnlyIfSource) {
if (file.isSourceArtifact()) {
stringBuilder.append(file.getRootRelativePathString());
stringBuilder.append("\n");
}
} else {
stringBuilder.append(file.getRootRelativePathString());
stringBuilder.append("\n");
}
}
return stringBuilder.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 the next blaze release (after 2017.07.30).
env.put("NEW_JAVA_COVERAGE_IMPL", "True");
// TODO(elenairina): Remove this after it reaches a blaze release.
env.put("NEW_JAVA_COVERAGE_IMPL", "released");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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;
Expand Down Expand Up @@ -261,6 +262,8 @@ public Artifact createStubAction(
List<String> jvmFlags,
Artifact executable,
String javaStartClass,
String coverageStartClass,
NestedSetBuilder<Artifact> filesBuilder,
String javaExecutable) {
Preconditions.checkState(ruleContext.getConfiguration().hasFragment(Jvm.class));

Expand Down Expand Up @@ -314,22 +317,35 @@ public Artifact createStubAction(
}
arguments.add(new ComputedClasspathSubstitution(classpath, workspacePrefix, isRunfilesEnabled));

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
: ""));
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, ""));
}

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));
Expand Down Expand Up @@ -660,49 +676,11 @@ 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,
JavaTargetAttributes.Builder attributes,
Artifact executable,
Artifact instrumentationMetadata,
JavaCompilationArtifacts.Builder javaArtifactsBuilder,
String mainClass)
public String addCoverageSupport(JavaCompilationHelper helper, Artifact executable)
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ 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"}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,6 @@ public static RuleConfiguredTargetBuilder createAndroidBinary(
ApkProvider.create(
zipAlignedApk,
unsignedApk,
androidCommon.getInstrumentedJar(),
applicationManifest.getManifest(),
debugKeystore))
.addProvider(AndroidPreDexJarProvider.class, AndroidPreDexJarProvider.create(jarToDex))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,12 +452,11 @@ private void compileResourceJar(
} else {
Artifact outputDepsProto =
javacHelper.createOutputDepsProtoArtifact(resourceClassJar, javaArtifactsBuilder);
javacHelper.createCompileActionWithInstrumentation(
javacHelper.createCompileAction(
resourceClassJar,
null /* manifestProtoOutput */,
null /* genSourceJar */,
outputDepsProto,
javaArtifactsBuilder);
outputDepsProto);
}
} else {
// Otherwise, it should have been the AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR.
Expand Down Expand Up @@ -705,8 +704,7 @@ private void initJava(
helper.createSourceJarAction(srcJar, genSourceJar);

outputDepsProto = helper.createOutputDepsProtoArtifact(classJar, javaArtifactsBuilder);
helper.createCompileActionWithInstrumentation(classJar, manifestProtoOutput, genSourceJar,
outputDepsProto, javaArtifactsBuilder);
helper.createCompileAction(classJar, manifestProtoOutput, genSourceJar, outputDepsProto);

if (isBinary) {
generatedExtensionRegistryProvider =
Expand Down Expand Up @@ -887,6 +885,10 @@ public ImmutableList<String> getJavacOpts() {
return javaCommon.getJavacOpts();
}

public Artifact getClassJar() {
return classJar;
}

public Artifact getGenClassJar() {
return genClassJar;
}
Expand All @@ -912,10 +914,6 @@ public NestedSet<Artifact> getJarsProducedForRuntime() {
return jarsProducedForRuntime;
}

public Artifact getInstrumentedJar() {
return javaCommon.getJavaCompilationArtifacts().getInstrumentedJar();
}

public NestedSet<Artifact> getTransitiveNeverLinkLibraries() {
return transitiveNeverlinkLibraries;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ 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()) {
Expand All @@ -154,7 +152,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
javaSemantics,
helper,
executable,
instrumentationMetadata,
null,
javaArtifactsBuilder,
attributesBuilder);

Expand All @@ -178,11 +176,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
helper.createOutputDepsProtoArtifact(classJar, javaArtifactsBuilder);
javaRuleOutputJarsProviderBuilder.setJdeps(outputDepsProtoArtifact);
helper.createCompileAction(
classJar,
manifestProtoOutput,
genSourceJar,
outputDepsProtoArtifact,
instrumentationMetadata);
classJar, manifestProtoOutput, genSourceJar, outputDepsProtoArtifact);
helper.createSourceJarAction(srcJar, genSourceJar);

setUpJavaCommon(javaCommon, helper, javaArtifactsBuilder.build());
Expand All @@ -202,6 +196,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
getJvmFlags(ruleContext, testClass),
executable,
mainClass,
"com.google.testing.junit.runner.GoogleTestRunner",
filesToBuildBuilder,
javaExecutable);

Artifact deployJar =
Expand Down Expand Up @@ -435,13 +431,6 @@ 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());
Expand Down Expand Up @@ -478,3 +467,4 @@ private static String getAndCheckTestClass(
return testClass;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
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
Expand All @@ -27,10 +26,9 @@ 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, coverageMetdata, mergedManifest, keystore);
return new AutoValue_ApkProvider(apk, unsignedApk, mergedManifest, keystore);
}

/** Returns the APK file built in the transitive closure. */
Expand All @@ -39,10 +37,6 @@ 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();

Expand Down
Loading

0 comments on commit 4110393

Please sign in to comment.