From 27085f44a168b502542cda6216863601e64da833 Mon Sep 17 00:00:00 2001
From: Ulf Adams <ulfjack@google.com>
Date: Thu, 6 Oct 2016 08:51:47 +0000
Subject: Remove JavaCpuSupplier; use the actual --cpu value instead.

This is a semantic change for Bazel, which now observes the --cpu flag when
looking up a Jvm. Use "-default" as a fallback to keep the change backwards
compatible.

RELNOTES[INC]: Bazel now uses the --cpu flag to look up Jvms; it falls back
    to "default" if it can't find a Jvm matching the CPU value.

--
MOS_MIGRATED_REVID=135333759
---
 src/main/java/com/google/devtools/build/lib/BUILD  |  1 -
 .../lib/bazel/rules/BazelRuleClassProvider.java    | 14 +---
 .../build/lib/bazel/rules/java/jdk.WORKSPACE       | 17 +++-
 .../build/lib/rules/java/JavaCpuSupplier.java      | 31 --------
 .../lib/rules/java/JvmConfigurationLoader.java     | 91 +++++++++++++---------
 5 files changed, 70 insertions(+), 84 deletions(-)
 delete mode 100644 src/main/java/com/google/devtools/build/lib/rules/java/JavaCpuSupplier.java

(limited to 'src/main/java/com/google')

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 110b4c1e42..d2f503ae24 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -742,7 +742,6 @@ java_library(
         "rules/java/JavaBinary.java",
         "rules/java/JavaCcLinkParamsProvider.java",
         "rules/java/JavaConfigurationLoader.java",
-        "rules/java/JavaCpuSupplier.java",
         "rules/java/JavaImport.java",
         "rules/java/JavaImportBaseRule.java",
         "rules/java/JavaLibrary.java",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index cedecad9d0..ad195b98d4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -25,10 +25,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.Prereq
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigRuleClasses;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
-import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule;
 import com.google.devtools.build.lib.bazel.rules.android.AarImportRule;
 import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryRule;
@@ -103,7 +100,6 @@ import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader;
 import com.google.devtools.build.lib.rules.cpp.CppOptions;
 import com.google.devtools.build.lib.rules.genquery.GenQueryRule;
 import com.google.devtools.build.lib.rules.java.JavaConfigurationLoader;
-import com.google.devtools.build.lib.rules.java.JavaCpuSupplier;
 import com.google.devtools.build.lib.rules.java.JavaImportBaseRule;
 import com.google.devtools.build.lib.rules.java.JavaOptions;
 import com.google.devtools.build.lib.rules.java.JavaToolchainRule;
@@ -170,14 +166,6 @@ public class BazelRuleClassProvider {
     return builder.build();
   }
 
-  public static final JavaCpuSupplier JAVA_CPU_SUPPLIER = new JavaCpuSupplier() {
-    @Override
-    public String getJavaCpu(BuildOptions buildOptions, ConfigurationEnvironment env)
-        throws InvalidConfigurationException {
-      return "default";
-    }
-  };
-
   private static class BazelPrerequisiteValidator implements PrerequisiteValidator {
     @Override
     public void validate(RuleContext.Builder context,
@@ -309,7 +297,7 @@ public class BazelRuleClassProvider {
 
   private static void initJava(ConfiguredRuleClassProvider.Builder builder) {
     builder.addConfigurationOptions(JavaOptions.class);
-    builder.addConfigurationFragment(new JvmConfigurationLoader(JAVA_CPU_SUPPLIER));
+    builder.addConfigurationFragment(new JvmConfigurationLoader());
     builder.addConfigurationFragment(new JavaConfigurationLoader());
 
     builder.addBuildInfoFactory(new BazelJavaBuildInfoFactory());
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE
index 6f85af1aec..3b78cac998 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE
@@ -49,12 +49,25 @@ bind(
     actual = "@local_jdk//:javac",
 )
 
+bind(
+    name = "jre",
+    actual = "@local_jdk//:jre",
+)
+
+bind(
+    name = "jdk",
+    actual = "@local_jdk//:jdk",
+)
+
+# TODO: Remove these two rules after we've migrated. In order to properly look
+# up Jdks/Jres for cross-platform builds, the lookup needs to happen in the Jdk
+# repository. For now, use an alias rule that redirects to //external:{jre,jdk}.
 bind(
     name = "jre-default",
-    actual = "@local_jdk//:jre-default",
+    actual = "@local_jdk//:jre",
 )
 
 bind(
     name = "jdk-default",
-    actual = "@local_jdk//:jdk-default",
+    actual = "@local_jdk//:jdk",
 )
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCpuSupplier.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCpuSupplier.java
deleted file mode 100644
index 6ca8e99861..0000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCpuSupplier.java
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.rules.java;
-
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
-import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
-
-/**
- * Determines the CPU to be used for Java compilation from the build options and the
- * configuration environment.
- */
-public interface JavaCpuSupplier {
-  /**
-   * Returns the Java CPU based on the buiold options and the configuration environment.
-   */
-  String getJavaCpu(BuildOptions buildOptions, ConfigurationEnvironment env)
-      throws InvalidConfigurationException;
-}
\ No newline at end of file
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
index fe97c33620..6d9c42da5b 100644
--- 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
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.java;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.RedirectChaser;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
@@ -49,12 +50,6 @@ import javax.annotation.Nullable;
  * <p>The loader also supports legacy mode, where the JVM can be defined with an abolute path.
  */
 public final class JvmConfigurationLoader implements ConfigurationFragmentFactory {
-  private final JavaCpuSupplier cpuSupplier;
-
-  public JvmConfigurationLoader(JavaCpuSupplier cpuSupplier) {
-    this.cpuSupplier = cpuSupplier;
-  }
-
   @Override
   public Jvm create(ConfigurationEnvironment env, BuildOptions buildOptions)
       throws InvalidConfigurationException, InterruptedException {
@@ -64,10 +59,7 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor
       return null;
     }
     String javaHome = javaOptions.javaBase;
-    String cpu = cpuSupplier.getJavaCpu(buildOptions, env);
-    if (cpu == null) {
-      return null;
-    }
+    String cpu = buildOptions.get(BuildConfiguration.Options.class).cpu;
 
     try {
       return createDefault(env, javaHome, cpu);
@@ -109,40 +101,65 @@ public final class JvmConfigurationLoader implements ConfigurationFragmentFactor
               + " is configurable. JAVABASE targets don't support configurable attributes");
         }
         List<Label> labels = javaHomeAttributes.get("srcs", BuildType.LABEL_LIST);
+        Label selectedJvmLabel = null;
+        Label defaultJvmLabel = null;
         for (Label jvmLabel : labels) {
           if (jvmLabel.getName().endsWith("-" + cpu)) {
-            jvmLabel = RedirectChaser.followRedirects(
-                lookup, jvmLabel, "Architecture-specific JDK");
-            if (jvmLabel == null) {
-              return null;
-            }
+            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(jvmLabel);
-            if (jvmTarget == null) {
-              return null;
-            }
+          Target jvmTarget = lookup.getTarget(selectedJvmLabel);
+          if (jvmTarget == null) {
+            return null;
+          }
 
-            PathFragment javaHomePath;
-            if (jvmTarget.getLabel().getPackageIdentifier().getRepository().isDefault()) {
-              javaHomePath = jvmLabel.getPackageFragment();
-            } else {
-              javaHomePath = jvmTarget.getLabel().getPackageIdentifier().getSourceRoot();
-            }
+          PathFragment javaHomePath;
+          if (jvmTarget.getLabel().getPackageIdentifier().getRepository().isDefault()) {
+            javaHomePath = selectedJvmLabel.getPackageFragment();
+          } else {
+            javaHomePath = jvmTarget.getLabel().getPackageIdentifier().getSourceRoot();
+          }
 
-            if ((jvmTarget instanceof Rule) &&
-                "filegroup".equals(((Rule) jvmTarget).getRuleClass())) {
-              RawAttributeMapper jvmTargetAttributes = RawAttributeMapper.of((Rule) jvmTarget);
-              if (jvmTargetAttributes.isConfigurable("path", Type.STRING)) {
-                throw new InvalidConfigurationException("\"path\" in " + jvmTarget
-                    + " is configurable. JVM targets don't support configurable attributes");
-              }
-              String path = jvmTargetAttributes.get("path", Type.STRING);
-              if (path != null) {
-                javaHomePath = javaHomePath.getRelative(path);
-              }
+          if ((jvmTarget instanceof Rule) &&
+              "filegroup".equals(((Rule) jvmTarget).getRuleClass())) {
+            RawAttributeMapper jvmTargetAttributes = RawAttributeMapper.of((Rule) jvmTarget);
+            if (jvmTargetAttributes.isConfigurable("path", Type.STRING)) {
+              throw new InvalidConfigurationException("\"path\" in " + jvmTarget
+                  + " is configurable. JVM targets don't support configurable attributes");
+            }
+            String path = jvmTargetAttributes.get("path", Type.STRING);
+            if (path != null) {
+              javaHomePath = javaHomePath.getRelative(path);
             }
-            return new Jvm(javaHomePath, jvmLabel);
           }
+          return new Jvm(javaHomePath, selectedJvmLabel);
         }
       }
       throw new InvalidConfigurationException("No JVM target found under " + javaHome
-- 
cgit v1.2.3