Skip to content

Commit

Permalink
Provide java_common.merge interface for Java compilation of proto lib…
Browse files Browse the repository at this point in the history 
…raries

Remove `exports`, `runtime_deps` parameters and introduce `merge_java_outputs`, `merge_source_jars` parameters. Drop corresponding providers.

This optimisation corresponds to optimisations done natively.

The new parameters are only available in builtins.

Natively only `JavaCompilationArgsProvider`, `JavaCcInfoProvider` are merged from the deps.

`java_common.merge` with `include_compilation_info` disabled
provides `JavaCompilationArgsProvider`, `JavaCcInfoProvider` and additionally `JavaPluginInfo`, `runtimeJars` and `constraints`. Additional providers are empty in case of Java proto libraries. (Exception are `constraints`, which are used in `java_lite_proto_library`).

In both cases `JavaRuleOutputJarsProvider`, `JavaSourceJarsProvider`, `JavaGenJarsProvider`, `JavaCompilationInfoProvider` are dropped (for optimisation).

PiperOrigin-RevId: 451966776
Change-Id: I7459f9f62b7a4e6d14c26a502544b1318e6bdad7
  • Loading branch information
@comius @copybara-github
comius authored and copybara-github committed May 31, 2022
1 parent a7ec710 commit 4c26c3a
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 124 deletions.
89 changes: 29 additions & 60 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package com.google.devtools.build.lib.rules.java;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType.RUNTIME_ONLY;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -109,82 +108,52 @@ public JavaPluginInfo getJavaPluginInfo() {
}

/**
* Merges the given providers and runtime dependencies into one {@link JavaInfo}. All the
* providers with the same type in the given list are merged into one provider that is added to
* the resulting {@link JavaInfo}.
* Merges the given providers into one {@link JavaInfo}. All the providers with the same type in
* the given list are merged into one provider that is added to the resulting {@link JavaInfo}.
*/
public static JavaInfo merge(
List<JavaInfo> providers,
List<JavaInfo> exports,
List<JavaInfo> runtimeDeps,
boolean includeSourceJarsFromExports) {
JavaCompilationArgsProvider.Builder javaCompilationArgsBuilder =
JavaCompilationArgsProvider.builder();
JavaInfo.fetchProvidersFromList(providers, JavaCompilationArgsProvider.class)
.forEach(javaCompilationArgsBuilder::addExports);
JavaInfo.fetchProvidersFromList(exports, JavaCompilationArgsProvider.class)
.forEach(javaCompilationArgsBuilder::addExports);
streamProviders(runtimeDeps, JavaCompilationArgsProvider.class)
.forEach(args -> javaCompilationArgsBuilder.addDeps(args, RUNTIME_ONLY));

JavaSourceJarsProvider.Builder javaSourceJarsBuilder = JavaSourceJarsProvider.builder();
JavaInfo.fetchProvidersFromList(providers, JavaSourceJarsProvider.class)
.forEach(javaSourceJarsBuilder::mergeFrom);
if (includeSourceJarsFromExports) {
JavaInfo.fetchProvidersFromList(exports, JavaSourceJarsProvider.class)
.forEach(export -> javaSourceJarsBuilder.addAllSourceJars(export.getSourceJars()));
}
// If transitive source jar doesn't include sourcejar at this level but they should.
NestedSetBuilder<Artifact> transitiveSourceJars = NestedSetBuilder.stableOrder();
streamProviders(runtimeDeps, JavaSourceJarsProvider.class)
.forEach(
dep -> {
transitiveSourceJars.addAll(dep.getSourceJars());
transitiveSourceJars.addTransitive(dep.getTransitiveSourceJars());
});
javaSourceJarsBuilder.addAllTransitiveSourceJars(transitiveSourceJars.build());

ImmutableList.Builder<JavaPluginInfo> javaPluginInfosBuilder = ImmutableList.builder();
javaPluginInfosBuilder.addAll(
List<JavaInfo> providers, boolean mergeJavaOutputs, boolean mergeSourceJars) {
ImmutableList<JavaCompilationArgsProvider> javaCompilationArgsProviders =
JavaInfo.fetchProvidersFromList(providers, JavaCompilationArgsProvider.class);

final ImmutableList<JavaSourceJarsProvider> javaSourceJarsProviders =
mergeSourceJars
? JavaInfo.fetchProvidersFromList(providers, JavaSourceJarsProvider.class)
: ImmutableList.of();
final ImmutableList<JavaRuleOutputJarsProvider> javaRuleOutputJarsProviders =
mergeJavaOutputs
? JavaInfo.fetchProvidersFromList(providers, JavaRuleOutputJarsProvider.class)
: ImmutableList.of();

ImmutableList<JavaPluginInfo> javaPluginInfos =
providers.stream()
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList()));
javaPluginInfosBuilder.addAll(
exports.stream()
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList()));

