Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ExecGroup to use ToolchainTypeRequirement #14812

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/packages/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ java_library(
name = "exec_group",
srcs = ["ExecGroup.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
"//third_party:auto_value",
Expand Down
61 changes: 46 additions & 15 deletions src/main/java/com/google/devtools/build/lib/packages/ExecGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@

package com.google.devtools.build.lib.packages;

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

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.starlarkbuildapi.ExecGroupApi;
import javax.annotation.Nullable;
Expand All @@ -31,7 +35,7 @@ public abstract class ExecGroup implements ExecGroupApi {
/** Returns a builder for a new ExecGroup. */
public static Builder builder() {
return new AutoValue_ExecGroup.Builder()
.requiredToolchains(ImmutableSet.of())
.toolchainTypes(ImmutableSet.of())
.execCompatibleWith(ImmutableSet.of());
}

Expand All @@ -41,7 +45,17 @@ public static ExecGroup copyFromDefault() {
}

/** Returns the required toolchain types for this exec group. */
public abstract ImmutableSet<Label> requiredToolchains();
public ImmutableSet<ToolchainTypeRequirement> toolchainTypes() {
return toolchainTypesMap().values();
}

@Nullable
public ToolchainTypeRequirement toolchainType(Label label) {
return toolchainTypesMap().get(label);
}

/** Returns the underlying map from label to ToolchainTypeRequirement. */
public abstract ImmutableBiMap<Label, ToolchainTypeRequirement> toolchainTypesMap();

/** Returns the execution constraints for this exec group. */
public abstract ImmutableSet<Label> execCompatibleWith();
Expand All @@ -52,29 +66,46 @@ public static ExecGroup copyFromDefault() {

/** Creates a new exec group that inherits from the given group and this group. */
public ExecGroup inheritFrom(ExecGroup other) {
ImmutableSet<Label> requiredToolchains =
new ImmutableSet.Builder<Label>()
.addAll(this.requiredToolchains())
.addAll(other.requiredToolchains())
.build();
ImmutableSet<Label> execCompatibleWith =
Builder builder = builder().copyFrom(null);
builder.toolchainTypesMapBuilder().putAll(this.toolchainTypesMap());
builder.toolchainTypesMapBuilder().putAll(other.toolchainTypesMap());

builder.execCompatibleWith(
new ImmutableSet.Builder<Label>()
.addAll(this.execCompatibleWith())
.addAll(other.execCompatibleWith())
.build();
return builder()
.requiredToolchains(requiredToolchains)
.execCompatibleWith(execCompatibleWith)
.copyFrom(null)
.build();
.build());

return builder.build();
}

/** A builder interface to create ExecGroup instances. */
@AutoValue.Builder
public interface Builder {

/** Sets the required toolchain types. */
Builder requiredToolchains(ImmutableSet<Label> toolchainTypes);
// TODO(katre): Remove this once all callers use toolchainTypes.
default Builder requiredToolchains(ImmutableSet<Label> toolchainTypes) {
ImmutableSet<ToolchainTypeRequirement> toolchainTypeRequirements =
toolchainTypes.stream()
.map(label -> ToolchainTypeRequirement.builder().toolchainType(label).build())
.collect(toImmutableSet());
return this.toolchainTypes(toolchainTypeRequirements);
}

/** Sets the toolchain type requirements. */
default Builder toolchainTypes(ImmutableSet<ToolchainTypeRequirement> toolchainTypes) {
toolchainTypes.forEach(this::addToolchainType);
return this;
}

ImmutableBiMap.Builder<Label, ToolchainTypeRequirement> toolchainTypesMapBuilder();

default Builder addToolchainType(ToolchainTypeRequirement toolchainTypeRequirement) {
this.toolchainTypesMapBuilder()
.put(toolchainTypeRequirement.toolchainType(), toolchainTypeRequirement);
return this;
}

/** Sets the execution constraints. */
Builder execCompatibleWith(ImmutableSet<Label> execCompatibleWith);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ private static ToolchainCollection<ToolchainContext> getToolchainContexts(
walkableGraph.getValue(
ToolchainContextKey.key()
.configurationKey(configurationKey)
.requiredToolchainTypeLabels(execGroup.requiredToolchains())
.toolchainTypes(execGroup.toolchainTypes())
.execConstraintLabels(execGroup.execCompatibleWith())
.debugTarget(debugTarget)
.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
name,
ToolchainContextKey.key()
.configurationKey(toolchainConfig)
.requiredToolchainTypeLabels(execGroup.requiredToolchains())
.toolchainTypes(execGroup.toolchainTypes())
.execConstraintLabels(execGroup.execCompatibleWith())
.debugTarget(debugTarget)
.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,12 @@ public void testInheritsRuleRequirements() throws Exception {
ExecGroupCollection execGroups = getRuleContext(ct).getExecGroups();
assertThat(execGroups).isNotNull();
assertThat(execGroups).hasExecGroup("watermelon");
assertThat(execGroups).execGroup("watermelon").hasRequiredToolchain("//rule:toolchain_type_1");
// TODO(https://github.com/bazelbuild/bazel/issues/14726): Add tests of optional toolchains.
assertThat(execGroups).execGroup("watermelon").hasToolchainType("//rule:toolchain_type_1");
assertThat(execGroups)
.execGroup("watermelon")
.toolchainType("//rule:toolchain_type_1")
.isMandatory();
assertThat(execGroups).execGroup("watermelon").hasExecCompatibleWith("//platform:constraint_1");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ java_library(
"ToolchainCollectionSubject.java",
"ToolchainContextSubject.java",
"ToolchainInfoSubject.java",
"ToolchainTypeRequirementSubject.java",
],
deps = [
"//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
"//src/main/java/com/google/devtools/build/lib/analysis:exec_group_collection",
"//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context",
"//src/main/java/com/google/devtools/build/lib/analysis:toolchain_collection",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IterableSubject;
import com.google.common.truth.MapSubject;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -46,17 +47,26 @@ protected ExecGroupSubject(FailureMetadata failureMetadata, ExecGroup subject) {
this.actual = subject;
}

public IterableSubject requiredToolchains() {
return check("requiredToolchainTypes()")
.that(actual.requiredToolchains().stream().collect(Collectors.toList()));
public MapSubject toolchainTypes() {
return check("toolchainTypes()").that(actual.toolchainTypesMap());
}

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

public void hasRequiredToolchain(Label toolchainType) {
requiredToolchains().contains(toolchainType);
public ToolchainTypeRequirementSubject toolchainType(Label toolchainType) {
return check("toolchainType(%s)", toolchainType)
.about(ToolchainTypeRequirementSubject.toolchainTypeRequirements())
.that(actual.toolchainType(toolchainType));
}

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

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

public IterableSubject execCompatibleWith() {
Expand Down
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/fragment",
"//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
"//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 @@ -1080,6 +1080,7 @@ public void testExecGroups() throws Exception {
Label toolchain = Label.parseAbsoluteUnchecked("//toolchain");
Label constraint = Label.parseAbsoluteUnchecked("//constraint");

// TODO(https://github.com/bazelbuild/bazel/issues/14726): Add tests of optional toolchains.
ruleClassBuilder.addExecGroups(
ImmutableMap.of(
"cherry",
Expand All @@ -1092,7 +1093,8 @@ public void testExecGroups() throws Exception {
RuleClass ruleClass = ruleClassBuilder.build();

assertThat(ruleClass.getExecGroups()).hasSize(1);
assertThat(ruleClass.getExecGroups().get("cherry")).hasRequiredToolchain(toolchain);
assertThat(ruleClass.getExecGroups().get("cherry")).hasToolchainType(toolchain);
assertThat(ruleClass.getExecGroups().get("cherry")).toolchainType(toolchain).isMandatory();
assertThat(ruleClass.getExecGroups().get("cherry")).hasExecCompatibleWith(constraint);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2338,7 +2338,11 @@ public void testRuleAddExecGroup() throws Exception {
")");
RuleClass plum = ((StarlarkRuleFunction) ev.lookup("plum")).getRuleClass();
assertThat(plum.getRequiredToolchains()).isEmpty();
assertThat(plum.getExecGroups().get("group")).hasRequiredToolchain("//test:my_toolchain_type");
// TODO(https://github.com/bazelbuild/bazel/issues/14726): Add tests of optional toolchains.
assertThat(plum.getExecGroups().get("group")).hasToolchainType("//test:my_toolchain_type");
assertThat(plum.getExecGroups().get("group"))
.toolchainType("//test:my_toolchain_type")
.isMandatory();
assertThat(plum.getExecutionPlatformConstraints()).isEmpty();
assertThat(plum.getExecGroups().get("group")).hasExecCompatibleWith("//constraint:cv1");
assertThat(plum.getExecGroups().get("group")).hasExecCompatibleWith("//constraint:cv2");
Expand Down Expand Up @@ -2388,7 +2392,9 @@ public void testCreateExecGroup() throws Exception {
" exec_compatible_with=['//constraint:cv1', '//constraint:cv2'],",
")");
ExecGroup group = ((ExecGroup) ev.lookup("group"));
assertThat(group).hasRequiredToolchain("//test:my_toolchain_type");
// TODO(https://github.com/bazelbuild/bazel/issues/14726): Add tests of optional toolchains.
assertThat(group).hasToolchainType("//test:my_toolchain_type");
assertThat(group).toolchainType("//test:my_toolchain_type").isMandatory();
assertThat(group).hasExecCompatibleWith("//constraint:cv1");
assertThat(group).hasExecCompatibleWith("//constraint:cv2");
}
Expand Down