diff options
author | 2017-04-26 21:30:50 +0200 | |
---|---|---|
committer | 2017-04-27 11:17:55 +0200 | |
commit | ce33ab7a49126a513d7d5a6bc16f86154d9a85b6 (patch) | |
tree | fac0eff07298229cc7547122e92e80fd3715427e /src/main/java/com/google/devtools/build/lib | |
parent | 46da1fca1b3b0b4cd4eb28c4ec4f3cb1b5a22dd4 (diff) |
Give RuleContext the ability to add Make Variables
This CL also makes CcToolchain responsible for adding the sysroot to CC_FLAGS.
PiperOrigin-RevId: 154329746
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
8 files changed, 122 insertions, 47 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 aa795bbea2..4d3b3b6be0 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 @@ import java.util.Map; */ 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 @@ public class ConfigurationMakeVariableContext implements MakeVariableExpander.Co 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 3ebdd78f76..f47b38d006 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.SkylarkProviderIdentifier; 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; @@ -1015,6 +1016,30 @@ public final class RuleContext extends TargetContext } } + 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). * @@ -1022,8 +1047,8 @@ public final class RuleContext extends TargetContext **/ public ConfigurationMakeVariableContext getConfigurationMakeVariableContext() { if (configurationMakeVariableContext == null) { - configurationMakeVariableContext = new ConfigurationMakeVariableContext( - getRule().getPackage(), getConfiguration()); + configurationMakeVariableContext = + new ConfigurationMakeVariableContext(this, getRule().getPackage(), getConfiguration()); } return configurationMakeVariableContext; } @@ -1086,8 +1111,8 @@ public final class RuleContext extends TargetContext */ 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 1c56c20bfb..15d4998c7f 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 897d77ac01..7ab8097ed9 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 final class SkylarkRuleContext implements SkylarkValue { 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 c4d3abc469..cc01e6c7f4 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 @@ -39,6 +39,7 @@ 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.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; @@ -49,6 +50,7 @@ import com.google.devtools.build.skyframe.SkyFunction; 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; @@ -204,7 +206,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { coverage = crosstool; } - CcToolchainProvider provider = + CcToolchainProvider ccProvider = new CcToolchainProvider( cppConfiguration, crosstool, @@ -231,10 +233,19 @@ public class CcToolchain implements RuleConfiguredTargetFactory { ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST), getEnvironment(ruleContext), cppConfiguration.getBuiltInIncludeDirectories()); + + // TODO(kmensah): Remove sysroot from cppConfiguration and calculate it here. + 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)) .setFilesToBuild(new NestedSetBuilder<Artifact>(Order.STABLE_ORDER).build()) .addProvider(RunfilesProvider.simple(Runfiles.EMPTY)); @@ -354,6 +365,22 @@ public class CcToolchain implements RuleConfiguredTargetFactory { : 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 95efb13dd9..d07ec2aaf8 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 @@ -718,6 +718,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { 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; @@ -1655,7 +1656,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { * <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 ef7062303b..f0bc1c7679 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 @@ public final class ExtraActionFactory implements RuleConfiguredTargetFactory { 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 ca50836a59..49372a9c3a 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.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.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 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { 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 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { } } 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); |