diff options
author | 2018-03-23 03:06:27 -0700 | |
---|---|---|
committer | 2018-03-23 03:07:25 -0700 | |
commit | 3ef288c0cf49783399c60ed9a33dcd98f3ae1210 (patch) | |
tree | f3e04fd8667549b7c27e4af2c041d79fd98e163b /src/main/java/com/google/devtools/build/lib | |
parent | 871d9f54dd45fd6383b60278788fb44bee7fd55c (diff) |
Victory lap: Remove all code that used to support the three-argument form of vardef().
RELNOTES: None.
PiperOrigin-RevId: 190196933
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
10 files changed, 14 insertions, 326 deletions
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 cb63624485..24ce6aa211 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.base.Preconditions; 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 @@ public class ConfigurationMakeVariableContext implements TemplateContext { .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 c349b80799..34836ab6bd 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.Collection; 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 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { 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 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { } /** - * 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 class ConfiguredRuleClassProvider implements RuleClassProvider { 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 273c167f6f..2d9426e441 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 @@ package com.google.devtools.build.lib.analysis; 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 @@ public interface MakeVariableSupplier { 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 4568a1c46d..54e7468ad5 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 @@ public class BuildConfiguration { } /** - * 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 @@ public class BuildConfiguration { private final Options options; private final String mnemonic; - private final String platformName; private final ImmutableMap<String, String> commandLineBuildVariables; @@ -1374,8 +1366,6 @@ public class BuildConfiguration { OutputDirectory.MIDDLEMAN.getRoot( RepositoryName.MAIN, outputDirName, directories, mainRepositoryName); - this.platformName = buildPlatformName(); - this.shellExecutable = computeShellExecutable(); this.actionEnv = setupActionEnvironment(); @@ -1506,26 +1496,6 @@ public class BuildConfiguration { 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 a208e11b38..0000000000 --- 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 25c375e6a0..ae191e8b32 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.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeMap; import javax.annotation.Nullable; /** @@ -122,7 +123,7 @@ public class Package { * 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 @@ public class Package { "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 @@ public class Package { } /** - * 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 @@ public class Package { 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 @@ public class Package { 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 @@ public class Package { 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 3a8a0d085c..c2f4c7436b 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 @@ public final class PackageFactory { private AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls; private final ThreadPoolExecutor threadPool; - private Map<String, String> platformSetRegexps; private int maxDirectoriesToEagerlyVisitInGlobbing; @@ -348,7 +347,6 @@ public final class PackageFactory { 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 @@ public final class PackageFactory { 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 final class PackageFactory { */ 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 @@ public final class PackageFactory { 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 @@ public final class PackageFactory { defaultVisibility, skylarkSemantics, false /* containsError */, - makeEnv, imports, skylarkFileDependencies); } catch (InterruptedException e) { @@ -1529,10 +1515,10 @@ public final class PackageFactory { } /** - * 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 @@ public final class PackageFactory { RuleVisibility defaultVisibility, SkylarkSemantics skylarkSemantics, boolean containsError, - MakeEnvironment.Builder pkgMakeEnv, Map<String, Extension> imports, ImmutableList<Label> skylarkFileDependencies) throws InterruptedException { @@ -1665,7 +1650,6 @@ public final class PackageFactory { 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 8ff1f8c065..85e875349b 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 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { 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 09467f1478..71714c3643 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 @@ public final class BlazeRuntime { 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 382010ee17..f5d393585d 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 @@ public abstract class AbstractPackageLoader implements PackageLoader { PackageFactory pkgFactory = new PackageFactory( ruleClassProvider, - null, AttributeContainer::new, getEnvironmentExtensions(), getName(), |