List<JavaRuleOutputJarsProvider> javaRuleOutputJarsProviders =
JavaInfo.fetchProvidersFromList(providers, JavaRuleOutputJarsProvider.class);

ImmutableList.Builder<JavaCcInfoProvider> javaCcInfoBuilder = ImmutableList.builder();
javaCcInfoBuilder.addAll(JavaInfo.fetchProvidersFromList(providers, JavaCcInfoProvider.class));
javaCcInfoBuilder.addAll(JavaInfo.fetchProvidersFromList(exports, JavaCcInfoProvider.class));
javaCcInfoBuilder.addAll(
JavaInfo.fetchProvidersFromList(runtimeDeps, JavaCcInfoProvider.class));
.collect(toImmutableList());
ImmutableList<JavaCcInfoProvider> javaCcInfoProviders =
JavaInfo.fetchProvidersFromList(providers, JavaCcInfoProvider.class);

NestedSetBuilder<Artifact> runtimeJars = NestedSetBuilder.stableOrder();
NestedSetBuilder<String> javaConstraints = NestedSetBuilder.stableOrder();
for (JavaInfo javaInfo : providers) {
runtimeJars.addAll(javaInfo.getDirectRuntimeJars());
javaConstraints.addAll(javaInfo.getJavaConstraints());
}
for (JavaInfo javaInfo : exports) {
if (mergeJavaOutputs) {
runtimeJars.addAll(javaInfo.getDirectRuntimeJars());
}
javaConstraints.addAll(javaInfo.getJavaConstraints());
}

JavaInfo.Builder javaInfoBuilder =
JavaInfo.Builder.create()
.addProvider(JavaCompilationArgsProvider.class, javaCompilationArgsBuilder.build())
.addProvider(JavaSourceJarsProvider.class, javaSourceJarsBuilder.build())
.addProvider(
JavaCompilationArgsProvider.class,
JavaCompilationArgsProvider.merge(javaCompilationArgsProviders))
.addProvider(
JavaSourceJarsProvider.class, JavaSourceJarsProvider.merge(javaSourceJarsProviders))
.addProvider(
JavaRuleOutputJarsProvider.class,
JavaRuleOutputJarsProvider.merge(javaRuleOutputJarsProviders))
.javaPluginInfo(JavaPluginInfo.mergeWithoutJavaOutputs(javaPluginInfosBuilder.build()))
.addProvider(
JavaCcInfoProvider.class, JavaCcInfoProvider.merge(javaCcInfoBuilder.build()))
.javaPluginInfo(JavaPluginInfo.mergeWithoutJavaOutputs(javaPluginInfos))
.addProvider(JavaCcInfoProvider.class, JavaCcInfoProvider.merge(javaCcInfoProviders))
// TODO(b/65618333): add merge function to JavaGenJarsProvider. See #3769
// TODO(iirina): merge or remove JavaCompilationInfoProvider
.setRuntimeJars(runtimeJars.build().toList())
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ public JavaPluginInfoApi<Artifact, JavaPluginData, JavaOutput> javaPluginInfo(
JavaInfo javaInfos =
JavaInfo.merge(
Sequence.cast(runtimeDeps, JavaInfo.class, "runtime_deps"),
ImmutableList.of(),
ImmutableList.of(),
false);
/* mergeJavaOutputs= */ true,
/* mergeSourceJars= */ true);

NestedSet<Artifact> processorClasspath =
javaInfos.getProvider(JavaCompilationArgsProvider.class).getRuntimeJars();
Expand Down
12 changes: 4 additions & 8 deletions src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,15 @@ public ImmutableList<String> getDefaultJavacOpts(JavaToolchainProvider javaToolc
@Override
public JavaInfo mergeJavaProviders(
Sequence<?> providers, /* <JavaInfo> expected. */
Sequence<?> exports, /* <JavaInfo> expected. */
Sequence<?> runtimeDeps, /* <JavaInfo> expected. */
boolean includeSourceJarsFromExports,
boolean mergeJavaOutputs,
boolean mergeSourceJars,
StarlarkThread thread)
throws EvalException {
if (!exports.isEmpty() || !runtimeDeps.isEmpty() || includeSourceJarsFromExports) {
if (!mergeJavaOutputs || !mergeSourceJars) {
checkPrivateAccess(thread);
}
return JavaInfo.merge(
Sequence.cast(providers, JavaInfo.class, "providers"),
Sequence.cast(exports, JavaInfo.class, "exports"),
Sequence.cast(runtimeDeps, JavaInfo.class, "runtime_deps"),
includeSourceJarsFromExports);
Sequence.cast(providers, JavaInfo.class, "providers"), mergeJavaOutputs, mergeSourceJars);
}

// TODO(b/65113771): Remove this method because it's incorrect.
Expand Down
22 changes: 5 additions & 17 deletions src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,29 +476,17 @@ FileApi packSources(
allowedTypes = {@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)},
doc = "The list of providers to merge."),
@Param(
name = "exports",
allowedTypes = {@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)},
named = true,
defaultValue = "[]",
doc = "A list of exports. Optional."),
@Param(
name = "runtime_deps",
allowedTypes = {@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)},
named = true,
defaultValue = "[]",
doc = "A list of runtime dependencies. Optional."),
@Param(
name = "include_source_jars_from_exports",
name = "merge_java_outputs",
positional = false,
named = true,
defaultValue = "False"),
defaultValue = "True"),
@Param(name = "merge_source_jars", positional = false, named = true, defaultValue = "True"),
},
useStarlarkThread = true)
JavaInfoT mergeJavaProviders(
Sequence<?> providers /* <JavaInfoT> expected. */,
Sequence<?> exports /* <JavaInfoT> expected. */,
Sequence<?> runtimeDeps /* <JavaInfoT> expected. */,
boolean includeSourceJarsFromExports,
boolean mergeJavaOutputs,
boolean mergeSourceJars,
StarlarkThread thread)
throws EvalException;

