diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build')
6 files changed, 69 insertions, 141 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 d988eb31ae..f5fa1ff7ae 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,6 +24,7 @@ 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; @@ -32,22 +33,18 @@ 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; @@ -84,8 +81,6 @@ 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() { } @@ -108,20 +103,10 @@ 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. @@ -135,8 +120,9 @@ public class BazelJavaSemantics implements JavaSemantics { if (mainClass.isEmpty()) { if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN) && !useLegacyJavaTest(ruleContext)) { - return BAZEL_TEST_RUNNER; + return "com.google.testing.junit.runner.BazelTestRunner"; } + mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources); } return mainClass; } @@ -178,55 +164,10 @@ 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, - JavaCommon javaCommon, + final JavaCommon javaCommon, List<String> jvmFlags, Artifact executable, String javaStartClass, @@ -247,26 +188,18 @@ 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 ComputedClasspathSubstitution( - "%classpath%", classpath, workspacePrefix, isRunfilesEnabled)); - arguments.add( - new ComputedClasspathSubstitution( - "%test_target_classpath%", testTargetClasspath, workspacePrefix, isRunfilesEnabled)); + 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(); + } + }); JavaCompilationArtifacts javaArtifacts = javaCommon.getJavaCompilationArtifacts(); String path = @@ -317,7 +250,40 @@ public class BazelJavaSemantics implements JavaSemantics { } } - @Nullable + /** + * 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("\""); + } + private TransitiveInfoCollection getTestSupport(RuleContext ruleContext) { if (!isJavaBinaryOrJavaTest(ruleContext)) { return null; @@ -334,25 +300,13 @@ 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) { - // Not using addTransitiveArtifacts() due to the mismatch in NestedSet ordering. - runfilesBuilder.addArtifacts(getRuntimeJarsForTargets(testSupport)); + runfilesBuilder.addTarget(testSupport, JavaRunfilesProvider.TO_RUNFILES); + runfilesBuilder.addTarget(testSupport, RunfilesProvider.DEFAULT_RUNFILES); } } @@ -362,16 +316,12 @@ public class BazelJavaSemantics implements JavaSemantics { @Override public void collectTargetsTreatedAsDeps( - 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; - } + RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder) { 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 312976f586..cdc7ee4aa7 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,10 +228,6 @@ 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 8056e7cb0d..03c6a594a3 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,6 +95,10 @@ 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()); @@ -106,10 +110,6 @@ 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 b6f0f2b448..344793423e 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, type); + semantics.collectTargetsTreatedAsDeps(ruleContext, builder); // 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 9f5034c1dc..ed638b2147 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,7 +37,6 @@ 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; @@ -263,7 +262,7 @@ public interface JavaSemantics { */ Artifact createStubAction( RuleContext ruleContext, - JavaCommon javaCommon, + final JavaCommon javaCommon, List<String> jvmFlags, Artifact executable, String javaStartClass, @@ -285,11 +284,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, - ClasspathType type); + RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder); /** * 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 e4193e8420..d4a0fa8140 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, - addPersistentRunnerVars(spawn.getEnvironment()), + spawn.getEnvironment(), startupArgs, actionExecutionContext.getExecutor().getExecRoot(), maxRetries); @@ -107,12 +107,6 @@ 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(); @@ -197,19 +191,6 @@ 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(); @@ -217,6 +198,8 @@ 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())); |