Remove support for configuring JDKs with filegroups

RELNOTES: Remove support for configuring JDKs with filegroups; use java_runtime and java_runtime_suite instead -- PiperOrigin-RevId: 147741653 MOS_MIGRATED_REVID=147741653
sigaev · Feb 17, 2017 · b6e2bf1 · b6e2bf1
1 parent 5a1af9c
commit b6e2bf1
Showing 1 changed file with 8 additions and 94 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/java/JvmConfigurationLoader.java
@@ -35,24 +35,15 @@
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
 
 /**
  * A provider to load jvm configurations from the package path.
  *
  * <p>If the given {@code javaHome} is a label, i.e. starts with {@code "//"}, then the loader will
- * look at the target it refers to.
- *
- * <ul>
- *   <li>If the target is a {@code java_runtime_suite}, the loader will select the runtime from the
- *       entry in {@code java_runtime_suite.runtimes} for {@code cpu}.
- *   <li>If the target is a filegroup, then the loader will look in it's srcs for a filegroup that
- *       ends with {@code -<cpu>}. It will use that filegroup to construct the actual {@link Jvm}
- *       instance, using the filegroups {@code path} attribute to construct the new {@code javaHome}
- *       path.
- * </ul>
+ * look at the {@code java_runtime_suite} it refers to, and select the runtime from the entry in
+ * {@code java_runtime_suite.runtimes} for {@code cpu}.
  *
  * <p>The loader also supports legacy mode, where the JVM can be defined with an abolute path.
  */
@@ -98,15 +89,11 @@ private static Jvm createDefault(ConfigurationEnvironment lookup, String javaHom
       }
       Target javaHomeTarget = lookup.getTarget(label);
       if (javaHomeTarget instanceof Rule) {
-        switch (((Rule) javaHomeTarget).getRuleClass()) {
-          case "filegroup":
-            return createFromFilegroup(lookup, javaHomeTarget, cpu);
-          case "java_runtime_suite":
-            return createFromRuntimeSuite(lookup, (Rule) javaHomeTarget, cpu);
-          default:
-            throw new InvalidConfigurationException(
-                "Unexpected javabase rule kind '" + ((Rule) javaHomeTarget).getRuleClass() + "'");
+        if (!((Rule) javaHomeTarget).getRuleClass().equals("java_runtime_suite")) {
+          throw new InvalidConfigurationException(
+              "Unexpected javabase rule kind '" + ((Rule) javaHomeTarget).getRuleClass() + "'");
         }
+        return createFromRuntimeSuite(lookup, (Rule) javaHomeTarget, cpu);
       }
       throw new InvalidConfigurationException(
           "No JVM target found under " + javaHome + " that would work for " + cpu);
@@ -116,81 +103,8 @@ private static Jvm createDefault(ConfigurationEnvironment lookup, String javaHom
     }
   }
 
-  private static Jvm createFromFilegroup(
-      ConfigurationEnvironment lookup, Target javaHomeTarget, String cpu)
-      throws InvalidConfigurationException, InterruptedException, NoSuchPackageException,
-          NoSuchTargetException {
-
-    RawAttributeMapper javaHomeAttributes = RawAttributeMapper.of((Rule) javaHomeTarget);
-    if (javaHomeAttributes.isConfigurable("srcs")) {
-      throw new InvalidConfigurationException(
-          String.format(
-              "\"srcs\" in %s is configurable. JAVABASE targets don't support configurable"
-                  + " attributes",
-              javaHomeTarget));
-    }
-    List<Label> labels = javaHomeAttributes.get("srcs", BuildType.LABEL_LIST);
-    Label selectedJvmLabel = null;
-    Label defaultJvmLabel = null;
-    for (Label jvmLabel : labels) {
-      if (jvmLabel.getName().endsWith("-" + cpu)) {
-        selectedJvmLabel = jvmLabel;
-        break;
-      }
-      // When we open sourced Bazel, we used the string "default" to look up the Jvm. This is
-      // incorrect for cross-platform builds, but works for purely local builds. Since we now
-      // need to support cross-platform builds, we need to look up by the CPU, rather than the
-      // hard-coded string "default". However, for local builds the Jvm is setup with a
-      // mechanism where we don't currently have access to the CPU value (this is different from
-      // C++, where we infer the CPU from the local machine). As such, looking up only by CPU
-      // breaks builds that currently work, unless we add alias rules for all possible CPU
-      // values (but this is problematic if Bazel is ported to more platforms). For now, we're
-      // working around this problem by falling back to -default if we can't find a Jvm ending
-      // in -<cpu>. This is backwards compatible, but still allows cross-platform builds. In the
-      // medium term, we should rewrite Jvm setup to use a Skylark remote repository, and also
-      // remove the necessity of having a Jvm defined for all platforms even if there's no Java
-      // code.
-      if (jvmLabel.getName().endsWith("-default")) {
-        defaultJvmLabel = jvmLabel;
-      }
-    }
-    if (selectedJvmLabel == null) {
-      selectedJvmLabel = defaultJvmLabel;
-    }
-    if (selectedJvmLabel != null) {
-      selectedJvmLabel =
-          RedirectChaser.followRedirects(lookup, selectedJvmLabel, "Architecture-specific JDK");
-      if (selectedJvmLabel == null) {
-        return null;
-      }
-
-      Target jvmTarget = lookup.getTarget(selectedJvmLabel);
-
-      PathFragment javaHomePath = defaultJavaHome(jvmTarget.getLabel());
-
-      if ((jvmTarget instanceof Rule) && "filegroup".equals(((Rule) jvmTarget).getRuleClass())) {
-        RawAttributeMapper jvmTargetAttributes = RawAttributeMapper.of((Rule) jvmTarget);
-        if (jvmTargetAttributes.isConfigurable("path")) {
-          throw new InvalidConfigurationException(
-              String.format(
-                  "\"path\" in %s is configurable. JVM targets don't support configurable"
-                      + " attributes",
-                  jvmTarget));
-        }
-        String path = jvmTargetAttributes.get("path", Type.STRING);
-        if (path != null) {
-          javaHomePath = javaHomePath.getRelative(path);
-        }
-      }
-      return new Jvm(javaHomePath, selectedJvmLabel);
-    }
-    throw new InvalidConfigurationException(
-        String.format("No JVM target found under %s that would work for %s", javaHomeTarget, cpu));
-  }
-
-  // TODO(b/34175492): this is temporary until support for filegroup-based javabases is removed.
-  // Eventually the Jvm fragement will containg only the label of a java_runtime rule, and all of
-  // the configuration will be accessed using JavaRuntimeProvider.
+  // TODO(b/34175492): eventually the Jvm fragement will containg only the label of a java_runtime
+  // rule, and all of the configuration will be accessed using JavaRuntimeProvider.
   private static Jvm createFromRuntimeSuite(
       ConfigurationEnvironment lookup, Rule javaRuntimeSuite, String cpu)
       throws InvalidConfigurationException, InterruptedException, NoSuchTargetException,