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).

Closes #14862.

PiperOrigin-RevId: 431546477
  • Loading branch information
katre authored and copybara-github committed Mar 1, 2022
1 parent 8778455 commit 6484373
Show file tree
Hide file tree
Showing 14 changed files with 163 additions and 55 deletions.
47 changes: 35 additions & 12 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,41 @@ 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.
*/
// TODO(katre): Remove this once all callers use addToolchainType.
public Builder addRequiredToolchains(Label... toolchainLabels) {
return this.addRequiredToolchains(Lists.newArrayList(toolchainLabels));
return this.addRequiredToolchains(ImmutableList.copyOf(toolchainLabels));
}

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

/**
Expand Down Expand Up @@ -1756,7 +1779,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 +1836,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 +1877,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 +2791,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
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;

import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -60,20 +59,6 @@ public SkyFunctionName functionName() {
public interface Builder {
Builder configurationKey(BuildConfigurationKey key);

// TODO(katre): Remove this once all callers use toolchainTypes.
default Builder requiredToolchainTypeLabels(Label... requiredToolchainTypeLabels) {
return this.requiredToolchainTypeLabels(ImmutableSet.copyOf(requiredToolchainTypeLabels));
}

// TODO(katre): Remove this once all callers use toolchainTypes.
default Builder requiredToolchainTypeLabels(ImmutableSet<Label> requiredToolchainTypeLabels) {
ImmutableSet<ToolchainTypeRequirement> toolchainTypeRequirements =
requiredToolchainTypeLabels.stream()
.map(label -> ToolchainTypeRequirement.create(label))
.collect(toImmutableSet());
return this.toolchainTypes(toolchainTypeRequirements);
}

Builder toolchainTypes(ImmutableSet<ToolchainTypeRequirement> toolchainTypes);

default Builder toolchainTypes(ToolchainTypeRequirement... toolchainTypes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ java_library(
"ExecGroupCollectionSubject.java",
"ExecGroupSubject.java",
"ResolvedToolchainContextSubject.java",
"RuleClassSubject.java",
"ToolchainCollectionSubject.java",
"ToolchainContextSubject.java",
"ToolchainInfoSubject.java",
Expand All @@ -33,6 +34,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages:exec_group",
"//src/main/java/net/starlark/java/eval",
"//third_party:guava",
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.collect.ImmutableMap.toImmutableMap;
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 final Map<Label, ToolchainTypeRequirement> toolchainTypesMap;

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

private static ImmutableMap<Label, ToolchainTypeRequirement> makeToolchainTypesMap(
RuleClass subject) {
return subject.getToolchainTypes().stream()
.collect(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.
}
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 Down Expand Up @@ -185,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 @@ -233,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 6484373

Please sign in to comment.