From 683c302129b66a8999f986be5ae7e642707e978c Mon Sep 17 00:00:00 2001 From: hlopko Date: Wed, 24 Oct 2018 08:33:08 -0700 Subject: [PATCH] Read the CROSSTOOL from the package of the current cc_toolchain, not 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 --- .../rules/cpp/CcSkyframeSupportFunction.java | 4 ++-- .../lib/rules/cpp/CcSkyframeSupportValue.java | 24 +++++++++---------- .../rules/cpp/CcToolchainProviderHelper.java | 6 ++--- .../build/lib/analysis/LicensingTests.java | 1 + .../rules/cpp/CcHostToolchainAliasTest.java | 15 ++++-------- .../build/lib/rules/cpp/CcToolchainTest.java | 12 ++++------ 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportFunction.java index 6334d1ef54b6ec..85ef28ce2fafea 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportFunction.java @@ -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()) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java index d097230a0a2ba3..d5ece4bc5ec507 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcSkyframeSupportValue.java @@ -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; @@ -49,22 +49,22 @@ public static class Key implements SkyKey { private static final Interner 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 @@ -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 @@ -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); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java index c95960895c2035..7f4b2df50e8e8b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java @@ -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; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LicensingTests.java b/src/test/java/com/google/devtools/build/lib/analysis/LicensingTests.java index c555efb3de0907..f397f0451a8f3f 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/LicensingTests.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/LicensingTests.java @@ -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 expected = licenses("//c:c", "notice"); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcHostToolchainAliasTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcHostToolchainAliasTest.java index 7b2e6753e28dfd..de24239c29e63e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcHostToolchainAliasTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcHostToolchainAliasTest.java @@ -42,9 +42,8 @@ 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 = {", @@ -52,13 +51,7 @@ public void testThatHostCrosstoolTopCommandLineArgumentWorks() throws Exception " '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',", @@ -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")); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java index 9b492fe03d0815..8933c551a0bf8e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java @@ -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; @@ -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(); @@ -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(); @@ -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',", @@ -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", @@ -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',",