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 ToolchainResolutionFunction to handle optional toolchain types #15357

Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ static ToolchainType create(
return new AutoValue_ToolchainResolutionFunction_ToolchainType(
toolchainTypeRequirement, toolchainTypeInfo);
}

public boolean mandatory() {
return toolchainTypeRequirement().mandatory();
}
}

/**
Expand Down Expand Up @@ -428,44 +432,40 @@ private static void determineToolchainImplementations(
// Determine the potential set of toolchains.
Table<ConfiguredTargetKey, ToolchainTypeInfo, Label> resolvedToolchains =
HashBasedTable.create();
ImmutableSet.Builder<ToolchainTypeInfo> requiredToolchainTypesBuilder = ImmutableSet.builder();
List<Label> missingToolchains = new ArrayList<>();
List<Label> missingMandatoryToolchains = new ArrayList<>();
for (SingleToolchainResolutionKey key : registeredToolchainKeys) {
SingleToolchainResolutionValue singleToolchainResolutionValue =
(SingleToolchainResolutionValue)
results.getOrThrow(key, InvalidToolchainLabelException.class);
if (singleToolchainResolutionValue == null) {
valuesMissing = true;
continue;
}
if (singleToolchainResolutionValue == null) {
valuesMissing = true;
continue;
}

if (singleToolchainResolutionValue.availableToolchainLabels().isEmpty()) {
// Save the missing type and continue looping to check for more.
// TODO(katre): Handle mandatory/optional.
missingToolchains.add(key.toolchainType().toolchainType());
} else {
if (!singleToolchainResolutionValue.availableToolchainLabels().isEmpty()) {
ToolchainTypeInfo requiredToolchainType = singleToolchainResolutionValue.toolchainType();
requiredToolchainTypesBuilder.add(requiredToolchainType);
resolvedToolchains.putAll(
findPlatformsAndLabels(requiredToolchainType, singleToolchainResolutionValue));
} else if (key.toolchainType().mandatory()) {
// Save the missing type and continue looping to check for more.
missingMandatoryToolchains.add(key.toolchainType().toolchainType());
}
// TODO(katre): track missing optional toolchains?
}

if (!missingToolchains.isEmpty()) {
throw new UnresolvedToolchainsException(missingToolchains);
// Verify that all mandatory toolchain types have a toolchain.
if (!missingMandatoryToolchains.isEmpty()) {
throw new UnresolvedToolchainsException(missingMandatoryToolchains);
}

if (valuesMissing) {
throw new ValueMissingException();
gregestren marked this conversation as resolved.
Show resolved Hide resolved
}

ImmutableSet<ToolchainTypeInfo> requiredToolchainTypes = requiredToolchainTypesBuilder.build();

// Find and return the first execution platform which has all required toolchains.
// TODO(katre): Handle mandatory/optional.
// Find and return the first execution platform which has all mandatory toolchains.
Optional<ConfiguredTargetKey> selectedExecutionPlatformKey =
findExecutionPlatformForToolchains(
requiredToolchainTypes,
toolchainTypes,
forcedExecutionPlatform,
platformKeys.executionPlatformKeys(),
resolvedToolchains);
Expand Down Expand Up @@ -520,43 +520,43 @@ private static Table<ConfiguredTargetKey, ToolchainTypeInfo, Label> findPlatform
* resolvedToolchains} and has all required toolchain types.
*/
private static Optional<ConfiguredTargetKey> findExecutionPlatformForToolchains(
ImmutableSet<ToolchainTypeInfo> requiredToolchainTypes,
ImmutableSet<ToolchainType> toolchainTypes,
Optional<ConfiguredTargetKey> forcedExecutionPlatform,
ImmutableList<ConfiguredTargetKey> availableExecutionPlatformKeys,
Table<ConfiguredTargetKey, ToolchainTypeInfo, Label> resolvedToolchains) {

if (forcedExecutionPlatform.isPresent()) {
// Is the forced platform suitable?
if (isPlatformSuitable(
forcedExecutionPlatform.get(), requiredToolchainTypes, resolvedToolchains)) {
if (isPlatformSuitable(forcedExecutionPlatform.get(), toolchainTypes, resolvedToolchains)) {
return forcedExecutionPlatform;
}
}

// Choose the first execution platform that has all mandatory toolchains.
return availableExecutionPlatformKeys.stream()
.filter(epk -> isPlatformSuitable(epk, requiredToolchainTypes, resolvedToolchains))
.filter(epk -> isPlatformSuitable(epk, toolchainTypes, resolvedToolchains))
.findFirst();
}

private static boolean isPlatformSuitable(
ConfiguredTargetKey executionPlatformKey,
ImmutableSet<ToolchainTypeInfo> requiredToolchainTypes,
ImmutableSet<ToolchainType> toolchainTypes,
Table<ConfiguredTargetKey, ToolchainTypeInfo, Label> resolvedToolchains) {
if (requiredToolchainTypes.isEmpty()) {
if (toolchainTypes.isEmpty()) {
// Since there aren't any toolchains, we should be able to use any execution platform that
// has made it this far.
return true;
}

if (!resolvedToolchains.containsRow(executionPlatformKey)) {
return false;
}

// Unless all toolchains are present, ignore this execution platform.
// Determine whether all mandatory toolchains are present.
return resolvedToolchains
.row(executionPlatformKey)
.keySet()
.containsAll(requiredToolchainTypes);
.containsAll(
toolchainTypes.stream()
.filter(ToolchainType::mandatory)
.map(ToolchainType::toolchainTypeInfo)
.collect(toImmutableSet()));
}

private static final class ValueMissingException extends Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.rules.platform.ToolchainTestCase;
import com.google.devtools.build.lib.skyframe.ConstraintValueLookupUtil.InvalidConstraintValueException;
Expand Down Expand Up @@ -86,6 +87,92 @@ public void resolve() throws Exception {
assertThat(unloadedToolchainContext).hasTargetPlatform("//platforms:linux");
}

// TODO(katre): Add further tests for optional/mandatory/mixed toolchains.

@Test
public void resolve_optional() throws Exception {
// This should select platform mac, toolchain extra_toolchain_mac, because platform
// mac is listed first.
addToolchain(
"extra",
"extra_toolchain_linux",
ImmutableList.of("//constraints:linux"),
ImmutableList.of("//constraints:linux"),
"baz");
addToolchain(
"extra",
"extra_toolchain_mac",
ImmutableList.of("//constraints:mac"),
ImmutableList.of("//constraints:linux"),
"baz");
rewriteWorkspace(
"register_toolchains('//extra:extra_toolchain_linux', '//extra:extra_toolchain_mac')",
"register_execution_platforms('//platforms:mac', '//platforms:linux')");

useConfiguration("--platforms=//platforms:linux");
ToolchainContextKey key =
ToolchainContextKey.key()
.configurationKey(targetConfigKey)
.toolchainTypes(testToolchainType)
.build();

EvaluationResult<UnloadedToolchainContext> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasNoError();
UnloadedToolchainContext unloadedToolchainContext = result.get(key);
assertThat(unloadedToolchainContext).isNotNull();

assertThat(unloadedToolchainContext).hasToolchainType(testToolchainTypeLabel);
assertThat(unloadedToolchainContext).hasResolvedToolchain("//extra:extra_toolchain_mac_impl");
assertThat(unloadedToolchainContext).hasExecutionPlatform("//platforms:mac");
assertThat(unloadedToolchainContext).hasTargetPlatform("//platforms:linux");
}

@Test
public void resolve_multiple() throws Exception {
Label secondToolchainTypeLabel = Label.parseAbsoluteUnchecked("//second:toolchain_type");
ToolchainTypeRequirement secondToolchainTypeRequirement =
ToolchainTypeRequirement.create(secondToolchainTypeLabel);
ToolchainTypeInfo secondToolchainTypeInfo = ToolchainTypeInfo.create(secondToolchainTypeLabel);
scratch.file("second/BUILD", "toolchain_type(name = 'toolchain_type')");

addToolchain(
"main",
"main_toolchain_linux",
ImmutableList.of("//constraints:linux"),
ImmutableList.of("//constraints:linux"),
"baz");
addToolchain(
"main",
"second_toolchain_linux",
secondToolchainTypeLabel,
ImmutableList.of("//constraints:linux"),
ImmutableList.of("//constraints:linux"),
"baz");
rewriteWorkspace(
"register_toolchains('//main:all',)", "register_execution_platforms('//platforms:linux')");

useConfiguration("--platforms=//platforms:linux");
ToolchainContextKey key =
ToolchainContextKey.key()
.configurationKey(targetConfigKey)
.toolchainTypes(testToolchainType, secondToolchainTypeRequirement)
.build();

EvaluationResult<UnloadedToolchainContext> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasNoError();
UnloadedToolchainContext unloadedToolchainContext = result.get(key);
assertThat(unloadedToolchainContext).isNotNull();

assertThat(unloadedToolchainContext).hasToolchainType(testToolchainTypeLabel);
assertThat(unloadedToolchainContext).hasResolvedToolchain("//main:main_toolchain_linux_impl");
assertThat(unloadedToolchainContext).hasToolchainType(secondToolchainTypeLabel);
assertThat(unloadedToolchainContext).hasResolvedToolchain("//main:second_toolchain_linux_impl");
assertThat(unloadedToolchainContext).hasExecutionPlatform("//platforms:linux");
assertThat(unloadedToolchainContext).hasTargetPlatform("//platforms:linux");
}

@Test
public void resolve_mandatory_missing() throws Exception {
// There is no toolchain for the requested type.
Expand All @@ -105,6 +192,91 @@ public void resolve_mandatory_missing() throws Exception {
.contains("no matching toolchains found for types //toolchain:test_toolchain");
}

@Test
public void resolve_multiple_optional() throws Exception {
Label secondToolchainTypeLabel = Label.parseAbsoluteUnchecked("//second:toolchain_type");
ToolchainTypeRequirement secondToolchainTypeRequirement =
ToolchainTypeRequirement.builder(secondToolchainTypeLabel).mandatory(false).build();
ToolchainTypeInfo secondToolchainTypeInfo = ToolchainTypeInfo.create(secondToolchainTypeLabel);
scratch.file("second/BUILD", "toolchain_type(name = 'toolchain_type')");

addToolchain(
"main",
"main_toolchain_linux",
ImmutableList.of("//constraints:linux"),
ImmutableList.of("//constraints:linux"),
"baz");
addToolchain(
"main",
"second_toolchain_linux",
secondToolchainTypeLabel,
ImmutableList.of("//constraints:linux"),
ImmutableList.of("//constraints:linux"),
"baz");
rewriteWorkspace(
"register_toolchains('//main:all',)", "register_execution_platforms('//platforms:linux')");

useConfiguration("--platforms=//platforms:linux");
ToolchainContextKey key =
ToolchainContextKey.key()
.configurationKey(targetConfigKey)
.toolchainTypes(testToolchainType, secondToolchainTypeRequirement)
.build();

EvaluationResult<UnloadedToolchainContext> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasNoError();
UnloadedToolchainContext unloadedToolchainContext = result.get(key);
assertThat(unloadedToolchainContext).isNotNull();

assertThat(unloadedToolchainContext).hasToolchainType(testToolchainTypeLabel);
assertThat(unloadedToolchainContext).hasResolvedToolchain("//main:main_toolchain_linux_impl");
assertThat(unloadedToolchainContext).hasToolchainType(secondToolchainTypeLabel);
assertThat(unloadedToolchainContext).hasResolvedToolchain("//main:second_toolchain_linux_impl");
assertThat(unloadedToolchainContext).hasExecutionPlatform("//platforms:linux");
assertThat(unloadedToolchainContext).hasTargetPlatform("//platforms:linux");
}

@Test
public void resolve_multiple_optional_missing() throws Exception {
Label secondToolchainTypeLabel = Label.parseAbsoluteUnchecked("//second:toolchain_type");
ToolchainTypeRequirement secondToolchainTypeRequirement =
ToolchainTypeRequirement.builder(secondToolchainTypeLabel).mandatory(false).build();
ToolchainTypeInfo secondToolchainTypeInfo = ToolchainTypeInfo.create(secondToolchainTypeLabel);
scratch.file("second/BUILD", "toolchain_type(name = 'toolchain_type')");

addToolchain(
"main",
"main_toolchain_linux",
ImmutableList.of("//constraints:linux"),
ImmutableList.of("//constraints:linux"),
"baz");
rewriteWorkspace(
"register_toolchains('//main:all',)", "register_execution_platforms('//platforms:linux')");

useConfiguration("--platforms=//platforms:linux");
ToolchainContextKey key =
ToolchainContextKey.key()
.configurationKey(targetConfigKey)
.toolchainTypes(testToolchainType, secondToolchainTypeRequirement)
.build();

EvaluationResult<UnloadedToolchainContext> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasNoError();
UnloadedToolchainContext unloadedToolchainContext = result.get(key);
assertThat(unloadedToolchainContext).isNotNull();

assertThat(unloadedToolchainContext).hasToolchainType(testToolchainTypeLabel);
assertThat(unloadedToolchainContext).hasResolvedToolchain("//main:main_toolchain_linux_impl");
assertThat(unloadedToolchainContext).hasToolchainType(secondToolchainTypeLabel);
assertThat(unloadedToolchainContext)
.resolvedToolchainLabels()
.doesNotContain(Label.parseAbsoluteUnchecked("//main:second_toolchain_linux_impl"));
assertThat(unloadedToolchainContext).hasExecutionPlatform("//platforms:linux");
assertThat(unloadedToolchainContext).hasTargetPlatform("//platforms:linux");
}

@Test
public void resolve_toolchainTypeAlias() throws Exception {
addToolchain(
Expand Down Expand Up @@ -220,6 +392,29 @@ public void resolve_unavailableToolchainType_single() throws Exception {
assertContainsEvent("no such target '//fake/toolchain:type_1'");
}

@Test
public void resolve_optional_unavailableToolchainType_single() throws Exception {
reporter.removeHandler(failFastHandler);
scratch.file("fake/toolchain/BUILD", "");
useConfiguration("--host_platform=//platforms:linux", "--platforms=//platforms:linux");
ToolchainContextKey key =
ToolchainContextKey.key()
.configurationKey(targetConfigKey)
.toolchainTypes(optionalToolchainType)
.build();

EvaluationResult<UnloadedToolchainContext> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasNoError();
UnloadedToolchainContext unloadedToolchainContext = result.get(key);
assertThat(unloadedToolchainContext).isNotNull();

assertThat(unloadedToolchainContext).hasToolchainType(optionalToolchainTypeLabel);
assertThat(unloadedToolchainContext).resolvedToolchainLabels().isEmpty();
assertThat(unloadedToolchainContext).hasExecutionPlatform("//platforms:linux");
assertThat(unloadedToolchainContext).hasTargetPlatform("//platforms:linux");
}

@Test
public void resolve_unavailableToolchainType_multiple() throws Exception {
reporter.removeHandler(failFastHandler);
Expand Down
Loading