From 8f08c63365d1e72d5f70285294d4df774d693a68 Mon Sep 17 00:00:00 2001 From: Lukacs Berki Date: Tue, 2 Aug 2016 10:04:22 +0000 Subject: Move the --define command line option from CppConfiguration to BuildConfiguration. It makes much more sense there because it does *not* specify C++ #defines but BUILD variables. Also rename the getter method to make that clearer. This allows us to remove BuildConfiguration.Fragment#getCommandLineDefines(). -- MOS_MIGRATED_REVID=129080014 --- .../analysis/ConfigurationMakeVariableContext.java | 3 +- .../lib/analysis/config/BuildConfiguration.java | 42 ++++++++++++---------- .../build/lib/rules/cpp/CppConfiguration.java | 26 -------------- .../devtools/build/lib/rules/cpp/CppOptions.java | 14 -------- 4 files changed, 25 insertions(+), 60 deletions(-) (limited to 'src/main') 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 5629c83a60..21fbab7537 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.devtools.build.lib.analysis.MakeVariableExpander.ExpansionExce import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.syntax.SkylarkDict; - import java.util.LinkedHashMap; import java.util.Map; @@ -35,7 +34,7 @@ public class ConfigurationMakeVariableContext implements MakeVariableExpander.Co public ConfigurationMakeVariableContext(Package pkg, BuildConfiguration configuration) { this.pkg = pkg; - commandLineEnv = configuration.getCommandLineDefines(); + commandLineEnv = configuration.getCommandLineBuildVariables(); globalEnv = configuration.getGlobalMakeEnvironment(); platform = configuration.getPlatformName(); } 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 c7a068b5a1..277552a353 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 @@ -68,7 +68,6 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParsingException; import com.google.devtools.common.options.TriState; - import java.io.Serializable; import java.lang.reflect.Field; import java.util.ArrayList; @@ -85,7 +84,6 @@ import java.util.Objects; import java.util.Queue; import java.util.Set; import java.util.TreeMap; - import javax.annotation.Nullable; /** @@ -165,13 +163,6 @@ public final class BuildConfiguration { return implicitLabels; } - /* - * Returns the command-line "Make" variable overrides. - */ - public ImmutableMap getCommandLineDefines() { - return ImmutableMap.of(); - } - /** * Returns a fragment of the output directory name for this configuration. The output * directory for the whole configuration contains all the short names by all fragments. @@ -451,6 +442,16 @@ public final class BuildConfiguration { return cpu; } + @Option( + name = "define", + converter = Converters.AssignmentConverter.class, + defaultValue = "", + category = "semantics", + allowMultiple = true, + help = "Each --define option specifies an assignment for a build variable." + ) + public List> commandLineBuildVariables; + @Option(name = "cpu", defaultValue = "null", category = "semantics", @@ -862,6 +863,7 @@ public final class BuildConfiguration { host.compilationMode = CompilationMode.OPT; host.isHost = true; host.useDynamicConfigurations = useDynamicConfigurations; + host.commandLineBuildVariables = commandLineBuildVariables; host.enforceConstraints = enforceConstraints; if (fallback) { @@ -1041,6 +1043,7 @@ public final class BuildConfiguration { private final String platformName; private final ImmutableMap testEnvironment; + private final ImmutableMap commandLineBuildVariables; /** * Helper container for {@link #transitiveOptionsMap} below. @@ -1203,6 +1206,15 @@ public final class BuildConfiguration { this.testEnvironment = ImmutableMap.copyOf(testEnv); + // We can't use an ImmutableMap.Builder here; we need the ability to add entries with keys that + // are already in the map so that the same define can be specified on the command line twice, + // and ImmutableMap.Builder does not support that. + Map commandLineDefinesBuilder = new TreeMap<>(); + for (Map.Entry define : options.commandLineBuildVariables) { + commandLineDefinesBuilder.put(define.getKey(), define.getValue()); + } + commandLineBuildVariables = ImmutableMap.copyOf(commandLineDefinesBuilder); + this.mnemonic = buildMnemonic(); String outputDirName = (options.outputDirectoryName != null) ? options.outputDirectoryName : mnemonic; @@ -2057,9 +2069,7 @@ public final class BuildConfiguration { public Map getMakeEnvironment() { Map makeEnvironment = new HashMap<>(); makeEnvironment.putAll(globalMakeEnv); - for (Fragment fragment : fragments.values()) { - makeEnvironment.putAll(fragment.getCommandLineDefines()); - } + makeEnvironment.putAll(commandLineBuildVariables); return ImmutableMap.copyOf(makeEnvironment); } @@ -2068,12 +2078,8 @@ public final class BuildConfiguration { * (Fragments, in particular the Google C++ support, can set variables through the * command line.) */ - public Map getCommandLineDefines() { - ImmutableMap.Builder builder = ImmutableMap.builder(); - for (Fragment fragment : fragments.values()) { - builder.putAll(fragment.getCommandLineDefines()); - } - return builder.build(); + public Map getCommandLineBuildVariables() { + return commandLineBuildVariables; } /** 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 1e14df9dc6..a02c7a92e5 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 @@ -331,7 +331,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // The dynamic mode for linking. private final DynamicMode dynamicMode; private final boolean stripBinaries; - private final ImmutableMap commandLineDefines; private final String solibDirectory; private final CompilationMode compilationMode; @@ -457,15 +456,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } } - // We can't use an ImmutableMap.Builder here; we need the ability (at least - // in tests) to add entries with keys that are already in the map, and only - // HashMap supports this (by replacing the existing entry under the key). - Map commandLineDefinesBuilder = new HashMap<>(); - for (Map.Entry define : cppOptions.commandLineDefinedVariables) { - commandLineDefinesBuilder.put(define.getKey(), define.getValue()); - } - commandLineDefines = ImmutableMap.copyOf(commandLineDefinesBuilder); - ListMultimap cFlags = ArrayListMultimap.create(); ListMultimap cxxFlags = ArrayListMultimap.create(); linkOptionsFromCompilationMode = ArrayListMultimap.create(); @@ -1576,22 +1566,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return cppOptions.stl; } - /* - * Returns the command-line "Make" variable overrides. - */ - @Override - public ImmutableMap getCommandLineDefines() { - return commandLineDefines; - } - - /** - * Returns the command-line override value for the specified "Make" variable - * for this configuration, or null if none. - */ - public String getMakeVariableOverride(String var) { - return commandLineDefines.get(var); - } - /** * Returns the currently active LIPO compilation mode. */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index 2195e3ff2d..47cd385670 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -31,11 +31,9 @@ import com.google.devtools.build.lib.util.OptionsUtils; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import com.google.devtools.common.options.Converter; -import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.EnumConverter; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionsParsingException; - import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -496,16 +494,6 @@ public class CppOptions extends FragmentOptions { ) public List hostCoptList; - @Option( - name = "define", - converter = Converters.AssignmentConverter.class, - defaultValue = "", - category = "semantics", - allowMultiple = true, - help = "Each --define option specifies an assignment for a build variable." - ) - public List> commandLineDefinedVariables; - @Option( name = "grte_top", defaultValue = "null", // The default value is chosen by the toolchain. @@ -572,8 +560,6 @@ public class CppOptions extends FragmentOptions { public FragmentOptions getHost(boolean fallback) { CppOptions host = (CppOptions) getDefault(); - host.commandLineDefinedVariables = commandLineDefinedVariables; - // The crosstool options are partially copied from the target configuration. if (!fallback) { if (hostCrosstoolTop == null) { -- cgit v1.2.3