Loosen java_library.exports and java_import.*

Stephen Twigg · iirina · commit 7e91ad631a66 · 2017-05-22T18:05:46.000+02:00
java_library.exports and java_import.runtime_deps|exports|deps will now
accept any label from a rule that has a JavaProvider declared provider.
Note that java_library.deps|runtime_deps already had this feature and
this simply extends that privilege.

This relies on the fact that both those targets (via JavaCommon) are
simply searching for jars via JavaProvider anyway.

Added test for passing a custom Skylark rule (that provides a
JavaProvider) can successfully be added to the deps, runtime_deps, and
exports of java_library, java_import, and java_binary (where
appropriate).

Added integration tests for java_library.exports|runtime_deps (the basic
sandwich already tests deps) and java_import.exports|runtime_deps.

Note that custom Skylark rules are still unable to provide or propagate
a JavaNativeLibraryProvider, which results from a cc dependency. Also,
the deps argument for java_import is somewhat odd.

Change-Id: I7b2c19c6b99516ce524e8c82193d0c73e2d66530
PiperOrigin-RevId: 156740729
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaImportRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaImportRule.java
@@ -44,26 +44,32 @@ public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
         The list of other libraries to be linked in to the target.
         See <a href="${link java_library.deps}">java_library.deps</a>.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("deps", LABEL_LIST)
-            .allowedRuleClasses(ALLOWED_DEPS)
-            .allowedFileTypes()  // none allowed
-            .validityPredicate(ANY_EDGE))
+        .add(
+            attr("deps", LABEL_LIST)
+                .allowedRuleClasses(ALLOWED_DEPS)
+                .allowedFileTypes() // none allowed
+                .validityPredicate(ANY_EDGE)
+                .mandatoryProvidersList(BazelJavaRuleClasses.MANDATORY_JAVA_PROVIDER_ONLY))
         /* <!-- #BLAZE_RULE(java_import).ATTRIBUTE(exports) -->
         Targets to make available to users of this rule.
         See <a href="${link java_library.exports}">java_library.exports</a>.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("exports", LABEL_LIST)
-            .allowedRuleClasses(ALLOWED_DEPS)
-            .allowedFileTypes()  // none allowed
-            .validityPredicate(ANY_EDGE))
+        .add(
+            attr("exports", LABEL_LIST)
+                .allowedRuleClasses(ALLOWED_DEPS)
+                .allowedFileTypes() // none allowed
+                .validityPredicate(ANY_EDGE)
+                .mandatoryProvidersList(BazelJavaRuleClasses.MANDATORY_JAVA_PROVIDER_ONLY))
         /* <!-- #BLAZE_RULE(java_import).ATTRIBUTE(runtime_deps) -->
         Libraries to make available to the final binary or test at runtime only.
         See <a href="${link java_library.runtime_deps}">java_library.runtime_deps</a>.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("runtime_deps", LABEL_LIST)
-            .allowedFileTypes(JavaSemantics.JAR)
-            .allowedRuleClasses(ALLOWED_DEPS)
-            .skipAnalysisTimeFileTypeCheck())
+        .add(
+            attr("runtime_deps", LABEL_LIST)
+                .allowedFileTypes(JavaSemantics.JAR)
+                .allowedRuleClasses(ALLOWED_DEPS)
+                .mandatoryProvidersList(BazelJavaRuleClasses.MANDATORY_JAVA_PROVIDER_ONLY)
+                .skipAnalysisTimeFileTypeCheck())
         .advertiseProvider(JavaSourceInfoProvider.class)
         .build();
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaLibraryRule.java
@@ -115,8 +115,8 @@ depended on these rules. This is not true for regular (non-exported) <code>deps<
         .add(
             attr("exports", LABEL_LIST)
                 .allowedRuleClasses(BazelJavaRuleClasses.ALLOWED_RULES_IN_DEPS)
-                .allowedFileTypes(/*May not have files in exports!*/ ))
-
+                .allowedFileTypes(/*May not have files in exports!*/ )
+                .mandatoryProvidersList(BazelJavaRuleClasses.MANDATORY_JAVA_PROVIDER_ONLY))
         /* <!-- #BLAZE_RULE(java_library).ATTRIBUTE(neverlink) -->
         Whether this library should only be used for compilation and not at runtime.
         Useful if the library will be provided by the runtime environment during execution. Examples
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaRuleClasses.java
@@ -74,6 +74,26 @@ public class BazelJavaRuleClasses {
           JavaSemantics.JAVA_LIBRARY_CLASS_JAR,
           JavaSemantics.JAVA_LIBRARY_SOURCE_JAR);
 
+  /**
+   * Meant to be an element of {@code mandatoryProvidersLists} in order to accept rules providing
+   * a {@link JavaProvider} through an attribute. Other providers can be included in
+   * {@code mandatoryProvidersLists} as well.
+   */
+  public static final ImmutableList<SkylarkProviderIdentifier> CONTAINS_JAVA_PROVIDER =
+      ImmutableList.of(SkylarkProviderIdentifier.forKey(JavaProvider.JAVA_PROVIDER.getKey()));
+
+  public static final ImmutableList<SkylarkProviderIdentifier> CONTAINS_CC_LINK_PARAMS =
+      ImmutableList.of(
+          SkylarkProviderIdentifier.forKey(CcLinkParamsProvider.CC_LINK_PARAMS.getKey()));
+
+  /**
+   * Meant to be the value of {@code mandatoryProvidersLists} in order for the rule to provide only
+   * a {@link JavaProvider} through an attribute.
+   */
+  public static final ImmutableList<ImmutableList<SkylarkProviderIdentifier>>
+      MANDATORY_JAVA_PROVIDER_ONLY = ImmutableList.of(CONTAINS_JAVA_PROVIDER);
+
+
   /**
    * Common attributes for rules that depend on ijar.
    */
