Victory lap: Remove all code that used to support the three-argument form of vardef().

RELNOTES: None.
PiperOrigin-RevId: 190196933
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java
index cb63624..24ce6aa 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java
@@ -18,7 +18,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.MakeVariableSupplier.MapBackedMakeVariableSupplier;
-import com.google.devtools.build.lib.analysis.MakeVariableSupplier.PackageBackedMakeVariableSupplier;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException;
 import com.google.devtools.build.lib.analysis.stringtemplate.TemplateContext;
@@ -80,7 +79,7 @@
             .addAll(Preconditions.checkNotNull(extraMakeVariableSuppliers))
             .add(new MapBackedMakeVariableSupplier(ruleMakeVariables))
             .add(new MapBackedMakeVariableSupplier(configuration.getCommandLineBuildVariables()))
-            .add(new PackageBackedMakeVariableSupplier(pkg, configuration.getPlatformName()))
+            .add(new MapBackedMakeVariableSupplier(pkg.getMakeEnvironment()))
             .add(new MapBackedMakeVariableSupplier(configuration.getGlobalMakeEnvironment()))
             .build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index c349b80..34836ab 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -69,8 +69,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeMap;
-import javax.annotation.Nullable;
 
 /**
  * Knows about every rule Blaze supports and the associated configuration options.
@@ -236,7 +234,6 @@
     private ImmutableList.Builder<NativeProvider> nativeProviders = ImmutableList.builder();
     private ImmutableBiMap.Builder<String, Class<? extends TransitiveInfoProvider>>
         registeredSkylarkProviders = ImmutableBiMap.builder();
-    private Map<String, String> platformRegexps = new TreeMap<>();
 
     // TODO(pcloudy): Remove this field after Bazel rule definitions are not used internally.
     private String nativeLauncherLabel;
@@ -365,26 +362,6 @@
     }
 
     /**
-     * Do not use - this only exists for backwards compatibility! Platform regexps are part of a
-     * legacy mechanism - {@code vardef} - that is not exposed in Bazel.
-     *
-     * <p>{@code vardef} needs explicit support in the rule implementations, and cannot express
-     * conditional dependencies, only conditional attribute values. This mechanism will be
-     * supplanted by configuration dependent attributes, and its effect can usually also be achieved
-     * with select().
-     *
-     * <p>This is a map of platform names to regexps. When a name is used as the third argument to
-     * {@code vardef}, the corresponding regexp is used to match on the C++ abi, and the variable is
-     * only set to that value if the regexp matches. For example, the entry
-     * {@code "oldlinux": "i[34]86-libc[345]-linux"} might define a set of platforms representing
-     * certain older linux releases.
-     */
-    public Builder addPlatformRegexps(Map<String, String> platformRegexps) {
-      this.platformRegexps.putAll(Preconditions.checkNotNull(platformRegexps));
-      return this;
-    }
-
-    /**
      * Sets the C++ LIPO data transition, as defined in {@link
      * com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition}.
      *
@@ -508,11 +485,6 @@
     public String getToolsRepository() {
       return toolsRepository;
     }
-
-    @Nullable
-    public Map<String, String> getPlatformRegexps() {
-      return platformRegexps.isEmpty() ? null : ImmutableMap.copyOf(platformRegexps);
-    }
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/MakeVariableSupplier.java b/src/main/java/com/google/devtools/build/lib/analysis/MakeVariableSupplier.java
index 273c167..2d9426e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/MakeVariableSupplier.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/MakeVariableSupplier.java
@@ -16,7 +16,6 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.packages.Package;
 import javax.annotation.Nullable;
 
 /**
@@ -52,27 +51,4 @@
       return makeVariables;
     }
   }
-
-  /** {@link MakeVariableSupplier} that reads variables it supplies from a {@link Package} */
-  class PackageBackedMakeVariableSupplier implements MakeVariableSupplier {
-
-    private final String platform;
-    private final Package pkg;
-
-    public PackageBackedMakeVariableSupplier(Package pkg, String platform) {
-      this.pkg = pkg;
-      this.platform = platform;
-    }
-
-    @Nullable
-    @Override
-    public String getMakeVariable(String variableName) {
-      return pkg.lookupMakeVariable(variableName, platform);
-    }
-
-    @Override
-    public ImmutableMap<String, String> getAllMakeVariables() {
-      return pkg.getAllMakeVariables(platform);
-    }
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 4568a1c..54e7468 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -158,13 +158,6 @@
     }
 
     /**
-     * The platform name is a concatenation of fragment platform names.
-     */
-    public String getPlatformName() {
-      return "";
-    }
-
-    /**
      * Add items to the action environment.
      *
      * @param builder the map to add environment variables to
@@ -1170,7 +1163,6 @@
   private final Options options;
 
   private final String mnemonic;
-  private final String platformName;
 
   private final ImmutableMap<String, String> commandLineBuildVariables;
 
@@ -1374,8 +1366,6 @@
         OutputDirectory.MIDDLEMAN.getRoot(
             RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
 
-    this.platformName = buildPlatformName();
-
     this.shellExecutable = computeShellExecutable();
 
     this.actionEnv = setupActionEnvironment();
@@ -1506,26 +1496,6 @@
     return Joiner.on('-').skipNulls().join(nameParts);
   }
 
-  private String buildPlatformName() {
-    StringBuilder platformNameBuilder = new StringBuilder();
-    for (Fragment fragment : fragments.values()) {
-      platformNameBuilder.append(fragment.getPlatformName());
-    }
-    return platformNameBuilder.toString();
-  }
-
-  /**
-   * The platform string, suitable for use as a key into a MakeEnvironment.
-   *
-   * <p>Is only there for platform-dependent vardefs. Use
-   * {@link com.google.devtools.build.lib.rules.cpp.CcToolchainProvider#getToolchainIdentifier()
-   * instead.
-   */
-  @Deprecated
-  public String getPlatformName() {
-    return platformName;
-  }
-
   /** Returns the output directory for this build configuration. */
   public ArtifactRoot getOutputDirectory(RepositoryName repositoryName) {
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/MakeEnvironment.java b/src/main/java/com/google/devtools/build/lib/packages/MakeEnvironment.java
deleted file mode 100644
index a208e11..0000000
--- a/src/main/java/com/google/devtools/build/lib/packages/MakeEnvironment.java
+++ /dev/null
@@ -1,178 +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.packages;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import java.util.HashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import javax.annotation.Nullable;
-
-/**
- * Environment for varref variables (formerly called "Makefile
- * variables").
- *
- * <p><code>update</code> emulates a very restricted subset of the behaviour of
- * GNU Make's environment. In particular, does not attempt to simulate Make's
- * complex range of assigment operators.
- */
-@Immutable @ThreadSafe
-public class MakeEnvironment {
-  /**
-   *  The platform set regexp that matches all platforms.  Canonical.
-   */
-  public static final String MATCH_ANY = ".*";
-
-  // A platform-specific binding of a value for a given variable.
-  static class Binding {
-    private final String value;
-    private final String platformSetRegexp;
-
-    Binding(String value, String platformSetRegexp) {
-      this.value = value;
-      this.platformSetRegexp = platformSetRegexp;
-    }
-
-    @Override
-    public String toString() {
-      return value + " (" + platformSetRegexp + ")";
-    }
-
-    String getValue() {
-      return value;
-    }
-
-    String getPlatformSetRegexp() {
-      return platformSetRegexp;
-    }
-  }
-
-  // Maps each variable name to the [short] list of platform-specific bindings
-  // for it. The first matching binding is definitive.
-  private final ImmutableMap<String, ImmutableList<Binding>> env;
-
-  private MakeEnvironment(ImmutableMap<String, ImmutableList<Binding>> env) {
-    this.env = env;
-  }
-
-  /**
-   * @return the "Make" value from the environment whose name is "varname", or
-   *   null iff the variable is not defined in the environment.
-   */
-  public String lookup(String varname, String platform) {
-    List<Binding> bindings = env.get(varname);
-    if (bindings == null) {
-      return null;
-    }
-    // First, look for a matching non-default binding.
-    // (The order in 'bindings' is the reverse of the order of vardefs in the BUILD file, so
-    // the first match in this for loop selects the last matching definition in the BUILD file.)
-    for (Binding binding : bindings) {
-      if (!binding.platformSetRegexp.equals(MATCH_ANY) &&
-          platform.matches(binding.platformSetRegexp)) {
-        return binding.value;
-      }
-    }
-    // If we didn't find a matching non-default binding,
-    // try using the last default binding.
-    for (Binding binding : bindings) {
-      if (binding.platformSetRegexp.equals(MATCH_ANY)) {
-        return binding.value;
-      }
-    }
-    return null;
-  }
-
-  Map<String, ImmutableList<Binding>> getBindings() {
-    return env;
-  }
-
-  /**
-   * Interface for creating a MakeEnvironment, settings its environment values,
-   * and exposing it in immutable state.
-   */
-  public static class Builder {
-    private final Map<String, LinkedList<Binding>> env = new HashMap<>();
-    private Map<String, String> platformSets = ImmutableMap.<String, String>of("any", MATCH_ANY);
-
-    /**
-     * Performs an update of Makefile variable 'var' to value 'value' for all
-     * platforms belonging to the specified 'platformSetRegexp'. Corresponds to
-     * vardef. We explicitly do not support the various complex nuances of
-     * Make's assignment operator.
-     *
-     * <p>The most recent binding for a particular variable takes precedence, even if
-     * a more specific binding came earlier.
-     *
-     * @param varname the name of the Makefile variable;
-     * @param value the string value to assign;
-     * @param platformSetRegexp a set of platforms for which this variable definition
-     *        should take effect.  This is expressed as a regexp over gplatform
-     *        strings.
-     */
-    public void update(String varname, String value, String platformSetRegexp) {
-      if (varname == null || value == null || platformSetRegexp == null) {
-        throw new NullPointerException();
-      }
-      LinkedList<Binding> bindings = env.computeIfAbsent(varname, k -> new LinkedList<>());
-      // push new bindings onto head of list (=> most recent binding is
-      // definitive):
-      bindings.addFirst(new Binding(value, platformSetRegexp));
-    }
-
-    /**
-     * Sets the nickname to regexp mapping for <tt>vardef</tt>.
-     */
-    public void setPlatformSetRegexps(Map<String, String> sets) {
-      this.platformSets = sets;
-    }
-
-    @Nullable
-    public String getPlatformSetRegexp(String nickname) {
-      return this.platformSets.get(nickname);
-    }
-
-    /**
-     * Returns a new MakeEnvironment with environment settings corresponding
-     * to the cumulative results of this builder's {@link #update} calls.
-     */
-    public MakeEnvironment build() {
-      Map<String, ImmutableList<Binding>> newMap = new HashMap<>();
-      for (Map.Entry<String, LinkedList<Binding>> entry : env.entrySet()) {
-        newMap.put(entry.getKey(), ImmutableList.copyOf(entry.getValue()));
-      }
-      return new MakeEnvironment(ImmutableMap.copyOf(newMap));
-    }
-  }
-
-  @Override
-  public int hashCode() {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean equals(Object that) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public String toString() {
-    return "MakeEnvironment=" + env;
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 25c375e..ae191e8 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -56,6 +56,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.TreeMap;
 import javax.annotation.Nullable;
 
 /**
@@ -122,7 +123,7 @@
    * The "Make" environment of this package, containing package-local
    * definitions of "Make" variables.
    */
-  private MakeEnvironment makeEnv;
+  private ImmutableMap<String, String> makeEnv;
 
   /** The collection of all targets defined in this package, indexed by name. */
   protected ImmutableSortedKeyMap<String, Target> targets;
@@ -322,7 +323,7 @@
           "Invalid BUILD file name for package '" + packageIdentifier + "': " + filename);
     }
 
-    this.makeEnv = builder.makeEnv.build();
+    this.makeEnv = ImmutableMap.copyOf(builder.makeEnv);
     this.targets = ImmutableSortedKeyMap.copyOf(builder.targets);
     this.defaultVisibility = builder.defaultVisibility;
     this.defaultVisibilitySet = builder.defaultVisibilitySet;
@@ -389,33 +390,10 @@
   }
 
   /**
-   * Returns the "Make" value from the package's make environment whose name
-   * is "varname", or null iff the variable is not defined in the environment.
-   */
-  public String lookupMakeVariable(String varname, String platform) {
-    return makeEnv.lookup(varname, platform);
-  }
-
-  /**
-   * Returns the make environment. This should only ever be used for serialization -- how the
-   * make variables are implemented is an implementation detail.
-   */
-  MakeEnvironment getMakeEnvironment() {
-    return makeEnv;
-  }
-
-  /**
    * Returns all make variables for a given platform.
    */
-  public ImmutableMap<String, String> getAllMakeVariables(String platform) {
-    ImmutableMap.Builder<String, String> map = ImmutableMap.builder();
-    for (String var : makeEnv.getBindings().keySet()) {
-      String value = makeEnv.lookup(var, platform);
-      if (value != null) {
-        map.put(var, value);
-      }
-    }
-    return map.build();
+  public ImmutableMap<String, String> getMakeEnvironment() {
+    return makeEnv;
   }
 
   /**
@@ -708,7 +686,6 @@
     Builder b = new Builder(helper.createFreshPackage(
         Label.EXTERNAL_PACKAGE_IDENTIFIER, runfilesPrefix));
     b.setFilename(workspacePath);
-    b.setMakeEnv(new MakeEnvironment.Builder());
     return b;
   }
 
@@ -768,7 +745,9 @@
     private Path filename = null;
     private Label buildFileLabel = null;
     private InputFile buildFile = null;
-    private MakeEnvironment.Builder makeEnv = null;
+    // TreeMap so that the iteration order of variables is predictable. This is useful so that the
+    // serialized representation is deterministic.
+    private TreeMap<String, String> makeEnv = new TreeMap<>();
     private RuleVisibility defaultVisibility = ConstantRuleVisibility.PRIVATE;
     private boolean defaultVisibilitySet;
     private List<String> defaultCopts = null;
@@ -867,18 +846,11 @@
       return events;
     }
 
-    /**
-     * Sets this package's Make environment.
-     */
-    Builder setMakeEnv(MakeEnvironment.Builder makeEnv) {
-      this.makeEnv = makeEnv;
+    Builder setMakeVariable(String name, String value) {
+      this.makeEnv.put(name, value);
       return this;
     }
 
-    MakeEnvironment.Builder getMakeEnvironment() {
-      return makeEnv;
-    }
-
     /**
      * Sets the default visibility for this package. Called at most once per
      * package from PackageFactory.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 3a8a0d0..c2f4c74 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -334,7 +334,6 @@
   private AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls;
 
   private final ThreadPoolExecutor threadPool;
-  private Map<String, String> platformSetRegexps;
 
   private int maxDirectoriesToEagerlyVisitInGlobbing;
 
@@ -348,7 +347,6 @@
   public abstract static class BuilderForTesting {
     protected final String version = "test";
     protected Iterable<EnvironmentExtension> environmentExtensions = ImmutableList.of();
-    protected Map<String, String> platformSetRegexps = null;
     protected Function<RuleClass, AttributeContainer> attributeContainerFactory =
         AttributeContainer::new;
     protected boolean doChecksForTesting = true;
@@ -359,11 +357,6 @@
       return this;
     }
 
-    public BuilderForTesting setPlatformSetRegexps(Map<String, String> platformSetRegexps) {
-      this.platformSetRegexps = platformSetRegexps;
-      return this;
-    }
-
     public BuilderForTesting disableChecks() {
       this.doChecksForTesting = false;
       return this;
@@ -388,12 +381,10 @@
    */
   public PackageFactory(
       RuleClassProvider ruleClassProvider,
-      Map<String, String> platformSetRegexps,
       Function<RuleClass, AttributeContainer> attributeContainerFactory,
       Iterable<EnvironmentExtension> environmentExtensions,
       String version,
       Package.Builder.Helper packageBuilderHelper) {
-    this.platformSetRegexps = platformSetRegexps;
     this.ruleFactory = new RuleFactory(ruleClassProvider, attributeContainerFactory);
     this.ruleFunctions = buildRuleFunctions(ruleFactory);
     this.ruleClassProvider = ruleClassProvider;
@@ -1343,10 +1334,6 @@
       SkylarkSemantics skylarkSemantics,
       Globber globber)
       throws InterruptedException {
-    MakeEnvironment.Builder makeEnv = new MakeEnvironment.Builder();
-    if (platformSetRegexps != null) {
-      makeEnv.setPlatformSetRegexps(platformSetRegexps);
-    }
     try {
       // At this point the package is guaranteed to exist.  It may have parse or
       // evaluation errors, resulting in a diminished number of rules.
@@ -1361,7 +1348,6 @@
           defaultVisibility,
           skylarkSemantics,
           false /* containsError */,
-          makeEnv,
           imports,
           skylarkFileDependencies);
     } catch (InterruptedException e) {
@@ -1529,10 +1515,10 @@
     }
 
     /**
-     * Returns the MakeEnvironment Builder of this Package.
+     * Sets a Make variable.
      */
-    public MakeEnvironment.Builder getMakeEnvironment() {
-      return pkgBuilder.getMakeEnvironment();
+    public void setMakeVariable(String name, String value) {
+      pkgBuilder.setMakeVariable(name, value);
     }
 
     /**
@@ -1645,7 +1631,6 @@
       RuleVisibility defaultVisibility,
       SkylarkSemantics skylarkSemantics,
       boolean containsError,
-      MakeEnvironment.Builder pkgMakeEnv,
       Map<String, Extension> imports,
       ImmutableList<Label> skylarkFileDependencies)
       throws InterruptedException {
@@ -1665,7 +1650,6 @@
       SkylarkUtils.setToolsRepository(pkgEnv, ruleClassProvider.getToolsRepository());
 
       pkgBuilder.setFilename(buildFilePath)
-          .setMakeEnv(pkgMakeEnv)
           .setDefaultVisibility(defaultVisibility)
           // "defaultVisibility" comes from the command line. Let's give the BUILD file a chance to
           // set default_visibility once, be reseting the PackageBuilder.defaultVisibilitySet flag.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 8ff1f8c..85e8753 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -1319,11 +1319,6 @@
     return toolchainPrefix + lipoSuffix;
   }
 
-  @Override
-  public String getPlatformName() {
-    return getToolchainIdentifier();
-  }
-
   /**
    * Returns true if we should share identical native libraries between different targets.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 09467f1..71714c3 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -1293,7 +1293,6 @@
       PackageFactory packageFactory =
           new PackageFactory(
               ruleClassProvider,
-              ruleClassBuilder.getPlatformRegexps(),
               serverBuilder.getAttributeContainerFactory(),
               serverBuilder.getEnvironmentExtensions(),
               BlazeVersionInfo.instance().getVersion(),
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index 382010e..f5d3935 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -360,7 +360,6 @@
     PackageFactory pkgFactory =
         new PackageFactory(
             ruleClassProvider,
-            null,
             AttributeContainer::new,
             getEnvironmentExtensions(),
             getName(),