Introduce cc_toolchain_type, which exports c++ make variables. Use //tools/cpp:toolchain_type as the canonical c++ toolchain. PiperOrigin-RevId: 174759558
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index ac85e07..2c0bd9e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -233,12 +233,14 @@ return this; } - public void addWorkspaceFilePrefix(String contents) { + public Builder addWorkspaceFilePrefix(String contents) { defaultWorkspaceFilePrefix.append(contents); + return this; } - public void addWorkspaceFileSuffix(String contents) { + public Builder addWorkspaceFileSuffix(String contents) { defaultWorkspaceFileSuffix.append(contents); + return this; } public Builder setPrelude(String preludeLabelString) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index a004998..4dcd5b6 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule; import com.google.devtools.build.lib.bazel.rules.BazelToolchainType.BazelToolchainTypeRule; +import com.google.devtools.build.lib.bazel.rules.CcToolchainType.CcToolchainTypeRule; import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryRule; import com.google.devtools.build.lib.bazel.rules.android.AndroidSdkRepositoryRule; import com.google.devtools.build.lib.bazel.rules.android.BazelAarImportRule; @@ -191,6 +192,7 @@ @Override public void init(Builder builder) { builder.addRuleDefinition(new BazelToolchainTypeRule()); + builder.addRuleDefinition(new CcToolchainTypeRule()); builder.addRuleDefinition(new GenRuleBaseRule()); builder.addRuleDefinition(new BazelGenRuleRule()); } @@ -217,6 +219,8 @@ builder.addConfigurationOptions(BuildConfiguration.Options.class); builder.addWorkspaceFileSuffix( "register_toolchains('@bazel_tools//tools/cpp:dummy_cc_toolchain')\n"); + builder.addWorkspaceFileSuffix( + "register_toolchains('@bazel_tools//tools/cpp:dummy_cc_toolchain_type')\n"); } @Override
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/CcToolchainType.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/CcToolchainType.java new file mode 100644 index 0000000..b865d72 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/CcToolchainType.java
@@ -0,0 +1,71 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.bazel.rules; + +import static com.google.devtools.build.lib.packages.Attribute.attr; +import static com.google.devtools.build.lib.syntax.Type.STRING; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.BaseRuleClasses; +import com.google.devtools.build.lib.analysis.PlatformConfiguration; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.RuleDefinition; +import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.RuleClass; +import com.google.devtools.build.lib.rules.ToolchainType; +import com.google.devtools.build.lib.rules.cpp.CppConfiguration; +import com.google.devtools.build.lib.rules.cpp.CppHelper; + +/** A toolchain type for the c++ toolchain. */ +public class CcToolchainType extends ToolchainType { + + /** Definition for {@code cc_toolchain_type}. */ + public static class CcToolchainTypeRule implements RuleDefinition { + @Override + public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) { + return builder + .requiresConfigurationFragments(CppConfiguration.class, PlatformConfiguration.class) + .addRequiredToolchains(CppHelper.getCcToolchainType(environment.getToolsRepository())) + .add(attr("$tools_repo", STRING).value(environment.getToolsRepository())) + .removeAttribute("licenses") + .removeAttribute("distribs") + .build(); + } + + @Override + public Metadata getMetadata() { + return Metadata.builder() + .name("cc_toolchain_type") + .factoryClass(CcToolchainType.class) + .ancestors(BaseRuleClasses.BaseRule.class) + .build(); + } + } + + private static ImmutableMap<Label, Class<? extends BuildConfiguration.Fragment>> + createFragmentMap(RuleContext ruleContext) { + return ImmutableMap.of( + Label.parseAbsoluteUnchecked( + ruleContext.attributes().get("$tools_repo", STRING) + "//tools/cpp:toolchain_type"), + CppConfiguration.class); + } + + public CcToolchainType() { + // Call constructor with a function that can infer fragment map from a RuleContext. + super(CcToolchainType::createFragmentMap, ImmutableMap.of()); + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java b/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java index d9dd120..46d2828 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java +++ b/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java
@@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules; +import com.google.common.base.Function; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; @@ -23,6 +24,7 @@ import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TemplateVariableInfo; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; import com.google.devtools.build.lib.cmdline.Label; import java.util.TreeMap; @@ -30,19 +32,37 @@ * Abstract base class for {@code toolchain_type}. */ public class ToolchainType implements RuleConfiguredTargetFactory { - private final ImmutableMap<Label, Class<? extends BuildConfiguration.Fragment>> fragmentMap; + private ImmutableMap<Label, Class<? extends BuildConfiguration.Fragment>> fragmentMap; + private final Function<RuleContext, ImmutableMap<Label, Class<? extends Fragment>>> + fragmentMapFromRuleContext; private final ImmutableMap<Label, ImmutableMap<String, String>> hardcodedVariableMap; protected ToolchainType( ImmutableMap<Label, Class<? extends BuildConfiguration.Fragment>> fragmentMap, ImmutableMap<Label, ImmutableMap<String, String>> hardcodedVariableMap) { this.fragmentMap = fragmentMap; + this.fragmentMapFromRuleContext = null; + this.hardcodedVariableMap = hardcodedVariableMap; + } + + // This constructor is required to allow for toolchains that depend on the tools repository, + // which depends on RuleContext. + protected ToolchainType( + Function<RuleContext, ImmutableMap<Label, Class<? extends Fragment>>> + fragmentMapFromRuleContext, + ImmutableMap<Label, ImmutableMap<String, String>> hardcodedVariableMap) { + this.fragmentMap = null; + this.fragmentMapFromRuleContext = fragmentMapFromRuleContext; this.hardcodedVariableMap = hardcodedVariableMap; } @Override public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException { + if (fragmentMap == null && fragmentMapFromRuleContext != null) { + this.fragmentMap = fragmentMapFromRuleContext.apply(ruleContext); + } + // This cannot be an ImmutableMap.Builder because that asserts when a key is duplicated TreeMap<String, String> makeVariables = new TreeMap<>(); Class<? extends BuildConfiguration.Fragment> fragmentClass =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index b7942b3..5746f7e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -92,7 +92,7 @@ ImmutableList.of("deps", "srcs"); /** Base label of the c++ toolchain category. */ - public static final String TOOLCHAIN_TYPE_LABEL = "//tools/cpp:toolchain_category"; + public static final String TOOLCHAIN_TYPE_LABEL = "//tools/cpp:toolchain_type"; /** Returns label used to select resolved cc_toolchain instances based on platform. */ public static Label getCcToolchainType(String toolsRepository) {
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index df43932..879a7e6 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1269,6 +1269,8 @@ "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/test/java/com/google/devtools/build/lib:packages_testutil", + "//src/test/java/com/google/devtools/build/lib:testutil", "//third_party:auto_value", "//third_party:guava", "//third_party:junit4",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java index f1d5209..37356a7 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java
@@ -78,7 +78,7 @@ "/bazel_tools_workspace/tools/cpp/BUILD", "package(default_visibility=['//visibility:public'])", "cc_library(name = 'stl')", - "toolchain_type(name = 'toolchain_type')", + "cc_toolchain_type(name = 'toolchain_type')", "cc_library(name = 'malloc')", "cc_toolchain_suite(", " name = 'toolchain',", @@ -156,14 +156,19 @@ " name = 'link_dynamic_library',", " srcs = ['link_dynamic_library.sh'],", ")", - "filegroup(name = 'toolchain_category')", "toolchain(", " name = 'toolchain_cc-compiler-piii',", - " toolchain_type = ':toolchain_category',", + " toolchain_type = ':toolchain_type',", " toolchain = '//third_party/crosstool/mock:cc-compiler-piii',", " target_compatible_with = [':mock_value'],", ")", "toolchain(", + " name = 'dummy_cc_toolchain_type',", + " toolchain_type = ':toolchain_type',", + " toolchain = ':dummy_cc_toolchain_impl',", + ")", + "filegroup(name = 'toolchain_category')", + "toolchain(", " name = 'dummy_cc_toolchain',", " toolchain_type = ':toolchain_category',", " toolchain = ':dummy_cc_toolchain_impl',",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java index 45aa949..c10bb59 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockPlatformSupport.java
@@ -50,9 +50,7 @@ ")", "toolchain(", " name = 'toolchain_cc-compiler-piii',", - " toolchain_type = '" - + TestConstants.TOOLS_REPOSITORY - + "//tools/cpp:toolchain_category',", + " toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type',", " toolchain = '" + crosstoolLabel.getRelative("cc-compiler-piii") + "',", " target_compatible_with = [':mock_value'],", ")");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java b/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java index 7f04325..9b6cc49 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java
@@ -18,6 +18,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TemplateVariableInfo; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.testutil.TestConstants; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -25,11 +26,19 @@ /** Unit tests for the {@code toolchain_type} rule. */ @RunWith(JUnit4.class) public class ToolchainTypeTest extends BuildViewTestCase { + @Test public void testSmoke() throws Exception { - ConfiguredTarget cc = getConfiguredTarget(getRuleClassProvider().getToolsRepository() - + "//tools/cpp:toolchain_type"); + ConfiguredTarget cc = + getConfiguredTarget(TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type"); assertThat(cc.get(TemplateVariableInfo.PROVIDER).getVariables()) .containsKey("TARGET_CPU"); } + + @Test + public void testCcToolchainDoesNotProvideJavaMakeVariables() throws Exception { + ConfiguredTarget cc = + getConfiguredTarget(TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type"); + assertThat(cc.get(TemplateVariableInfo.PROVIDER).getVariables()).doesNotContainKey("JAVABASE"); + } }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java index e966cc0..f6e7119 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
@@ -969,7 +969,7 @@ @Override public Metadata getMetadata() { return Metadata.builder() - .name("toolchain_type") + .name("cc_toolchain_type") .factoryClass(BazelToolchainType.class) .ancestors(BaseRuleClasses.BaseRule.class) .build();
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSelectionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSelectionTest.java index f5f22a2..e0d92a4 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSelectionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSelectionTest.java
@@ -55,7 +55,7 @@ } private static final String CPP_TOOLCHAIN_TYPE = - TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_category"; + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type"; @Test public void testResolvedCcToolchain() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java index 49d928c..8cd0e30 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java
@@ -41,8 +41,8 @@ assertThatEvaluationResult(result).hasEntryThat(toolchainsKey).isNotNull(); RegisteredToolchainsValue value = result.get(toolchainsKey); - // We have two registered toolchains, and one default for c++ - assertThat(value.registeredToolchains()).hasSize(3); + // We have two registered toolchains, and two default for c++ + assertThat(value.registeredToolchains()).hasSize(4); assertThat(value.registeredToolchains().stream().anyMatch(toolchain -> (toolchain.toolchainType().equals(testToolchainType))
diff --git a/tools/cpp/BUILD b/tools/cpp/BUILD index a693b76..5e5b6ff 100644 --- a/tools/cpp/BUILD +++ b/tools/cpp/BUILD
@@ -205,12 +205,20 @@ srcs = ["link_dynamic_library.sh"], ) -filegroup(name = "toolchain_category") +filegroup(name = "toolchain_type") # A dummy toolchain is necessary to satisfy toolchain resolution until platforms # are used in c++ by default. # TODO(b/64754003): Remove once platforms are used in c++ by default. toolchain( + name = "dummy_cc_toolchain_type", + toolchain = "dummy_cc_toolchain_impl", + toolchain_type = ":toolchain_type", +) + +filegroup(name = "toolchain_category") + +toolchain( name = "dummy_cc_toolchain", toolchain = "dummy_cc_toolchain_impl", toolchain_type = ":toolchain_category",
diff --git a/tools/cpp/BUILD.static b/tools/cpp/BUILD.static index 65bbac1..d5fc9a3 100644 --- a/tools/cpp/BUILD.static +++ b/tools/cpp/BUILD.static
@@ -116,12 +116,19 @@ srcs = ["link_dynamic_library.sh"], ) -filegroup(name = "toolchain_category") +filegroup(name = "toolchain_type") # A dummy toolchain is necessary to satisfy toolchain resolution until platforms # are used in c++ by default. # TODO(b/64754003): Remove once platforms are used in c++ by default. toolchain( + name = "dummy_cc_toolchain_type", + toolchain = "dummy_cc_toolchain_impl", + toolchain_type = ":toolchain_type", +) + +filegroup(name = "toolchain_category") +toolchain( name = "dummy_cc_toolchain", toolchain = "dummy_cc_toolchain_impl", toolchain_type = ":toolchain_category",
diff --git a/tools/cpp/BUILD.tpl b/tools/cpp/BUILD.tpl index 32b796d..f3281cf 100644 --- a/tools/cpp/BUILD.tpl +++ b/tools/cpp/BUILD.tpl
@@ -76,12 +76,19 @@ supports_param_files = 1, ) -filegroup(name = "toolchain_category") +filegroup(name = "toolchain_type") # A dummy toolchain is necessary to satisfy toolchain resolution until platforms # are used in c++ by default. # TODO(b/64754003): Remove once platforms are used in c++ by default. toolchain( + name = "dummy_cc_toolchain_type", + toolchain = "dummy_cc_toolchain_impl", + toolchain_type = ":toolchain_type", +) + +filegroup(name = "toolchain_category") +toolchain( name = "dummy_cc_toolchain", toolchain = "dummy_cc_toolchain_impl", toolchain_type = ":toolchain_category",