Skip to content

Commit

Permalink
Accept JavaPluginInfo as plugins and exported_plugins parameter of ja…
Browse files Browse the repository at this point in the history
…va_common.compile and add exported_plugins parameter to JavaInfo constructor

java_common.compile now accepts either a list of JavaInfo or a list of JavaPluginInfo. A flag --incompatible_require_javaplugininfo_in_javacommon is added that disables supports for lists of JavaInfo, and at the same time also stops returning it from java_plugin rule.

JavaInfo constructor was missing exported_plugins parameter and it is added for consistency. Because it's new there is no support for a list of JavaInfo.

PiperOrigin-RevId: 373301285
  • Loading branch information
comius authored and copybara-github committed May 12, 2021
1 parent e3c78c4 commit ba4435c
Show file tree
Hide file tree
Showing 10 changed files with 172 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ public enum ImportDepsCheckingLevel {
private final boolean experimentalTurbineAnnotationProcessing;
private final boolean experimentalEnableJspecify;
private final boolean dontCollectDataLibraries;
private final boolean requireJavaPluginInfo;

// TODO(dmarting): remove once we have a proper solution for #2539
private final boolean useLegacyBazelJavaTest;
Expand Down Expand Up @@ -145,6 +146,7 @@ public JavaConfiguration(BuildOptions buildOptions) throws InvalidConfigurationE
this.runAndroidLint = javaOptions.runAndroidLint;
this.limitAndroidLintToAndroidCompatible = javaOptions.limitAndroidLintToAndroidCompatible;
this.dontCollectDataLibraries = javaOptions.dontCollectDataLibraries;
this.requireJavaPluginInfo = javaOptions.requireJavaPluginInfo;

Map<String, Label> optimizers = javaOptions.bytecodeOptimizers;
if (optimizers.size() > 1) {
Expand Down Expand Up @@ -399,4 +401,8 @@ public boolean experimentalEnableJspecify() {
public boolean dontCollectDataLibraries() {
return dontCollectDataLibraries;
}

public boolean requireJavaPluginInfo() {
return requireJavaPluginInfo;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ public JavaInfo javaInfo(
Sequence<?> deps,
Sequence<?> runtimeDeps,
Sequence<?> exports,
Sequence<?> exportedPlugins,
Object jdepsApi,
Sequence<?> nativeLibraries,
StarlarkThread thread)
Expand All @@ -499,7 +500,6 @@ public JavaInfo javaInfo(
@Nullable Artifact nativeHeadersJar = nullIfNone(nativeHeadersJarApi, Artifact.class);
@Nullable Artifact manifestProto = nullIfNone(manifestProtoApi, Artifact.class);
@Nullable Artifact jdeps = nullIfNone(jdepsApi, Artifact.class);

return JavaInfoBuildHelper.getInstance()
.createJavaInfo(
JavaOutput.builder()
Expand All @@ -517,6 +517,7 @@ public JavaInfo javaInfo(
Sequence.cast(deps, JavaInfo.class, "deps"),
Sequence.cast(runtimeDeps, JavaInfo.class, "runtime_deps"),
Sequence.cast(exports, JavaInfo.class, "exports"),
Sequence.cast(exportedPlugins, JavaPluginInfo.class, "exported_plugins"),
Sequence.cast(nativeLibraries, CcInfo.class, "native_libraries"),
thread.getCallerLocation(),
thread.getSemantics().getBool(INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public static JavaInfoBuildHelper getInstance() {
* @param exports libraries to make available for users of this library. <a
* href="https://docs.bazel.build/versions/master/be/java.html#java_library"
* target="_top">java_library.exports</a>
* @param exportedPlugins A list of exported plugins.
* @param nativeLibraries CC library dependencies that are needed for this library
* @return new created JavaInfo instance
*/
Expand All @@ -84,6 +85,7 @@ JavaInfo createJavaInfo(
Sequence<JavaInfo> compileTimeDeps,
Sequence<JavaInfo> runtimeDeps,
Sequence<JavaInfo> exports,
Sequence<JavaPluginInfo> exportedPlugins,
Sequence<CcInfo> nativeLibraries,
Location location,
boolean withExportsProvider) {
Expand Down Expand Up @@ -126,7 +128,7 @@ JavaInfo createJavaInfo(
javaInfoBuilder.addProvider(JavaExportsProvider.class, createJavaExportsProvider(exports));
}

javaInfoBuilder.javaPluginInfo(mergeExportedJavaPluginInfo(exports));
javaInfoBuilder.javaPluginInfo(mergeExportedJavaPluginInfo(exportedPlugins, exports));

javaInfoBuilder.addProvider(
JavaSourceJarsProvider.class,
Expand Down Expand Up @@ -230,12 +232,15 @@ private JavaExportsProvider createJavaExportsProvider(Iterable<JavaInfo> exports
JavaInfo.fetchProvidersFromList(exports, JavaExportsProvider.class));
}

private JavaPluginInfo mergeExportedJavaPluginInfo(Iterable<JavaInfo> javaInfos) {
private JavaPluginInfo mergeExportedJavaPluginInfo(
Iterable<JavaPluginInfo> plugins, Iterable<JavaInfo> javaInfos) {
return JavaPluginInfo.merge(
stream(javaInfos)
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList()));
concat(
plugins,
stream(javaInfos)
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList())));
}

public JavaInfo createJavaCompileAction(
Expand All @@ -249,8 +254,8 @@ public JavaInfo createJavaCompileAction(
List<JavaInfo> runtimeDeps,
List<JavaInfo> experimentalLocalCompileTimeDeps,
List<JavaInfo> exports,
List<JavaInfo> plugins,
List<JavaInfo> exportedPlugins,
List<JavaPluginInfo> plugins,
List<JavaPluginInfo> exportedPlugins,
List<CcInfo> nativeLibraries,
List<Artifact> annotationProcessorAdditionalInputs,
List<Artifact> annotationProcessorAdditionalOutputs,
Expand Down Expand Up @@ -289,7 +294,7 @@ public JavaInfo createJavaCompileAction(
streamProviders(deps, JavaCompilationArgsProvider.class).forEach(helper::addDep);
streamProviders(exports, JavaCompilationArgsProvider.class).forEach(helper::addExport);
helper.setCompilationStrictDepsMode(getStrictDepsMode(Ascii.toUpperCase(strictDepsMode)));
helper.setPlugins(mergeExportedJavaPluginInfo(concat(plugins, deps)));
helper.setPlugins(mergeExportedJavaPluginInfo(plugins, deps));
helper.setNeverlink(neverlink);

NestedSet<Artifact> localCompileTimeDeps =
Expand Down Expand Up @@ -344,7 +349,7 @@ public JavaInfo createJavaCompileAction(
JavaSourceJarsProvider.class,
createJavaSourceJarsProvider(outputSourceJars, concat(runtimeDeps, exports, deps)))
.addProvider(JavaRuleOutputJarsProvider.class, outputJarsBuilder.build())
.javaPluginInfo(mergeExportedJavaPluginInfo(concat(exportedPlugins, exports)))
.javaPluginInfo(mergeExportedJavaPluginInfo(exportedPlugins, exports))
.addProvider(JavaExportsProvider.class, createJavaExportsProvider(exports))
.addProvider(JavaCcInfoProvider.class, JavaCcInfoProvider.merge(transitiveNativeLibraries))
.addTransitiveOnlyRuntimeJarsToJavaInfo(deps)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ final ConfiguredTarget init(
JavaCommon.getRunfiles(ruleContext, semantics, javaArtifacts, neverLink)))
.setFilesToBuild(filesToBuild)
.addNativeDeclaredProvider(new ProguardSpecProvider(proguardSpecs))
.addNativeDeclaredProvider(javaInfo)
.addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars)
.addOutputGroup(
JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP,
Expand All @@ -207,6 +206,10 @@ final ConfiguredTarget init(
if (isJavaPluginRule) {
builder.addStarlarkDeclaredProvider(javaPluginInfo);
}
if (!isJavaPluginRule || !javaConfig.requireJavaPluginInfo()) {
// After javaConfig.requireJavaPluginInfo is flipped JavaInfo is not returned from java_plugin
builder.addNativeDeclaredProvider(javaInfo);
}

Artifact validation =
AndroidLintActionBuilder.create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,18 @@ public ImportDepsCheckingLevelConverter() {
help = "When enabled the native libraries in the data attribute are not collected.")
public boolean dontCollectDataLibraries;

@Option(
name = "incompatible_require_javaplugininfo_in_javacommon",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.UNKNOWN},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "When enabled java_common.compile only accepts JavaPluginInfo for plugins.")
public boolean requireJavaPluginInfo;

@Option(
name = "experimental_publish_javacclinkparamsinfo",
defaultValue = "true",
Expand Down Expand Up @@ -753,6 +765,7 @@ public FragmentOptions getHost() {

host.dontCollectSoArtifacts = dontCollectSoArtifacts;
host.dontCollectDataLibraries = dontCollectDataLibraries;
host.requireJavaPluginInfo = requireJavaPluginInfo;

return host;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.java;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER;

import com.google.common.collect.ImmutableList;
Expand All @@ -27,6 +28,7 @@
import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi;
import com.google.devtools.build.lib.starlarkbuildapi.java.JavaCommonApi;
import com.google.devtools.build.lib.starlarkbuildapi.java.JavaToolchainStarlarkApiProviderApi;
import java.util.Objects;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
Expand Down Expand Up @@ -65,8 +67,8 @@ public JavaInfo createJavaCompileAction(
Sequence<?> runtimeDeps, // <JavaInfo> expected
Sequence<?> experimentalLocalCompileTimeDeps, // <JavaInfo> expected
Sequence<?> exports, // <JavaInfo> expected
Sequence<?> plugins, // <JavaInfo> expected
Sequence<?> exportedPlugins, // <JavaInfo> expected
Sequence<?> plugins, // <JavaPluginInfo> expected
Sequence<?> exportedPlugins, // <JavaPluginInfo> expected
Sequence<?> nativeLibraries, // <CcInfo> expected.
Sequence<?> annotationProcessorAdditionalInputs, // <Artifact> expected
Sequence<?> annotationProcessorAdditionalOutputs, // <Artifact> expected
Expand All @@ -79,6 +81,40 @@ public JavaInfo createJavaCompileAction(
StarlarkThread thread)
throws EvalException, InterruptedException {

boolean acceptJavaInfo =
!starlarkRuleContext
.getRuleContext()
.getFragment(JavaConfiguration.class)
.requireJavaPluginInfo();

final ImmutableList<JavaPluginInfo> pluginsParam;
if (acceptJavaInfo && !plugins.isEmpty() && plugins.get(0) instanceof JavaInfo) {
// Handle deprecated case where plugins is given a list of JavaInfos
pluginsParam =
Sequence.cast(plugins, JavaInfo.class, "plugins").stream()
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList());
} else {
pluginsParam = Sequence.cast(plugins, JavaPluginInfo.class, "plugins").getImmutableList();
}

final ImmutableList<JavaPluginInfo> exportedPluginsParam;
if (acceptJavaInfo
&& !exportedPlugins.isEmpty()
&& exportedPlugins.get(0) instanceof JavaInfo) {
// Handle deprecated case where exported_plugins is given a list of JavaInfos
exportedPluginsParam =
Sequence.cast(exportedPlugins, JavaInfo.class, "exported_plugins").stream()
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList());
} else {
exportedPluginsParam =
Sequence.cast(exportedPlugins, JavaPluginInfo.class, "exported_plugins")
.getImmutableList();
}

return JavaInfoBuildHelper.getInstance()
.createJavaCompileAction(
starlarkRuleContext,
Expand All @@ -94,8 +130,8 @@ public JavaInfo createJavaCompileAction(
JavaInfo.class,
"experimental_local_compile_time_deps"),
Sequence.cast(exports, JavaInfo.class, "exports"),
Sequence.cast(plugins, JavaInfo.class, "plugins"),
Sequence.cast(exportedPlugins, JavaInfo.class, "exported_plugins"),
pluginsParam,
exportedPluginsParam,
Sequence.cast(nativeLibraries, CcInfo.class, "native_libraries"),
Sequence.cast(
annotationProcessorAdditionalInputs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,20 @@ public interface JavaCommonApi<
name = "plugins",
positional = false,
named = true,
allowedTypes = {@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)},
allowedTypes = {
@ParamType(type = Sequence.class, generic1 = JavaPluginInfoApi.class),
@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)
},
defaultValue = "[]",
doc = "A list of plugins. Optional."),
@Param(
name = "exported_plugins",
positional = false,
named = true,
allowedTypes = {@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)},
allowedTypes = {
@ParamType(type = Sequence.class, generic1 = JavaPluginInfoApi.class),
@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)
},
defaultValue = "[]",
doc = "A list of exported plugins. Optional."),
@Param(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,14 @@ interface JavaInfoProviderApi extends ProviderApi {
"Libraries to make available for users of this library. See also "
+ "<a class=\"anchor\" href=\"https://docs.bazel.build/versions/"
+ "master/be/java.html#java_library.exports\">java_library.exports</a>."),
@Param(
name = "exported_plugins",
named = true,
allowedTypes = {
@ParamType(type = Sequence.class, generic1 = JavaPluginInfoApi.class)
},
defaultValue = "[]",
doc = "A list of exported plugins. Optional."),
@Param(
name = "jdeps",
allowedTypes = {
Expand Down Expand Up @@ -355,6 +363,7 @@ interface JavaInfoProviderApi extends ProviderApi {
Sequence<?> deps,
Sequence<?> runtimeDeps,
Sequence<?> exports,
Sequence<?> exportedPlugins,
Object jdepsApi,
Sequence<?> nativeLibraries,
StarlarkThread thread)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,16 +623,8 @@ public void buildHelperCreateJavaInfoExportProvider001() throws Exception {
}

@Test
public void buildHelperCreateJavaInfoPlugins() throws Exception {
public void buildHelperCreateJavaInfoPluginsFromExports() throws Exception {
ruleBuilder().build();
scratch.file("java/test/lib.jar");
scratch.file(
"java/test/BUILD",
"load(':custom_rule.bzl', 'java_custom_library')",
"java_custom_library(",
" name = 'custom',",
" export = ':export',",
")");
scratch.file(
"foo/BUILD",
"load(':extension.bzl', 'my_rule')",
Expand All @@ -656,6 +648,28 @@ public void buildHelperCreateJavaInfoPlugins() throws Exception {
.containsExactly("com.google.process.stuff");
}

@Test
public void buildHelperCreateJavaInfoWithPlugins() throws Exception {
ruleBuilder().build();
scratch.file(
"foo/BUILD",
"load(':extension.bzl', 'my_rule')",
"java_library(name = 'plugin_dep',",
" srcs = [ 'ProcessorDep.java'])",
"java_plugin(name = 'plugin',",
" srcs = ['AnnotationProcessor.java'],",
" processor_class = 'com.google.process.stuff',",
" deps = [ ':plugin_dep' ])",
"my_rule(name = 'my_starlark_rule',",
" output_jar = 'my_starlark_rule_lib.jar',",
" dep_exported_plugins = [':plugin']",
")");
assertNoEvents();

assertThat(fetchJavaInfo().getJavaPluginInfo().plugins().processorClasses().toList())
.containsExactly("com.google.process.stuff");
}

@Test
public void buildHelperCreateJavaInfoWithOutputJarAndStampJar() throws Exception {
ruleBuilder().withStampJar().build();
Expand Down Expand Up @@ -895,6 +909,7 @@ private String[] newJavaInfo() {
" dp = [dep[java_common.provider] for dep in ctx.attr.dep]",
" dp_runtime = [dep[java_common.provider] for dep in ctx.attr.dep_runtime]",
" dp_exports = [dep[java_common.provider] for dep in ctx.attr.dep_exports]",
" dp_exported_plugins = [dep[JavaPluginInfo] for dep in ctx.attr.dep_exported_plugins]",
" dp_libs = [dep[CcInfo] for dep in ctx.attr.cc_dep]");

if (useIJar) {
Expand Down Expand Up @@ -941,6 +956,7 @@ private String[] newJavaInfo() {
" deps = dp,",
" runtime_deps = dp_runtime,",
" exports = dp_exports,",
" exported_plugins = dp_exported_plugins,",
" jdeps = ctx.file.jdeps,",
" compile_jdeps = ctx.file.compile_jdeps,",
" generated_class_jar = ctx.file.generated_class_jar,",
Expand Down Expand Up @@ -968,6 +984,7 @@ private void build() throws Exception {
" 'cc_dep' : attr.label_list(),",
" 'dep_runtime' : attr.label_list(),",
" 'dep_exports' : attr.label_list(),",
" 'dep_exported_plugins' : attr.label_list(),",
" 'output_jar' : attr.output(mandatory=True),",
" 'source_jars' : attr.label_list(allow_files=['.jar']),",
" 'sources' : attr.label_list(allow_files=['.java']),",
Expand Down
Loading

0 comments on commit ba4435c

Please sign in to comment.