Skip to content

Commit

Permalink
Extract logic from CROSSTOOL in CrosstoolInfo provider
Browse files Browse the repository at this point in the history 
CrosstoolInfo carries the necessary information from the CROSSTOOL text proto. Later on, instead from the CROSSTOOL file and the corresponding CToolchain, CrosstoolInfo will be derived from a skylark_crosstool rule implemented in Skylark. Therefore CToolchain involvement in CppConfiguration and CcToolchain creation needs to be eliminated.

Work towards issue bazelbuild#5380

RELNOTES: None.
PiperOrigin-RevId: 201491207
  • Loading branch information
@scentini
scentini authored and George Gensure committed Aug 2, 2018
1 parent b5e85c4 commit 3fe1c00
Show file tree
Hide file tree
Showing 8 changed files with 799 additions and 139 deletions.
12 changes: 10 additions & 2 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -654,10 +654,18 @@ private CppToolchainInfo getCppToolchainInfo(

// If we found a toolchain, use it.
try {
toolchain =
CppToolchainInfo.addLegacyFeatures(
toolchain, cppConfiguration.getCrosstoolTopPathFragment());
CrosstoolInfo crosstoolInfo =
CrosstoolInfo.fromToolchain(
cppConfiguration.getCrosstoolFile().getProto(),
toolchain,
cppConfiguration.getCrosstoolTopPathFragment());
return CppToolchainInfo.create(
toolchain,
cppConfiguration.getCrosstoolTopPathFragment(),
cppConfiguration.getCcToolchainRuleLabel());
cppConfiguration.getCcToolchainRuleLabel(),
crosstoolInfo);
} catch (InvalidConfigurationException e) {
throw ruleContext.throwWithRuleError(e.getMessage());
}
Expand Down
53 changes: 22 additions & 31 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ static class Feature implements Serializable, CrosstoolSelectable {
private final ImmutableList<String> implies;
private final ImmutableList<String> provides;

private Feature(CToolchain.Feature feature) throws InvalidConfigurationException {
Feature(CToolchain.Feature feature) throws InvalidConfigurationException {
this.name = feature.getName();
ImmutableList.Builder<FlagSet> flagSetBuilder = ImmutableList.builder();
for (CToolchain.FlagSet flagSet : feature.getFlagSetList()) {
Expand Down Expand Up @@ -888,7 +888,7 @@ static class ActionConfig implements Serializable, CrosstoolSelectable {
private final boolean enabled;
private final ImmutableList<String> implies;

private ActionConfig(CToolchain.ActionConfig actionConfig, PathFragment crosstoolTop)
ActionConfig(CToolchain.ActionConfig actionConfig, PathFragment crosstoolTop)
throws InvalidConfigurationException {
this.configName = actionConfig.getConfigName();
this.actionName = actionConfig.getActionName();
Expand Down Expand Up @@ -995,13 +995,13 @@ public ImmutableList<String> getImplies() {

/** A description of how artifacts of a certain type are named. */
@Immutable
private static class ArtifactNamePattern {
static class ArtifactNamePattern {

private final ArtifactCategory artifactCategory;
private final String prefix;
private final String extension;

private ArtifactNamePattern(CToolchain.ArtifactNamePattern artifactNamePattern)
ArtifactNamePattern(CToolchain.ArtifactNamePattern artifactNamePattern)
throws InvalidConfigurationException {

ArtifactCategory foundCategory = null;
Expand Down Expand Up @@ -1269,14 +1269,13 @@ public ImmutableSet<String> getEnabledFeatureNames() {
buildConfigurationCache();

/**
* Constructs the feature configuration from a {@code CToolchain} protocol buffer.
* Constructs the feature configuration from a {@code crosstoolInfo}.
*
* @param toolchain the toolchain configuration as specified by the user.
* @param crosstoolInfo the toolchain information as specified by the user.
* @throws InvalidConfigurationException if the configuration has logical errors.
*/
@VisibleForTesting
public CcToolchainFeatures(CToolchain toolchain, PathFragment crosstoolTop)
throws InvalidConfigurationException {
public CcToolchainFeatures(CrosstoolInfo crosstoolInfo) throws InvalidConfigurationException {
// Build up the feature/action config graph. We refer to features/action configs as
// 'selectables'.
// First, we build up the map of name -> selectables in one pass, so that earlier selectables
Expand All @@ -1288,17 +1287,15 @@ public CcToolchainFeatures(CToolchain toolchain, PathFragment crosstoolTop)
ImmutableMap.Builder<String, ActionConfig> actionConfigsByActionName = ImmutableMap.builder();

ImmutableList.Builder<String> defaultSelectablesBuilder = ImmutableList.builder();
for (CToolchain.Feature toolchainFeature : toolchain.getFeatureList()) {
Feature feature = new Feature(toolchainFeature);
for (Feature feature : crosstoolInfo.getFeatures()) {
selectablesBuilder.add(feature);
selectablesByName.put(feature.getName(), feature);
if (feature.isEnabled()) {
defaultSelectablesBuilder.add(feature.getName());
}
}

for (CToolchain.ActionConfig toolchainActionConfig : toolchain.getActionConfigList()) {
ActionConfig actionConfig = new ActionConfig(toolchainActionConfig, crosstoolTop);
for (ActionConfig actionConfig : crosstoolInfo.getActionConfigs()) {
selectablesBuilder.add(actionConfig);
selectablesByName.put(actionConfig.getName(), actionConfig);
actionConfigsByActionName.put(actionConfig.getActionName(), actionConfig);
Expand All @@ -1311,18 +1308,12 @@ public CcToolchainFeatures(CToolchain toolchain, PathFragment crosstoolTop)
this.selectables = selectablesBuilder.build();
this.selectablesByName = ImmutableMap.copyOf(selectablesByName);

checkForActionNameDups(toolchain.getActionConfigList());
checkForActionNameDups(crosstoolInfo.getActionConfigs());
checkForActivatableDups(this.selectables);

this.actionConfigsByActionName = actionConfigsByActionName.build();

ImmutableList.Builder<ArtifactNamePattern> artifactNamePatternsBuilder =
ImmutableList.builder();
for (CToolchain.ArtifactNamePattern artifactNamePattern :
toolchain.getArtifactNamePatternList()) {
artifactNamePatternsBuilder.add(new ArtifactNamePattern(artifactNamePattern));
}
this.artifactNamePatterns = artifactNamePatternsBuilder.build();
this.artifactNamePatterns = crosstoolInfo.getArtifactNamePatterns();

// Next, we build up all forward references for 'implies', 'requires', and 'provides' edges.
ImmutableMultimap.Builder<CrosstoolSelectable, CrosstoolSelectable> implies =
Expand All @@ -1336,32 +1327,32 @@ public CcToolchainFeatures(CToolchain toolchain, PathFragment crosstoolTop)
ImmutableMultimap.Builder<CrosstoolSelectable, CrosstoolSelectable> requiredBy =
ImmutableMultimap.builder();

for (CToolchain.Feature toolchainFeature : toolchain.getFeatureList()) {
String name = toolchainFeature.getName();
for (Feature feature : crosstoolInfo.getFeatures()) {
String name = feature.getName();
CrosstoolSelectable selectable = selectablesByName.get(name);
for (CToolchain.FeatureSet requiredFeatures : toolchainFeature.getRequiresList()) {
for (ImmutableSet<String> requiredFeatures : feature.getRequires()) {
ImmutableSet.Builder<CrosstoolSelectable> allOf = ImmutableSet.builder();
for (String requiredName : requiredFeatures.getFeatureList()) {
for (String requiredName : requiredFeatures) {
CrosstoolSelectable required = getActivatableOrFail(requiredName, name);
allOf.add(required);
requiredBy.put(required, selectable);
}
requires.put(selectable, allOf.build());
}
for (String impliedName : toolchainFeature.getImpliesList()) {
for (String impliedName : feature.getImplies()) {
CrosstoolSelectable implied = getActivatableOrFail(impliedName, name);
impliedBy.put(implied, selectable);
implies.put(selectable, implied);
}
for (String providesName : toolchainFeature.getProvidesList()) {
for (String providesName : feature.getProvides()) {
provides.put(selectable, providesName);
}
}

for (CToolchain.ActionConfig toolchainActionConfig : toolchain.getActionConfigList()) {
String name = toolchainActionConfig.getConfigName();
for (ActionConfig actionConfig : crosstoolInfo.getActionConfigs()) {
String name = actionConfig.getName();
CrosstoolSelectable selectable = selectablesByName.get(name);
for (String impliedName : toolchainActionConfig.getImpliesList()) {
for (String impliedName : actionConfig.getImplies()) {
CrosstoolSelectable implied = getActivatableOrFail(impliedName, name);
impliedBy.put(implied, selectable);
implies.put(selectable, implied);
Expand Down Expand Up @@ -1389,10 +1380,10 @@ private static void checkForActivatableDups(Iterable<CrosstoolSelectable> select
}
}

private static void checkForActionNameDups(Iterable<CToolchain.ActionConfig> actionConfigs)
private static void checkForActionNameDups(Iterable<ActionConfig> actionConfigs)
throws InvalidConfigurationException {
Collection<String> actionNames = new HashSet<>();
for (CToolchain.ActionConfig actionConfig : actionConfigs) {
for (ActionConfig actionConfig : actionConfigs) {
if (!actionNames.add(actionConfig.getActionName())) {
throw new InvalidConfigurationException(
"Invalid toolchain configuration: multiple action "
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig;
import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain;
import java.util.Map;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -219,12 +218,12 @@ public String toString() {

static CppConfiguration create(CppConfigurationParameters params)
throws InvalidConfigurationException {
CrosstoolConfig.CToolchain toolchain = params.toolchain;
CppOptions cppOptions = params.cppOptions;
PathFragment crosstoolTopPathFragment =
params.crosstoolTop.getPackageIdentifier().getPathUnderExecRoot();
CppToolchainInfo cppToolchainInfo =
CppToolchainInfo.create(toolchain, crosstoolTopPathFragment, params.ccToolchainLabel);
CppToolchainInfo.create(
crosstoolTopPathFragment, params.ccToolchainLabel, params.crosstoolInfo);

CompilationMode compilationMode = params.commonOptions.compilationMode;

Expand Down Expand Up @@ -267,7 +266,7 @@ static CppConfiguration create(CppConfigurationParameters params)
params.sysrootLabel,
coptsBuilder.build(),
cxxOpts,
ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()),
cppToolchainInfo.getUnfilteredCompilerOptions(/* sysroot= */ null),
ImmutableList.copyOf(cppOptions.conlyoptList),
cppToolchainInfo.configureAllLegacyLinkOptions(compilationMode, LinkingMode.STATIC),
cppToolchainInfo.configureAllLegacyLinkOptions(
Expand Down Expand Up @@ -1125,8 +1124,7 @@ public boolean useLLVMCoverageMapFormat() {
return cppOptions.useLLVMCoverageMapFormat;
}

public static PathFragment computeDefaultSysroot(CToolchain toolchain) {
String builtInSysroot = toolchain.getBuiltinSysroot();
public static PathFragment computeDefaultSysroot(String builtInSysroot) {
if (builtInSysroot.isEmpty()) {
return null;
}
Expand Down
24 changes: 17 additions & 7 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ public CppConfiguration create(ConfigurationEnvironment env, BuildOptions option
* Value class for all the data needed to create a {@link CppConfiguration}.
*/
public static class CppConfigurationParameters {
protected final CrosstoolConfig.CToolchain toolchain;
protected final CrosstoolConfigurationLoader.CrosstoolFile crosstoolFile;
protected final String cacheKeySuffix;
protected final BuildConfiguration.Options commonOptions;
Expand All @@ -90,9 +89,9 @@ public static class CppConfigurationParameters {
protected final Label fdoOptimizeLabel;
protected final Label sysrootLabel;
protected final CpuTransformer cpuTransformer;
protected final CrosstoolInfo crosstoolInfo;

CppConfigurationParameters(
CrosstoolConfig.CToolchain toolchain,
CrosstoolConfigurationLoader.CrosstoolFile crosstoolFile,
String cacheKeySuffix,
BuildOptions buildOptions,
Expand All @@ -102,8 +101,8 @@ public static class CppConfigurationParameters {
Label ccToolchainLabel,
Label stlLabel,
Label sysrootLabel,
CpuTransformer cpuTransformer) {
this.toolchain = toolchain;
CpuTransformer cpuTransformer,
CrosstoolInfo crosstoolInfo) {
this.crosstoolFile = crosstoolFile;
this.cacheKeySuffix = cacheKeySuffix;
this.commonOptions = buildOptions.get(BuildConfiguration.Options.class);
Expand All @@ -115,6 +114,7 @@ public static class CppConfigurationParameters {
this.stlLabel = stlLabel;
this.sysrootLabel = sysrootLabel;
this.cpuTransformer = cpuTransformer;
this.crosstoolInfo = crosstoolInfo;
}
}

Expand Down Expand Up @@ -241,11 +241,19 @@ protected CppConfigurationParameters createParameters(
: CrosstoolConfigurationLoader.getToolchainByIdentifier(
file.getProto(), identifier, desiredCpu, cppOptions.cppCompiler);
}
toolchain =
CppToolchainInfo.addLegacyFeatures(
toolchain, crosstoolTopLabel.getPackageIdentifier().getPathUnderExecRoot());

CrosstoolInfo crosstoolInfo =
CrosstoolInfo.fromToolchain(
file.getProto(),
toolchain,
crosstoolTopLabel.getPackageIdentifier().getPathUnderExecRoot());

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

return new CppConfigurationParameters(
toolchain,
file,
file.getMd5(),
options,
Expand All @@ -255,13 +263,15 @@ protected CppConfigurationParameters createParameters(
ccToolchainLabel,
stlLabel,
sysrootLabel,
cpuTransformer);
cpuTransformer,
crosstoolInfo);
}

@Nullable
public static Label getSysrootLabel(CrosstoolConfig.CToolchain toolchain, Label libcTopLabel)
throws InvalidConfigurationException {
PathFragment defaultSysroot = CppConfiguration.computeDefaultSysroot(toolchain);
PathFragment defaultSysroot =
CppConfiguration.computeDefaultSysroot(toolchain.getBuiltinSysroot());

if ((libcTopLabel != null) && (defaultSysroot == null)) {
throw new InvalidConfigurationException(
Expand Down
Loading

0 comments on commit 3fe1c00

Please sign in to comment.