From d852e484d8114829ad2d7a98f075a823889f5469 Mon Sep 17 00:00:00 2001 From: cpeyser Date: Thu, 7 Sep 2017 22:16:06 +0200 Subject: Add a new toolchain type for c++. In order to do this, PlatformConfiguration is made a legal configuration fragment for every rule class. Add a default "dummy" c++ toolchain to prevent resolution errors when legacy toolchain selection logic is used. Add toolchain mocks to java and shell tests. PiperOrigin-RevId: 167901210 --- compile.sh | 4 + .../build/lib/analysis/BaseRuleClasses.java | 77 +++++++++++------- .../build/lib/analysis/PlatformOptions.java | 4 +- .../build/lib/analysis/PlatformSemantics.java | 2 + .../skylark/SkylarkRuleClassFunctions.java | 8 +- .../lib/bazel/rules/cpp/BazelCppRuleClasses.java | 2 + .../devtools/build/lib/packages/RuleClass.java | 6 ++ .../devtools/build/lib/rules/cpp/CppHelper.java | 5 ++ .../skyframe/SkyFunctionEnvironmentForTesting.java | 12 ++- .../build/lib/skyframe/SkyframeExecutor.java | 4 +- .../devtools/build/lib/analysis/BuildViewTest.java | 4 +- .../lib/packages/util/BazelMockCcSupport.java | 27 ++++++- .../lib/packages/util/MockPlatformSupport.java | 6 +- .../devtools/build/lib/rules/cpp/CcCommonTest.java | 1 + .../skyframe/RegisteredToolchainsFunctionTest.java | 11 +-- .../build/lib/skyframe/RuleContextTest.java | 2 - .../build/lib/testutil/TestRuleClassProvider.java | 3 +- .../shell/integration/discard_graph_edges_test.sh | 2 +- tools/BUILD | 2 +- tools/cpp/BUILD | 15 ++++ tools/cpp/BUILD.static | 15 ++++ tools/cpp/BUILD.tpl | 15 ++++ tools/cpp/cc_configure.bzl | 2 + tools/cpp/dummy_toolchain.bzl | 23 ++++++ tools/platforms/BUILD | 92 ++++++++++++++++++++++ 25 files changed, 285 insertions(+), 59 deletions(-) create mode 100644 tools/cpp/dummy_toolchain.bzl diff --git a/compile.sh b/compile.sh index dc414b18b6..c72a8026d1 100755 --- a/compile.sh +++ b/compile.sh @@ -81,7 +81,11 @@ source scripts/bootstrap/bootstrap.sh new_step 'Building Bazel with Bazel' display "." log "Building output/bazel" +# We set host and target platform directly since the defaults in @bazel_tools +# have not yet been generated. bazel_build "src:bazel${EXE_EXT}" \ + --experimental_host_platform=//tools/platforms:host_platform \ + --experimental_platforms=//tools/platforms:target_platform \ || fail "Could not build Bazel" bazel_bin_path="$(get_bazel_bin_path)/src/bazel${EXE_EXT}" [ -e "$bazel_bin_path" ] \ diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index 817f4d163e..f52d66f209 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -182,41 +182,60 @@ public class BaseRuleClasses { * Share common attributes across both base and Skylark base rules. */ public static RuleClass.Builder commonCoreAndSkylarkAttributes(RuleClass.Builder builder) { - return builder + return PlatformSemantics.platformAttributes(builder) // The visibility attribute is special: it is a nodep label, and loading the // necessary package groups is handled by {@link LabelVisitor#visitTargetVisibility}. // Package groups always have the null configuration so that they are not duplicated // needlessly. - .add(attr("visibility", NODEP_LABEL_LIST).orderIndependent().cfg(HOST) - .nonconfigurable("special attribute integrated more deeply into Bazel's core logic")) - .add(attr("deprecation", STRING).value(deprecationDefault) - .nonconfigurable("Used in core loading phase logic with no access to configs")) - .add(attr("tags", STRING_LIST).orderIndependent().taggable() - .nonconfigurable("low-level attribute, used in TargetUtils without configurations")) - .add(attr("generator_name", STRING).undocumented("internal") - .nonconfigurable("static structure of a rule")) - .add(attr("generator_function", STRING).undocumented("internal") - .nonconfigurable("static structure of a rule")) - .add(attr("generator_location", STRING).undocumented("internal") - .nonconfigurable("static structure of a rule")) - .add(attr("testonly", BOOLEAN).value(testonlyDefault) - .nonconfigurable("policy decision: rules testability should be consistent")) + .add( + attr("visibility", NODEP_LABEL_LIST) + .orderIndependent() + .cfg(HOST) + .nonconfigurable( + "special attribute integrated more deeply into Bazel's core logic")) + .add( + attr("deprecation", STRING) + .value(deprecationDefault) + .nonconfigurable("Used in core loading phase logic with no access to configs")) + .add( + attr("tags", STRING_LIST) + .orderIndependent() + .taggable() + .nonconfigurable("low-level attribute, used in TargetUtils without configurations")) + .add( + attr("generator_name", STRING) + .undocumented("internal") + .nonconfigurable("static structure of a rule")) + .add( + attr("generator_function", STRING) + .undocumented("internal") + .nonconfigurable("static structure of a rule")) + .add( + attr("generator_location", STRING) + .undocumented("internal") + .nonconfigurable("static structure of a rule")) + .add( + attr("testonly", BOOLEAN) + .value(testonlyDefault) + .nonconfigurable("policy decision: rules testability should be consistent")) .add(attr("features", STRING_LIST).orderIndependent()) .add(attr(":action_listener", LABEL_LIST).cfg(HOST).value(ACTION_LISTENER)) - .add(attr(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST) - .allowedRuleClasses(EnvironmentRule.RULE_NAME) - .cfg(Attribute.ConfigurationTransition.HOST) - .allowedFileTypes(FileTypeSet.NO_FILE) - .dontCheckConstraints() - .nonconfigurable("special logic for constraints and select: see ConstraintSemantics") - ) - .add(attr(RuleClass.RESTRICTED_ENVIRONMENT_ATTR, LABEL_LIST) - .allowedRuleClasses(EnvironmentRule.RULE_NAME) - .cfg(Attribute.ConfigurationTransition.HOST) - .allowedFileTypes(FileTypeSet.NO_FILE) - .dontCheckConstraints() - .nonconfigurable("special logic for constraints and select: see ConstraintSemantics") - ); + .add( + attr(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR, LABEL_LIST) + .allowedRuleClasses(EnvironmentRule.RULE_NAME) + .cfg(Attribute.ConfigurationTransition.HOST) + .allowedFileTypes(FileTypeSet.NO_FILE) + .dontCheckConstraints() + .nonconfigurable( + "special logic for constraints and select: see ConstraintSemantics")) + .add( + attr(RuleClass.RESTRICTED_ENVIRONMENT_ATTR, LABEL_LIST) + .allowedRuleClasses(EnvironmentRule.RULE_NAME) + .cfg(Attribute.ConfigurationTransition.HOST) + .allowedFileTypes(FileTypeSet.NO_FILE) + .dontCheckConstraints() + .nonconfigurable( + "special logic for constraints and select: see ConstraintSemantics")); } public static RuleClass.Builder nameAttribute(RuleClass.Builder builder) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java index 3376e9609d..e2bf25b98b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java @@ -60,7 +60,7 @@ public class PlatformOptions extends FragmentOptions { @Option( name = "extra_toolchains", converter = LabelListConverter.class, - defaultValue = "", + defaultValue = "@bazel_tools//tools/cpp:dummy_cc_toolchain", documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, effectTags = {OptionEffectTag.UNKNOWN}, metadataTags = {OptionMetadataTag.HIDDEN}, @@ -98,6 +98,8 @@ public class PlatformOptions extends FragmentOptions { public PlatformOptions getHost(boolean fallback) { PlatformOptions host = (PlatformOptions) getDefault(); host.platforms = ImmutableList.of(this.hostPlatform); + host.hostPlatform = this.hostPlatform; + host.extraToolchains = this.extraToolchains; return host; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformSemantics.java index bc47c69d57..45c967e551 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformSemantics.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import java.util.List; +import javax.annotation.Nullable; /** Helper class to manage rules' use of platforms. */ public class PlatformSemantics { @@ -49,6 +50,7 @@ public class PlatformSemantics { }; /** Implementation for the :execution_platform attribute. */ + @Nullable public static final Attribute.LateBoundLabel EXECUTION_PLATFORM = new Attribute.LateBoundLabel(PlatformConfiguration.class) { @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index 3e850d966d..6c8487364d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.analysis.ActionsProvider; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.DefaultInfo; import com.google.devtools.build.lib.analysis.OutputGroupProvider; -import com.google.devtools.build.lib.analysis.PlatformSemantics; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.skylark.SkylarkAttr.Descriptor; import com.google.devtools.build.lib.analysis.test.TestConfiguration; @@ -129,10 +128,9 @@ public class SkylarkRuleClassFunctions { /** Parent rule class for non-executable non-test Skylark rules. */ public static final RuleClass baseRule = BaseRuleClasses.commonCoreAndSkylarkAttributes( - PlatformSemantics.platformAttributes( - BaseRuleClasses.nameAttribute( - new RuleClass.Builder("$base_rule", RuleClassType.ABSTRACT, true)) - .add(attr("expect_failure", STRING)))) + BaseRuleClasses.nameAttribute( + new RuleClass.Builder("$base_rule", RuleClassType.ABSTRACT, true)) + .add(attr("expect_failure", STRING))) .build(); /** Parent rule class for executable non-test Skylark rules. */ diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java index 99abec3c14..8d494c8720 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java @@ -57,6 +57,7 @@ import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.cpp.CcToolchain; import com.google.devtools.build.lib.rules.cpp.CppConfiguration; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; +import com.google.devtools.build.lib.rules.cpp.CppHelper; import com.google.devtools.build.lib.rules.cpp.CppRuleClasses; import com.google.devtools.build.lib.rules.cpp.CppRuleClasses.LipoTransition; import com.google.devtools.build.lib.util.FileTypeSet; @@ -139,6 +140,7 @@ public class BazelCppRuleClasses { attr(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, LABEL) .value(CppRuleClasses.ccToolchainAttribute(env))) .setPreferredDependencyPredicate(Predicates.or(CPP_SOURCE, C_SOURCE, CPP_HEADER)) + .addRequiredToolchains(CppHelper.getCcToolchainType(env.getToolsRepository())) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index dcf609261e..daa2e8ec1b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; import com.google.common.collect.Ordering; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -997,6 +998,11 @@ public class RuleClass { return this; } + public Builder addRequiredToolchains(Label... toolchainLabels) { + Iterables.addAll(this.requiredToolchains, Lists.newArrayList(toolchainLabels)); + return this; + } + /** * Returns an Attribute.Builder object which contains a replica of the * same attribute in the parent rule if exists. 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 1100576151..87f43cc057 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 @@ -87,6 +87,11 @@ public class CppHelper { private static final ImmutableList LINKOPTS_PREREQUISITE_LABEL_KINDS = ImmutableList.of("deps", "srcs"); + /** Returns label used to select resolved cc_toolchain instances based on platform. */ + public static Label getCcToolchainType(String toolsRepository) { + return Label.parseAbsoluteUnchecked(toolsRepository + "//tools/cpp:toolchain_type"); + } + private CppHelper() { // prevents construction } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctionEnvironmentForTesting.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctionEnvironmentForTesting.java index 7df8afddd5..0acb49c1c6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctionEnvironmentForTesting.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctionEnvironmentForTesting.java @@ -14,7 +14,9 @@ package com.google.devtools.build.lib.skyframe; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.util.ResourceUsage; import com.google.devtools.build.skyframe.AbstractSkyFunctionEnvironment; @@ -33,21 +35,27 @@ public class SkyFunctionEnvironmentForTesting extends AbstractSkyFunctionEnviron private final BuildDriver buildDriver; private final ExtendedEventHandler eventHandler; + private final SkyframeExecutor skyframeExecutor; /** Creates a SkyFunctionEnvironmentForTesting that uses a BuildDriver to evaluate skykeys. */ public SkyFunctionEnvironmentForTesting( - BuildDriver buildDriver, ExtendedEventHandler eventHandler) { + BuildDriver buildDriver, + ExtendedEventHandler eventHandler, + SkyframeExecutor skyframeExecutor) { this.buildDriver = buildDriver; this.eventHandler = eventHandler; + this.skyframeExecutor = skyframeExecutor; } @Override protected Map getValueOrUntypedExceptions( Iterable depKeys) throws InterruptedException { ImmutableMap.Builder resultMap = ImmutableMap.builder(); + Iterable keysToEvaluate = ImmutableList.copyOf(depKeys); EvaluationResult evaluationResult = + skyframeExecutor.evaluateSkyKeys(eventHandler, keysToEvaluate, true); buildDriver.evaluate(depKeys, true, ResourceUsage.getAvailableProcessors(), eventHandler); - for (SkyKey depKey : depKeys) { + for (SkyKey depKey : ImmutableSet.copyOf(depKeys)) { resultMap.put(depKey, ValueOrExceptionUtils.ofValue(evaluationResult.get(depKey))); } return resultMap.build(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index f592d39268..9c952acb78 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -837,7 +837,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { Set