Expand Down
8 changes: 2 additions & 6 deletions src/main/starlark/builtins_bzl/common/java/proto/java_proto_library.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def java_compile_for_protos(ctx, output_jar_suffix, source_jar = None, deps = []
jars = [source_jar, output_jar]
else:
# If there are no proto sources just pass along the compilation dependencies.
java_info = java_common.merge([], exports = deps + exports)
java_info = java_common.merge(deps + exports, merge_java_outputs = False, merge_source_jars = False)
jars = []
return java_info, jars

Expand Down Expand Up @@ -152,11 +152,7 @@ def bazel_java_proto_library_rule(ctx):
([JavaInfo, DefaultInfo, OutputGroupInfo])
"""

java_info = java_common.merge(
[],
exports = [dep[JavaInfo] for dep in ctx.attr.deps],
include_source_jars_from_exports = True,
)
java_info = java_common.merge([dep[JavaInfo] for dep in ctx.attr.deps], merge_java_outputs = False)

transitive_src_and_runtime_jars = depset(transitive = [dep[JavaProtoAspectInfo].jars for dep in ctx.attr.deps])
transitive_runtime_jars = depset(transitive = [java_info.transitive_runtime_jars])
Expand Down
34 changes: 4 additions & 30 deletions src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3155,14 +3155,14 @@ public void testEnableJSpecifyIsPrivateApi() throws Exception {
}

@Test
public void testExportsIsPrivateApi() throws Exception {
public void testMergeJavaOutputsIsPrivateApi() throws Exception {
scratch.file(
"foo/custom_rule.bzl",
"def _impl(ctx):",
" output_jar = ctx.actions.declare_file('lib.jar')",
" java_info = JavaInfo(output_jar = output_jar, compile_jar = None)",
" java_common.merge([java_info],",
" exports = [java_info]",
" merge_java_outputs = False",
" )",
" return []",
"java_custom_library = rule(",
Expand All @@ -3181,40 +3181,14 @@ public void testExportsIsPrivateApi() throws Exception {
}

@Test
public void testRuntimeDepsIsPrivateApi() throws Exception {
public void testMergeSourceJarsIsPrivateApi() throws Exception {
scratch.file(
"foo/custom_rule.bzl",
"def _impl(ctx):",
" output_jar = ctx.actions.declare_file('lib.jar')",
" java_info = JavaInfo(output_jar = output_jar, compile_jar = None)",
" java_common.merge([java_info],",
" runtime_deps = [java_info]",
" )",
" return []",
"java_custom_library = rule(",
" implementation = _impl,",
" fragments = ['java']",
")");
scratch.file(
"foo/BUILD",
"load(':custom_rule.bzl', 'java_custom_library')",
"java_custom_library(name = 'custom')");
reporter.removeHandler(failFastHandler);

getConfiguredTarget("//foo:custom");

assertContainsEvent("Rule in 'foo' cannot use private API");
}

@Test
public void testIncludeSourceJarsIsPrivateApi() throws Exception {
scratch.file(
"foo/custom_rule.bzl",
"def _impl(ctx):",
" output_jar = ctx.actions.declare_file('lib.jar')",
" java_info = JavaInfo(output_jar = output_jar, compile_jar = None)",
" java_common.merge([java_info],",
" include_source_jars_from_exports = True",
" merge_source_jars = False",
" )",
" return []",
"java_custom_library = rule(",
Expand Down

0 comments on commit 4c26c3a

Please sign in to comment.