Skip to content

Commit

Permalink
Update SingleToolchainValue to properly handle missing toolchains.
Browse files Browse the repository at this point in the history 
This can no longer throw an exception, but needs to signal in the
returned value.

Part of Optional Toolchains (#14726).

Closes #15338.

PiperOrigin-RevId: 444343903
  • Loading branch information
@katre @copybara-github
katre authored and copybara-github committed Apr 25, 2022
1 parent 89b42a2 commit 05747cf
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 62 deletions.
28 changes: 3 additions & 25 deletions src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.skyframe.PlatformLookupUtil.InvalidPlatformException;
import com.google.devtools.build.lib.skyframe.RegisteredToolchainsFunction.InvalidToolchainLabelException;
import com.google.devtools.build.lib.skyframe.SingleToolchainResolutionValue.SingleToolchainResolutionKey;
Expand Down Expand Up @@ -90,6 +89,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
// Find the right one.
return resolveConstraints(
key.toolchainType(),
key.toolchainTypeInfo(),
key.availableExecutionPlatformKeys(),
key.targetPlatformKey(),
toolchains.registeredToolchains(),
Expand All @@ -105,6 +105,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
@Nullable
private static SingleToolchainResolutionValue resolveConstraints(
ToolchainTypeRequirement toolchainType,
ToolchainTypeInfo toolchainTypeInfo,
List<ConfiguredTargetKey> availableExecutionPlatformKeys,
ConfiguredTargetKey targetPlatformKey,
ImmutableList<DeclaredToolchainInfo> toolchains,
Expand Down Expand Up @@ -136,7 +137,6 @@ private static SingleToolchainResolutionValue resolveConstraints(
// check whether a platform has already been seen during processing.
Set<ConfiguredTargetKey> platformKeysSeen = new HashSet<>();
ImmutableMap.Builder<ConfiguredTargetKey, Label> builder = ImmutableMap.builder();
ToolchainTypeInfo toolchainTypeInfo = null;

// Pre-filter for the correct toolchain type. This simplifies the loop and makes debugging
// toolchain resolution much, much easier.
Expand Down Expand Up @@ -202,20 +202,18 @@ private static SingleToolchainResolutionValue resolveConstraints(
targetPlatform.label(),
executionPlatformKey.getLabel(),
toolchain.toolchainLabel());
toolchainTypeInfo = toolchain.toolchainType();
builder.put(executionPlatformKey, toolchain.toolchainLabel());
platformKeysSeen.add(executionPlatformKey);
}
}

ImmutableMap<ConfiguredTargetKey, Label> resolvedToolchainLabels = builder.buildOrThrow();
if (toolchainType == null || resolvedToolchainLabels.isEmpty()) {
if (resolvedToolchainLabels.isEmpty()) {
debugMessage(
eventHandler,
" Type %s: target platform %s: No toolchains found.",
toolchainType.toolchainType(),
targetPlatform.label());
throw new ToolchainResolutionFunctionException(new NoToolchainFoundException(toolchainType));
}

return SingleToolchainResolutionValue.create(toolchainTypeInfo, resolvedToolchainLabels);
Expand Down Expand Up @@ -296,30 +294,10 @@ private static boolean checkConstraints(
return mismatchSettingsWithDefault.isEmpty();
}

/** Used to indicate that a toolchain was not found for the current request. */
public static final class NoToolchainFoundException extends NoSuchThingException {
private final ToolchainTypeRequirement missingToolchainType;

public NoToolchainFoundException(ToolchainTypeRequirement missingToolchainType) {
super(
String.format(
"no matching toolchain found for %s", missingToolchainType.toolchainType()));
this.missingToolchainType = missingToolchainType;
}

public ToolchainTypeRequirement missingToolchainType() {
return missingToolchainType;
}
}

/**
* Used to indicate errors during the computation of an {@link SingleToolchainResolutionValue}.
*/
private static final class ToolchainResolutionFunctionException extends SkyFunctionException {
ToolchainResolutionFunctionException(NoToolchainFoundException e) {
super(e, Transience.PERSISTENT);
}

ToolchainResolutionFunctionException(InvalidToolchainLabelException e) {
super(e, Transience.PERSISTENT);
}
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,29 @@ public abstract class SingleToolchainResolutionValue implements SkyValue {
public static SingleToolchainResolutionKey key(
BuildConfigurationKey configurationKey,
ToolchainTypeRequirement toolchainType,
ToolchainTypeInfo toolchainTypeInfo,
ConfiguredTargetKey targetPlatformKey,
List<ConfiguredTargetKey> availableExecutionPlatformKeys) {
return key(
configurationKey, toolchainType, targetPlatformKey, availableExecutionPlatformKeys, false);
configurationKey,
toolchainType,
toolchainTypeInfo,
targetPlatformKey,
availableExecutionPlatformKeys,
false);
}

public static SingleToolchainResolutionKey key(
BuildConfigurationKey configurationKey,
ToolchainTypeRequirement toolchainType,
ToolchainTypeInfo toolchainTypeInfo,
ConfiguredTargetKey targetPlatformKey,
List<ConfiguredTargetKey> availableExecutionPlatformKeys,
boolean debugTarget) {
return SingleToolchainResolutionKey.create(
configurationKey,
toolchainType,
toolchainTypeInfo,
targetPlatformKey,
availableExecutionPlatformKeys,
debugTarget);
Expand All @@ -70,6 +78,8 @@ public SkyFunctionName functionName() {

public abstract ToolchainTypeRequirement toolchainType();

public abstract ToolchainTypeInfo toolchainTypeInfo();

abstract ConfiguredTargetKey targetPlatformKey();

abstract ImmutableList<ConfiguredTargetKey> availableExecutionPlatformKeys();
Expand All @@ -79,12 +89,14 @@ public SkyFunctionName functionName() {
static SingleToolchainResolutionKey create(
BuildConfigurationKey configurationKey,
ToolchainTypeRequirement toolchainType,
ToolchainTypeInfo toolchainTypeInfo,
ConfiguredTargetKey targetPlatformKey,
List<ConfiguredTargetKey> availableExecutionPlatformKeys,
boolean debugTarget) {
return new AutoValue_SingleToolchainResolutionValue_SingleToolchainResolutionKey(
configurationKey,
toolchainType,
toolchainTypeInfo,
targetPlatformKey,
ImmutableList.copyOf(availableExecutionPlatformKeys),
debugTarget);
Expand Down
74 changes: 46 additions & 28 deletions src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java
View file
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.ImmutableList.builder;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.stream.Collectors.joining;
Expand All @@ -40,13 +39,12 @@
import com.google.devtools.build.lib.skyframe.PlatformLookupUtil.InvalidPlatformException;
import com.google.devtools.build.lib.skyframe.RegisteredExecutionPlatformsFunction.InvalidExecutionPlatformLabelException;
import com.google.devtools.build.lib.skyframe.RegisteredToolchainsFunction.InvalidToolchainLabelException;
import com.google.devtools.build.lib.skyframe.SingleToolchainResolutionFunction.NoToolchainFoundException;
import com.google.devtools.build.lib.skyframe.SingleToolchainResolutionValue.SingleToolchainResolutionKey;
import com.google.devtools.build.lib.skyframe.ToolchainTypeLookupUtil.InvalidToolchainTypeException;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -92,15 +90,15 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
// Load the configured target for the toolchain types to ensure that they are valid and
// resolve aliases.
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypeInfos =
loadToolchainTypes(
loadToolchainTypeInfos(
env,
configuration,
key.toolchainTypes().stream()
.map(ToolchainTypeRequirement::toolchainType)
.collect(toImmutableSet()));
builder.setRequestedLabelToToolchainType(resolvedToolchainTypeInfos);
ImmutableSet<ToolchainTypeRequirement> resolvedToolchainTypes =
loadToolchainTypeRequirements(resolvedToolchainTypeInfos, key.toolchainTypes());
ImmutableSet<ToolchainType> resolvedToolchainTypes =
loadToolchainTypes(resolvedToolchainTypeInfos, key.toolchainTypes());

// Create keys for all platforms that will be used, and validate them early.
PlatformKeys platformKeys =
Expand Down Expand Up @@ -152,11 +150,24 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
}
}

@AutoValue
abstract static class ToolchainType {
abstract ToolchainTypeRequirement toolchainTypeRequirement();

abstract ToolchainTypeInfo toolchainTypeInfo();

static ToolchainType create(
ToolchainTypeRequirement toolchainTypeRequirement, ToolchainTypeInfo toolchainTypeInfo) {
return new AutoValue_ToolchainResolutionFunction_ToolchainType(
toolchainTypeRequirement, toolchainTypeInfo);
}
}

/**
* Returns a map from the requested toolchain type Label (after any alias chains) to the {@link
* ToolchainTypeInfo} provider.
*/
private static ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypes(
private static ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypeInfos(
Environment environment,
BuildConfigurationValue configuration,
ImmutableSet<Label> toolchainTypeLabels)
Expand All @@ -182,16 +193,17 @@ private static ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypes(
/**
* Returns a map from the actual post-alias Label to the ToolchainTypeRequirement for that type.
*/
private ImmutableSet<ToolchainTypeRequirement> loadToolchainTypeRequirements(
private ImmutableSet<ToolchainType> loadToolchainTypes(
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypeInfos,
ImmutableSet<ToolchainTypeRequirement> toolchainTypes) {
ImmutableSet.Builder<ToolchainTypeRequirement> resolved = new ImmutableSet.Builder<>();
ImmutableSet.Builder<ToolchainType> resolved = new ImmutableSet.Builder<>();

for (ToolchainTypeRequirement toolchainTypeRequirement : toolchainTypes) {
// Find the actual Label.
Label toolchainTypeLabel = toolchainTypeRequirement.toolchainType();
if (resolvedToolchainTypeInfos.containsKey(toolchainTypeLabel)) {
toolchainTypeLabel = resolvedToolchainTypeInfos.get(toolchainTypeLabel).typeLabel();
ToolchainTypeInfo toolchainTypeInfo = resolvedToolchainTypeInfos.get(toolchainTypeLabel);
if (toolchainTypeInfo != null) {
toolchainTypeLabel = toolchainTypeInfo.typeLabel();
}

// If the labels don't match, re-build the TTR.
Expand All @@ -200,7 +212,7 @@ private ImmutableSet<ToolchainTypeRequirement> loadToolchainTypeRequirements(
toolchainTypeRequirement.toBuilder().toolchainType(toolchainTypeLabel).build();
}

resolved.add(toolchainTypeRequirement);
resolved.add(ToolchainType.create(toolchainTypeRequirement, toolchainTypeInfo));
}
return resolved.build();
}
Expand Down Expand Up @@ -388,7 +400,7 @@ private static boolean filterPlatform(
private static void determineToolchainImplementations(
Environment environment,
BuildConfigurationKey configurationKey,
ImmutableSet<ToolchainTypeRequirement> toolchainTypes,
ImmutableSet<ToolchainType> toolchainTypes,
Optional<ConfiguredTargetKey> forcedExecutionPlatform,
UnloadedToolchainContextImpl.Builder builder,
PlatformKeys platformKeys,
Expand All @@ -399,43 +411,43 @@ private static void determineToolchainImplementations(

// Find the toolchains for the requested toolchain types.
List<SingleToolchainResolutionKey> registeredToolchainKeys = new ArrayList<>();
for (ToolchainTypeRequirement toolchainType : toolchainTypes) {
for (ToolchainType toolchainType : toolchainTypes) {
registeredToolchainKeys.add(
SingleToolchainResolutionValue.key(
configurationKey,
toolchainType,
toolchainType.toolchainTypeRequirement(),
toolchainType.toolchainTypeInfo(),
platformKeys.targetPlatformKey(),
platformKeys.executionPlatformKeys(),
debugTarget));
}

SkyframeIterableResult results =
environment.getOrderedValuesAndExceptions(registeredToolchainKeys);
SkyframeLookupResult results = environment.getValuesAndExceptions(registeredToolchainKeys);
boolean valuesMissing = false;

// Determine the potential set of toolchains.
Table<ConfiguredTargetKey, ToolchainTypeInfo, Label> resolvedToolchains =
HashBasedTable.create();
ImmutableSet.Builder<ToolchainTypeInfo> requiredToolchainTypesBuilder = ImmutableSet.builder();
List<Label> missingToolchains = new ArrayList<>();
while (results.hasNext()) {
try {
SingleToolchainResolutionValue singleToolchainResolutionValue =
(SingleToolchainResolutionValue)
results.nextOrThrow(
NoToolchainFoundException.class, InvalidToolchainLabelException.class);
for (SingleToolchainResolutionKey key : registeredToolchainKeys) {
SingleToolchainResolutionValue singleToolchainResolutionValue =
(SingleToolchainResolutionValue)
results.getOrThrow(key, InvalidToolchainLabelException.class);
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 {
ToolchainTypeInfo requiredToolchainType = singleToolchainResolutionValue.toolchainType();
requiredToolchainTypesBuilder.add(requiredToolchainType);
resolvedToolchains.putAll(
findPlatformsAndLabels(requiredToolchainType, singleToolchainResolutionValue));
} catch (NoToolchainFoundException e) {
// Save the missing type and continue looping to check for more.
missingToolchains.add(e.missingToolchainType().toolchainType());
}
}

Expand All @@ -458,9 +470,15 @@ private static void determineToolchainImplementations(
platformKeys.executionPlatformKeys(),
resolvedToolchains);

ImmutableSet<ToolchainTypeRequirement> toolchainTypeRequirements =
toolchainTypes.stream()
.map(ToolchainType::toolchainTypeRequirement)
.collect(toImmutableSet());
if (selectedExecutionPlatformKey.isEmpty()) {
throw new NoMatchingPlatformException(
toolchainTypes, platformKeys.executionPlatformKeys(), platformKeys.targetPlatformKey());
toolchainTypeRequirements,
platformKeys.executionPlatformKeys(),
platformKeys.targetPlatformKey());
}

Map<ConfiguredTargetKey, PlatformInfo> platforms =
Expand All @@ -471,7 +489,7 @@ private static void determineToolchainImplementations(
throw new ValueMissingException();
}

builder.setToolchainTypes(toolchainTypes);
builder.setToolchainTypes(toolchainTypeRequirements);
builder.setExecutionPlatform(platforms.get(selectedExecutionPlatformKey.get()));
builder.setTargetPlatform(platforms.get(platformKeys.targetPlatformKey()));

Expand Down
25 changes: 17 additions & 8 deletions ...st/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ private EvaluationResult<SingleToolchainResolutionValue> invokeToolchainResoluti
public void testResolution_singleExecutionPlatform() throws Exception {
SkyKey key =
SingleToolchainResolutionValue.key(
targetConfigKey, testToolchainType, linuxCtkey, ImmutableList.of(macCtkey));
targetConfigKey,
testToolchainType,
testToolchainTypeInfo,
linuxCtkey,
ImmutableList.of(macCtkey));
EvaluationResult<SingleToolchainResolutionValue> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasNoError();
Expand All @@ -104,7 +108,11 @@ public void testResolution_multipleExecutionPlatforms() throws Exception {

SkyKey key =
SingleToolchainResolutionValue.key(
targetConfigKey, testToolchainType, linuxCtkey, ImmutableList.of(linuxCtkey, macCtkey));
targetConfigKey,
testToolchainType,
testToolchainTypeInfo,
linuxCtkey,
ImmutableList.of(linuxCtkey, macCtkey));
EvaluationResult<SingleToolchainResolutionValue> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasNoError();
Expand All @@ -125,14 +133,15 @@ public void testResolution_noneFound() throws Exception {

SkyKey key =
SingleToolchainResolutionValue.key(
targetConfigKey, testToolchainType, linuxCtkey, ImmutableList.of(macCtkey));
targetConfigKey,
testToolchainType,
testToolchainTypeInfo,
linuxCtkey,
ImmutableList.of(macCtkey));
EvaluationResult<SingleToolchainResolutionValue> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result)
.hasErrorEntryForKeyThat(key)
.hasExceptionThat()
.hasMessageThat()
.contains("no matching toolchain found for //toolchain:test_toolchain");
SingleToolchainResolutionValue singleToolchainResolutionValue = result.get(key);
assertThat(singleToolchainResolutionValue.availableToolchainLabels()).isEmpty();
}

@Test
Expand Down
Loading

0 comments on commit 05747cf

Please sign in to comment.