@@ -158,13 +178,7 @@ public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
                   .allowedFileTypes(JavaSemantics.JAR)
                   .allowedRuleClasses(ALLOWED_RULES_IN_DEPS)
                   .mandatoryProvidersList(
-                      ImmutableList.of(
-                          ImmutableList.of(
-                              SkylarkProviderIdentifier.forKey(
-                                  CcLinkParamsProvider.CC_LINK_PARAMS.getKey())),
-                          ImmutableList.of(
-                              SkylarkProviderIdentifier.forKey(
-                                  JavaProvider.JAVA_PROVIDER.getKey()))))
+                      ImmutableList.of(CONTAINS_CC_LINK_PARAMS, CONTAINS_JAVA_PROVIDER))
                   .skipAnalysisTimeFileTypeCheck())
           /* <!-- #BLAZE_RULE($java_rule).ATTRIBUTE(runtime_deps) -->
           Libraries to make available to the final binary or test at runtime only.
@@ -177,11 +191,7 @@ public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
               attr("runtime_deps", LABEL_LIST)
                   .allowedFileTypes(JavaSemantics.JAR)
                   .allowedRuleClasses(ALLOWED_RULES_IN_DEPS)
-                  .mandatoryProvidersList(
-                      ImmutableList.of(
-                          ImmutableList.of(
-                              SkylarkProviderIdentifier.forKey(
-                                  JavaProvider.JAVA_PROVIDER.getKey()))))
+                  .mandatoryProvidersList(MANDATORY_JAVA_PROVIDER_ONLY)
                   .skipAnalysisTimeFileTypeCheck())
 
           /* <!-- #BLAZE_RULE($java_rule).ATTRIBUTE(srcs) -->
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
@@ -324,6 +324,102 @@ public void javaProviderPropagation() throws Exception {
         jlTopJavaProvider.getProvider(JavaCompilationArgsProvider.class).getJavaCompilationArgs());
   }
 
+  @Test
+  public void skylarkJavaToJavaLibraryAttributes() throws Exception {
+    scratch.file(
+        "foo/extension.bzl",
+        "def _impl(ctx):",
+        "  dep_params = ctx.attr.dep[java_common.provider]",
+        "  return struct(providers = [dep_params])",
+        "my_rule = rule(_impl, attrs = { 'dep' : attr.label() })");
+    scratch.file(
+        "foo/BUILD",
+        "load(':extension.bzl', 'my_rule')",
+        "java_library(name = 'jl_bottom_for_deps', srcs = ['java/A.java'])",
+        "java_library(name = 'jl_bottom_for_exports', srcs = ['java/A2.java'])",
+        "java_library(name = 'jl_bottom_for_runtime_deps', srcs = ['java/A2.java'])",
+        "my_rule(name = 'mya', dep = ':jl_bottom_for_deps')",
+        "my_rule(name = 'myb', dep = ':jl_bottom_for_exports')",
+        "my_rule(name = 'myc', dep = ':jl_bottom_for_runtime_deps')",
+        "java_library(name = 'lib_exports', srcs = ['java/B.java'], deps = [':mya'],",
+        "  exports = [':myb'], runtime_deps = [':myc'])",
+        "java_library(name = 'lib_interm', srcs = ['java/C.java'], deps = [':lib_exports'])",
+        "java_library(name = 'lib_top', srcs = ['java/D.java'], deps = [':lib_interm'])");
+    assertNoEvents();
+
+    // Test that all bottom jars are on the runtime classpath of lib_exports.
+    ConfiguredTarget jlExports = getConfiguredTarget("//foo:lib_exports");
+    JavaCompilationArgsProvider jlExportsProvider =
+        JavaProvider.getProvider(JavaCompilationArgsProvider.class, jlExports);
+    assertThat(prettyJarNames(jlExportsProvider.getRecursiveJavaCompilationArgs().getRuntimeJars()))
+        .containsAllOf(
+            "foo/libjl_bottom_for_deps.jar",
+            "foo/libjl_bottom_for_runtime_deps.jar",
+            "foo/libjl_bottom_for_exports.jar");
+
+    // Test that libjl_bottom_for_exports.jar is in the recursive java compilation args of lib_top.
+    ConfiguredTarget jlTop = getConfiguredTarget("//foo:lib_interm");
+    JavaCompilationArgsProvider jlTopProvider =
+        JavaProvider.getProvider(JavaCompilationArgsProvider.class, jlTop);
+    assertThat(prettyJarNames(jlTopProvider.getRecursiveJavaCompilationArgs().getRuntimeJars()))
+        .contains("foo/libjl_bottom_for_exports.jar");
+  }
+
+  @Test
+  public void skylarkJavaToJavaBinaryAttributes() throws Exception {
+    scratch.file(
+        "foo/extension.bzl",
+        "def _impl(ctx):",
+        "  dep_params = ctx.attr.dep[java_common.provider]",
+        "  return struct(providers = [dep_params])",
+        "my_rule = rule(_impl, attrs = { 'dep' : attr.label() })");
+    scratch.file(
+        "foo/BUILD",
+        "load(':extension.bzl', 'my_rule')",
+        "java_library(name = 'jl_bottom_for_deps', srcs = ['java/A.java'])",
+        "java_library(name = 'jl_bottom_for_runtime_deps', srcs = ['java/A2.java'])",
+        "my_rule(name = 'mya', dep = ':jl_bottom_for_deps')",
+        "my_rule(name = 'myb', dep = ':jl_bottom_for_runtime_deps')",
+        "java_binary(name = 'binary', srcs = ['java/B.java'], main_class = 'foo.A',",
+        "  deps = [':mya'], runtime_deps = [':myb'])");
+    assertNoEvents();
+
+    // Test that all bottom jars are on the runtime classpath.
+    ConfiguredTarget binary = getConfiguredTarget("//foo:binary");
+    assertThat(prettyJarNames(
+        binary.getProvider(JavaRuntimeClasspathProvider.class).getRuntimeClasspath()))
+            .containsAllOf(
+                "foo/libjl_bottom_for_deps.jar", "foo/libjl_bottom_for_runtime_deps.jar");
+  }
+
+  @Test
+  public void skylarkJavaToJavaImportAttributes() throws Exception {
+    scratch.file(
+        "foo/extension.bzl",
+        "def _impl(ctx):",
+        "  dep_params = ctx.attr.dep[java_common.provider]",
+        "  return struct(providers = [dep_params])",
+        "my_rule = rule(_impl, attrs = { 'dep' : attr.label() })");
+    scratch.file(
+        "foo/BUILD",
+        "load(':extension.bzl', 'my_rule')",
+        "java_library(name = 'jl_bottom_for_deps', srcs = ['java/A.java'])",
+        "java_library(name = 'jl_bottom_for_runtime_deps', srcs = ['java/A2.java'])",
+        "my_rule(name = 'mya', dep = ':jl_bottom_for_deps')",
+        "my_rule(name = 'myb', dep = ':jl_bottom_for_runtime_deps')",
+        "java_import(name = 'import', jars = ['B.jar'], deps = [':mya'], runtime_deps = [':myb'])");
+    assertNoEvents();
+
+    // Test that all bottom jars are on the runtime classpath.
+    ConfiguredTarget importTarget = getConfiguredTarget("//foo:import");
+    JavaCompilationArgsProvider compilationProvider =
+        JavaProvider.getProvider(JavaCompilationArgsProvider.class, importTarget);
+    assertThat(prettyJarNames(
+        compilationProvider.getRecursiveJavaCompilationArgs().getRuntimeJars()))
+        .containsAllOf(
+            "foo/libjl_bottom_for_deps.jar", "foo/libjl_bottom_for_runtime_deps.jar");
+  }
+
   @Test
   public void strictDepsEnabled() throws Exception {
     scratch.file(
diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh
@@ -59,6 +59,111 @@ public class HelloLibrary {
 EOF
 }
 
+function write_files_for_java_provider_in_attr() {
+  mkdir -p java/com/google/sandwich
+  cd java/com/google/sandwich
+
+  touch BUILD A.java B.java Main.java java_custom_library.bzl
+
+  rule_type="$1" # java_library / java_import
+  attribute_name="$2" # exports / runtime_deps
+  srcs_attribute_row="srcs = ['A.java']"
+  if [ "$rule_type" = "java_import" ]; then
+    srcs_attribute_row="jars = []"
+  fi
+
+  cat > BUILD <<EOF
+load(':java_custom_library.bzl', 'java_custom_library')
+
+java_binary(
+  name = "Main",
+EOF
+
+  if [ "$attribute_name" = "runtime_deps" ]; then
+    cat >> BUILD <<EOF
+  main_class = "com.google.sandwich.Main",
+  runtime_deps = [":top"]
+)
+
+EOF
+  else
+    cat >> BUILD <<EOF
+  srcs = ["Main.java"],
+  deps = [":top"]
+)
+
+EOF
+  fi
+
+  echo "$rule_type(" >> BUILD
+
+  cat >> BUILD <<EOF
+  name = "top",
+EOF
+
+  echo "  $srcs_attribute_row," >> BUILD
+  echo "  $attribute_name = [':middle']" >> BUILD
+
+  cat >> BUILD <<EOF
+)
+
+java_custom_library(
+  name = "middle",
+EOF
+
+  if [ "$attribute_name" = "runtime_deps" ]; then
+    cat >> BUILD <<EOF
+  srcs = ["B.java", "Main.java"],
+)
+EOF
+  else
+    cat >> BUILD <<EOF
+  srcs = ["B.java"],
+)
+EOF
+  fi
+
+  cat > B.java <<EOF
+package com.google.sandwich;
+class B {
+  public void printB() {
+    System.out.println("Message from B");
+  }
+}
+EOF
+
+if [ "$rule_type" = "java_library" ]; then
+  cat > A.java <<EOF
+package com.google.sandwich;
+class A {
+  public void printA() {
+    System.out.println("Message from A");
+  }
+}
+EOF
+fi
+
+cat > Main.java <<EOF
+package com.google.sandwich;
+class Main {
+  public static void main(String[] args) {
+EOF
+
+if [[ "$rule_type" = "java_library" && "$attribute_name" = "exports" ]]; then
+  cat >> Main.java <<EOF
+    A myObjectA = new A();
+    myObjectA.printA();
+EOF
+fi
+
+cat >> Main.java <<EOF
+    B myObjectB = new B();
+    myObjectB.printB();
+  }
+}
+EOF
+}
+
 function write_java_custom_rule() {
   cat > java_custom_library.bzl << EOF
 def _impl(ctx):
@@ -494,6 +599,39 @@ EOF
   expect_log "Message from C"
 }
 
+function test_java_library_exports_java_sandwich() {
+  write_files_for_java_provider_in_attr "java_library" "exports"
+  write_java_custom_rule
+
+  bazel run :Main > $TEST_log || fail "Java sandwich build failed"
+  expect_log "Message from A"
+  expect_log "Message from B"
+}
+
+function test_java_library_runtime_deps_java_sandwich() {
+  write_files_for_java_provider_in_attr "java_library" "runtime_deps"
+  write_java_custom_rule
+
+  bazel run :Main > $TEST_log || fail "Java sandwich build failed"
+  expect_log "Message from B"
+}
+
+function test_java_import_exports_java_sandwich() {
+  write_files_for_java_provider_in_attr "java_import" "exports"
+  write_java_custom_rule
+
+  bazel run :Main > $TEST_log || fail "Java sandwich build failed"
+  expect_log "Message from B"
+}
+
+function test_java_import_runtime_deps_java_sandwich() {
+  write_files_for_java_provider_in_attr "java_import" "runtime_deps"
+  write_java_custom_rule
+
+  bazel run :Main > $TEST_log || fail "Java sandwich build failed"
+  expect_log "Message from B"
+}
+
 function test_java_binary_deps_java_sandwich() {
   mkdir -p java/com/google/sandwich
   cd java/com/google/sandwich
