diff options
Diffstat (limited to 'src/main')
6 files changed, 141 insertions, 69 deletions
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 f5fa1ff7ae..d988eb31ae 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 @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; -import com.google.devtools.build.lib.analysis.RunfilesProvider; 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; @@ -33,18 +32,22 @@ import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Su import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Template; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.bazel.rules.BazelConfiguration; +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.packages.BuildType; +import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression; import com.google.devtools.build.lib.rules.java.JavaCommon; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaCompilationHelper; import com.google.devtools.build.lib.rules.java.JavaConfiguration; import com.google.devtools.build.lib.rules.java.JavaHelper; import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider; -import com.google.devtools.build.lib.rules.java.JavaRunfilesProvider; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; import com.google.devtools.build.lib.rules.java.JavaTargetAttributes; @@ -81,6 +84,8 @@ public class BazelJavaSemantics implements JavaSemantics { private static final String JACOCO_COVERAGE_RUNNER_MAIN_CLASS = "com.google.testing.coverage.JacocoCoverageRunner"; + private static final String BAZEL_TEST_RUNNER = "com.google.testing.junit.runner.BazelTestRunner"; + private BazelJavaSemantics() { } @@ -103,10 +108,20 @@ public class BazelJavaSemantics implements JavaSemantics { private static final String JUNIT4_RUNNER = "org.junit.runner.JUnitCore"; + @Nullable private String getMainClassInternal(RuleContext ruleContext, ImmutableList<Artifact> sources) { if (!ruleContext.attributes().get("create_executable", Type.BOOLEAN)) { return null; } + String mainClass = determineMainClassFromRule(ruleContext); + + if (mainClass.isEmpty()) { + mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources); + } + return mainClass; + } + + private String determineMainClassFromRule(RuleContext ruleContext) { String mainClass = ruleContext.attributes().get("main_class", Type.STRING); // Legacy behavior for java_test rules: main_class defaulted to JUnit4 runner. @@ -120,9 +135,8 @@ public class BazelJavaSemantics implements JavaSemantics { if (mainClass.isEmpty()) { if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN) && !useLegacyJavaTest(ruleContext)) { - return "com.google.testing.junit.runner.BazelTestRunner"; + return BAZEL_TEST_RUNNER; } - mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources); } return mainClass; } @@ -164,10 +178,55 @@ public class BazelJavaSemantics implements JavaSemantics { return ruleContext.getPrerequisiteArtifacts("resources", Mode.TARGET).list(); } + /** + * Used to generate the Classpaths contained within the stub. + */ + private static class ComputedClasspathSubstitution extends ComputedSubstitution { + private final NestedSet<Artifact> jars; + private final String workspacePrefix; + private final boolean isRunfilesEnabled; + + ComputedClasspathSubstitution( + String key, NestedSet<Artifact> jars, String workspacePrefix, boolean isRunfilesEnabled) { + super(key); + this.jars = jars; + this.workspacePrefix = workspacePrefix; + this.isRunfilesEnabled = isRunfilesEnabled; + } + + /** + * Builds a class path by concatenating the root relative paths of the artifacts. Each relative + * path entry is prepended with "${RUNPATH}" which will be expanded by the stub script at + * runtime, to either "${JAVA_RUNFILES}/" or if we are lucky, the empty string. + */ + @Override + public String getValue() { + StringBuilder buffer = new StringBuilder(); + buffer.append("\""); + for (Artifact artifact : jars) { + if (buffer.length() > 1) { + buffer.append(File.pathSeparatorChar); + } + if (!isRunfilesEnabled) { + buffer.append("$(rlocation "); + PathFragment runfilePath = + new PathFragment(new PathFragment(workspacePrefix), artifact.getRunfilesPath()); + buffer.append(runfilePath.normalize().getPathString()); + buffer.append(")"); + } else { + buffer.append("${RUNPATH}"); + buffer.append(artifact.getRunfilesPath().getPathString()); + } + } + buffer.append("\""); + return buffer.toString(); + } + } + @Override public Artifact createStubAction( RuleContext ruleContext, - final JavaCommon javaCommon, + JavaCommon javaCommon, List<String> jvmFlags, Artifact executable, String javaStartClass, @@ -188,18 +247,26 @@ public class BazelJavaSemantics implements JavaSemantics { arguments.add(Substitution.of("%javabin%", javaExecutable)); arguments.add(Substitution.of("%needs_runfiles%", ruleContext.getFragment(Jvm.class).getJavaExecutable().isAbsolute() ? "0" : "1")); + + NestedSet<Artifact> classpath = javaCommon.getRuntimeClasspath(); + NestedSet<Artifact> testTargetClasspath = NestedSetBuilder.emptySet(Order.STABLE_ORDER); + TransitiveInfoCollection testSupport = getTestSupport(ruleContext); + if (TargetUtils.isTestRule(ruleContext.getRule()) + && determineMainClassFromRule(ruleContext).equals(BAZEL_TEST_RUNNER) + && testSupport != null) { + // Keep only the locations containing the classes to start the test runner itself within, + // classpath variable, and place all the paths required for the test run in + // testTargetClasspath, so that the classes for the test target may be loaded by a separate + // ClassLoader. + testTargetClasspath = classpath; + classpath = getRuntimeJarsForTargets(testSupport); + } arguments.add( - new ComputedSubstitution("%classpath%") { - @Override - public String getValue() { - StringBuilder buffer = new StringBuilder(); - Iterable<Artifact> jars = javaCommon.getRuntimeClasspath(); - char delimiter = File.pathSeparatorChar; - appendRunfilesRelativeEntries( - buffer, jars, workspacePrefix, delimiter, isRunfilesEnabled); - return buffer.toString(); - } - }); + new ComputedClasspathSubstitution( + "%classpath%", classpath, workspacePrefix, isRunfilesEnabled)); + arguments.add( + new ComputedClasspathSubstitution( + "%test_target_classpath%", testTargetClasspath, workspacePrefix, isRunfilesEnabled)); JavaCompilationArtifacts javaArtifacts = javaCommon.getJavaCompilationArtifacts(); String path = @@ -250,40 +317,7 @@ public class BazelJavaSemantics implements JavaSemantics { } } - /** - * Builds a class path by concatenating the root relative paths of the artifacts separated by the - * delimiter. Each relative path entry is prepended with "${RUNPATH}" which will be expanded by - * the stub script at runtime, to either "${JAVA_RUNFILES}/" or if we are lucky, the empty string. - * - * @param buffer the buffer to use for concatenating the entries - * @param artifacts the entries to concatenate in the buffer - * @param delimiter the delimiter character to separate the entries - */ - private static void appendRunfilesRelativeEntries( - StringBuilder buffer, - Iterable<Artifact> artifacts, - String workspacePrefix, - char delimiter, - boolean isRunfilesEnabled) { - buffer.append("\""); - for (Artifact artifact : artifacts) { - if (buffer.length() > 1) { - buffer.append(delimiter); - } - if (!isRunfilesEnabled) { - buffer.append("$(rlocation "); - PathFragment runfilePath = - new PathFragment(new PathFragment(workspacePrefix), artifact.getRunfilesPath()); - buffer.append(runfilePath.normalize().getPathString()); - buffer.append(")"); - } else { - buffer.append("${RUNPATH}"); - buffer.append(artifact.getRunfilesPath().getPathString()); - } - } - buffer.append("\""); - } - + @Nullable private TransitiveInfoCollection getTestSupport(RuleContext ruleContext) { if (!isJavaBinaryOrJavaTest(ruleContext)) { return null; @@ -300,13 +334,25 @@ public class BazelJavaSemantics implements JavaSemantics { } } + private static NestedSet<Artifact> getRuntimeJarsForTargets(TransitiveInfoCollection... deps) { + // The dep may be a simple JAR and not a java rule, hence we can't simply do + // dep.getProvider(JavaCompilationArgsProvider.class).getRecursiveJavaCompilationArgs(), + // so we reuse the logic within JavaCompilationArgs to handle both scenarios. + JavaCompilationArgs args = + JavaCompilationArgs.builder() + .addTransitiveTargets( + ImmutableList.copyOf(deps), /*recursive=*/ true, ClasspathType.RUNTIME_ONLY) + .build(); + return args.getRuntimeJars(); + } + @Override public void addRunfilesForBinary(RuleContext ruleContext, Artifact launcher, Runfiles.Builder runfilesBuilder) { TransitiveInfoCollection testSupport = getTestSupport(ruleContext); if (testSupport != null) { - runfilesBuilder.addTarget(testSupport, JavaRunfilesProvider.TO_RUNFILES); - runfilesBuilder.addTarget(testSupport, RunfilesProvider.DEFAULT_RUNFILES); + // Not using addTransitiveArtifacts() due to the mismatch in NestedSet ordering. + runfilesBuilder.addArtifacts(getRuntimeJarsForTargets(testSupport)); } } @@ -316,12 +362,16 @@ public class BazelJavaSemantics implements JavaSemantics { @Override public void collectTargetsTreatedAsDeps( - RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder) { + RuleContext ruleContext, + ImmutableList.Builder<TransitiveInfoCollection> builder, + ClasspathType type) { + if (type == ClasspathType.COMPILE_ONLY) { + // We add the test support below, but the test framework's deps are not relevant for + // COMPILE_ONLY, hence we return here. + return; + } TransitiveInfoCollection testSupport = getTestSupport(ruleContext); if (testSupport != null) { - // TODO(bazel-team): The testsupport is used as the test framework - // and really only needs to be on the runtime, not compile-time - // classpath. builder.add(testSupport); } } 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 cdc7ee4aa7..312976f586 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 @@ -228,6 +228,10 @@ else CLASSPATH=%classpath% fi +# Export the classpaths which would be used to load the classes of the test target as an +# environment variable. +export TEST_TARGET_CLASSPATH=%test_target_classpath% + # If using Jacoco in offline instrumentation mode, the CLASSPATH contains instrumented files. # 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. 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 03c6a594a3..8056e7cb0d 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 @@ -95,10 +95,6 @@ public class JavaBinary implements RuleConfiguredTargetFactory { } } - List<TransitiveInfoCollection> deps = - // Do not remove <TransitiveInfoCollection>: workaround for Java 7 type inference. - Lists.<TransitiveInfoCollection>newArrayList( - common.targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY)); semantics.checkRule(ruleContext, common); semantics.checkForProtoLibraryAndJavaProtoLibraryOnSameProto(ruleContext, common); String mainClass = semantics.getMainClass(ruleContext, common.getSrcsArtifacts()); @@ -110,6 +106,10 @@ public class JavaBinary implements RuleConfiguredTargetFactory { // Collect the transitive dependencies. JavaCompilationHelper helper = new JavaCompilationHelper( ruleContext, semantics, common.getJavacOpts(), attributesBuilder); + List<TransitiveInfoCollection> deps = + // Do not remove <TransitiveInfoCollection>: workaround for Java 7 type inference. + Lists.<TransitiveInfoCollection>newArrayList( + common.targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY)); helper.addLibrariesToAttributes(deps); attributesBuilder.addNativeLibraries( collectNativeLibraries(common.targetsTreatedAsDeps(ClasspathType.BOTH))); 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 344793423e..b6f0f2b448 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 @@ -664,7 +664,7 @@ public class JavaCommon { } builder.addAll(ruleContext.getPrerequisites("deps", Mode.TARGET)); - semantics.collectTargetsTreatedAsDeps(ruleContext, builder); + semantics.collectTargetsTreatedAsDeps(ruleContext, builder, type); // Implicitly add dependency on java launcher cc_binary when --java_launcher= is enabled, // or when launcher attribute is specified in a build rule. 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 ed638b2147..9f5034c1dc 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 @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.vfs.PathFragment; @@ -262,7 +263,7 @@ public interface JavaSemantics { */ Artifact createStubAction( RuleContext ruleContext, - final JavaCommon javaCommon, + JavaCommon javaCommon, List<String> jvmFlags, Artifact executable, String javaStartClass, @@ -284,11 +285,11 @@ public interface JavaSemantics { */ Iterable<String> getExtraJavacOpts(RuleContext ruleContext); - /** - * Add additional targets to be treated as direct dependencies. - */ + /** Add additional targets to be treated as direct dependencies. */ void collectTargetsTreatedAsDeps( - RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder); + RuleContext ruleContext, + ImmutableList.Builder<TransitiveInfoCollection> builder, + ClasspathType type); /** * Enables coverage support for the java target - adds instrumented jar to the classpath and diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java index d4a0fa8140..e4193e8420 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java @@ -92,7 +92,7 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { return execInWorker( action, actionExecutionContext, - spawn.getEnvironment(), + addPersistentRunnerVars(spawn.getEnvironment()), startupArgs, actionExecutionContext.getExecutor().getExecRoot(), maxRetries); @@ -107,6 +107,12 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { int retriesLeft) throws ExecException, InterruptedException, IOException { Executor executor = actionExecutionContext.getExecutor(); + // TODO(kush): Remove once we're out of the experimental phase. + executor + .getEventHandler() + .handle( + Event.warn( + "RUNNING TEST IN AN EXPERIMENTAL PERSISTENT WORKER. RESULTS MAY BE INACCURATE")); TestResultData.Builder builder = TestResultData.newBuilder(); Path testLogPath = action.getTestLog().getPath(); @@ -191,6 +197,19 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { } } + private static Map<String, String> addPersistentRunnerVars(Map<String, String> originalEnv) + throws UserExecException { + if (originalEnv.containsKey("PERSISTENT_TEST_RUNNER")) { + throw new UserExecException( + "Found clashing environment variable with persistent_test_runner." + + " Please use another test strategy"); + } + return ImmutableMap.<String, String>builder() + .putAll(originalEnv) + .put("PERSISTENT_TEST_RUNNER", "true") + .build(); + } + private List<String> getStartUpArgs(TestRunnerAction action) throws ExecException { List<String> args = getArgs(/*coverageScript=*/ "", action); ImmutableList.Builder<String> startupArgs = ImmutableList.builder(); @@ -198,8 +217,6 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { startupArgs.add(args.get(0)).add("--no_echo"); // Add remaining of the original args. startupArgs.addAll(args.subList(1, args.size())); - // Make the Test runner run persistently. - startupArgs.add("--persistent_test_runner"); // Add additional flags requested for this invocation. startupArgs.addAll(MoreObjects.firstNonNull( extraFlags.get(action.getMnemonic()), ImmutableList.<String>of())); |