diff options
7 files changed, 58 insertions, 125 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 8aaaaa44e0..9a114f544f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -960,6 +960,18 @@ public final class RuleContext extends TargetContext return result; } + public void initConfigurationMakeVariableContext( + Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) { + Preconditions.checkState(configurationMakeVariableContext == null); + configurationMakeVariableContext = + new ConfigurationMakeVariableContext( + this, getRule().getPackage(), getConfiguration(), makeVariableSuppliers); + } + + public void initConfigurationMakeVariableContext(MakeVariableSupplier... makeVariableSuppliers) { + initConfigurationMakeVariableContext(ImmutableList.copyOf(makeVariableSuppliers)); + } + /** Indicates whether a string list attribute should be tokenized. */ public enum Tokenize { YES, @@ -974,22 +986,7 @@ public final class RuleContext extends TargetContext * @return a list of strings containing the expanded and tokenized values for the attribute */ public ImmutableList<String> getTokenizedStringListAttr(String attributeName) { - return getExpandedStringListAttr( - attributeName, Tokenize.YES, getConfigurationMakeVariableContext()); - } - - /** - * Gets an attribute of type STRING_LIST expanding Make variables, $(location) tags into the - * dependency location (see {@link LocationExpander} for details) and tokenizes the result. - * - * @param attributeName the name of the attribute to process - * @param makeVariableContext the make variable context - * @return a list of strings containing the expanded and tokenized values for the attribute - */ - public ImmutableList<String> getTokenizedStringListAttr( - String attributeName, ConfigurationMakeVariableContext makeVariableContext) { - return getExpandedStringListAttr( - attributeName, Tokenize.YES, makeVariableContext); + return getExpandedStringListAttr(attributeName, Tokenize.YES); } /** @@ -1000,8 +997,7 @@ public final class RuleContext extends TargetContext * @return a list of strings containing the processed values for the attribute */ public ImmutableList<String> getExpandedStringListAttr(String attributeName) { - return getExpandedStringListAttr( - attributeName, Tokenize.NO, getConfigurationMakeVariableContext()); + return getExpandedStringListAttr(attributeName, Tokenize.NO); } /** @@ -1009,13 +1005,11 @@ public final class RuleContext extends TargetContext * optionally tokenizes the result. * * @param attributeName the name of the attribute to process - * @param makeVariableContext the make variable context * @return a list of strings containing the processed values for the attribute */ private ImmutableList<String> getExpandedStringListAttr( String attributeName, - Tokenize tokenize, - ConfigurationMakeVariableContext makeVariableContext) { + Tokenize tokenize) { if (!getRule().isAttrDefined(attributeName, Type.STRING_LIST)) { // TODO(bazel-team): This should be an error. return ImmutableList.of(); @@ -1029,27 +1023,21 @@ public final class RuleContext extends TargetContext new LocationExpander(this, LocationExpander.Options.ALLOW_DATA); for (String token : original) { - expandValue(tokens, attributeName, token, locationExpander, tokenize, makeVariableContext); + expandValue(tokens, attributeName, token, locationExpander, tokenize); } return ImmutableList.copyOf(tokens); } /** * Expands make variables in value and tokenizes the result into tokens. - * - * @param makeVariableContext the make variable context - * <p>This methods should be called only during initialization. */ public void tokenizeAndExpandMakeVars( List<String> result, String attributeName, - String value, - ConfigurationMakeVariableContext makeVariableContext) { + String value) { LocationExpander locationExpander = new LocationExpander(this, Options.ALLOW_DATA, Options.EXEC_PATHS); - expandValue( - result, attributeName, value, locationExpander, Tokenize.YES, - makeVariableContext); + expandValue(result, attributeName, value, locationExpander, Tokenize.YES); } /** @@ -1060,12 +1048,11 @@ public final class RuleContext extends TargetContext String attributeName, String value, @Nullable LocationExpander locationExpander, - Tokenize tokenize, - ConfigurationMakeVariableContext makeVariableContext) { + Tokenize tokenize) { if (locationExpander != null) { value = locationExpander.expandAttribute(attributeName, value); } - value = expandMakeVariables(attributeName, value, makeVariableContext); + value = expandMakeVariables(attributeName, value); if (tokenize == Tokenize.YES) { try { ShellUtils.tokenize(tokens, value); @@ -1098,33 +1085,12 @@ public final class RuleContext extends TargetContext } /** - * Returns a (cached! read on) context that maps Make variable names (string) to values (string) - * without any extra {@link MakeVariableSupplier}. - * - * <p>Beware!!! {@link ConfigurationMakeVariableContext} instance is cached, so if you call it - * first with some list of {@link MakeVariableSupplier} and then with other list, you will always - * get the first instance back. TODO(hlopko): Extract Make variable expansion from RuleContext and - * fix all the callers - * - * @return a ConfigurationMakeVariableContext. + * Returns a cached context that maps Make variable names (string) to values (string) without any + * extra {@link MakeVariableSupplier}. */ public ConfigurationMakeVariableContext getConfigurationMakeVariableContext() { - return getConfigurationMakeVariableContext(ImmutableList.<MakeVariableSupplier>of()); - } - - /** - * Returns a (cached! read on) context that maps Make variable names (string) to values (string). - * - * @see #getConfigurationMakeVariableContext() to understand how the instance is cached! - * @param makeVariableSuppliers to be used with {@link ConfigurationMakeVariableContext} - * @return a ConfigurationMakeVariableContext. - */ - public ConfigurationMakeVariableContext getConfigurationMakeVariableContext( - Iterable<? extends MakeVariableSupplier> makeVariableSuppliers) { if (configurationMakeVariableContext == null) { - configurationMakeVariableContext = - new ConfigurationMakeVariableContext( - this, getRule().getPackage(), getConfiguration(), makeVariableSuppliers); + initConfigurationMakeVariableContext(ImmutableList.<MakeVariableSupplier>of()); } return configurationMakeVariableContext; } @@ -1138,7 +1104,7 @@ public final class RuleContext extends TargetContext */ public String expandedMakeVariables(String attributeName) { String expression = attributes().get(attributeName, Type.STRING); - return expandMakeVariables(attributeName, expression, getConfigurationMakeVariableContext()); + return expandMakeVariables(attributeName, expression); } /** @@ -1181,8 +1147,7 @@ public final class RuleContext extends TargetContext public List<String> expandedMakeVariablesList(String attrName) { List<String> variables = new ArrayList<>(); for (String variable : attributes().get(attrName, Type.STRING_LIST)) { - variables.add( - expandMakeVariables(attrName, variable, getConfigurationMakeVariableContext())); + variables.add(expandMakeVariables(attrName, variable)); } return variables; } @@ -1194,16 +1159,13 @@ public final class RuleContext extends TargetContext * @param attrName the name of the attribute from which "expression" comes; used for error * reporting. * @param expression the string to expand. - * @param makeVariableContext the make variable context * @return the expansion of "expression", or null. */ @Nullable - public String expandSingleMakeVariable( - String attrName, - String expression, - ConfigurationMakeVariableContext makeVariableContext) { + public String expandSingleMakeVariable(String attrName, String expression) { try { - return MakeVariableExpander.expandSingleVariable(expression, makeVariableContext); + return MakeVariableExpander + .expandSingleVariable(expression, getConfigurationMakeVariableContext()); } catch (MakeVariableExpander.ExpansionException e) { attributeError(attrName, e.getMessage()); return expression; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index d8cc52f7c2..7f4c12d14a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -395,10 +395,7 @@ public final class RunfilesSupport { ruleContext, executable, runfiles, - computeArgs( - ruleContext, - CommandLine.EMPTY, - ruleContext.getConfigurationMakeVariableContext())); + computeArgs(ruleContext, CommandLine.EMPTY)); } /** @@ -411,10 +408,7 @@ public final class RunfilesSupport { ruleContext, executable, runfiles, - computeArgs( - ruleContext, - CommandLine.of(appendingArgs), - ruleContext.getConfigurationMakeVariableContext())); + computeArgs(ruleContext, CommandLine.of(appendingArgs))); } /** @@ -427,26 +421,13 @@ public final class RunfilesSupport { ruleContext, executable, runfiles, - computeArgs(ruleContext, appendingArgs, ruleContext.getConfigurationMakeVariableContext())); - } - - public static RunfilesSupport withExecutable( - RuleContext ruleContext, - Runfiles runfiles, - Artifact executable, - ConfigurationMakeVariableContext makeVariableContext) { - return new RunfilesSupport( - ruleContext, - executable, - runfiles, - computeArgs(ruleContext, CommandLine.EMPTY, makeVariableContext)); + computeArgs(ruleContext, appendingArgs)); } private static CommandLine computeArgs( RuleContext ruleContext, - CommandLine additionalArgs, - ConfigurationMakeVariableContext makeVariableContext) { + CommandLine additionalArgs) { return CommandLine.concat( - ruleContext.getTokenizedStringListAttr("args", makeVariableContext), additionalArgs); + ruleContext.getTokenizedStringListAttr("args"), additionalArgs); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index b341c4b825..742e5dd37c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.apple.ApplePlatform; +import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier; import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; @@ -166,6 +167,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleContext, boolean fake) throws InterruptedException, RuleErrorException { ruleContext.checkSrcsSamePackage(true); + ruleContext.initConfigurationMakeVariableContext(new CcFlagsSupplier(ruleContext)); + CcCommon common = new CcCommon(ruleContext); CcToolchainProvider ccToolchain = common.getToolchain(); FdoSupportProvider fdoSupport = common.getFdoSupport(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 602b5d4ed6..36278ef9c9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -150,8 +150,6 @@ public final class CcCommon { } public ImmutableList<String> getCopts() { - Preconditions.checkState(hasAttribute("copts", Type.STRING_LIST)); - if (!getCoptsFilter(ruleContext).apply("-Wno-future-warnings")) { ruleContext.attributeWarning( "nocopts", @@ -164,8 +162,8 @@ public final class CcCommon { } return ImmutableList.<String>builder() - .addAll(getPackageCopts(ruleContext)) - .addAll(CppHelper.getAttributeCopts(ruleContext, "copts")) + .addAll(CppHelper.getPackageCopts(ruleContext)) + .addAll(CppHelper.getAttributeCopts(ruleContext)) .build(); } @@ -336,11 +334,6 @@ public final class CcCommon { } } - private static ImmutableList<String> getPackageCopts(RuleContext ruleContext) { - List<String> unexpanded = ruleContext.getRule().getPackage().getDefaultCopts(); - return ImmutableList.copyOf(CppHelper.expandMakeVariables(ruleContext, "copts", unexpanded)); - } - /** Returns copts filter built from the make variable expanded nocopts attribute. */ Predicate<String> getCoptsFilter() { return getCoptsFilter(ruleContext); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index 0b4c2dfef2..96681370bd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; import com.google.devtools.build.lib.packages.RawAttributeMapper; +import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; @@ -104,6 +105,8 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { boolean linkStatic, boolean addDynamicRuntimeInputArtifactsToRunfiles) throws RuleErrorException, InterruptedException { + ruleContext.initConfigurationMakeVariableContext(new CcFlagsSupplier(ruleContext)); + final CcCommon common = new CcCommon(ruleContext); CcToolchainProvider ccToolchain = common.getToolchain(); FdoSupportProvider fdoSupport = common.getFdoSupport(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index feaad45b72..5bc125633e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -25,9 +25,7 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MiddlemanFactory; import com.google.devtools.build.lib.analysis.AnalysisUtils; -import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext; import com.google.devtools.build.lib.analysis.FileProvider; -import com.google.devtools.build.lib.analysis.MakeVariableSupplier; import com.google.devtools.build.lib.analysis.PlatformConfiguration; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; @@ -46,7 +44,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.RuleErrorConsumer; -import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier; import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Tool; @@ -149,32 +146,25 @@ public class CppHelper { * @return a list of strings containing the expanded and tokenized values for the * attribute */ - // TODO(bazel-team): Move to CcCommon; refactor CcPlugin to use either CcLibraryHelper or - // CcCommon. - static List<String> expandMakeVariables( + private static List<String> expandMakeVariables( RuleContext ruleContext, String attributeName, List<String> input) { boolean tokenization = !ruleContext.getFeatures().contains("no_copts_tokenization"); List<String> tokens = new ArrayList<>(); - ImmutableList<? extends MakeVariableSupplier> makeVariableSuppliers = - ImmutableList.of(new CcFlagsSupplier(ruleContext)); - ConfigurationMakeVariableContext makeVariableContext = - ruleContext.getConfigurationMakeVariableContext(makeVariableSuppliers); for (String token : input) { try { // Legacy behavior: tokenize all items. if (tokenization) { - ruleContext.tokenizeAndExpandMakeVars( - tokens, attributeName, token, makeVariableContext); + ruleContext.tokenizeAndExpandMakeVars(tokens, attributeName, token); } else { String exp = - ruleContext.expandSingleMakeVariable(attributeName, token, makeVariableContext); + ruleContext.expandSingleMakeVariable(attributeName, token); if (exp != null) { ShellUtils.tokenize(tokens, exp); } else { tokens.add( - ruleContext.expandMakeVariables(attributeName, token, makeVariableContext)); + ruleContext.expandMakeVariables(attributeName, token)); } } } catch (ShellUtils.TokenizationException e) { @@ -185,15 +175,22 @@ public class CppHelper { } /** - * Appends the tokenized values of the copts attribute to copts. + * Returns the tokenized values of the copts attribute to copts. */ - public static ImmutableList<String> getAttributeCopts(RuleContext ruleContext, String attr) { + // Called from CcCommon and CcSupport (Google's internal version of proto_library). + public static ImmutableList<String> getAttributeCopts(RuleContext ruleContext) { + String attr = "copts"; Preconditions.checkArgument(ruleContext.getRule().isAttrDefined(attr, Type.STRING_LIST)); List<String> unexpanded = ruleContext.attributes().get(attr, Type.STRING_LIST); - return ImmutableList.copyOf(expandMakeVariables(ruleContext, attr, unexpanded)); } + // Called from CcCommon. + static ImmutableList<String> getPackageCopts(RuleContext ruleContext) { + List<String> unexpanded = ruleContext.getRule().getPackage().getDefaultCopts(); + return ImmutableList.copyOf(expandMakeVariables(ruleContext, "copts", unexpanded)); + } + /** * Expands attribute value either using label expansion * (if attemptLabelExpansion == {@code true} and it does not look like make @@ -202,9 +199,6 @@ public class CppHelper { public static List<String> expandLinkopts( RuleContext ruleContext, String attrName, Iterable<String> values) { List<String> result = new ArrayList<>(); - ConfigurationMakeVariableContext makeVariableContext = - ruleContext.getConfigurationMakeVariableContext( - ImmutableList.of(new CcFlagsSupplier(ruleContext))); for (String value : values) { if (isLinkoptLabel(value)) { if (!expandLabel(ruleContext, result, value)) { @@ -215,8 +209,7 @@ public class CppHelper { .tokenizeAndExpandMakeVars( result, attrName, - value, - makeVariableContext); + value); } } return result; diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java index e1e779a13b..56c5185dc0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.rules.python; -import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; @@ -57,6 +56,7 @@ public abstract class PyBinary implements RuleConfiguredTargetFactory { static RuleConfiguredTargetBuilder init(RuleContext ruleContext, PythonSemantics semantics, PyCommon common) throws InterruptedException { + ruleContext.initConfigurationMakeVariableContext(new CcFlagsSupplier(ruleContext)); CcLinkParamsStore ccLinkParamsStore = initializeCcLinkParamStore(ruleContext); List<Artifact> srcs = common.validateSrcs(); @@ -91,9 +91,7 @@ public abstract class PyBinary implements RuleConfiguredTargetFactory { RunfilesSupport.withExecutable( ruleContext, defaultRunfiles, - common.getExecutable(), - ruleContext.getConfigurationMakeVariableContext( - ImmutableList.of(new CcFlagsSupplier(ruleContext)))); + common.getExecutable()); if (ruleContext.hasErrors()) { return null; |