Skip to content

Commit 05747cf

Browse files
katrecopybara-github
katre
authored and
copybara-github
committed
Update SingleToolchainValue to properly handle missing toolchains.
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
1 parent 89b42a2 commit 05747cf

File tree

5 files changed

+98
-62
lines changed

5 files changed

+98
-62
lines changed

‎src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunction.java‎

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.google.devtools.build.lib.cmdline.Label;
3333
import com.google.devtools.build.lib.events.Event;
3434
import com.google.devtools.build.lib.events.EventHandler;
35-
import com.google.devtools.build.lib.packages.NoSuchThingException;
3635
import com.google.devtools.build.lib.skyframe.PlatformLookupUtil.InvalidPlatformException;
3736
import com.google.devtools.build.lib.skyframe.RegisteredToolchainsFunction.InvalidToolchainLabelException;
3837
import com.google.devtools.build.lib.skyframe.SingleToolchainResolutionValue.SingleToolchainResolutionKey;
@@ -90,6 +89,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
9089
// Find the right one.
9190
return resolveConstraints(
9291
key.toolchainType(),
92+
key.toolchainTypeInfo(),
9393
key.availableExecutionPlatformKeys(),
9494
key.targetPlatformKey(),
9595
toolchains.registeredToolchains(),
@@ -105,6 +105,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
105105
@Nullable
106106
private static SingleToolchainResolutionValue resolveConstraints(
107107
ToolchainTypeRequirement toolchainType,
108+
ToolchainTypeInfo toolchainTypeInfo,
108109
List<ConfiguredTargetKey> availableExecutionPlatformKeys,
109110
ConfiguredTargetKey targetPlatformKey,
110111
ImmutableList<DeclaredToolchainInfo> toolchains,
@@ -136,7 +137,6 @@ private static SingleToolchainResolutionValue resolveConstraints(
136137
// check whether a platform has already been seen during processing.
137138
Set<ConfiguredTargetKey> platformKeysSeen = new HashSet<>();
138139
ImmutableMap.Builder<ConfiguredTargetKey, Label> builder = ImmutableMap.builder();
139-
ToolchainTypeInfo toolchainTypeInfo = null;
140140

141141
// Pre-filter for the correct toolchain type. This simplifies the loop and makes debugging
142142
// toolchain resolution much, much easier.
@@ -202,20 +202,18 @@ private static SingleToolchainResolutionValue resolveConstraints(
202202
targetPlatform.label(),
203203
executionPlatformKey.getLabel(),
204204
toolchain.toolchainLabel());
205-
toolchainTypeInfo = toolchain.toolchainType();
206205
builder.put(executionPlatformKey, toolchain.toolchainLabel());
207206
platformKeysSeen.add(executionPlatformKey);
208207
}
209208
}
210209

211210
ImmutableMap<ConfiguredTargetKey, Label> resolvedToolchainLabels = builder.buildOrThrow();
212-
if (toolchainType == null || resolvedToolchainLabels.isEmpty()) {
211+
if (resolvedToolchainLabels.isEmpty()) {
213212
debugMessage(
214213
eventHandler,
215214
" Type %s: target platform %s: No toolchains found.",
216215
toolchainType.toolchainType(),
217216
targetPlatform.label());
218-
throw new ToolchainResolutionFunctionException(new NoToolchainFoundException(toolchainType));
219217
}
220218

221219
return SingleToolchainResolutionValue.create(toolchainTypeInfo, resolvedToolchainLabels);
@@ -296,30 +294,10 @@ private static boolean checkConstraints(
296294
return mismatchSettingsWithDefault.isEmpty();
297295
}
298296

299-
/** Used to indicate that a toolchain was not found for the current request. */
300-
public static final class NoToolchainFoundException extends NoSuchThingException {
301-
private final ToolchainTypeRequirement missingToolchainType;
302-
303-
public NoToolchainFoundException(ToolchainTypeRequirement missingToolchainType) {
304-
super(
305-
String.format(
306-
"no matching toolchain found for %s", missingToolchainType.toolchainType()));
307-
this.missingToolchainType = missingToolchainType;
308-
}
309-
310-
public ToolchainTypeRequirement missingToolchainType() {
311-
return missingToolchainType;
312-
}
313-
}
314-
315297
/**
316298
* Used to indicate errors during the computation of an {@link SingleToolchainResolutionValue}.
317299
*/
318300
private static final class ToolchainResolutionFunctionException extends SkyFunctionException {
319-
ToolchainResolutionFunctionException(NoToolchainFoundException e) {
320-
super(e, Transience.PERSISTENT);
321-
}
322-
323301
ToolchainResolutionFunctionException(InvalidToolchainLabelException e) {
324302
super(e, Transience.PERSISTENT);
325303
}

‎src/main/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionValue.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,29 @@ public abstract class SingleToolchainResolutionValue implements SkyValue {
3737
public static SingleToolchainResolutionKey key(
3838
BuildConfigurationKey configurationKey,
3939
ToolchainTypeRequirement toolchainType,
40+
ToolchainTypeInfo toolchainTypeInfo,
4041
ConfiguredTargetKey targetPlatformKey,
4142
List<ConfiguredTargetKey> availableExecutionPlatformKeys) {
4243
return key(
43-
configurationKey, toolchainType, targetPlatformKey, availableExecutionPlatformKeys, false);
44+
configurationKey,
45+
toolchainType,
46+
toolchainTypeInfo,
47+
targetPlatformKey,
48+
availableExecutionPlatformKeys,
49+
false);
4450
}
4551

4652
public static SingleToolchainResolutionKey key(
4753
BuildConfigurationKey configurationKey,
4854
ToolchainTypeRequirement toolchainType,
55+
ToolchainTypeInfo toolchainTypeInfo,
4956
ConfiguredTargetKey targetPlatformKey,
5057
List<ConfiguredTargetKey> availableExecutionPlatformKeys,
5158
boolean debugTarget) {
5259
return SingleToolchainResolutionKey.create(
5360
configurationKey,
5461
toolchainType,
62+
toolchainTypeInfo,
5563
targetPlatformKey,
5664
availableExecutionPlatformKeys,
5765
debugTarget);
@@ -70,6 +78,8 @@ public SkyFunctionName functionName() {
7078

7179
public abstract ToolchainTypeRequirement toolchainType();
7280

81+
public abstract ToolchainTypeInfo toolchainTypeInfo();
82+
7383
abstract ConfiguredTargetKey targetPlatformKey();
7484

7585
abstract ImmutableList<ConfiguredTargetKey> availableExecutionPlatformKeys();
@@ -79,12 +89,14 @@ public SkyFunctionName functionName() {
7989
static SingleToolchainResolutionKey create(
8090
BuildConfigurationKey configurationKey,
8191
ToolchainTypeRequirement toolchainType,
92+
ToolchainTypeInfo toolchainTypeInfo,
8293
ConfiguredTargetKey targetPlatformKey,
8394
List<ConfiguredTargetKey> availableExecutionPlatformKeys,
8495
boolean debugTarget) {
8596
return new AutoValue_SingleToolchainResolutionValue_SingleToolchainResolutionKey(
8697
configurationKey,
8798
toolchainType,
99+
toolchainTypeInfo,
88100
targetPlatformKey,
89101
ImmutableList.copyOf(availableExecutionPlatformKeys),
90102
debugTarget);

‎src/main/java/com/google/devtools/build/lib/skyframe/ToolchainResolutionFunction.java‎

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.skyframe;
1515

16-
import static com.google.common.collect.ImmutableList.builder;
1716
import static com.google.common.collect.ImmutableList.toImmutableList;
1817
import static com.google.common.collect.ImmutableSet.toImmutableSet;
1918
import static java.util.stream.Collectors.joining;
@@ -40,13 +39,12 @@
4039
import com.google.devtools.build.lib.skyframe.PlatformLookupUtil.InvalidPlatformException;
4140
import com.google.devtools.build.lib.skyframe.RegisteredExecutionPlatformsFunction.InvalidExecutionPlatformLabelException;
4241
import com.google.devtools.build.lib.skyframe.RegisteredToolchainsFunction.InvalidToolchainLabelException;
43-
import com.google.devtools.build.lib.skyframe.SingleToolchainResolutionFunction.NoToolchainFoundException;
4442
import com.google.devtools.build.lib.skyframe.SingleToolchainResolutionValue.SingleToolchainResolutionKey;
4543
import com.google.devtools.build.lib.skyframe.ToolchainTypeLookupUtil.InvalidToolchainTypeException;
4644
import com.google.devtools.build.skyframe.SkyFunction;
4745
import com.google.devtools.build.skyframe.SkyFunctionException;
4846
import com.google.devtools.build.skyframe.SkyKey;
49-
import com.google.devtools.build.skyframe.SkyframeIterableResult;
47+
import com.google.devtools.build.skyframe.SkyframeLookupResult;
5048
import java.util.ArrayList;
5149
import java.util.List;
5250
import java.util.Map;
@@ -92,15 +90,15 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
9290
// Load the configured target for the toolchain types to ensure that they are valid and
9391
// resolve aliases.
9492
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypeInfos =
95-
loadToolchainTypes(
93+
loadToolchainTypeInfos(
9694
env,
9795
configuration,
9896
key.toolchainTypes().stream()
9997
.map(ToolchainTypeRequirement::toolchainType)
10098
.collect(toImmutableSet()));
10199
builder.setRequestedLabelToToolchainType(resolvedToolchainTypeInfos);
102-
ImmutableSet<ToolchainTypeRequirement> resolvedToolchainTypes =
103-
loadToolchainTypeRequirements(resolvedToolchainTypeInfos, key.toolchainTypes());
100+
ImmutableSet<ToolchainType> resolvedToolchainTypes =
101+
loadToolchainTypes(resolvedToolchainTypeInfos, key.toolchainTypes());
104102

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

153+
@AutoValue
154+
abstract static class ToolchainType {
155+
abstract ToolchainTypeRequirement toolchainTypeRequirement();
156+
157+
abstract ToolchainTypeInfo toolchainTypeInfo();
158+
159+
static ToolchainType create(
160+
ToolchainTypeRequirement toolchainTypeRequirement, ToolchainTypeInfo toolchainTypeInfo) {
161+
return new AutoValue_ToolchainResolutionFunction_ToolchainType(
162+
toolchainTypeRequirement, toolchainTypeInfo);
163+
}
164+
}
165+
155166
/**
156167
* Returns a map from the requested toolchain type Label (after any alias chains) to the {@link
157168
* ToolchainTypeInfo} provider.
158169
*/
159-
private static ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypes(
170+
private static ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypeInfos(
160171
Environment environment,
161172
BuildConfigurationValue configuration,
162173
ImmutableSet<Label> toolchainTypeLabels)
@@ -182,16 +193,17 @@ private static ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypes(
182193
/**
183194
* Returns a map from the actual post-alias Label to the ToolchainTypeRequirement for that type.
184195
*/
185-
private ImmutableSet<ToolchainTypeRequirement> loadToolchainTypeRequirements(
196+
private ImmutableSet<ToolchainType> loadToolchainTypes(
186197
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypeInfos,
187198
ImmutableSet<ToolchainTypeRequirement> toolchainTypes) {
188-
ImmutableSet.Builder<ToolchainTypeRequirement> resolved = new ImmutableSet.Builder<>();
199+
ImmutableSet.Builder<ToolchainType> resolved = new ImmutableSet.Builder<>();
189200

190201
for (ToolchainTypeRequirement toolchainTypeRequirement : toolchainTypes) {
191202
// Find the actual Label.
192203
Label toolchainTypeLabel = toolchainTypeRequirement.toolchainType();
193-
if (resolvedToolchainTypeInfos.containsKey(toolchainTypeLabel)) {
194-
toolchainTypeLabel = resolvedToolchainTypeInfos.get(toolchainTypeLabel).typeLabel();
204+
ToolchainTypeInfo toolchainTypeInfo = resolvedToolchainTypeInfos.get(toolchainTypeLabel);
205+
if (toolchainTypeInfo != null) {
206+
toolchainTypeLabel = toolchainTypeInfo.typeLabel();
195207
}
196208

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

203-
resolved.add(toolchainTypeRequirement);
215+
resolved.add(ToolchainType.create(toolchainTypeRequirement, toolchainTypeInfo));
204216
}
205217
return resolved.build();
206218
}
@@ -388,7 +400,7 @@ private static boolean filterPlatform(
388400
private static void determineToolchainImplementations(
389401
Environment environment,
390402
BuildConfigurationKey configurationKey,
391-
ImmutableSet<ToolchainTypeRequirement> toolchainTypes,
403+
ImmutableSet<ToolchainType> toolchainTypes,
392404
Optional<ConfiguredTargetKey> forcedExecutionPlatform,
393405
UnloadedToolchainContextImpl.Builder builder,
394406
PlatformKeys platformKeys,
@@ -399,43 +411,43 @@ private static void determineToolchainImplementations(
399411

400412
// Find the toolchains for the requested toolchain types.
401413
List<SingleToolchainResolutionKey> registeredToolchainKeys = new ArrayList<>();
402-
for (ToolchainTypeRequirement toolchainType : toolchainTypes) {
414+
for (ToolchainType toolchainType : toolchainTypes) {
403415
registeredToolchainKeys.add(
404416
SingleToolchainResolutionValue.key(
405417
configurationKey,
406-
toolchainType,
418+
toolchainType.toolchainTypeRequirement(),
419+
toolchainType.toolchainTypeInfo(),
407420
platformKeys.targetPlatformKey(),
408421
platformKeys.executionPlatformKeys(),
409422
debugTarget));
410423
}
411424

412-
SkyframeIterableResult results =
413-
environment.getOrderedValuesAndExceptions(registeredToolchainKeys);
425+
SkyframeLookupResult results = environment.getValuesAndExceptions(registeredToolchainKeys);
414426
boolean valuesMissing = false;
415427

416428
// Determine the potential set of toolchains.
417429
Table<ConfiguredTargetKey, ToolchainTypeInfo, Label> resolvedToolchains =
418430
HashBasedTable.create();
419431
ImmutableSet.Builder<ToolchainTypeInfo> requiredToolchainTypesBuilder = ImmutableSet.builder();
420432
List<Label> missingToolchains = new ArrayList<>();
421-
while (results.hasNext()) {
422-
try {
423-
SingleToolchainResolutionValue singleToolchainResolutionValue =
424-
(SingleToolchainResolutionValue)
425-
results.nextOrThrow(
426-
NoToolchainFoundException.class, InvalidToolchainLabelException.class);
433+
for (SingleToolchainResolutionKey key : registeredToolchainKeys) {
434+
SingleToolchainResolutionValue singleToolchainResolutionValue =
435+
(SingleToolchainResolutionValue)
436+
results.getOrThrow(key, InvalidToolchainLabelException.class);
427437
if (singleToolchainResolutionValue == null) {
428438
valuesMissing = true;
429439
continue;
430440
}
431441

442+
if (singleToolchainResolutionValue.availableToolchainLabels().isEmpty()) {
443+
// Save the missing type and continue looping to check for more.
444+
// TODO(katre): Handle mandatory/optional.
445+
missingToolchains.add(key.toolchainType().toolchainType());
446+
} else {
432447
ToolchainTypeInfo requiredToolchainType = singleToolchainResolutionValue.toolchainType();
433448
requiredToolchainTypesBuilder.add(requiredToolchainType);
434449
resolvedToolchains.putAll(
435450
findPlatformsAndLabels(requiredToolchainType, singleToolchainResolutionValue));
436-
} catch (NoToolchainFoundException e) {
437-
// Save the missing type and continue looping to check for more.
438-
missingToolchains.add(e.missingToolchainType().toolchainType());
439451
}
440452
}
441453

@@ -458,9 +470,15 @@ private static void determineToolchainImplementations(
458470
platformKeys.executionPlatformKeys(),
459471
resolvedToolchains);
460472

473+
ImmutableSet<ToolchainTypeRequirement> toolchainTypeRequirements =
474+
toolchainTypes.stream()
475+
.map(ToolchainType::toolchainTypeRequirement)
476+
.collect(toImmutableSet());
461477
if (selectedExecutionPlatformKey.isEmpty()) {
462478
throw new NoMatchingPlatformException(
463-
toolchainTypes, platformKeys.executionPlatformKeys(), platformKeys.targetPlatformKey());
479+
toolchainTypeRequirements,
480+
platformKeys.executionPlatformKeys(),
481+
platformKeys.targetPlatformKey());
464482
}
465483

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

474-
builder.setToolchainTypes(toolchainTypes);
492+
builder.setToolchainTypes(toolchainTypeRequirements);
475493
builder.setExecutionPlatform(platforms.get(selectedExecutionPlatformKey.get()));
476494
builder.setTargetPlatform(platforms.get(platformKeys.targetPlatformKey()));
477495

‎src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,11 @@ private EvaluationResult<SingleToolchainResolutionValue> invokeToolchainResoluti
7878
public void testResolution_singleExecutionPlatform() throws Exception {
7979
SkyKey key =
8080
SingleToolchainResolutionValue.key(
81-
targetConfigKey, testToolchainType, linuxCtkey, ImmutableList.of(macCtkey));
81+
targetConfigKey,
82+
testToolchainType,
83+
testToolchainTypeInfo,
84+
linuxCtkey,
85+
ImmutableList.of(macCtkey));
8286
EvaluationResult<SingleToolchainResolutionValue> result = invokeToolchainResolution(key);
8387

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

105109
SkyKey key =
106110
SingleToolchainResolutionValue.key(
107-
targetConfigKey, testToolchainType, linuxCtkey, ImmutableList.of(linuxCtkey, macCtkey));
111+
targetConfigKey,
112+
testToolchainType,
113+
testToolchainTypeInfo,
114+
linuxCtkey,
115+
ImmutableList.of(linuxCtkey, macCtkey));
108116
EvaluationResult<SingleToolchainResolutionValue> result = invokeToolchainResolution(key);
109117

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

126134
SkyKey key =
127135
SingleToolchainResolutionValue.key(
128-
targetConfigKey, testToolchainType, linuxCtkey, ImmutableList.of(macCtkey));
136+
targetConfigKey,
137+
testToolchainType,
138+
testToolchainTypeInfo,
139+
linuxCtkey,
140+
ImmutableList.of(macCtkey));
129141
EvaluationResult<SingleToolchainResolutionValue> result = invokeToolchainResolution(key);
130142

131-
assertThatEvaluationResult(result)
132-
.hasErrorEntryForKeyThat(key)
133-
.hasExceptionThat()
134-
.hasMessageThat()
135-
.contains("no matching toolchain found for //toolchain:test_toolchain");
143+
SingleToolchainResolutionValue singleToolchainResolutionValue = result.get(key);
144+
assertThat(singleToolchainResolutionValue.availableToolchainLabels()).isEmpty();
136145
}
137146

138147
@Test

0 commit comments

Comments
 (0)