Populate JavaInfo's annotation_processing field in java_common.compile RELNOTES: None. PiperOrigin-RevId: 200471197
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 cb4b607..9557005 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
@@ -59,7 +59,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import javax.annotation.Nullable; @@ -213,22 +212,6 @@ return javaArtifacts; } - public NestedSet<Artifact> getProcessorClasspathJars() { - NestedSetBuilder<Artifact> builder = NestedSetBuilder.naiveLinkOrder(); - for (JavaPluginInfoProvider plugin : activePlugins) { - builder.addTransitive(plugin.getProcessorClasspath()); - } - return builder.build(); - } - - public ImmutableList<String> getProcessorClassNames() { - Set<String> processorNames = new LinkedHashSet<>(); - for (JavaPluginInfoProvider plugin : activePlugins) { - processorNames.addAll(plugin.getProcessorClasses()); - } - return ImmutableList.copyOf(processorNames); - } - /** * Creates the java.library.path from a list of the native libraries. * Concatenates the parent directories of the shared libraries into a Java @@ -411,37 +394,6 @@ } /** - * Collects transitive gen jars for the current rule. - */ - private JavaGenJarsProvider collectTransitiveGenJars( - boolean usesAnnotationProcessing, - @Nullable Artifact genClassJar, - @Nullable Artifact genSourceJar) { - NestedSetBuilder<Artifact> classJarsBuilder = NestedSetBuilder.stableOrder(); - NestedSetBuilder<Artifact> sourceJarsBuilder = NestedSetBuilder.stableOrder(); - - if (genClassJar != null) { - classJarsBuilder.add(genClassJar); - } - if (genSourceJar != null) { - sourceJarsBuilder.add(genSourceJar); - } - for (JavaGenJarsProvider dep : getDependencies(JavaGenJarsProvider.class)) { - classJarsBuilder.addTransitive(dep.getTransitiveGenClassJars()); - sourceJarsBuilder.addTransitive(dep.getTransitiveGenSourceJars()); - } - return new JavaGenJarsProvider( - usesAnnotationProcessing, - genClassJar, - genSourceJar, - getProcessorClasspathJars(), - getProcessorClassNames(), - classJarsBuilder.build(), - sourceJarsBuilder.build() - ); - } - - /** * Collects labels of targets and artifacts reached transitively via the "exports" attribute. */ protected JavaExportsProvider collectTransitiveExports() { @@ -749,16 +701,20 @@ JavaInfo.Builder javaInfoBuilder, @Nullable Artifact genClassJar, @Nullable Artifact genSourceJar) { - JavaGenJarsProvider genJarsProvider = collectTransitiveGenJars( - javaCompilationHelper.usesAnnotationProcessing(), - genClassJar, genSourceJar); + JavaGenJarsProvider genJarsProvider = + JavaGenJarsProvider.create( + javaCompilationHelper.usesAnnotationProcessing(), + genClassJar, + genSourceJar, + activePlugins, + getDependencies(JavaGenJarsProvider.class)); NestedSetBuilder<Artifact> genJarsBuilder = NestedSetBuilder.stableOrder(); genJarsBuilder.addTransitive(genJarsProvider.getTransitiveGenClassJars()); genJarsBuilder.addTransitive(genJarsProvider.getTransitiveGenSourceJars()); builder - .add(JavaGenJarsProvider.class, genJarsProvider) + .addProvider(JavaGenJarsProvider.class, genJarsProvider) .addOutputGroup(JavaSemantics.GENERATED_JARS_OUTPUT_GROUP, genJarsBuilder.build()); javaInfoBuilder.addProvider(JavaGenJarsProvider.class, genJarsProvider);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java index d58281c..c592377 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaGenJarsProvider.java
@@ -18,9 +18,13 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skylarkbuildapi.java.JavaAnnotationProcessingApi; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; import javax.annotation.Nullable; /** The collection of gen jars from the transitive closure. */ @@ -41,6 +45,44 @@ private final NestedSet<Artifact> transitiveGenClassJars; private final NestedSet<Artifact> transitiveGenSourceJars; + static JavaGenJarsProvider create( + boolean usesAnnotationProcessing, + @Nullable Artifact genClassJar, + @Nullable Artifact genSourceJar, + List<JavaPluginInfoProvider> plugins, + Iterable<JavaGenJarsProvider> transitiveJavaGenJars) { + NestedSetBuilder<Artifact> classJarsBuilder = NestedSetBuilder.stableOrder(); + NestedSetBuilder<Artifact> sourceJarsBuilder = NestedSetBuilder.stableOrder(); + + if (genClassJar != null) { + classJarsBuilder.add(genClassJar); + } + if (genSourceJar != null) { + sourceJarsBuilder.add(genSourceJar); + } + for (JavaGenJarsProvider dep : transitiveJavaGenJars) { + classJarsBuilder.addTransitive(dep.getTransitiveGenClassJars()); + sourceJarsBuilder.addTransitive(dep.getTransitiveGenSourceJars()); + } + + NestedSetBuilder<Artifact> processorClasspathsBuilder = NestedSetBuilder.naiveLinkOrder(); + Set<String> processorNames = new LinkedHashSet<>(); + for (JavaPluginInfoProvider plugin : plugins) { + processorClasspathsBuilder.addTransitive(plugin.getProcessorClasspath()); + processorNames.addAll(plugin.getProcessorClasses()); + } + + return new JavaGenJarsProvider( + usesAnnotationProcessing, + genClassJar, + genSourceJar, + processorClasspathsBuilder.build(), + ImmutableList.copyOf(processorNames), + classJarsBuilder.build(), + sourceJarsBuilder.build()); + } + + // Package-private for @AutoCodec JavaGenJarsProvider( boolean usesAnnotationProcessing, @Nullable Artifact genClassJar,
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 e5c2cde..369e198 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
@@ -437,7 +437,7 @@ SkylarkList<Artifact> resources, Boolean neverlink, JavaSemantics javaSemantics) - throws EvalException, InterruptedException { + throws EvalException { if (sourceJars.isEmpty() && sourceFiles.isEmpty() && exports.isEmpty()) { throw new EvalException( null, "source_jars, sources and exports cannot be simultaneous empty"); @@ -499,9 +499,15 @@ javaRuntimeInfo, SkylarkList.createImmutable(ImmutableList.of()), outputJarsBuilder, - /*createOutputSourceJar*/ generateMergedSourceJar, + /*createOutputSourceJar=*/ generateMergedSourceJar, outputSourceJar, - javaInfoBuilder); + javaInfoBuilder, + // Include JavaGenJarsProviders from both deps and exports in the JavaGenJarsProvider + // added to javaInfoBuilder for this target. + NestedSetBuilder.<JavaGenJarsProvider>stableOrder() + .addAll(JavaInfo.fetchProvidersFromList(deps, JavaGenJarsProvider.class)) + .addAll(JavaInfo.fetchProvidersFromList(exports, JavaGenJarsProvider.class)) + .build()); JavaCompilationArgsProvider javaCompilationArgsProvider = helper.buildCompilationArgsProvider(artifacts, true, neverlink);
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 f39e5b1..81de790 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
@@ -195,7 +195,8 @@ outputJarsBuilder, createOutputSourceJar, outputSourceJar, - /* javaInfoBuilder= */ null); + /* javaInfoBuilder= */ null, + ImmutableList.of()); // ignored when javaInfoBuilder is null } public JavaCompilationArtifacts build( @@ -206,7 +207,8 @@ JavaRuleOutputJarsProvider.Builder outputJarsBuilder, boolean createOutputSourceJar, @Nullable Artifact outputSourceJar, - @Nullable JavaInfo.Builder javaInfoBuilder) { + @Nullable JavaInfo.Builder javaInfoBuilder, + Iterable<JavaGenJarsProvider> transitiveJavaGenJars) { Preconditions.checkState(output != null, "must have an output file; use setOutput()"); Preconditions.checkState( !createOutputSourceJar || outputSourceJar != null, @@ -272,12 +274,12 @@ JavaCompilationArtifacts javaArtifacts = artifactsBuilder.build(); if (javaInfoBuilder != null) { - ClasspathConfiguredFragment classpathFragment = new ClasspathConfiguredFragment( - javaArtifacts, - attributes.build(), - neverlink, - JavaCompilationHelper.getBootClasspath(javaToolchainProvider) - ); + ClasspathConfiguredFragment classpathFragment = + new ClasspathConfiguredFragment( + javaArtifacts, + attributes.build(), + neverlink, + JavaCompilationHelper.getBootClasspath(javaToolchainProvider)); javaInfoBuilder.addProvider( JavaCompilationInfoProvider.class, @@ -287,11 +289,28 @@ .setCompilationClasspath(classpathFragment.getCompileTimeClasspath()) .setRuntimeClasspath(classpathFragment.getRuntimeClasspath()) .build()); + + javaInfoBuilder.addProvider( + JavaGenJarsProvider.class, + createJavaGenJarsProvider(helper, genClassJar, genSourceJar, transitiveJavaGenJars)); } return javaArtifacts; } + private JavaGenJarsProvider createJavaGenJarsProvider( + JavaCompilationHelper helper, + @Nullable Artifact genClassJar, + @Nullable Artifact genSourceJar, + Iterable<JavaGenJarsProvider> transitiveJavaGenJars) { + return JavaGenJarsProvider.create( + helper.usesAnnotationProcessing(), + genClassJar, + genSourceJar, + plugins, + transitiveJavaGenJars); + } + /** * Returns a JavaCompilationArgsProvider that fully encapsulates this compilation, based on the * result of a call to build(). (that is, it contains the compile-time and runtime jars, separated
diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java index 6a28c8c..47036d7 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
@@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames; -import com.google.common.base.Function; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; @@ -309,6 +308,52 @@ assertThat(outputs.getJdeps().getFilename()).isEqualTo("libdep.jdeps"); } + /** + * Tests that JavaInfo.java_annotation_processing returned from java_common.compile looks as + * expected, and specifically, looks as if java_library was used instead. + */ + @Test + public void testJavaCommonCompileExposesAnnotationProcessingInfo() throws Exception { + // Set up a Skylark rule that uses java_common.compile and supports annotation processing in + // the same way as java_library, then use a helper method to test that the custom rule produces + // the same annotation processing information as java_library would. + writeBuildFileForJavaToolchain(); + scratch.file( + "java/test/custom_rule.bzl", + "def _impl(ctx):", + " output_jar = ctx.actions.declare_file('lib' + ctx.label.name + '.jar')", + " return java_common.compile(", + " ctx,", + " source_files = ctx.files.srcs,", + " deps = [d[JavaInfo] for d in ctx.attr.deps],", + " exports = [e[JavaInfo] for e in ctx.attr.exports],", + " plugins = [p[JavaInfo] for p in ctx.attr.plugins],", + " output = output_jar,", + " java_toolchain = ctx.attr._java_toolchain,", + " host_javabase = ctx.attr._host_javabase", + " )", + "java_custom_library = rule(", + " implementation = _impl,", + " outputs = {", + " 'my_output': 'lib%{name}.jar'", + " },", + " attrs = {", + " 'srcs': attr.label_list(allow_files=['.java']),", + " 'deps': attr.label_list(),", + " 'exports': attr.label_list(),", + " 'plugins': attr.label_list(),", + " '_java_toolchain': attr.label(default = Label('//java/com/google/test:toolchain')),", + " '_host_javabase': attr.label(", + " default = Label('" + HOST_JAVA_RUNTIME_LABEL + "'))", + " },", + " fragments = ['java']", + ")"); + + testAnnotationProcessingInfoIsSkylarkAccessible( + /*toBeProcessedRuleName=*/ "java_custom_library", + /*extraLoad=*/ "load(':custom_rule.bzl', 'java_custom_library')"); + } + @Test public void testJavaCommonCompileCompilationInfo() throws Exception { writeBuildFileForJavaToolchain(); @@ -702,19 +747,38 @@ return result; } + /** + * Tests that a java_library exposes java_processing_info as expected when annotation processing + * is used. + */ @Test public void testJavaPlugin() throws Exception { + testAnnotationProcessingInfoIsSkylarkAccessible( + /*toBeProcessedRuleName=*/ "java_library", /*extraLoad=*/ ""); + } + + /** + * Tests that JavaInfo's java_annotation_processing looks as expected with a target that's + * assumed to use annotation processing itself and has a dep and an export that likewise use + * annotation processing. + */ + private void testAnnotationProcessingInfoIsSkylarkAccessible( + String toBeProcessedRuleName, String extraLoad) throws Exception { scratch.file( - "java/test/extension.bzl", - "result = provider()", - "def impl(ctx):", - " depj = ctx.attr.dep.java", - " return [result(", - " processor_classpath = depj.annotation_processing.processor_classpath,", - " processor_classnames = depj.annotation_processing.processor_classnames,", - " )]", - "my_rule = rule(impl, attrs = { 'dep' : attr.label() })" - ); + "java/test/extension.bzl", + "result = provider()", + "def impl(ctx):", + " depj = ctx.attr.dep[JavaInfo]", + " return [result(", + " enabled = depj.annotation_processing.enabled,", + " class_jar = depj.annotation_processing.class_jar,", + " source_jar = depj.annotation_processing.source_jar,", + " processor_classpath = depj.annotation_processing.processor_classpath,", + " processor_classnames = depj.annotation_processing.processor_classnames,", + " transitive_class_jars = depj.annotation_processing.transitive_class_jars,", + " transitive_source_jars = depj.annotation_processing.transitive_source_jars,", + " )]", + "my_rule = rule(impl, attrs = { 'dep' : attr.label() })"); scratch.file( "java/test/BUILD", "load(':extension.bzl', 'my_rule')", @@ -724,27 +788,53 @@ " srcs = ['AnnotationProcessor.java'],", " processor_class = 'com.google.process.stuff',", " deps = [ ':plugin_dep' ])", - "java_library(name = 'to_be_processed',", + extraLoad, + toBeProcessedRuleName + "(", + " name = 'to_be_processed',", " plugins = [':plugin'],", - " srcs = ['ToBeProcessed.java'])", + " srcs = ['ToBeProcessed.java'],", + " deps = [':dep'],", + " exports = [':export'],", + ")", + "java_library(", + " name = 'dep',", + " srcs = ['Dep.java'],", + " plugins = [':plugin']", + ")", + "java_library(", + " name = 'export',", + " srcs = ['Export.java'],", + " plugins = [':plugin']", + ")", "my_rule(name = 'my', dep = ':to_be_processed')"); + + // Assert that java_annotation_processing for :to_be_processed looks as expected: + // the target itself uses :plugin as the processor, and transitive information includes + // the target's own as well as :dep's and :export's annotation processing outputs, since those + // two targets also use annotation processing. ConfiguredTarget configuredTarget = getConfiguredTarget("//java/test:my"); Info info = configuredTarget.get( new SkylarkKey(Label.parseAbsolute("//java/test:extension.bzl"), "result")); + assertThat(info.getValue("enabled")).isEqualTo(Boolean.TRUE); + assertThat(info.getValue("class_jar")).isNotNull(); + assertThat(info.getValue("source_jar")).isNotNull(); assertThat((List<?>) info.getValue("processor_classnames")) .containsExactly("com.google.process.stuff"); assertThat( Iterables.transform( ((SkylarkNestedSet) info.getValue("processor_classpath")).toCollection(), - new Function<Object, String>() { - @Override - public String apply(Object o) { - return ((Artifact) o).getFilename(); - } - })) + o -> ((Artifact) o).getFilename())) .containsExactly("libplugin.jar", "libplugin_dep.jar"); + assertThat(((SkylarkNestedSet) info.getValue("transitive_class_jars")).toCollection()) + .hasSize(3); // from to_be_processed, dep, and export + assertThat(((SkylarkNestedSet) info.getValue("transitive_class_jars")).toCollection()) + .contains(info.getValue("class_jar")); + assertThat(((SkylarkNestedSet) info.getValue("transitive_source_jars")).toCollection()) + .hasSize(3); // from to_be_processed, dep, and export + assertThat(((SkylarkNestedSet) info.getValue("transitive_source_jars")).toCollection()) + .contains(info.getValue("source_jar")); } @Test