From 8d166b5bc7c7154bf001958faf9119d224b1dc67 Mon Sep 17 00:00:00 2001 From: ishikhman Date: Fri, 30 Aug 2019 14:12:22 +0200 Subject: [PATCH 1/2] added propagation for java rules --- .../lib/rules/java/JavaCompilationHelper.java | 31 ++-- .../java/JavaHeaderCompileActionBuilder.java | 30 +++- .../lib/rules/java/JavaInfoBuildHelper.java | 2 +- .../lib/rules/java/JavaLibraryHelper.java | 4 +- .../rules/java/proto/JavaLiteProtoAspect.java | 2 +- .../lib/rules/java/proto/JavaProtoAspect.java | 2 +- .../java/proto/JavaProtoAspectCommon.java | 2 +- .../bazel/tags_propagation_native_test.sh | 142 +++++++++++++++++- 8 files changed, 190 insertions(+), 25 deletions(-) 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 cfc20bcc494691..0a31a9daa54a57 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 @@ -38,12 +38,15 @@ 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.TargetUtils; import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import javax.annotation.Nullable; /** @@ -189,7 +192,8 @@ public JavaCompileAction createCompileAction( Artifact outputJar, Artifact manifestProtoOutput, @Nullable Artifact gensrcOutputJar, - @Nullable Artifact nativeHeaderOutput) { + @Nullable Artifact nativeHeaderOutput) + throws InterruptedException { JavaTargetAttributes attributes = getAttributes(); @@ -252,13 +256,19 @@ && getTranslations().isEmpty()) { return builder.build(ruleContext, semantics); } - private ImmutableMap getExecutionInfo() { - return getConfiguration() - .modifiedExecutionInfo( - javaToolchain.getJavacSupportsWorkers() - ? ExecutionRequirements.WORKER_MODE_ENABLED - : ImmutableMap.of(), - JavaCompileActionBuilder.MNEMONIC); + private ImmutableMap getExecutionInfo() throws InterruptedException { + Map executionInfo = new LinkedHashMap<>(); + executionInfo.putAll( + getConfiguration() + .modifiedExecutionInfo( + javaToolchain.getJavacSupportsWorkers() + ? ExecutionRequirements.WORKER_MODE_ENABLED + : ImmutableMap.of(), + JavaCompileActionBuilder.MNEMONIC)); + executionInfo.putAll( + TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation())); + + return ImmutableMap.copyOf(executionInfo); } /** Returns the bootclasspath explicit set in attributes if present, or else the default. */ @@ -352,7 +362,8 @@ private boolean shouldUseHeaderCompilation() { * for new artifacts. */ private Artifact createHeaderCompilationAction( - Artifact runtimeJar, JavaCompilationArtifacts.Builder artifactBuilder) { + Artifact runtimeJar, JavaCompilationArtifacts.Builder artifactBuilder) + throws InterruptedException { Artifact headerJar = getAnalysisEnvironment() @@ -628,7 +639,7 @@ public void createSourceJarAction(Artifact outputJar, @Nullable Artifact gensrcJ * @return the header jar (if requested), or ijar (if requested), or else the class jar */ public Artifact createCompileTimeJarAction( - Artifact runtimeJar, JavaCompilationArtifacts.Builder builder) { + Artifact runtimeJar, JavaCompilationArtifacts.Builder builder) throws InterruptedException { Artifact jar; boolean isFullJar = false; if (shouldUseHeaderCompilation()) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java index 06e0187cdaeec3..00e4d5ebb1fbd1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java @@ -44,6 +44,7 @@ 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.TargetUtils; import com.google.devtools.build.lib.rules.java.JavaCompileAction.ProgressMessage; import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode; import com.google.devtools.build.lib.rules.java.JavaPluginInfoProvider.JavaPluginInfo; @@ -53,7 +54,9 @@ import com.google.devtools.build.lib.view.proto.Deps; import java.io.IOException; import java.io.InputStream; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.function.Consumer; import javax.annotation.Nullable; @@ -215,7 +218,7 @@ public JavaHeaderCompileActionBuilder setToolsJars(NestedSet toolsJars } /** Builds and registers the action for a header compilation. */ - public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavabase) { + public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavabase) throws InterruptedException { checkNotNull(outputDepsProto, "outputDepsProto must not be null"); checkNotNull(sourceFiles, "sourceFiles must not be null"); checkNotNull(sourceJars, "sourceJars must not be null"); @@ -369,9 +372,7 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab /* commandLineLimits= */ ruleContext.getConfiguration().getCommandLineLimits(), /* isShellCommand= */ false, /* env= */ actionEnvironment, - /* executionInfo= */ ruleContext - .getConfiguration() - .modifiedExecutionInfo(executionInfo, "Turbine"), + /* executionInfo= */ getExecutionInfo(executionInfo), /* progressMessage= */ progressMessage, /* runfilesSupplier= */ CompositeRunfilesSupplier.fromSuppliers(runfilesSuppliers), /* mnemonic= */ "Turbine", @@ -417,7 +418,7 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab /* transitiveInputs= */ classpathEntries, /* directJars= */ directJars, /* outputs= */ outputs, - /* executionInfo= */ executionInfo, + /* executionInfo= */ addTags(executionInfo), /* extraActionInfoSupplier= */ null, /* executableLine= */ executableLine, /* flagLine= */ commandLine.build(), @@ -454,9 +455,7 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab /* commandLineLimits= */ ruleContext.getConfiguration().getCommandLineLimits(), /* isShellCommand= */ false, /* env= */ actionEnvironment, - /* executionInfo= */ ruleContext - .getConfiguration() - .modifiedExecutionInfo(executionInfo, "JavacTurbine"), + /* executionInfo= */ getExecutionInfo(executionInfo), /* progressMessage= */ progressMessage, /* runfilesSupplier= */ CompositeRunfilesSupplier.fromSuppliers(runfilesSuppliers), /* mnemonic= */ "JavacTurbine", @@ -465,6 +464,21 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab /* resultConsumer= */ resultConsumer)); } + private ImmutableMap addTags(ImmutableMap executionInfo) throws InterruptedException { + Map executionInfoBuilder = new LinkedHashMap<>(); + + executionInfoBuilder.putAll(executionInfo); + executionInfoBuilder.putAll(TargetUtils.getExecutionInfo(ruleContext.getRule(), ruleContext.isAllowTagsPropagation())); + + return ImmutableMap.copyOf(executionInfoBuilder); + } + + private ImmutableMap getExecutionInfo(ImmutableMap execInfo) throws InterruptedException { + return addTags(ruleContext.getConfiguration() + .modifiedExecutionInfo(execInfo, "JavacTurbine")); + + } + /** * Creates a consumer that reads the produced .jdeps file into memory. Pulled out into a separate * function to avoid capturing a data member, which would keep the entire builder instance alive. diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java index d5184b75c40e1f..be6208cafe29df 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java @@ -292,7 +292,7 @@ public JavaInfo createJavaCompileAction( JavaSemantics javaSemantics, Location location, Environment environment) - throws EvalException { + throws EvalException, InterruptedException { if (sourceJars.isEmpty() && sourceFiles.isEmpty() 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 ece3c58bfa4570..55326445a553b5 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 @@ -182,7 +182,7 @@ public JavaCompilationArtifacts build( JavaRuntimeInfo hostJavabase, JavaRuleOutputJarsProvider.Builder outputJarsBuilder, boolean createOutputSourceJar, - @Nullable Artifact outputSourceJar) { + @Nullable Artifact outputSourceJar) throws InterruptedException { return build( semantics, javaToolchainProvider, @@ -204,7 +204,7 @@ public JavaCompilationArtifacts build( @Nullable Artifact outputSourceJar, @Nullable JavaInfo.Builder javaInfoBuilder, Iterable transitiveJavaGenJars, - ImmutableList additionalJavaBaseInputs) { + ImmutableList additionalJavaBaseInputs) throws InterruptedException { Preconditions.checkState(output != null, "must have an output file; use setOutput()"); Preconditions.checkState( diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java index da899079159f1d..f4f4ae52933a8c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java @@ -172,7 +172,7 @@ private static class Impl { "exports", RuleConfiguredTarget.Mode.TARGET, JavaCompilationArgsProvider.class)); } - void addProviders(ConfiguredAspect.Builder aspect) { + void addProviders(ConfiguredAspect.Builder aspect) throws InterruptedException { JavaInfo.Builder javaInfo = JavaInfo.Builder.create(); // Represents the result of compiling the code generated for this proto, including all of its // dependencies. diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java index 93ac592dd93965..98df054f5eeef5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java @@ -192,7 +192,7 @@ private static class Impl { JavaCompilationArgsProvider.class)); } - void addProviders(ConfiguredAspect.Builder aspect) { + void addProviders(ConfiguredAspect.Builder aspect) throws InterruptedException { // Represents the result of compiling the code generated for this proto, including all of its // dependencies. JavaInfo.Builder javaInfo = JavaInfo.Builder.create(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java index 422ce7b994398c..5318467f4c1d0e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java @@ -108,7 +108,7 @@ public JavaCompilationArgsProvider createJavaCompileAction( String injectingRuleKind, Artifact sourceJar, Artifact outputJar, - JavaCompilationArgsProvider dep) { + JavaCompilationArgsProvider dep) throws InterruptedException { JavaLibraryHelper helper = new JavaLibraryHelper(ruleContext) .setInjectingRuleKind(injectingRuleKind) diff --git a/src/test/shell/bazel/tags_propagation_native_test.sh b/src/test/shell/bazel/tags_propagation_native_test.sh index 7ffee2fab58800..969f3fbf549761 100755 --- a/src/test/shell/bazel/tags_propagation_native_test.sh +++ b/src/test/shell/bazel/tags_propagation_native_test.sh @@ -98,6 +98,119 @@ EOF assert_contains "no-remote:" output1 } +function test_java_library_tags_propagated() { + mkdir -p test + cat > test/BUILD < test/Hello.java < output1 2> $TEST_log \ + || fail "should have generated output successfully" + + assert_contains "ExecutionInfo: {" output1 + assert_contains "local:" output1 + assert_contains "no-cache:" output1 + assert_contains "no-remote:" output1 +} + +function test_java_binary_tags_propagated() { + mkdir -p test + cat > test/BUILD < test/Hello.java < output1 2> $TEST_log \ + || fail "should have generated output successfully" + + assert_contains "ExecutionInfo: {" output1 + assert_contains "local:" output1 + assert_contains "no-cache:" output1 + assert_contains "no-remote:" output1 +} + +function write_hello_library_files() { + local -r pkg="$1" + mkdir -p $pkg/java/main || fail "mkdir" + cat >$pkg/java/main/BUILD <$pkg/java/main/Main.java <$pkg/java/hello_library/BUILD <$pkg/java/hello_library/HelloLibrary.java < output1 2> $TEST_log \ + || fail "should have generated output successfully" + + assert_contains "ExecutionInfo: {" output1 + assert_contains "local:" output1 + assert_contains "no-cache:" output1 + assert_contains "no-remote:" output1 + +} + function test_genrule_tags_propagated() { mkdir -p test cat > test/BUILD < test/BUILD < test/Hello.java < output1 2> $TEST_log \ + || fail "should have generated output successfully" + + assert_not_contains "local:" output1 + assert_not_contains "no-cache:" output1 + assert_not_contains "no-remote:" output1 +} + +run_suite "tags propagation: native rule tests" From a8efac40d7307736a665859636170170106f4640 Mon Sep 17 00:00:00 2001 From: ishikhman Date: Fri, 30 Aug 2019 15:02:28 +0200 Subject: [PATCH 2/2] small fix --- .../rules/java/JavaHeaderCompileActionBuilder.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java index 00e4d5ebb1fbd1..90234c95af5b69 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java @@ -372,7 +372,9 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab /* commandLineLimits= */ ruleContext.getConfiguration().getCommandLineLimits(), /* isShellCommand= */ false, /* env= */ actionEnvironment, - /* executionInfo= */ getExecutionInfo(executionInfo), + /* executionInfo= */ addTags(ruleContext + .getConfiguration() + .modifiedExecutionInfo(executionInfo, "Turbine")), /* progressMessage= */ progressMessage, /* runfilesSupplier= */ CompositeRunfilesSupplier.fromSuppliers(runfilesSuppliers), /* mnemonic= */ "Turbine", @@ -455,7 +457,9 @@ public void build(JavaToolchainProvider javaToolchain, JavaRuntimeInfo hostJavab /* commandLineLimits= */ ruleContext.getConfiguration().getCommandLineLimits(), /* isShellCommand= */ false, /* env= */ actionEnvironment, - /* executionInfo= */ getExecutionInfo(executionInfo), + /* executionInfo= */ addTags(ruleContext + .getConfiguration() + .modifiedExecutionInfo(executionInfo, "JavacTurbine")), /* progressMessage= */ progressMessage, /* runfilesSupplier= */ CompositeRunfilesSupplier.fromSuppliers(runfilesSuppliers), /* mnemonic= */ "JavacTurbine", @@ -473,12 +477,6 @@ private ImmutableMap addTags(ImmutableMap execut return ImmutableMap.copyOf(executionInfoBuilder); } - private ImmutableMap getExecutionInfo(ImmutableMap execInfo) throws InterruptedException { - return addTags(ruleContext.getConfiguration() - .modifiedExecutionInfo(execInfo, "JavacTurbine")); - - } - /** * Creates a consumer that reads the produced .jdeps file into memory. Pulled out into a separate * function to avoid capturing a data member, which would keep the entire builder instance alive.