Skip to content

Commit

Permalink
Update RuleClass to use ToolchainTypeRequirement.
Browse files Browse the repository at this point in the history
A future change will update rules.

Part of Optional Toolchains (#14726).
  • Loading branch information
katre committed Feb 18, 2022
1 parent be3fcb6 commit f9f8571
Show file tree
Hide file tree
Showing 13 changed files with 156 additions and 41 deletions.
40 changes: 27 additions & 13 deletions src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
import com.google.devtools.build.lib.analysis.config.Fragment;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
Expand Down Expand Up @@ -74,6 +75,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import net.starlark.java.eval.EvalException;
Expand Down Expand Up @@ -831,7 +833,7 @@ public enum ThirdPartyLicenseExistencePolicy {
private ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy;

private final Map<String, Attribute> attributes = new LinkedHashMap<>();
private final Set<Label> requiredToolchains = new HashSet<>();
private final Set<ToolchainTypeRequirement> toolchainTypes = new HashSet<>();
private ToolchainResolutionMode useToolchainResolution = ToolchainResolutionMode.INHERIT;
private ToolchainTransitionMode useToolchainTransition = ToolchainTransitionMode.INHERIT;
private final Set<Label> executionPlatformConstraints = new HashSet<>();
Expand Down Expand Up @@ -868,7 +870,7 @@ public Builder(String name, RuleClassType type, boolean starlark, RuleClass... p
.includeConfigurationFragmentsFrom(parent.getConfigurationFragmentPolicy());
supportsConstraintChecking = parent.supportsConstraintChecking;

addRequiredToolchains(parent.getRequiredToolchains());
addToolchainTypes(parent.getToolchainTypes());
this.useToolchainResolution =
this.useToolchainResolution.apply(name, parent.useToolchainResolution);
this.useToolchainTransition =
Expand Down Expand Up @@ -997,7 +999,7 @@ public RuleClass build(String name, String key) {
configurationFragmentPolicy.build(),
supportsConstraintChecking,
thirdPartyLicenseExistencePolicy,
requiredToolchains,
toolchainTypes,
useToolchainResolution,
useToolchainTransition,
executionPlatformConstraints,
Expand Down Expand Up @@ -1528,20 +1530,32 @@ public Builder setOptionReferenceFunctionForConfigSettingOnly(
}

/**
* Causes rules of this type to require the specified toolchains be available via toolchain
* Cause rules of this type to request the specified toolchains be available via toolchain
* resolution when a target is configured.
*/
public Builder addRequiredToolchains(Iterable<Label> toolchainLabels) {
Iterables.addAll(this.requiredToolchains, toolchainLabels);
public Builder addToolchainTypes(Iterable<ToolchainTypeRequirement> toolchainTypes) {
Iterables.addAll(this.toolchainTypes, toolchainTypes);
return this;
}

/**
* Cause rules of this type to request the specified toolchains be available via toolchain
* resolution when a target is configured.
*/
public Builder addToolchainTypes(ToolchainTypeRequirement... toolchainTypes) {
return addToolchainTypes(ImmutableList.copyOf(toolchainTypes));
}

/**
* Causes rules of this type to require the specified toolchains be available via toolchain
* resolution when a target is configured.
*/
public Builder addRequiredToolchains(Label... toolchainLabels) {
return this.addRequiredToolchains(Lists.newArrayList(toolchainLabels));
// TODO(katre): Remove this once all callers use addToolchainType.
public Builder addRequiredToolchains(Collection<Label> toolchainLabels) {
return this.addToolchainTypes(
toolchainLabels.stream()
.map(label -> ToolchainTypeRequirement.builder().toolchainType(label).build())
.collect(Collectors.toList()));
}

/**
Expand Down Expand Up @@ -1756,7 +1770,7 @@ public Attribute.Builder<?> copy(String name) {

private final ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy;

private final ImmutableSet<Label> requiredToolchains;
private final ImmutableSet<ToolchainTypeRequirement> toolchainTypes;
private final ToolchainResolutionMode useToolchainResolution;
private final ToolchainTransitionMode useToolchainTransition;
private final ImmutableSet<Label> executionPlatformConstraints;
Expand Down Expand Up @@ -1813,7 +1827,7 @@ public Attribute.Builder<?> copy(String name) {
ConfigurationFragmentPolicy configurationFragmentPolicy,
boolean supportsConstraintChecking,
ThirdPartyLicenseExistencePolicy thirdPartyLicenseExistencePolicy,
Set<Label> requiredToolchains,
Set<ToolchainTypeRequirement> toolchainTypes,
ToolchainResolutionMode useToolchainResolution,
ToolchainTransitionMode useToolchainTransition,
Set<Label> executionPlatformConstraints,
Expand Down Expand Up @@ -1854,7 +1868,7 @@ public Attribute.Builder<?> copy(String name) {
this.configurationFragmentPolicy = configurationFragmentPolicy;
this.supportsConstraintChecking = supportsConstraintChecking;
this.thirdPartyLicenseExistencePolicy = thirdPartyLicenseExistencePolicy;
this.requiredToolchains = ImmutableSet.copyOf(requiredToolchains);
this.toolchainTypes = ImmutableSet.copyOf(toolchainTypes);
this.useToolchainResolution = useToolchainResolution;
this.useToolchainTransition = useToolchainTransition;
this.executionPlatformConstraints = ImmutableSet.copyOf(executionPlatformConstraints);
Expand Down Expand Up @@ -2768,8 +2782,8 @@ public boolean ignoreLicenses() {
return ignoreLicenses;
}

public ImmutableSet<Label> getRequiredToolchains() {
return requiredToolchains;
public ImmutableSet<ToolchainTypeRequirement> getToolchainTypes() {
return toolchainTypes;
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/query2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/starlark_defined_config_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/null_transition",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.devtools.build.lib.analysis.ToolchainContext;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -215,7 +216,8 @@ private static ToolchainCollection<ToolchainContext> getToolchainContexts(
return null;
}

ImmutableSet<Label> requiredToolchains = rule.getRuleClassObject().getRequiredToolchains();
ImmutableSet<ToolchainTypeRequirement> toolchainTypes =
rule.getRuleClassObject().getToolchainTypes();
// Collect local (target, rule) constraints for filtering out execution platforms.
ImmutableSet<Label> execConstraintLabels =
getExecutionPlatformConstraints(rule, config.getFragment(PlatformConfiguration.class));
Expand Down Expand Up @@ -249,7 +251,7 @@ private static ToolchainCollection<ToolchainContext> getToolchainContexts(
walkableGraph.getValue(
ToolchainContextKey.key()
.configurationKey(configurationKey)
.requiredToolchainTypeLabels(requiredToolchains)
.toolchainTypes(toolchainTypes)
.execConstraintLabels(execConstraintLabels)
.debugTarget(debugTarget)
.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
Expand Down Expand Up @@ -550,8 +551,8 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
Rule rule = ((Rule) targetAndConfig.getTarget());
BuildConfigurationValue configuration = targetAndConfig.getConfiguration();

ImmutableSet<Label> requiredDefaultToolchains =
rule.getRuleClassObject().getRequiredToolchains();
ImmutableSet<ToolchainTypeRequirement> toolchainTypes =
rule.getRuleClassObject().getToolchainTypes();
// Collect local (target, rule) constraints for filtering out execution platforms.
ImmutableSet<Label> defaultExecConstraintLabels =
getExecutionPlatformConstraints(
Expand All @@ -560,7 +561,7 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
// Create a merged version of the exec groups that handles exec group inheritance properly.
ExecGroup defaultExecGroup =
ExecGroup.builder()
.requiredToolchains(requiredDefaultToolchains)
.toolchainTypes(toolchainTypes)
.execCompatibleWith(defaultExecConstraintLabels)
.copyFrom(null)
.build();
Expand Down Expand Up @@ -619,7 +620,7 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
ToolchainContextKey.Builder toolchainContextKeyBuilder =
ToolchainContextKey.key()
.configurationKey(toolchainConfig)
.requiredToolchainTypeLabels(requiredDefaultToolchains)
.toolchainTypes(toolchainTypes)
.execConstraintLabels(defaultExecConstraintLabels)
.debugTarget(debugTarget);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// Copyright 2022 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.analysis.testing;

import static com.google.common.truth.Truth.assertAbout;

import com.google.common.base.Functions;
import com.google.common.collect.ImmutableMap;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.MapSubject;
import com.google.common.truth.Subject;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.RuleClass;
import java.util.Map;

/** A Truth {@link Subject} for {@link RuleClass}. */
public class RuleClassSubject extends Subject {
// Static data.

/** Entry point for test assertions related to {@link RuleClass}. */
public static RuleClassSubject assertThat(RuleClass ruleClass) {
return assertAbout(RuleClassSubject::new).that(ruleClass);
}

/** Static method for getting the subject factory (for use with assertAbout()). */
public static Subject.Factory<RuleClassSubject, RuleClass> ruleClasses() {
return RuleClassSubject::new;
}

// Instance fields.

private final RuleClass actual;
private Map<Label, ToolchainTypeRequirement> toolchainTypesMap;

protected RuleClassSubject(FailureMetadata failureMetadata, RuleClass subject) {
super(failureMetadata, subject);
this.actual = subject;
this.toolchainTypesMap = makeToolchainTypesMap(subject);
}

private static Map<Label, ToolchainTypeRequirement> makeToolchainTypesMap(RuleClass subject) {
return subject.getToolchainTypes().stream()
.collect(
ImmutableMap.toImmutableMap(
ToolchainTypeRequirement::toolchainType, Functions.identity()));
}

public MapSubject toolchainTypes() {
return check("toolchainTypes()").that(toolchainTypesMap);
}

public ToolchainTypeRequirementSubject toolchainType(String toolchainTypeLabel) {
return toolchainType(Label.parseAbsoluteUnchecked(toolchainTypeLabel));
}

public ToolchainTypeRequirementSubject toolchainType(Label toolchainType) {
return check("toolchainType(%s)", toolchainType)
.about(ToolchainTypeRequirementSubject.toolchainTypeRequirements())
.that(toolchainTypesMap.get(toolchainType));
}

public void hasToolchainType(String toolchainTypeLabel) {
toolchainType(toolchainTypeLabel).isNotNull();
}

public void hasToolchainType(Label toolchainType) {
toolchainType(toolchainType).isNotNull();
}

// TODO(blaze-team): Add more useful methods.
}
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/packages/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
"//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.analysis.testing.ExecGroupSubject.assertThat;
import static com.google.devtools.build.lib.analysis.testing.RuleClassSubject.assertThat;
import static com.google.devtools.build.lib.packages.Attribute.attr;
import static com.google.devtools.build.lib.packages.Type.BOOLEAN;
import static com.google.devtools.build.lib.packages.Type.INTEGER;
Expand All @@ -27,6 +28,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassNamePredicate;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
Expand Down Expand Up @@ -184,14 +186,15 @@ public void testRequiredToolchainsAreInherited() throws Exception {
RuleClass parent =
new RuleClass.Builder("$parent", RuleClassType.ABSTRACT, false)
.add(attr("tags", STRING_LIST))
.addRequiredToolchains(ImmutableList.of(mockToolchainType))
.addToolchainTypes(ToolchainTypeRequirement.create(mockToolchainType))
.build();
RuleClass child =
new RuleClass.Builder("child", RuleClassType.NORMAL, false, parent)
.factory(DUMMY_CONFIGURED_TARGET_FACTORY)
.add(attr("attr", STRING))
.build();
assertThat(child.getRequiredToolchains()).contains(mockToolchainType);

assertThat(child).hasToolchainType(mockToolchainType);
}

@Test
Expand Down Expand Up @@ -232,18 +235,23 @@ public void testDuplicateExecGroupsThatInheritFromRuleIsOk() throws Exception {
.factory(DUMMY_CONFIGURED_TARGET_FACTORY)
.addExecGroups(ImmutableMap.of("blueberry", ExecGroup.copyFromDefault()))
.add(attr("tags", STRING_LIST))
.addRequiredToolchains(Label.parseAbsoluteUnchecked("//some/toolchain"))
.addToolchainTypes(
ToolchainTypeRequirement.create(Label.parseAbsoluteUnchecked("//some/toolchain")))
.build();
RuleClass b =
new RuleClass.Builder("ruleB", RuleClassType.NORMAL, false)
.factory(DUMMY_CONFIGURED_TARGET_FACTORY)
.addExecGroups(ImmutableMap.of("blueberry", ExecGroup.copyFromDefault()))
.add(attr("tags", STRING_LIST))
.addRequiredToolchains(Label.parseAbsoluteUnchecked("//some/other/toolchain"))
.addToolchainTypes(
ToolchainTypeRequirement.create(
Label.parseAbsoluteUnchecked("//some/other/toolchain")))
.build();
RuleClass c =
new RuleClass.Builder("$ruleC", RuleClassType.ABSTRACT, false, a, b)
.addRequiredToolchains(Label.parseAbsoluteUnchecked("//actual/toolchain/we/care/about"))
.addToolchainTypes(
ToolchainTypeRequirement.create(
Label.parseAbsoluteUnchecked("//actual/toolchain/we/care/about")))
.build();
assertThat(c.getExecGroups()).containsKey("blueberry");
ExecGroup blueberry = c.getExecGroups().get("blueberry");
Expand Down
Loading

0 comments on commit f9f8571

Please sign in to comment.