Provide java_common.compile interface for Java compilation of proto l…

…ibraries Remove `create_output_source_jar` parameter and infer its value. The value is `false` when precisely 1 source jar is on the input which is also the `output_source_jar`. Introduce `include_compilation_info` parameter. It may be disabled only in builtins. When disabled the function drops `JavaCompilationInfo` and `JavaGenJarsProvider`.This corresponds to optimisations done natively. Native Java proto compilation code provides `JavaRuleOutputJarsProvider`, truncated `JavaSourceJarsProvider`, `JavaCcInfoProvider`. Benchmarks show that truncating `JavaSourceJarsProvider` is unnecessary. PiperOrigin-RevId: 451967306 Change-Id: I033e690ffb1df36e00203f5447675861bfdfdb52
databricks · May 31, 2022 · 3649324 · 3649324
1 parent 4c26c3a
commit 3649324
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 17 deletions.
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
@@ -265,7 +265,7 @@ public JavaInfo createJavaCompileAction(
       Boolean enableAnnotationProcessing,
       Boolean enableCompileJarAction,
       boolean enableJSpecify,
-      boolean createOutputSourceJar,
+      boolean includeCompilationInfo,
       JavaSemantics javaSemantics,
       Object injectingRuleKind,
       List<String> addExports,
@@ -319,6 +319,10 @@ public JavaInfo createJavaCompileAction(
 
     JavaRuleOutputJarsProvider.Builder outputJarsBuilder = JavaRuleOutputJarsProvider.builder();
 
+    boolean createOutputSourceJar =
+        !(sourceJars.size() == 1
+            && sourceFiles.isEmpty()
+            && sourceJars.get(0).equals(outputSourceJar));
     if (outputSourceJar == null) {
       outputSourceJar = getDerivedSourceJar(starlarkRuleContext.getRuleContext(), outputJar);
     }
@@ -330,6 +334,7 @@ public JavaInfo createJavaCompileAction(
             toolchainProvider,
             outputJarsBuilder,
             createOutputSourceJar,
+            includeCompilationInfo,
             outputSourceJar,
             enableCompileJarAction,
             javaInfoBuilder,
@@ -358,15 +363,12 @@ public JavaInfo createJavaCompileAction(
                 streamProviders(deps, JavaCcInfoProvider.class),
                 Stream.of(new JavaCcInfoProvider(CcInfo.merge(nativeLibraries))))
             .collect(toImmutableList());
+
     return javaInfoBuilder
         .addProvider(JavaCompilationArgsProvider.class, javaCompilationArgsProvider)
         .addProvider(
             JavaSourceJarsProvider.class,
-            createOutputSourceJar
-                ? createJavaSourceJarsProvider(outputSourceJars, concat(runtimeDeps, exports, deps))
-                : JavaSourceJarsProvider.create(
-                    // TODO(b/207058960): Refactor. This is used for proto optimisation.
-                    NestedSetBuilder.wrap(Order.STABLE_ORDER, sourceJars), sourceJars))
+            createJavaSourceJarsProvider(outputSourceJars, concat(runtimeDeps, exports, deps)))
         .addProvider(JavaRuleOutputJarsProvider.class, outputJarsBuilder.build())
         .javaPluginInfo(mergeExportedJavaPluginInfo(exportedPlugins, exports))
         .addProvider(JavaCcInfoProvider.class, JavaCcInfoProvider.merge(transitiveNativeLibraries))

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
@@ -213,6 +213,7 @@ public JavaCompilationArtifacts build(
         javaToolchainProvider,
         outputJarsBuilder,
         createOutputSourceJar,
+        true,
         outputSourceJar,
         true,
         /* javaInfoBuilder= */ null,
@@ -225,6 +226,7 @@ public JavaCompilationArtifacts build(
       JavaToolchainProvider javaToolchainProvider,
       JavaRuleOutputJarsProvider.Builder outputJarsBuilder,
       boolean createOutputSourceJar,
+      boolean includeCompilationInfo,
       @Nullable Artifact outputSourceJar,
       boolean enableCompileJarAction,
       @Nullable JavaInfo.Builder javaInfoBuilder,
@@ -291,13 +293,13 @@ public JavaCompilationArtifacts build(
     JavaCompilationArtifacts javaArtifacts = artifactsBuilder.build();
     outputJarsBuilder.addJavaOutput(
         JavaOutput.builder()
-            .fromJavaCompileOutputs(outputs)
+            .fromJavaCompileOutputs(outputs, includeCompilationInfo)
             .setCompileJar(iJar)
             .setCompileJdeps(javaArtifacts.getCompileTimeDependencyArtifact())
             .addSourceJar(outputSourceJar)
             .build());
 
-    if (javaInfoBuilder != null) {
+    if (javaInfoBuilder != null && includeCompilationInfo) {
       ClasspathConfiguredFragment classpathFragment =
           new ClasspathConfiguredFragment(
               javaArtifacts,

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleOutputJarsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleOutputJarsProvider.java
@@ -166,8 +166,15 @@ public Builder addSourceJars(Iterable<Artifact> values) {
 
       /** Populates the builder with outputs from {@link JavaCompileOutputs}. */
       public Builder fromJavaCompileOutputs(JavaCompileOutputs<Artifact> value) {
+        return fromJavaCompileOutputs(value, true);
+      }
+
+      public Builder fromJavaCompileOutputs(
+          JavaCompileOutputs<Artifact> value, boolean includeJdeps) {
         setClassJar(value.output());
-        setJdeps(value.depsProto());
+        if (includeJdeps) {
+          setJdeps(value.depsProto());
+        }
         setGeneratedClassJar(value.genClass());
         setGeneratedSourceJar(value.genSource());
         setNativeHeadersJar(value.nativeHeader());

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java
@@ -91,7 +91,7 @@ public JavaInfo createJavaCompileAction(
       Boolean enableAnnotationProcessing,
       Boolean enableCompileJarAction,
       Boolean enableJSpecify,
-      boolean createOutputSourceJar,
+      boolean includeCompilationInfo,
       Object injectingRuleKind,
       Sequence<?> addExports, // <String> expected
       Sequence<?> addOpens, // <String> expected
@@ -134,7 +134,7 @@ public JavaInfo createJavaCompileAction(
     // checks for private API access
     if (!enableCompileJarAction
         || !enableJSpecify
-        || !createOutputSourceJar
+        || !includeCompilationInfo
         || !classpathResources.isEmpty()
         || injectingRuleKind != Starlark.NONE) {
       checkPrivateAccess(thread);
@@ -170,7 +170,7 @@ public JavaInfo createJavaCompileAction(
             enableAnnotationProcessing,
             enableCompileJarAction,
             enableJSpecify,
-            createOutputSourceJar,
+            includeCompilationInfo,
             javaSemantics,
             injectingRuleKind,
             Sequence.cast(addExports, String.class, "add_exports"),

diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java
@@ -236,7 +236,7 @@ public interface JavaCommonApi<
             defaultValue = "True",
             documented = false),
         @Param(
-            name = "create_output_source_jar",
+            name = "include_compilation_info",
             positional = false,
             named = true,
             defaultValue = "True",
@@ -294,7 +294,7 @@ JavaInfoT createJavaCompileAction(
       Boolean enableAnnotationProcessing,
       Boolean enableCompileJarAction,
       Boolean enableJSpecify,
-      boolean createOutputSourceJar,
+      boolean includeCompilationInfo,
       Object injectingRuleKind,
       Sequence<?> addExports, // <String> expected.
       Sequence<?> addOpens, // <String> expected.

diff --git a/src/main/starlark/builtins_bzl/common/java/proto/java_proto_library.bzl b/src/main/starlark/builtins_bzl/common/java/proto/java_proto_library.bzl
@@ -117,8 +117,8 @@ def java_compile_for_protos(ctx, output_jar_suffix, source_jar = None, deps = []
             injecting_rule_kind = injecting_rule_kind,
             javac_opts = java_toolchain.compatible_javacopts("proto"),
             enable_jspecify = False,
-            create_output_source_jar = False,
             java_toolchain = java_toolchain,
+            include_compilation_info = False,
         )
         jars = [source_jar, output_jar]
     else:

diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java
@@ -3207,7 +3207,7 @@ public void testMergeSourceJarsIsPrivateApi() throws Exception {
   }
 
   @Test
-  public void testCreateOutputSourceJarIsPrivateApi() throws Exception {
+  public void testCompileIncludeCompilationInfoIsPrivateApi() throws Exception {
     JavaToolchainTestUtil.writeBuildFileForJavaToolchain(scratch);
     scratch.file(
         "foo/custom_rule.bzl",
@@ -3216,7 +3216,7 @@ public void testCreateOutputSourceJarIsPrivateApi() throws Exception {
         "    ctx,",
         "    output = ctx.actions.declare_file('output.jar'),",
         "    java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo],",
-        "    create_output_source_jar = False,",
+        "    include_compilation_info = False,",
         "  )",
         "  return []",
         "java_custom_library = rule(",