Skip to content

Commit

Permalink
Read the CROSSTOOL from the package of the current cc_toolchain, not …
Browse files Browse the repository at this point in the history
…from --crosstool_top

This will make the behavior work properly when using platforms, and it will fix
recurring issues when cc_toolchain_suite and cc_toolchain are built as a top
level targets, without setting --crosstool_top (that resulted in trying to
associate cc_toolchain with an unrelated CROSSTOOL file and fail the build).
This could also happen when using `bazel cquery` or `bazel aquery`.

RELNOTES: CROSSTOOL file is now read from the package of cc_toolchain, not from
the package of cc_toolchain_suite. This is not expected to break anybody since
cc_toolchain_suite and cc_toolchain are commonly in the same package.
PiperOrigin-RevId: 218516709
  • Loading branch information
hlopko authored and Copybara-Service committed Oct 24, 2018
1 parent da94144 commit 683c302
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}

CrosstoolRelease crosstoolRelease = null;
if (key.getCcToolchainSuiteLabel() != null) {
if (key.getPackageWithCrosstoolInIt() != null) {
try {
// 1. Lookup the package to handle multiple package roots (PackageLookupValue)
PackageIdentifier packageIdentifier = key.getCcToolchainSuiteLabel().getPackageIdentifier();
PackageIdentifier packageIdentifier = key.getPackageWithCrosstoolInIt();
PackageLookupValue crosstoolPackageValue =
(PackageLookupValue) env.getValue(PackageLookupValue.key(packageIdentifier));
if (env.valuesMissing()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
package com.google.devtools.build.lib.rules.cpp;

import com.google.common.collect.Interner;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
Expand Down Expand Up @@ -49,22 +49,22 @@ public static class Key implements SkyKey {
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();

@Nullable private final PathFragment fdoZipPath;
@Nullable private final Label ccToolchainSuiteLabel;
@Nullable private final PackageIdentifier packageWithCrosstoolInIt;

private Key(PathFragment fdoZipPath, Label ccToolchainSuiteLabel) {
private Key(PathFragment fdoZipPath, PackageIdentifier packageWithCrosstoolInIt) {
this.fdoZipPath = fdoZipPath;
this.ccToolchainSuiteLabel = ccToolchainSuiteLabel;
this.packageWithCrosstoolInIt = packageWithCrosstoolInIt;
}

@AutoCodec.Instantiator
@AutoCodec.VisibleForSerialization
static Key of(PathFragment fdoZipPath, Label ccToolchainSuiteLabel) {
return interner.intern(new Key(fdoZipPath, ccToolchainSuiteLabel));
static Key of(PathFragment fdoZipPath, PackageIdentifier packageWithCrosstoolInIt) {
return interner.intern(new Key(fdoZipPath, packageWithCrosstoolInIt));
}

@Nullable
public Label getCcToolchainSuiteLabel() {
return ccToolchainSuiteLabel;
public PackageIdentifier getPackageWithCrosstoolInIt() {
return packageWithCrosstoolInIt;
}

@Nullable
Expand All @@ -82,13 +82,13 @@ public boolean equals(Object o) {
}
Key key = (Key) o;
return Objects.equals(fdoZipPath, key.fdoZipPath)
&& Objects.equals(ccToolchainSuiteLabel, key.ccToolchainSuiteLabel);
&& Objects.equals(packageWithCrosstoolInIt, key.packageWithCrosstoolInIt);
}

@Override
public int hashCode() {

return Objects.hash(fdoZipPath, ccToolchainSuiteLabel);
return Objects.hash(fdoZipPath, packageWithCrosstoolInIt);
}

@Override
Expand Down Expand Up @@ -118,7 +118,7 @@ public CrosstoolRelease getCrosstoolRelease() {
return crosstoolRelease;
}

public static SkyKey key(PathFragment fdoZipPath, Label ccToolchainSuiteLabel) {
return Key.of(fdoZipPath, ccToolchainSuiteLabel);
public static SkyKey key(PathFragment fdoZipPath, PackageIdentifier packageWithCrosstoolInIt) {
return Key.of(fdoZipPath, packageWithCrosstoolInIt);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -385,12 +385,12 @@ static CcToolchainProvider getCcToolchainProvider(

// Is there a toolchain proto available on the target directly?
CToolchain toolchain = parseToolchainFromAttributes(ruleContext, attributes);
Label ccToolchainSuiteLabelIfNeeded = null;
PackageIdentifier packageWithCrosstoolInIt = null;
if (toolchain == null && cppConfiguration.getCrosstoolFromCcToolchainProtoAttribute() == null) {
ccToolchainSuiteLabelIfNeeded = cppConfiguration.getCrosstoolTop();
packageWithCrosstoolInIt = ruleContext.getLabel().getPackageIdentifier();
}

SkyKey ccSupportKey = CcSkyframeSupportValue.key(fdoZip, ccToolchainSuiteLabelIfNeeded);
SkyKey ccSupportKey = CcSkyframeSupportValue.key(fdoZip, packageWithCrosstoolInIt);

SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
CcSkyframeSupportValue ccSkyframeSupportValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ public void testCcToolchainLicenseOverride() throws Exception {
" all_files = ':every-file',",
" dynamic_runtime_libs = ['dynamic-runtime-libs-cherry'],",
" static_runtime_libs = ['static-runtime-libs-cherry'])");
scratch.file("c/CROSSTOOL", AnalysisMock.get().ccSupport().readCrosstoolFile());

ConfiguredTarget target = getConfiguredTarget("//c:c");
Map<Label, License> expected = licenses("//c:c", "notice");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,16 @@ public void testCcHostToolchainAliasRuleHasHostConfiguration() throws Exception

@Test
public void testThatHostCrosstoolTopCommandLineArgumentWorks() throws Exception {

scratch.file(
"t/BUILD",
"b/BUILD",
"cc_toolchain_suite(",
" name = 'my_custom_toolchain_suite',",
" toolchains = {",
" 'k8|gcc-4.4.0': '//b:toolchain_b',",
" 'k8|compiler': '//b:toolchain_b',",
" 'x64_windows|windows_msys64': '//b:toolchain_b',",
" 'darwin|compiler': '//b:toolchain_b',",

"})");

scratch.file("t/CROSSTOOL", AnalysisMock.get().ccSupport().readCrosstoolFile());

scratch.file(
"b/BUILD",
"})",
"cc_toolchain(",
" name = 'toolchain_b',",
" cpu = 'ED-E',",
Expand All @@ -72,9 +65,11 @@ public void testThatHostCrosstoolTopCommandLineArgumentWorks() throws Exception
" objcopy_files = ':empty',",
" dynamic_runtime_libs = [':empty'],",
" static_runtime_libs = [':empty'])");
scratch.file("b/CROSSTOOL", AnalysisMock.get().ccSupport().readCrosstoolFile());

scratch.file("a/BUILD", "cc_host_toolchain_alias(name='current_cc_host_toolchain')");

useConfiguration("--host_crosstool_top=//t:my_custom_toolchain_suite");
useConfiguration("--host_crosstool_top=//b:my_custom_toolchain_suite");
ConfiguredTarget target = getConfiguredTarget("//a:current_cc_host_toolchain");

assertThat(target.getLabel()).isEqualTo(Label.parseAbsoluteUnchecked("//b:toolchain_b"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.packages.util.MockCcSupport;
import com.google.devtools.build.lib.packages.util.ResourceLoader;
Expand Down Expand Up @@ -265,6 +266,7 @@ public void testDynamicMode() throws Exception {
" objcopy_files = ':empty',",
" dynamic_runtime_libs = [':empty'],",
" static_runtime_libs = [':empty'])");
scratch.file("a/CROSSTOOL", AnalysisMock.get().ccSupport().readCrosstoolFile());

// Check defaults.
useConfiguration();
Expand Down Expand Up @@ -529,6 +531,7 @@ public void testZipperInclusionDependsOnFdoOptimization() throws Exception {
"fdo/BUILD",
"exports_files(['my_profile.afdo'])",
"fdo_profile(name = 'fdo', profile = ':my_profile.profdata')");
scratch.file("a/CROSSTOOL", AnalysisMock.get().ccSupport().readCrosstoolFile());

useConfiguration();
assertThat(getPrerequisites(getConfiguredTarget("//a:b"), ":zipper")).isEmpty();
Expand Down Expand Up @@ -670,10 +673,6 @@ private void writeStarlarkRule() throws IOException {
"cc_toolchain_config_rule(name = 'toolchain_config')",
"filegroup(",
" name='empty')",
"cc_toolchain_suite(",
" name = 'a',",
" toolchains = { 'k8': ':b' },",
")",
"cc_toolchain(",
" name = 'b',",
" cpu = 'banana',",
Expand All @@ -688,6 +687,7 @@ private void writeStarlarkRule() throws IOException {
" dynamic_runtime_libs = [':empty'],",
" static_runtime_libs = [':empty'],",
" toolchain_config = ':toolchain_config')");
scratch.file("a/CROSSTOOL", AnalysisMock.get().ccSupport().readCrosstoolFile());

scratch.file(
"a/crosstool_rule.bzl",
Expand Down Expand Up @@ -887,10 +887,6 @@ public void testSysroot_fromCcToolchain() throws Exception {
"a/BUILD",
"filegroup(",
" name='empty')",
"cc_toolchain_suite(",
" name = 'a',",
" toolchains = { 'k8': ':b' },",
")",
"cc_toolchain(",
" name = 'b',",
" cpu = 'banana',",
Expand Down

0 comments on commit 683c302

Please sign in to comment.