Skip to content

Commit

Permalink
Remove uses of CppToolchainInfo from CppConfiguration
Browse files Browse the repository at this point in the history 
It's used in tests only.

RELNOTES: None.
PiperOrigin-RevId: 222623580
  • Loading branch information
@hlopko
hlopko authored and Copybara-Service committed Nov 23, 2018
1 parent 8bbc058 commit ed1c01f
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 299 deletions.
8 changes: 3 additions & 5 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ static CcToolchainProvider getCcToolchainProvider(
for (String s : toolchainInfo.getRawBuiltInIncludeDirectories()) {
try {
builtInIncludeDirectoriesBuilder.add(
resolveIncludeDir(s, sysroot, toolchainInfo.getCrosstoolTopPathFragment()));
resolveIncludeDir(s, sysroot, toolchainInfo.getToolsDirectory()));
} catch (InvalidConfigurationException e) {
ruleContext.ruleError(e.getMessage());
}
Expand Down Expand Up @@ -581,7 +581,7 @@ static CcToolchainProvider getCcToolchainProvider(
getToolchainForSkylark(toolchainInfo),
cppConfiguration,
toolchainInfo,
cppConfiguration.getCrosstoolTopPathFragment(),
toolchainInfo.getToolsDirectory(),
attributes.getCrosstool(),
attributes.getFullInputsForCrosstool(),
attributes.getCompile(),
Expand Down Expand Up @@ -672,7 +672,6 @@ private static CppToolchainInfo getCppToolchainInfo(
if (configInfo != null) {
try {
return CppToolchainInfo.create(
ruleContext.getLabel().getPackageIdentifier().getPathUnderExecRoot(),
ruleContext.getLabel(),
configInfo,
cppConfiguration.disableLegacyCrosstoolFields(),
Expand Down Expand Up @@ -700,10 +699,9 @@ private static CppToolchainInfo getCppToolchainInfo(
try {
toolchain =
CppToolchainInfo.addLegacyFeatures(
toolchain, cppConfiguration.getCrosstoolTopPathFragment());
toolchain, CppToolchainInfo.getToolsDirectory(attributes.getCcToolchainLabel()));
CcToolchainConfigInfo ccToolchainConfigInfo = CcToolchainConfigInfo.fromToolchain(toolchain);
return CppToolchainInfo.create(
cppConfiguration.getCrosstoolTopPathFragment(),
attributes.getCcToolchainLabel(),
ccToolchainConfigInfo,
cppConfiguration.disableLegacyCrosstoolFields(),
Expand Down
122 changes: 17 additions & 105 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 @@ -14,13 +14,13 @@

package com.google.devtools.build.lib.rules.cpp;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.config.AutoCpuConverter;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.CompilationMode;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
Expand All @@ -32,12 +32,9 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader.CppConfigurationParameters;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.skylarkbuildapi.cpp.CppConfigurationApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
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 javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -170,16 +167,14 @@ public String toString() {
*/
public static final String FDO_STAMP_MACRO = "BUILD_FDO_TYPE";

private final Label crosstoolTop;

private final String transformedCpuFromOptions;
private final String compilerFromOptions;
// TODO(lberki): desiredCpu *should* be always the same as targetCpu, except that we don't check
// that the CPU we get from the toolchain matches BuildConfiguration.Options.cpu . So we store
// it here so that the output directory doesn't depend on the CToolchain. When we will eventually
// verify that the two are the same, we can remove one of desiredCpu and targetCpu.
private final String desiredCpu;

private final Label redirectChasedCrosstoolTop;
private final PathFragment fdoPath;
private final Label fdoOptimizeLabel;

Expand All @@ -198,26 +193,11 @@ public String toString() {
private final boolean stripBinaries;
private final CompilationMode compilationMode;

private final CppToolchainInfo cppToolchainInfo;

static CppConfiguration create(CppConfigurationParameters params)
throws InvalidConfigurationException {
static CppConfiguration create(CppConfigurationParameters params) {
CppOptions cppOptions = params.cppOptions;
CppToolchainInfo cppToolchainInfo;
try {
cppToolchainInfo =
CppToolchainInfo.create(
params.crosstoolTop.getPackageIdentifier().getPathUnderExecRoot(),
params.ccToolchainLabel,
params.ccToolchainConfigInfo,
cppOptions.disableLegacyCrosstoolFields,
cppOptions.disableCompilationModeFlags,
cppOptions.disableLinkingModeFlags);
} catch (EvalException e) {
throw new InvalidConfigurationException(e);
}

CompilationMode compilationMode = params.commonOptions.compilationMode;
Options commonOptions = params.commonOptions;
CompilationMode compilationMode = commonOptions.compilationMode;

ImmutableList.Builder<String> linkoptsBuilder = ImmutableList.builder();
linkoptsBuilder.addAll(cppOptions.linkoptList);
Expand All @@ -226,10 +206,9 @@ static CppConfiguration create(CppConfigurationParameters params)
}

return new CppConfiguration(
params.crosstoolTop,
params.transformedCpu,
params.compiler,
Preconditions.checkNotNull(params.commonOptions.cpu),
params.crosstoolTop,
Preconditions.checkNotNull(commonOptions.cpu),
params.fdoPath,
params.fdoOptimizeLabel,
ImmutableList.copyOf(cppOptions.conlyoptList),
Expand All @@ -242,14 +221,12 @@ static CppConfiguration create(CppConfigurationParameters params)
(cppOptions.stripBinaries == StripMode.ALWAYS
|| (cppOptions.stripBinaries == StripMode.SOMETIMES
&& compilationMode == CompilationMode.FASTBUILD)),
compilationMode,
cppToolchainInfo);
compilationMode);
}

private CppConfiguration(
Label crosstoolTop,
String transformedCpuFromOptions,
String compilerFromOptions,
Label redirectChasedCrosstoolTop,
String desiredCpu,
PathFragment fdoPath,
Label fdoOptimizeLabel,
Expand All @@ -261,11 +238,9 @@ private CppConfiguration(
ImmutableList<String> ltobackendOptions,
CppOptions cppOptions,
boolean stripBinaries,
CompilationMode compilationMode,
CppToolchainInfo cppToolchainInfo) {
this.crosstoolTop = crosstoolTop;
CompilationMode compilationMode) {
this.transformedCpuFromOptions = transformedCpuFromOptions;
this.compilerFromOptions = compilerFromOptions;
this.redirectChasedCrosstoolTop = redirectChasedCrosstoolTop;
this.desiredCpu = desiredCpu;
this.fdoPath = fdoPath;
this.fdoOptimizeLabel = fdoOptimizeLabel;
Expand All @@ -278,65 +253,6 @@ private CppConfiguration(
this.cppOptions = cppOptions;
this.stripBinaries = stripBinaries;
this.compilationMode = compilationMode;
this.cppToolchainInfo = cppToolchainInfo;
}

@VisibleForTesting
static LinkingMode importLinkingMode(CrosstoolConfig.LinkingMode mode) {
switch (mode.name()) {
case "FULLY_STATIC":
return LinkingMode.LEGACY_FULLY_STATIC;
case "MOSTLY_STATIC_LIBRARIES":
return LinkingMode.LEGACY_MOSTLY_STATIC_LIBRARIES;
case "MOSTLY_STATIC":
return LinkingMode.STATIC;
case "DYNAMIC":
return LinkingMode.DYNAMIC;
default:
throw new IllegalArgumentException(
String.format("Linking mode '%s' not known.", mode.name()));
}
}

/** Returns the {@link CppToolchainInfo} used by this configuration. */
public CppToolchainInfo getCppToolchainInfo() {
return cppToolchainInfo;
}

/**
* Returns the toolchain identifier, which uniquely identifies the compiler version, target libc
* version, and target cpu.
*/
public String getToolchainIdentifier() {
return cppToolchainInfo.getToolchainIdentifier();
}

/** Returns the label of the CROSSTOOL for this configuration. */
public Label getCrosstoolTop() {
return crosstoolTop;
}

/**
* Returns the path of the crosstool.
*/
public PathFragment getCrosstoolTopPathFragment() {
return cppToolchainInfo.getCrosstoolTopPathFragment();
}

@Override
public String toString() {
return cppToolchainInfo.toString();
}

/**
* Returns the path fragment that is either absolute or relative to the execution root that can be
* used to execute the given tool.
*
* <p>Deprecated: Use {@link CcToolchainProvider#getToolPathFragment(Tool)}
*/
@Deprecated
public PathFragment getToolPathFragment(CppConfiguration.Tool tool) {
return cppToolchainInfo.getToolPathFragment(tool);
}

/** Returns the label of the <code>cc_compiler</code> rule for the C++ configuration. */
Expand All @@ -346,15 +262,7 @@ public PathFragment getToolPathFragment(CppConfiguration.Tool tool) {
defaultLabel = "//tools/cpp:crosstool",
defaultInToolRepository = true)
public Label getRuleProvidingCcToolchainProvider() {
return crosstoolTop;
}

/**
* Returns the configured features of the toolchain. Rules should not call this directly, but
* instead use {@code CcToolchainProvider.getFeatures}.
*/
public CcToolchainFeatures getFeatures() {
return cppToolchainInfo.getFeatures();
return redirectChasedCrosstoolTop;
}

/**
Expand Down Expand Up @@ -502,7 +410,7 @@ public String getTransformedCpuFromOptions() {
/** @return value from --compiler option, null if the option was not passed. */
@Nullable
public String getCompilerFromOptions() {
return compilerFromOptions;
return cppOptions.cppCompiler;
}

public boolean legacyWholeArchive() {
Expand Down Expand Up @@ -685,6 +593,10 @@ boolean enableCcToolchainConfigInfoFromSkylark() {
return cppOptions.enableCcToolchainConfigInfoFromSkylark;
}

public Label getCrosstoolTop() {
return cppOptions.crosstoolTop;
}

/**
* Returns the value of the libc top-level directory (--grte_top) as specified on the command line
*/
Expand Down
44 changes: 3 additions & 41 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 @@ -33,11 +33,8 @@
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
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.CrosstoolRelease;
import java.util.Map;
import javax.annotation.Nullable;
Expand All @@ -53,7 +50,7 @@ public Class<? extends Fragment> creates() {

@Override
public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
return ImmutableSet.<Class<? extends FragmentOptions>>of(CppOptions.class);
return ImmutableSet.of(CppOptions.class);
}

private final CpuTransformer cpuTransformer;
Expand Down Expand Up @@ -86,7 +83,6 @@ public static class CppConfigurationParameters {
protected final Label ccToolchainLabel;
protected final PathFragment fdoPath;
protected final Label fdoOptimizeLabel;
protected final CcToolchainConfigInfo ccToolchainConfigInfo;
protected final String transformedCpu;
protected final String compiler;

Expand All @@ -97,8 +93,7 @@ public static class CppConfigurationParameters {
PathFragment fdoPath,
Label fdoOptimizeLabel,
Label crosstoolTop,
Label ccToolchainLabel,
CcToolchainConfigInfo ccToolchainConfigInfo) {
Label ccToolchainLabel) {
this.transformedCpu = transformedCpu;
this.compiler = compiler;
this.commonOptions = buildOptions.get(BuildConfiguration.Options.class);
Expand All @@ -107,7 +102,6 @@ public static class CppConfigurationParameters {
this.fdoOptimizeLabel = fdoOptimizeLabel;
this.crosstoolTop = crosstoolTop;
this.ccToolchainLabel = ccToolchainLabel;
this.ccToolchainConfigInfo = ccToolchainConfigInfo;
}
}

Expand Down Expand Up @@ -161,37 +155,6 @@ protected CppConfigurationParameters createParameters(
return null;
}

// If cc_toolchain_suite contains an entry for the given --cpu and --compiler options, we
// select the toolchain by its identifier if "toolchain_identifier" attribute is present.
// Otherwise, we fall back to going through the CROSSTOOL file to select the toolchain using
// the legacy selection mechanism.
String identifierAttribute =
NonconfigurableAttributeMapper.of((Rule) ccToolchain)
.get("toolchain_identifier", Type.STRING);
String cpuAttribute =
NonconfigurableAttributeMapper.of((Rule) ccToolchain).get("cpu", Type.STRING);
String compilerAttribute =
NonconfigurableAttributeMapper.of((Rule) ccToolchain).get("compiler", Type.STRING);

CToolchain cToolchain =
CToolchainSelectionUtils.selectCToolchain(
identifierAttribute,
cpuAttribute,
compilerAttribute,
transformedCpu,
cppOptions.cppCompiler,
crosstoolRelease);

cToolchain =
CppToolchainInfo.addLegacyFeatures(
cToolchain, crosstoolTopLabel.getPackageIdentifier().getPathUnderExecRoot());
CcToolchainConfigInfo ccToolchainConfigInfo;
try {
ccToolchainConfigInfo = CcToolchainConfigInfo.fromToolchain(cToolchain);
} catch (EvalException e) {
throw new InvalidConfigurationException(e);
}

PathFragment fdoPath = null;
Label fdoProfileLabel = null;
if (cppOptions.getFdoOptimize() != null) {
Expand Down Expand Up @@ -219,8 +182,7 @@ protected CppConfigurationParameters createParameters(
fdoPath,
fdoProfileLabel,
crosstoolTopLabel,
ccToolchainLabel,
ccToolchainConfigInfo);
ccToolchainLabel);
}

private Target loadCcToolchainTarget(ConfigurationEnvironment env, Label ccToolchainLabel)
Expand Down
9 changes: 4 additions & 5 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ private static ImmutableList<String> computeAllLinkstampDefines(
String labelReplacement,
String outputReplacement,
Iterable<String> additionalLinkstampDefines,
CppConfiguration cppConfiguration,
CcToolchainProvider ccToolchainProvider,
String fdoBuildStamp,
boolean codeCoverageEnabled) {
String labelPattern = Pattern.quote("${LABEL}");
String outputPathPattern = Pattern.quote("${OUTPUT_PATH}");
ImmutableList.Builder<String> defines =
ImmutableList.<String>builder()
.add("GPLATFORM=\"" + cppConfiguration + "\"")
.add("GPLATFORM=\"" + ccToolchainProvider.getToolchainIdentifier() + "\"")
.add("BUILD_COVERAGE_ENABLED=" + (codeCoverageEnabled ? "1" : "0"))
// G3_TARGET_NAME is a C string literal that normally contain the label of the target
// being linked. However, they are set differently when using shared native deps. In
Expand Down Expand Up @@ -150,8 +150,7 @@ private static CcToolchainVariables getVariables(
/* gcnoFile= */ null,
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
buildInfoHeaderArtifacts
.stream()
buildInfoHeaderArtifacts.stream()
.map(Artifact::getExecPathString)
.collect(ImmutableList.toImmutableList()),
CcCompilationHelper.getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()),
Expand All @@ -170,7 +169,7 @@ private static CcToolchainVariables getVariables(
labelReplacement,
outputReplacement,
additionalLinkstampDefines,
cppConfiguration,
ccToolchainProvider,
fdoBuildStamp,
codeCoverageEnabled));
}
Expand Down
Loading

0 comments on commit ed1c01f

Please sign in to comment.