Give RuleContext the ability to add make variables.
This CL also makes CcToolchain responsible for adding the sysroot to CC_FLAGS.
PiperOrigin-RevId: 155171725
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 aa795bb..4d3b3b6 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
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.MakeVariableExpander.ExpansionException;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -29,27 +30,35 @@
*/
public class ConfigurationMakeVariableContext implements MakeVariableExpander.Context {
private final Package pkg;
- private final Map<String, String> toolchainEnv;
- private final Map<String, String> commandLineEnv;
- private final Map<String, String> globalEnv;
+ private final ImmutableMap<String, String> ruleEnv;
+ private final ImmutableMap<String, String> commandLineEnv;
+ private final ImmutableMap<String, String> globalEnv;
private final String platform;
- public ConfigurationMakeVariableContext(Package pkg, BuildConfiguration configuration) {
- this(pkg, configuration, ImmutableMap.<String, String>of());
+ // TODO(b/37567440): Remove when Skylark callers can be updated to get this from
+ // CcToolchainProvider.
+ private static final ImmutableList<String> defaultMakeVariableAttributes =
+ ImmutableList.of(":cc_toolchain");
+
+ public ConfigurationMakeVariableContext(
+ RuleContext ruleContext, Package pkg, BuildConfiguration configuration) {
+ this(ruleContext.getMakeVariables(defaultMakeVariableAttributes), pkg, configuration);
}
- public ConfigurationMakeVariableContext(Package pkg, BuildConfiguration configuration,
- ImmutableMap<String, String> toolchainEnv) {
+ public ConfigurationMakeVariableContext(
+ ImmutableMap<String, String> ruleMakeVariables,
+ Package pkg,
+ BuildConfiguration configuration) {
this.pkg = pkg;
- this.toolchainEnv = toolchainEnv;
- commandLineEnv = configuration.getCommandLineBuildVariables();
- globalEnv = configuration.getGlobalMakeEnvironment();
- platform = configuration.getPlatformName();
+ this.ruleEnv = ruleMakeVariables;
+ this.commandLineEnv = ImmutableMap.copyOf(configuration.getCommandLineBuildVariables());
+ this.globalEnv = ImmutableMap.copyOf(configuration.getGlobalMakeEnvironment());
+ this.platform = configuration.getPlatformName();
}
@Override
public String lookupMakeVariable(String var) throws ExpansionException {
- String value = toolchainEnv.get(var);
+ String value = ruleEnv.get(var);
if (value == null) {
value = commandLineEnv.get(var);
}
@@ -73,6 +82,7 @@
map.putAll(pkg.getAllMakeVariables(platform));
map.putAll(globalEnv);
map.putAll(commandLineEnv);
+ map.putAll(ruleEnv);
return SkylarkDict.<String, String>copyOf(null, map);
}
}
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 13ed25c..a6c43d4 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
@@ -77,6 +77,7 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.AliasProvider;
+import com.google.devtools.build.lib.rules.MakeVariableProvider;
import com.google.devtools.build.lib.rules.fileset.FilesetProvider;
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.syntax.ClassObject;
@@ -1017,6 +1018,30 @@
}
}
+ public ImmutableMap<String, String> getMakeVariables(Iterable<String> attributeNames) {
+ // Using an ImmutableBuilder to complain about duplicate keys. This traversal order of
+ // getPrerequisites isn't well-defined, so this makes sure providers don't seceretly stomp on
+ // each other.
+ ImmutableMap.Builder<String, String> makeVariableBuilder = ImmutableMap.builder();
+ ImmutableSet.Builder<MakeVariableProvider> makeVariableProvidersBuilder =
+ ImmutableSet.builder();
+
+ for (String attributeName : attributeNames) {
+ // TODO(b/37567440): Remove this continue statement.
+ if (!attributes().has(attributeName)) {
+ continue;
+ }
+ makeVariableProvidersBuilder.addAll(
+ getPrerequisites(attributeName, Mode.TARGET, MakeVariableProvider.class));
+ }
+
+ for (MakeVariableProvider makeVariableProvider : makeVariableProvidersBuilder.build()) {
+ makeVariableBuilder.putAll(makeVariableProvider.getMakeVariables());
+ }
+
+ return makeVariableBuilder.build();
+ }
+
/**
* Return a context that maps Make variable names (string) to values (string).
*
@@ -1024,8 +1049,8 @@
**/
public ConfigurationMakeVariableContext getConfigurationMakeVariableContext() {
if (configurationMakeVariableContext == null) {
- configurationMakeVariableContext = new ConfigurationMakeVariableContext(
- getRule().getPackage(), getConfiguration());
+ configurationMakeVariableContext =
+ new ConfigurationMakeVariableContext(this, getRule().getPackage(), getConfiguration());
}
return configurationMakeVariableContext;
}
@@ -1088,8 +1113,8 @@
*/
public String expandSingleMakeVariable(String attrName, String expression) {
try {
- return MakeVariableExpander.expandSingleVariable(expression,
- new ConfigurationMakeVariableContext(getRule().getPackage(), getConfiguration()));
+ 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/rules/MakeVariableProvider.java b/src/main/java/com/google/devtools/build/lib/rules/MakeVariableProvider.java
index 1c56c20..15d4998 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/MakeVariableProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/MakeVariableProvider.java
@@ -15,34 +15,37 @@
package com.google.devtools.build.lib.rules;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
-import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import java.util.TreeMap;
+import com.google.devtools.build.lib.packages.ClassObjectConstructor;
+import com.google.devtools.build.lib.packages.NativeClassObjectConstructor;
+import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
/** Provides access to make variables from the current fragments. */
+@SkylarkModule(name = "MakeVariables", doc = "Make variables exposed by the current target.")
@Immutable
-public final class MakeVariableProvider implements TransitiveInfoProvider {
+public final class MakeVariableProvider extends SkylarkClassObject
+ implements TransitiveInfoProvider {
+ public static final String SKYLARK_NAME = "MakeVariableInfo";
+
+ public static final ClassObjectConstructor SKYLARK_CONSTRUCTOR =
+ new NativeClassObjectConstructor(SKYLARK_NAME) {};
+
private final ImmutableMap<String, String> makeVariables;
public MakeVariableProvider(ImmutableMap<String, String> makeVariables) {
+ super(SKYLARK_CONSTRUCTOR, ImmutableMap.<String, Object>of());
this.makeVariables = makeVariables;
}
+ @SkylarkCallable(
+ name = "make_variables",
+ doc = "Returns the make variables defined by this target.",
+ structField = true
+ )
public ImmutableMap<String, String> getMakeVariables() {
return makeVariables;
}
-
- public static ImmutableMap<String, String> getToolchainMakeVariables(
- RuleContext ruleContext, String attributeName) {
- // Cannot be an ImmutableMap.Builder because we want to support duplicate keys
- TreeMap<String, String> result = new TreeMap<>();
- for (MakeVariableProvider provider :
- ruleContext.getPrerequisites(attributeName, Mode.TARGET, MakeVariableProvider.class)) {
- result.putAll(provider.getMakeVariables());
- }
-
- return ImmutableMap.copyOf(result);
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
index 897d77a..7ab8097 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
@@ -986,9 +986,11 @@
public String expandMakeVariables(String attributeName, String command,
final Map<String, String> additionalSubstitutions) throws EvalException {
checkMutable("expand_make_variables");
- return ruleContext.expandMakeVariables(attributeName,
- command, new ConfigurationMakeVariableContext(ruleContext.getRule().getPackage(),
- ruleContext.getConfiguration()) {
+ return ruleContext.expandMakeVariables(
+ attributeName,
+ command,
+ new ConfigurationMakeVariableContext(
+ ruleContext, ruleContext.getRule().getPackage(), ruleContext.getConfiguration()) {
@Override
public String lookupMakeVariable(String name) throws ExpansionException {
if (additionalSubstitutions.containsKey(name)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
index 69924f7..6fc4c92 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
@@ -41,6 +41,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.License;
+import com.google.devtools.build.lib.rules.MakeVariableProvider;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException;
import com.google.devtools.build.lib.util.Pair;
@@ -52,6 +53,7 @@
import com.google.devtools.build.skyframe.SkyKey;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -282,7 +284,7 @@
coverage = crosstool;
}
- CcToolchainProvider provider =
+ CcToolchainProvider ccProvider =
new CcToolchainProvider(
cppConfiguration,
crosstool,
@@ -309,10 +311,18 @@
ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST),
getEnvironment(ruleContext),
cppConfiguration.getBuiltInIncludeDirectories());
+
+ PathFragment sysroot = cppConfiguration.getSysroot();
+
+ MakeVariableProvider makeVariableProvider =
+ createMakeVariableProvider(cppConfiguration, sysroot);
+
RuleConfiguredTargetBuilder builder =
new RuleConfiguredTargetBuilder(ruleContext)
- .addProvider(provider)
- .addNativeDeclaredProvider(provider)
+ .addProvider(ccProvider)
+ .addNativeDeclaredProvider(ccProvider)
+ .addProvider(makeVariableProvider)
+ .addNativeDeclaredProvider(makeVariableProvider)
.addProvider(
fdoSupport.getFdoSupport().createFdoSupportProvider(ruleContext, profileArtifact))
.setFilesToBuild(new NestedSetBuilder<Artifact>(Order.STABLE_ORDER).build())
@@ -433,6 +443,22 @@
: NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
}
+ private MakeVariableProvider createMakeVariableProvider(
+ CppConfiguration cppConfiguration, PathFragment sysroot) {
+
+ HashMap<String, String> makeVariables =
+ new HashMap<>(cppConfiguration.getAdditionalMakeVariables());
+
+ // Overwrite the CC_FLAGS variable to include sysroot, if it's available.
+ if (sysroot != null) {
+ String sysrootFlag = "--sysroot=" + sysroot;
+ String ccFlags = makeVariables.get("CC_FLAGS");
+ ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag;
+ makeVariables.put("CC_FLAGS", ccFlags);
+ }
+ return new MakeVariableProvider(ImmutableMap.copyOf(makeVariables));
+ }
+
/**
* Returns a map that should be templated into the crosstool as build variables. Is meant to
* be overridden by subclasses of CcToolchain.
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 f9ddfba..b911026 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
@@ -727,6 +727,7 @@
for (CrosstoolConfig.MakeVariable variable : toolchain.getMakeVariableList()) {
makeVariablesBuilder.put(variable.getName(), variable.getValue());
}
+ // TODO(kmensah): Remove once targets can depend on the cc_toolchain in skylark.
if (sysrootFlag != null) {
String ccFlags = makeVariablesBuilder.get("CC_FLAGS");
ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag;
@@ -1664,7 +1665,6 @@
* <p>The returned map must contain an entry for {@code STACK_FRAME_UNLIMITED},
* though the entry may be an empty string.
*/
- @VisibleForTesting
public ImmutableMap<String, String> getAdditionalMakeVariables() {
return additionalMakeVariables;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
index ef70623..f0bc1c7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java
@@ -64,9 +64,11 @@
command = command.replace("$(OWNER_LABEL_DIGEST)", "$$(OWNER_LABEL_DIGEST)");
command = command.replace("$(output ", "$$(output ");
try {
- command = MakeVariableExpander.expand(
- command, new ConfigurationMakeVariableContext(
- context.getTarget().getPackage(), context.getConfiguration()));
+ command =
+ MakeVariableExpander.expand(
+ command,
+ new ConfigurationMakeVariableContext(
+ context, context.getTarget().getPackage(), context.getConfiguration()));
} catch (MakeVariableExpander.ExpansionException e) {
context.ruleError(String.format("Unable to expand make variables: %s",
e.getMessage()));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
index ca50836..49372a9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
@@ -39,7 +39,6 @@
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.AliasProvider;
-import com.google.devtools.build.lib.rules.MakeVariableProvider;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.rules.cpp.CppHelper;
import com.google.devtools.build.lib.rules.java.JavaHelper;
@@ -290,12 +289,17 @@
private final NestedSet<Artifact> resolvedSrcs;
private final NestedSet<Artifact> filesToBuild;
- public CommandResolverContext(RuleContext ruleContext, NestedSet<Artifact> resolvedSrcs,
+ private static final ImmutableList<String> makeVariableAttributes =
+ ImmutableList.of(":cc_toolchain", "toolchains");
+
+ public CommandResolverContext(
+ RuleContext ruleContext,
+ NestedSet<Artifact> resolvedSrcs,
NestedSet<Artifact> filesToBuild) {
super(
+ ruleContext.getMakeVariables(makeVariableAttributes),
ruleContext.getRule().getPackage(),
- ruleContext.getConfiguration(),
- MakeVariableProvider.getToolchainMakeVariables(ruleContext, "toolchains"));
+ ruleContext.getConfiguration());
this.ruleContext = ruleContext;
this.resolvedSrcs = resolvedSrcs;
this.filesToBuild = filesToBuild;
@@ -344,7 +348,9 @@
}
} else if (JDK_MAKE_VARIABLE.matcher("$(" + name + ")").find()) {
return new ConfigurationMakeVariableContext(
- ruleContext.getTarget().getPackage(), ruleContext.getHostConfiguration())
+ ruleContext.getMakeVariables(makeVariableAttributes),
+ ruleContext.getTarget().getPackage(),
+ ruleContext.getHostConfiguration())
.lookupMakeVariable(name);
} else {
return super.lookupMakeVariable(name);
