Skip to content

Commit

Permalink
Pass cc_toolchain location to FeatureConfiguration
Browse files Browse the repository at this point in the history
Paths to tools in CROSSTOOL are either absolute or relative to the CROSSTOOL location (which is the same as cc_toolchain location). As in the future CROSSTOOL will be gone, and the new skylark rule that will replace CROSSTOOL will not have to be in the same location as cc_toolchain, we need to pass information to FeatureConfiguration about the location of the cc_toolchain in use, so we can calculate the workspace relative paths to the tools.

RELNOTES: None.
PiperOrigin-RevId: 207695703
  • Loading branch information
scentini authored and Copybara-Service committed Aug 7, 2018
1 parent 6eefeb0 commit 473ea10
Show file tree
Hide file tree
Showing 15 changed files with 92 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,7 @@ public FeatureConfiguration configureFeatures(

@Override
public String getToolForAction(FeatureConfiguration featureConfiguration, String actionName) {
return featureConfiguration
.getToolForAction(actionName)
.getToolPathFragment()
.getSafePathString();
return featureConfiguration.getToolPathForAction(actionName);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -659,9 +659,7 @@ private CppToolchainInfo getCppToolchainInfo(
toolchain, cppConfiguration.getCrosstoolTopPathFragment());
CcToolchainConfigInfo ccToolchainConfigInfo =
CcToolchainConfigInfo.fromToolchain(
cppConfiguration.getCrosstoolFile().getProto(),
toolchain,
cppConfiguration.getCrosstoolTopPathFragment());
cppConfiguration.getCrosstoolFile().getProto(), toolchain);
return CppToolchainInfo.create(
cppConfiguration.getCrosstoolTopPathFragment(),
cppConfiguration.getCcToolchainRuleLabel(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Feature;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CompilationModeFlags;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CrosstoolRelease;
Expand Down Expand Up @@ -188,13 +187,12 @@ protected CcToolchainConfigInfo(
this.hasDynamicLinkingModeFlags = hasDynamicLinkingModeFlags;
}

public static CcToolchainConfigInfo fromToolchain(
CrosstoolRelease file, CToolchain toolchain, PathFragment crosstoolTop)
public static CcToolchainConfigInfo fromToolchain(CrosstoolRelease file, CToolchain toolchain)
throws InvalidConfigurationException {

ImmutableList.Builder<ActionConfig> actionConfigBuilder = ImmutableList.builder();
for (CToolchain.ActionConfig actionConfig : toolchain.getActionConfigList()) {
actionConfigBuilder.add(new ActionConfig(actionConfig, crosstoolTop));
actionConfigBuilder.add(new ActionConfig(actionConfig));
}

ImmutableList.Builder<Feature> featureBuilder = ImmutableList.builder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -832,10 +832,9 @@ public static class Tool {

private Tool(
CToolchain.Tool tool,
PathFragment crosstoolTop,
ImmutableSet<WithFeatureSet> withFeatureSetSets) {
this.withFeatureSetSets = withFeatureSetSets;
toolPathFragment = crosstoolTop.getRelative(tool.getToolPath());
this.toolPathFragment = PathFragment.create(tool.getToolPath());
executionRequirements = ImmutableSet.copyOf(tool.getExecutionRequirementList());
}

Expand All @@ -850,8 +849,8 @@ public Tool(
}

/** Returns the path to this action's tool relative to the provided crosstool path. */
public PathFragment getToolPathFragment() {
return toolPathFragment;
public String getToolPathString(PathFragment ccToolchainPath) {
return ccToolchainPath.getRelative(toolPathFragment).getSafePathString();
}

/**
Expand Down Expand Up @@ -907,8 +906,7 @@ static class ActionConfig implements Serializable, CrosstoolSelectable {
private final boolean enabled;
private final ImmutableList<String> implies;

ActionConfig(CToolchain.ActionConfig actionConfig, PathFragment crosstoolTop)
throws InvalidConfigurationException {
ActionConfig(CToolchain.ActionConfig actionConfig) throws InvalidConfigurationException {
this.configName = actionConfig.getConfigName();
this.actionName = actionConfig.getActionName();
this.tools =
Expand All @@ -919,7 +917,6 @@ static class ActionConfig implements Serializable, CrosstoolSelectable {
t ->
new Tool(
t,
crosstoolTop,
t.getWithFeatureList()
.stream()
.map(f -> new WithFeatureSet(f))
Expand Down Expand Up @@ -1117,6 +1114,8 @@ public static class FeatureConfiguration implements FeatureConfigurationApi {

private final ImmutableMap<String, ActionConfig> actionConfigByActionName;

private final PathFragment ccToolchainPath;

/**
* {@link FeatureConfiguration} instance that doesn't produce any command lines. This is to be
* used when creation of the real {@link FeatureConfiguration} failed, the rule error was
Expand All @@ -1126,23 +1125,32 @@ public static class FeatureConfiguration implements FeatureConfigurationApi {
FEATURE_CONFIGURATION_INTERNER.intern(new FeatureConfiguration());

protected FeatureConfiguration() {
this(ImmutableList.of(), ImmutableSet.of(), ImmutableMap.of());
this(
/* enabledFeatures= */ ImmutableList.of(),
/* enabledActionConfigActionNames= */ ImmutableSet.of(),
/* actionConfigByActionName= */ ImmutableMap.of(),
/* ccToolchainPath= */ PathFragment.EMPTY_FRAGMENT);
}

@AutoCodec.Instantiator
static FeatureConfiguration createForSerialization(
ImmutableList<Feature> enabledFeatures,
ImmutableSet<String> enabledActionConfigActionNames,
ImmutableMap<String, ActionConfig> actionConfigByActionName) {
ImmutableMap<String, ActionConfig> actionConfigByActionName,
PathFragment ccToolchainPath) {
return FEATURE_CONFIGURATION_INTERNER.intern(
new FeatureConfiguration(
enabledFeatures, enabledActionConfigActionNames, actionConfigByActionName));
enabledFeatures,
enabledActionConfigActionNames,
actionConfigByActionName,
ccToolchainPath));
}

FeatureConfiguration(
ImmutableList<Feature> enabledFeatures,
ImmutableSet<String> enabledActionConfigActionNames,
ImmutableMap<String, ActionConfig> actionConfigByActionName) {
ImmutableMap<String, ActionConfig> actionConfigByActionName,
PathFragment ccToolchainPath) {
this.enabledFeatures = enabledFeatures;

this.actionConfigByActionName = actionConfigByActionName;
Expand All @@ -1152,6 +1160,7 @@ static FeatureConfiguration createForSerialization(
}
this.enabledFeatureNames = featureBuilder.build();
this.enabledActionConfigActionNames = enabledActionConfigActionNames;
this.ccToolchainPath = ccToolchainPath;
}

/**
Expand Down Expand Up @@ -1228,14 +1237,22 @@ public ImmutableMap<String, String> getEnvironmentVariables(
return envBuilder.build();
}

/** Returns a given action's tool under this FeatureConfiguration. */
public Tool getToolForAction(String actionName) {
String getToolPathForAction(String actionName) {
Preconditions.checkArgument(
actionConfigByActionName.containsKey(actionName),
"Action %s does not have an enabled configuration in the toolchain.",
actionName);
ActionConfig actionConfig = actionConfigByActionName.get(actionName);
return actionConfig.getTool(enabledFeatureNames).getToolPathString(ccToolchainPath);
}

ImmutableSet<String> getToolRequirementsForAction(String actionName) {
Preconditions.checkArgument(
actionConfigByActionName.containsKey(actionName),
"Action %s does not have an enabled configuration in the toolchain.",
actionName);
ActionConfig actionConfig = actionConfigByActionName.get(actionName);
return actionConfig.getTool(enabledFeatureNames);
return actionConfig.getTool(enabledFeatureNames).getExecutionRequirements();
}

@Override
Expand Down Expand Up @@ -1335,14 +1352,18 @@ public ImmutableSet<String> getEnabledFeatureNames() {
private transient LoadingCache<ImmutableSet<String>, FeatureConfiguration> configurationCache =
buildConfigurationCache();

private PathFragment ccToolchainPath;

/**
* Constructs the feature configuration from a {@code crosstoolInfo}.
* Constructs the feature configuration from a {@link CcToolchainConfigInfo}.
*
* @param ccToolchainConfigInfo the toolchain information as specified by the user.
* @param ccToolchainPath location of the cc_toolchain.
* @throws InvalidConfigurationException if the configuration has logical errors.
*/
@VisibleForTesting
public CcToolchainFeatures(CcToolchainConfigInfo ccToolchainConfigInfo)
public CcToolchainFeatures(
CcToolchainConfigInfo ccToolchainConfigInfo, PathFragment ccToolchainPath)
throws InvalidConfigurationException {
// Build up the feature/action config graph. We refer to features/action configs as
// 'selectables'.
Expand Down Expand Up @@ -1432,6 +1453,7 @@ public CcToolchainFeatures(CcToolchainConfigInfo ccToolchainConfigInfo)
this.provides = provides.build().inverse();
this.impliedBy = impliedBy.build();
this.requiredBy = requiredBy.build();
this.ccToolchainPath = ccToolchainPath;
}

private static void checkForActivatableDups(Iterable<CrosstoolSelectable> selectables)
Expand Down Expand Up @@ -1529,7 +1551,8 @@ public FeatureConfiguration computeFeatureConfiguration(ImmutableSet<String> req
impliedBy,
requires,
requiredBy,
actionConfigsByActionName)
actionConfigsByActionName,
ccToolchainPath)
.run();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public String getToolPath() {
featureConfiguration.actionIsConfigured(actionName),
"Expected action_config for '%s' to be configured",
actionName);
return featureConfiguration.getToolForAction(actionName).getToolPathFragment().getPathString();
return featureConfiguration.getToolPathForAction(actionName);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public CppCompileAction buildAndVerify(Consumer<String> errorCollector) {

if (featureConfiguration.actionIsConfigured(getActionName())) {
for (String executionRequirement :
featureConfiguration.getToolForAction(getActionName()).getExecutionRequirements()) {
featureConfiguration.getToolRequirementsForAction(getActionName())) {
executionInfo.put(executionRequirement, "");
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,7 @@ protected CppConfigurationParameters createParameters(
toolchain, crosstoolTopLabel.getPackageIdentifier().getPathUnderExecRoot());

CcToolchainConfigInfo ccToolchainConfigInfo =
CcToolchainConfigInfo.fromToolchain(
file.getProto(),
toolchain,
crosstoolTopLabel.getPackageIdentifier().getPathUnderExecRoot());
CcToolchainConfigInfo.fromToolchain(file.getProto(), toolchain);

Label sysrootLabel = getSysrootLabel(toolchain, cppOptions.libcTopLabel);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
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;
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.syntax.Type;
Expand Down Expand Up @@ -669,8 +668,6 @@ public static void createStripAction(
return;
}

Tool stripTool =
Preconditions.checkNotNull(featureConfiguration.getToolForAction(CppActionNames.STRIP));
CcToolchainVariables variables =
new CcToolchainVariables.Builder(toolchain.getBuildVariables())
.addStringVariable(
Expand All @@ -682,7 +679,8 @@ public static void createStripAction(
ImmutableList<String> commandLine =
ImmutableList.copyOf(featureConfiguration.getCommandLine(CppActionNames.STRIP, variables));
ImmutableMap.Builder<String, String> executionInfoBuilder = ImmutableMap.builder();
for (String executionRequirement : stripTool.getExecutionRequirements()) {
for (String executionRequirement :
featureConfiguration.getToolRequirementsForAction(CppActionNames.STRIP)) {
executionInfoBuilder.put(executionRequirement, "");
}
Action[] stripAction =
Expand All @@ -691,7 +689,9 @@ public static void createStripAction(
.addTransitiveInputs(toolchain.getStrip())
.addOutput(output)
.useDefaultShellEnvironment()
.setExecutable(stripTool.getToolPathFragment())
.setExecutable(
PathFragment.create(
featureConfiguration.getToolPathForAction(CppActionNames.STRIP)))
.setExecutionInfo(executionInfoBuilder.build())
.setProgressMessage("Stripping %s for %s", output.prettyPrint(), context.getLabel())
.setMnemonic("CcStrip")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,7 @@ public CppLinkAction build() throws InterruptedException {
ImmutableSet.Builder<String> executionRequirements = ImmutableSet.builder();
if (featureConfiguration.actionIsConfigured(getActionName())) {
executionRequirements.addAll(
featureConfiguration.getToolForAction(getActionName()).getExecutionRequirements());
featureConfiguration.getToolRequirementsForAction(getActionName()));
}

if (!isLtoIndexing) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,8 @@ public static CppToolchainInfo create(
this.crosstoolTopPathFragment = crosstoolTopPathFragment;
this.toolchainIdentifier = toolchainIdentifier;
// Since this field can be derived from `crosstoolInfo`, it is re-derived instead of serialized.
this.toolchainFeatures = new CcToolchainFeatures(ccToolchainConfigInfo);
this.toolchainFeatures =
new CcToolchainFeatures(ccToolchainConfigInfo, crosstoolTopPathFragment);
this.toolPaths = toolPaths;
this.compiler = compiler;
this.abiGlibcVersion = abiGlibcVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.CrosstoolSelectable;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Feature;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -101,6 +102,9 @@ class FeatureSelection {
*/
private final ImmutableMultimap<CrosstoolSelectable, CrosstoolSelectable> requiredBy;

/** Location of the cc_toolchain in use. */
private final PathFragment ccToolchainPath;

FeatureSelection(
ImmutableSet<String> requestedFeatures,
ImmutableMap<String, CrosstoolSelectable> selectablesByName,
Expand All @@ -110,7 +114,8 @@ class FeatureSelection {
ImmutableMultimap<CrosstoolSelectable, CrosstoolSelectable> impliedBy,
ImmutableMultimap<CrosstoolSelectable, ImmutableSet<CrosstoolSelectable>> requires,
ImmutableMultimap<CrosstoolSelectable, CrosstoolSelectable> requiredBy,
ImmutableMap<String, ActionConfig> actionConfigsByActionName) {
ImmutableMap<String, ActionConfig> actionConfigsByActionName,
PathFragment ccToolchainPath) {
ImmutableSet.Builder<CrosstoolSelectable> builder = ImmutableSet.builder();
for (String name : requestedFeatures) {
if (selectablesByName.containsKey(name)) {
Expand All @@ -125,6 +130,7 @@ class FeatureSelection {
this.requires = requires;
this.requiredBy = requiredBy;
this.actionConfigsByActionName = actionConfigsByActionName;
this.ccToolchainPath = ccToolchainPath;
}

/**
Expand Down Expand Up @@ -179,7 +185,10 @@ FeatureConfiguration run() throws CollidingProvidesException {
}

return new FeatureConfiguration(
enabledFeaturesInOrder, enabledActionConfigNames.build(), actionConfigsByActionName);
enabledFeaturesInOrder,
enabledActionConfigNames.build(),
actionConfigsByActionName,
ccToolchainPath);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ public ImmutableList<String> getLinkopts() {

/** Returns the path to the linker. */
public String getLinkerPathString() {
return featureConfiguration
.getToolForAction(linkTargetType.getActionName())
.getToolPathFragment()
.getPathString();
return featureConfiguration.getToolPathForAction(linkTargetType.getActionName());
}

/**
Expand Down Expand Up @@ -377,11 +374,7 @@ private static List<String> getRawLinkArgv(
Preconditions.checkArgument(
featureConfiguration.actionIsConfigured(actionName),
String.format("Expected action_config for '%s' to be configured", actionName));
argv.add(
featureConfiguration
.getToolForAction(linkTargetType.getActionName())
.getToolPathFragment()
.getPathString());
argv.add(featureConfiguration.getToolPathForAction(linkTargetType.getActionName()));
}
argv.addAll(featureConfiguration.getCommandLine(actionName, variables, expander));
return argv;
Expand Down
Loading

0 comments on commit 473ea10

Please sign in to comment.