diff options
Diffstat (limited to 'src/main/java')
18 files changed, 222 insertions, 281 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWritePathsFileAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWritePathsFileAction.java new file mode 100644 index 0000000000..a43f09e6d9 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWritePathsFileAction.java @@ -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(); + } +}
\ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index f7d2fc184e..8b42e96721 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -461,8 +461,8 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa 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"); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java index de646ac546..424e16cb8c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.RuleContext; 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; @@ -261,6 +262,8 @@ public class BazelJavaSemantics implements JavaSemantics { List<String> jvmFlags, Artifact executable, String javaStartClass, + String coverageStartClass, + NestedSetBuilder<Artifact> filesBuilder, String javaExecutable) { Preconditions.checkState(ruleContext.getConfiguration().hasFragment(Jvm.class)); @@ -314,22 +317,35 @@ public class BazelJavaSemantics implements JavaSemantics { } 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)); @@ -660,49 +676,11 @@ public class BazelJavaSemantics implements JavaSemantics { 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); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt index dbe84b4fae..b549b34353 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt @@ -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"} diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 1385b16a7b..d4cdd637c4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -670,7 +670,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { ApkProvider.create( zipAlignedApk, unsignedApk, - androidCommon.getInstrumentedJar(), applicationManifest.getManifest(), debugKeystore)) .addProvider(AndroidPreDexJarProvider.class, AndroidPreDexJarProvider.create(jarToDex)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java index 0796ae2db6..b9b1986b08 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java @@ -452,12 +452,11 @@ public class AndroidCommon { } 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. @@ -705,8 +704,7 @@ public class AndroidCommon { 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 = @@ -887,6 +885,10 @@ public class AndroidCommon { return javaCommon.getJavacOpts(); } + public Artifact getClassJar() { + return classJar; + } + public Artifact getGenClassJar() { return genClassJar; } @@ -912,10 +914,6 @@ public class AndroidCommon { return jarsProducedForRuntime; } - public Artifact getInstrumentedJar() { - return javaCommon.getJavaCompilationArtifacts().getInstrumentedJar(); - } - public NestedSet<Artifact> getTransitiveNeverLinkLibraries() { return transitiveNeverlinkLibraries; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java index 6ac746f904..423b13af36 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java @@ -126,8 +126,6 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor 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()) { @@ -154,7 +152,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor javaSemantics, helper, executable, - instrumentationMetadata, + null, javaArtifactsBuilder, attributesBuilder); @@ -178,11 +176,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor 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()); @@ -202,6 +196,8 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor getJvmFlags(ruleContext, testClass), executable, mainClass, + "com.google.testing.junit.runner.GoogleTestRunner", + filesToBuildBuilder, javaExecutable); Artifact deployJar = @@ -435,13 +431,6 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor 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()); @@ -478,3 +467,4 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor return testClass; } } + diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkProvider.java index d265fd3160..70676b701d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkProvider.java @@ -17,7 +17,6 @@ import com.google.auto.value.AutoValue; 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 @@ -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. */ @@ -39,10 +37,6 @@ public abstract class ApkProvider implements TransitiveInfoProvider { /** 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(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java index 10f42eee77..e94c4e1aea 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java @@ -79,7 +79,6 @@ public class JavaBinary implements RuleConfiguredTargetFactory { JavaTargetAttributes.Builder attributesBuilder = common.initCommon(); attributesBuilder.addClassPathResources( ruleContext.getPrerequisiteArtifacts("classpath_resources", Mode.TARGET).list()); - // Add Java8 timezone resource data addTimezoneResourceForJavaBinaries(ruleContext, attributesBuilder); @@ -148,8 +147,6 @@ public class JavaBinary implements RuleConfiguredTargetFactory { } JavaCompilationArtifacts.Builder javaArtifactsBuilder = new JavaCompilationArtifacts.Builder(); - Artifact instrumentationMetadata = - helper.createInstrumentationMetadata(classJar, javaArtifactsBuilder); NestedSetBuilder<Artifact> filesBuilder = NestedSetBuilder.stableOrder(); Artifact executableForRunfiles = null; @@ -166,14 +163,7 @@ public class JavaBinary implements RuleConfiguredTargetFactory { filesBuilder.add(classJar).add(executableForRunfiles); if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { - mainClass = - semantics.addCoverageSupport( - helper, - attributesBuilder, - executableForRunfiles, - instrumentationMetadata, - javaArtifactsBuilder, - mainClass); + mainClass = semantics.addCoverageSupport(helper, executableForRunfiles); } } else { filesBuilder.add(classJar); @@ -224,8 +214,7 @@ public class JavaBinary implements RuleConfiguredTargetFactory { helper.createGenJarAction(classJar, manifestProtoOutput, genClassJar); } - helper.createCompileAction( - classJar, manifestProtoOutput, genSourceJar, outputDepsProto, instrumentationMetadata); + helper.createCompileAction(classJar, manifestProtoOutput, genSourceJar, outputDepsProto); helper.createSourceJarAction(srcJar, genSourceJar); common.setClassPathFragment( @@ -260,6 +249,8 @@ public class JavaBinary implements RuleConfiguredTargetFactory { jvmFlags, executableForRunfiles, mainClass, + originalMainClass, + filesBuilder, javaExecutable); if (!executableToRun.equals(executableForRunfiles)) { filesBuilder.add(executableToRun); @@ -511,13 +502,6 @@ public class JavaBinary implements RuleConfiguredTargetFactory { builder.addTargets(runtimeDeps, RunfilesProvider.DEFAULT_RUNFILES); semantics.addDependenciesForRunfiles(ruleContext, builder); - if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { - 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). @@ -600,3 +584,4 @@ public class JavaBinary implements RuleConfiguredTargetFactory { } } } + diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index d75277cf2b..73e4f91b3c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -173,28 +173,6 @@ public class JavaCommon { } } - /** - * Creates an action to aggregate all metadata artifacts into a single - * <target_name>_instrumented.jar file. - */ - public static void createInstrumentedJarAction( - RuleContext ruleContext, - JavaSemantics semantics, - List<Artifact> metadataArtifacts, - Artifact instrumentedJar, - String mainClass) - throws InterruptedException { - // In Jacoco's setup, metadata artifacts are real jars. - new DeployArchiveBuilder(semantics, ruleContext) - .setOutputJar(instrumentedJar) - // We need to save the original mainClass because we're going to run inside CoverageRunner - .setJavaStartClass(mainClass) - .setAttributes(new JavaTargetAttributes.Builder(semantics).build()) - .addRuntimeJars(ImmutableList.copyOf(metadataArtifacts)) - .setCompression(DeployArchiveBuilder.Compression.UNCOMPRESSED) - .build(); - } - public static ImmutableList<String> getConstraints(RuleContext ruleContext) { return ruleContext.getRule().isAttrDefined("constraints", Type.STRING_LIST) ? ImmutableList.copyOf(ruleContext.attributes().get("constraints", Type.STRING_LIST)) @@ -756,7 +734,6 @@ public class JavaCommon { .addTransitiveTargets(runtimeDepInfo, true, ClasspathType.RUNTIME_ONLY) .build(); attributes.addRuntimeClassPathEntries(args.getRuntimeJars()); - attributes.addInstrumentationMetadataEntries(args.getInstrumentationMetadata()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java index f9198eee58..a83bf36137 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java @@ -22,7 +22,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.util.FileType; -import java.util.Collection; /** A container of Java compilation artifacts. */ @AutoValue @@ -43,16 +42,13 @@ public abstract class JavaCompilationArgs { JavaCompilationArgs.create( NestedSetBuilder.<Artifact>create(Order.NAIVE_LINK_ORDER), NestedSetBuilder.<Artifact>create(Order.NAIVE_LINK_ORDER), - NestedSetBuilder.<Artifact>create(Order.NAIVE_LINK_ORDER), NestedSetBuilder.<Artifact>create(Order.NAIVE_LINK_ORDER)); private static JavaCompilationArgs create( NestedSet<Artifact> runtimeJars, NestedSet<Artifact> compileTimeJars, - NestedSet<Artifact> fullCompileTimeJars, - NestedSet<Artifact> instrumentationMetadata) { - return new AutoValue_JavaCompilationArgs( - runtimeJars, compileTimeJars, fullCompileTimeJars, instrumentationMetadata); + NestedSet<Artifact> fullCompileTimeJars) { + return new AutoValue_JavaCompilationArgs(runtimeJars, compileTimeJars, fullCompileTimeJars); } /** Returns transitive runtime jars. */ @@ -67,9 +63,6 @@ public abstract class JavaCompilationArgs { */ public abstract NestedSet<Artifact> getFullCompileTimeJars(); - /** Returns transitive instrumentation metadata jars. */ - public abstract NestedSet<Artifact> getInstrumentationMetadata(); - /** * Returns a new builder instance. */ @@ -87,8 +80,6 @@ public abstract class JavaCompilationArgs { NestedSetBuilder.naiveLinkOrder(); private final NestedSetBuilder<Artifact> fullCompileTimeJarsBuilder = NestedSetBuilder.naiveLinkOrder(); - private final NestedSetBuilder<Artifact> instrumentationMetadataBuilder = - NestedSetBuilder.naiveLinkOrder(); /** * Use {@code TransitiveJavaCompilationArgs#builder()} to instantiate the builder. @@ -107,7 +98,6 @@ public abstract class JavaCompilationArgs { } addCompileTimeJars(other.getCompileTimeJars()); addFullCompileTimeJars(other.getFullCompileTimeJars()); - addInstrumentationMetadata(other.getInstrumentationMetadata()); return this; } @@ -165,16 +155,6 @@ public abstract class JavaCompilationArgs { return this; } - public Builder addInstrumentationMetadata(Artifact instrumentationMetadata) { - this.instrumentationMetadataBuilder.add(instrumentationMetadata); - return this; - } - - public Builder addInstrumentationMetadata(Collection<Artifact> instrumentationMetadata) { - this.instrumentationMetadataBuilder.addAll(instrumentationMetadata); - return this; - } - public Builder addTransitiveCompilationArgs( JavaCompilationArgsProvider dep, boolean recursive, ClasspathType type) { JavaCompilationArgs args = recursive @@ -268,8 +248,6 @@ public abstract class JavaCompilationArgs { if (!ClasspathType.COMPILE_ONLY.equals(type)) { runtimeJarsBuilder.addTransitive(args.getRuntimeJars()); } - instrumentationMetadataBuilder.addTransitive( - args.getInstrumentationMetadata()); return this; } @@ -280,8 +258,7 @@ public abstract class JavaCompilationArgs { return JavaCompilationArgs.create( runtimeJarsBuilder.build(), compileTimeJarsBuilder.build(), - fullCompileTimeJarsBuilder.build(), - instrumentationMetadataBuilder.build()); + fullCompileTimeJarsBuilder.build()); } } @@ -301,3 +278,4 @@ public abstract class JavaCompilationArgs { JavaCompilationArgs() {} } + diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java index a6d346a11e..0d313033da 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArtifacts.java @@ -20,10 +20,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; - import java.util.LinkedHashSet; import java.util.Set; - import javax.annotation.Nullable; /** @@ -49,9 +47,7 @@ public abstract class JavaCompilationArtifacts { public abstract ImmutableList<Artifact> getRuntimeJars(); public abstract ImmutableList<Artifact> getCompileTimeJars(); abstract ImmutableList<Artifact> getFullCompileTimeJars(); - public abstract ImmutableList<Artifact> getInstrumentationMetadata(); @Nullable public abstract Artifact getCompileTimeDependencyArtifact(); - @Nullable public abstract Artifact getInstrumentedJar(); /** Returns a builder for a {@link JavaCompilationArtifacts}. */ public static Builder builder() { @@ -65,9 +61,7 @@ public abstract class JavaCompilationArtifacts { private final Set<Artifact> runtimeJars = new LinkedHashSet<>(); private final Set<Artifact> compileTimeJars = new LinkedHashSet<>(); private final Set<Artifact> fullCompileTimeJars = new LinkedHashSet<>(); - private final Set<Artifact> instrumentationMetadata = new LinkedHashSet<>(); private Artifact compileTimeDependencies; - private Artifact instrumentedJar; public JavaCompilationArtifacts build() { Preconditions.checkState(fullCompileTimeJars.size() == compileTimeJars.size()); @@ -75,9 +69,7 @@ public abstract class JavaCompilationArtifacts { ImmutableList.copyOf(runtimeJars), ImmutableList.copyOf(compileTimeJars), ImmutableList.copyOf(fullCompileTimeJars), - ImmutableList.copyOf(instrumentationMetadata), - compileTimeDependencies, - instrumentedJar); + compileTimeDependencies); } public Builder addRuntimeJar(Artifact jar) { @@ -112,19 +104,9 @@ public abstract class JavaCompilationArtifacts { return this; } - public Builder addInstrumentationMetadata(Artifact instrumentationMetadata) { - this.instrumentationMetadata.add(instrumentationMetadata); - return this; - } - public Builder setCompileTimeDependencies(@Nullable Artifact compileTimeDependencies) { this.compileTimeDependencies = compileTimeDependencies; return this; } - - public Builder setInstrumentedJar(@Nullable Artifact instrumentedJar) { - this.instrumentedJar = instrumentedJar; - return this; - } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java index b1629f0063..49d3e97c66 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java @@ -165,18 +165,16 @@ public final class JavaCompilationHelper { * * @param outputJar the class jar Artifact to create with the Action * @param manifestProtoOutput the output artifact for the manifest proto emitted from JavaBuilder - * @param gensrcOutputJar the generated sources jar Artifact to create with the Action - * (null if no sources will be generated). - * @param outputDepsProto the compiler-generated jdeps file to create with the Action - * (null if not requested) - * @param outputMetadata metadata file (null if no instrumentation is needed). + * @param gensrcOutputJar the generated sources jar Artifact to create with the Action (null if no + * sources will be generated). + * @param outputDepsProto the compiler-generated jdeps file to create with the Action (null if not + * requested) */ public void createCompileAction( Artifact outputJar, Artifact manifestProtoOutput, @Nullable Artifact gensrcOutputJar, - @Nullable Artifact outputDepsProto, - @Nullable Artifact outputMetadata) { + @Nullable Artifact outputDepsProto) { JavaTargetAttributes attributes = getAttributes(); @@ -209,7 +207,7 @@ public final class JavaCompilationHelper { builder.setGensrcOutputJar(gensrcOutputJar); builder.setOutputDepsProto(outputDepsProto); builder.setAdditionalOutputs(attributes.getAdditionalOutputs()); - builder.setMetadata(outputMetadata); + builder.setFileWithPathsForCoverage(maybeCreateFileWithPathsForCoverage(outputJar)); builder.setInstrumentationJars(jacocoInstrumentation); builder.setSourceFiles(attributes.getSourceFiles()); builder.addSourceJars(attributes.getSourceJars()); @@ -253,65 +251,15 @@ public final class JavaCompilationHelper { } } - /** - * Returns the instrumentation metadata files to be generated for a given output jar. - * - * <p>Only called if the output jar actually needs to be instrumented. - */ - @Nullable - private static Artifact createInstrumentationMetadataArtifact( - RuleContext ruleContext, Artifact outputJar) { - PathFragment packageRelativePath = outputJar.getRootRelativePath().relativeTo( - ruleContext.getPackageDirectory()); - return ruleContext.getPackageRelativeArtifact( - FileSystemUtils.replaceExtension(packageRelativePath, ".em"), outputJar.getRoot()); - } - - /** - * Creates the Action that compiles Java source files and optionally instruments them for - * coverage. - * - * @param outputJar the class jar Artifact to create with the Action - * @param manifestProtoOutput the output artifact for the manifest proto emitted from JavaBuilder - * @param gensrcJar the generated sources jar Artifact to create with the Action - * @param outputDepsProto the compiler-generated jdeps file to create with the Action - * @param javaArtifactsBuilder the build to store the instrumentation metadata in - */ - public void createCompileActionWithInstrumentation( - Artifact outputJar, - Artifact manifestProtoOutput, - @Nullable Artifact gensrcJar, - @Nullable Artifact outputDepsProto, - JavaCompilationArtifacts.Builder javaArtifactsBuilder) { - createCompileAction( - outputJar, - manifestProtoOutput, - gensrcJar, - outputDepsProto, - createInstrumentationMetadata(outputJar, javaArtifactsBuilder)); - } - - /** - * Creates the instrumentation metadata artifact if needed. - * - * @return the instrumentation metadata artifact or null if instrumentation is - * disabled - */ - @Nullable - public Artifact createInstrumentationMetadata(Artifact outputJar, - JavaCompilationArtifacts.Builder javaArtifactsBuilder) { - // If we need to instrument the jar, add additional output (the coverage metadata file) to the - // JavaCompileAction. - Artifact instrumentationMetadata = null; - if (shouldInstrumentJar()) { - instrumentationMetadata = createInstrumentationMetadataArtifact( - getRuleContext(), outputJar); - - if (instrumentationMetadata != null) { - javaArtifactsBuilder.addInstrumentationMetadata(instrumentationMetadata); - } + private Artifact maybeCreateFileWithPathsForCoverage(Artifact outputJar) { + if (!shouldInstrumentJar()) { + return null; } - return instrumentationMetadata; + PathFragment packageRelativePath = + outputJar.getRootRelativePath().relativeTo(ruleContext.getPackageDirectory()); + PathFragment path = + FileSystemUtils.replaceExtension(packageRelativePath, "-paths-for-coverage.txt"); + return ruleContext.getPackageRelativeArtifact(path, outputJar.getRoot()); } private boolean shouldInstrumentJar() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index 7f04049806..fc59b2f64c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.CustomMultiArgv; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg; +import com.google.devtools.build.lib.analysis.actions.LazyWritePathsFileAction; import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -500,7 +501,7 @@ public final class JavaCompileAction extends SpawnAction { private Artifact outputDepsProto; private Collection<Artifact> additionalOutputs; private Artifact paramFile; - private Artifact metadata; + private Artifact fileWithPathsForCoverage; private ImmutableSet<Artifact> sourceFiles = ImmutableSet.of(); private final Collection<Artifact> sourceJars = new ArrayList<>(); private BuildConfiguration.StrictDepsMode strictJavaDeps = @@ -592,7 +593,6 @@ public final class JavaCompileAction extends SpawnAction { .addAll( new ArrayList<>(Collections2.filter(Arrays.asList( outputJar, - metadata, gensrcOutputJar, manifestProtoOutput, outputDepsProto), Predicates.notNull()))); @@ -615,6 +615,11 @@ public final class JavaCompileAction extends SpawnAction { semantics.getJavaBuilderMainClass(), pathSeparator); + if (fileWithPathsForCoverage != null) { + analysisEnvironment.registerAction(new LazyWritePathsFileAction( + owner, fileWithPathsForCoverage, sourceFiles, true)); + } + // The actual params-file-based command line executed for a compile action. CommandLine javaBuilderCommandLine = CustomCommandLine.builder() @@ -630,7 +635,7 @@ public final class JavaCompileAction extends SpawnAction { .addAll(instrumentationJars) .build(); - NestedSet<Artifact> inputs = + NestedSetBuilder<Artifact> inputsBuilder = NestedSetBuilder.<Artifact>stableOrder() .addTransitive(classpathEntries) .addTransitive(compileTimeDependencyArtifacts) @@ -642,8 +647,12 @@ public final class JavaCompileAction extends SpawnAction { .addAll(sourcePathEntries) .addAll(extdirInputs) .add(paramFile) - .addTransitive(tools) - .build(); + .addTransitive(tools); + if (fileWithPathsForCoverage != null) { + inputsBuilder.add(fileWithPathsForCoverage); + } + + NestedSet<Artifact> inputs = inputsBuilder.build(); return new JavaCompileAction( owner, @@ -757,9 +766,9 @@ public final class JavaCompileAction extends SpawnAction { } } } - if (metadata != null) { + if (fileWithPathsForCoverage != null) { result.add("--post_processor"); - result.addExecPath(JACOCO_INSTRUMENTATION_PROCESSOR, metadata); + result.addExecPath(JACOCO_INSTRUMENTATION_PROCESSOR, fileWithPathsForCoverage); result.addPath( configuration .getCoverageMetadataDirectory(targetLabel.getPackageIdentifier().getRepository()) @@ -869,8 +878,8 @@ public final class JavaCompileAction extends SpawnAction { return this; } - public Builder setMetadata(Artifact metadata) { - this.metadata = metadata; + public Builder setFileWithPathsForCoverage(Artifact fileWithExecPathsForCoverage) { + this.fileWithPathsForCoverage = fileWithExecPathsForCoverage; return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java index 5932b19526..9326f735aa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java @@ -126,8 +126,7 @@ public class JavaLibrary implements RuleConfiguredTargetFactory { Artifact outputDepsProto = helper.createOutputDepsProtoArtifact(classJar, javaArtifactsBuilder); - helper.createCompileActionWithInstrumentation(classJar, manifestProtoOutput, genSourceJar, - outputDepsProto, javaArtifactsBuilder); + helper.createCompileAction(classJar, manifestProtoOutput, genSourceJar, outputDepsProto); helper.createSourceJarAction(srcJar, genSourceJar); Artifact iJar = null; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java index 1be54d5358..8b43e9491c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java @@ -188,11 +188,7 @@ public final class JavaLibraryHelper { jacocoInstrumental); Artifact outputDepsProto = helper.createOutputDepsProtoArtifact(output, artifactsBuilder); helper.createCompileAction( - output, - null /* manifestProtoOutput */, - null /* gensrcOutputJar */, - outputDepsProto, - null /* outputMetadata */); + output, null /* manifestProtoOutput */, null /* gensrcOutputJar */, outputDepsProto); helper.createCompileTimeJarAction(output, artifactsBuilder); artifactsBuilder.addRuntimeJar(output); @@ -244,7 +240,6 @@ public final class JavaLibraryHelper { .build(); attributes.addCompileTimeClassPathEntries(args.getCompileTimeJars()); attributes.addRuntimeClassPathEntries(args.getRuntimeJars()); - attributes.addInstrumentationMetadataEntries(args.getInstrumentationMetadata()); } private NestedSet<Artifact> getNonRecursiveCompileTimeJarsFromDeps() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java index 0dd4ab0b87..7289d1afc6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java @@ -21,6 +21,7 @@ import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.fro import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Streams; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.LanguageDependentFragment.LibraryLanguage; import com.google.devtools.build.lib.analysis.OutputGroupProvider; @@ -31,6 +32,7 @@ import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.Runfiles.Builder; 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.TemplateExpansionAction.ComputedSubstitution; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -44,8 +46,10 @@ import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistry import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.vfs.PathFragment; +import java.io.File; import java.util.Collection; import java.util.List; +import java.util.stream.Collectors; import javax.annotation.Nullable; /** @@ -65,6 +69,9 @@ public interface JavaSemantics { SafeImplicitOutputsFunction JAVA_BINARY_SOURCE_JAR = fromTemplates("%{name}-src.jar"); + SafeImplicitOutputsFunction JAVA_COVERAGE_RUNTIME_CLASS_PATH_TXT = + fromTemplates("%{name}-runtime-classpath.txt"); + SafeImplicitOutputsFunction JAVA_BINARY_DEPLOY_JAR = fromTemplates("%{name}_deploy.jar"); SafeImplicitOutputsFunction JAVA_BINARY_MERGED_JAR = @@ -209,7 +216,34 @@ public interface JavaSemantics { Optional.presentInstances(javaConfig.getBytecodeOptimizers().values())); }); - String IJAR_LABEL = "//tools/defaults:ijar"; + String JACOCO_METADATA_PLACEHOLDER = "%set_jacoco_metadata%"; + String JACOCO_MAIN_CLASS_PLACEHOLDER = "%set_jacoco_main_class%"; + String JACOCO_JAVA_RUNFILES_ROOT_PLACEHOLDER = "%set_jacoco_java_runfiles_root%"; + + /** + * Substitution for exporting the jars needed for jacoco coverage. + */ + class ComputedJacocoSubstitution extends ComputedSubstitution { + private final NestedSet<Artifact> jars; + private final String pathPrefix; + + public ComputedJacocoSubstitution(NestedSet<Artifact> jars, String workspacePrefix) { + super(JACOCO_METADATA_PLACEHOLDER); + this.jars = jars; + this.pathPrefix = "${JAVA_RUNFILES}/" + workspacePrefix; + } + + /** + * Concatenating the root relative paths of the artifacts. Each relative path entry is prepended + * with "${JAVA_RUNFILES}" and the workspace prefix. + */ + @Override + public String getValue() { + return Streams.stream(jars) + .map(artifact -> pathPrefix + "/" + artifact.getRootRelativePathString()) + .collect(Collectors.joining(File.pathSeparator, "export JACOCO_METADATA_JARS=", "")); + } + } /** * Verifies if the rule contains any errors. @@ -286,6 +320,8 @@ public interface JavaSemantics { List<String> jvmFlags, Artifact executable, String javaStartClass, + String coverageStartClass, + NestedSetBuilder<Artifact> filesBuilder, String javaExecutable); /** @@ -331,13 +367,7 @@ public interface JavaSemantics { * * @return new main class */ - String addCoverageSupport( - JavaCompilationHelper helper, - JavaTargetAttributes.Builder attributes, - Artifact executable, - Artifact instrumentationMetadata, - JavaCompilationArtifacts.Builder javaArtifactsBuilder, - String mainClass) + String addCoverageSupport(JavaCompilationHelper helper, Artifact executable) throws InterruptedException; /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java index 28f6c86f5c..2cf3a36a03 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java @@ -81,7 +81,6 @@ public class JavaTargetAttributes { private final Map<PathFragment, Artifact> resources = new LinkedHashMap<>(); private final NestedSetBuilder<Artifact> resourceJars = NestedSetBuilder.stableOrder(); private final List<Artifact> messages = new ArrayList<>(); - private final List<Artifact> instrumentationMetadata = new ArrayList<>(); private final List<Artifact> sourceJars = new ArrayList<>(); private final List<Artifact> classPathResources = new ArrayList<>(); @@ -141,7 +140,6 @@ public class JavaTargetAttributes { Preconditions.checkArgument(!built); addCompileTimeClassPathEntries(context.getCompileTimeJars()); addRuntimeClassPathEntries(context.getRuntimeJars()); - addInstrumentationMetadataEntries(context.getInstrumentationMetadata()); return this; } @@ -255,12 +253,6 @@ public class JavaTargetAttributes { return this; } - public Builder addInstrumentationMetadataEntries(Iterable<Artifact> metadataEntries) { - Preconditions.checkArgument(!built); - Iterables.addAll(instrumentationMetadata, metadataEntries); - return this; - } - public Builder addNativeLibrary(Artifact nativeLibrary) { Preconditions.checkArgument(!built); String name = nativeLibrary.getFilename(); @@ -400,12 +392,6 @@ public class JavaTargetAttributes { return !sourceFiles.isEmpty(); } - /** @deprecated prefer {@link JavaTargetAttributes#getInstrumentationMetadata} */ - @Deprecated - public List<Artifact> getInstrumentationMetadata() { - return instrumentationMetadata; - } - /** @deprecated prefer {@link JavaTargetAttributes#hasSourceJars} */ @Deprecated public boolean hasSourceJars() { |