Skip to content

Commit

Permalink
Report the toolchain rather than <lang>_toolchain label in debug log
Browse files Browse the repository at this point in the history
When debugging toolchain resolution, there is an important difference between a `toolchain` target (more relevant, as it specifies the constraints checked during resolution) and the `<lang>_toolchain` target it resolves to if selected (less relevant during resolution and also potentially shared between multiple `toolchain` targets with different constraints).

This change starts tracking both labels and emits them in the debug logs where it makes sense.

Closes bazelbuild#25109.

PiperOrigin-RevId: 721050780
Change-Id: I03942ebe5bc137e7e5886ec71607f41d5777a846
  • Loading branch information
fmeum authored and copybara-github committed Jan 29, 2025
1 parent f1cdde3 commit 4d29590
Show file tree
Hide file tree
Showing 13 changed files with 108 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,32 +35,37 @@
* @param execConstraints The constraints describing the execution environment.
* @param targetConstraints The constraints describing the target environment.
* @param targetSettings The setting, that target build configuration needs to satisfy.
* @param toolchainLabel The label of the toolchain to resolve for use in toolchain-aware rules.
* @param targetLabel The label of the {@code toolchain} target itself.
* @param resolvedToolchainLabel The label of the toolchain to resolve for use in toolchain-aware
* rules.
*/
@AutoCodec
public record DeclaredToolchainInfo(
ToolchainTypeInfo toolchainType,
ConstraintCollection execConstraints,
ConstraintCollection targetConstraints,
ImmutableList<ConfigMatchingProvider> targetSettings,
Label toolchainLabel)
Label targetLabel,
Label resolvedToolchainLabel)
implements TransitiveInfoProvider {
public DeclaredToolchainInfo {
requireNonNull(toolchainType, "toolchainType");
requireNonNull(execConstraints, "execConstraints");
requireNonNull(targetConstraints, "targetConstraints");
requireNonNull(targetSettings, "targetSettings");
requireNonNull(toolchainLabel, "toolchainLabel");
requireNonNull(targetLabel, "targetLabel");
requireNonNull(resolvedToolchainLabel, "resolvedToolchainLabel");
}

/** Builder class to assist in creating {@link DeclaredToolchainInfo} instances. */
public static class Builder {
private ToolchainTypeInfo toolchainType;
private ConstraintCollection.Builder execConstraints = ConstraintCollection.builder();
private ConstraintCollection.Builder targetConstraints = ConstraintCollection.builder();
private ImmutableList.Builder<ConfigMatchingProvider> targetSettings =
private final ConstraintCollection.Builder execConstraints = ConstraintCollection.builder();
private final ConstraintCollection.Builder targetConstraints = ConstraintCollection.builder();
private final ImmutableList.Builder<ConfigMatchingProvider> targetSettings =
new ImmutableList.Builder<>();
private Label toolchainLabel;
private Label targetLabel;
private Label resolvedToolchainLabel;

/** Sets the type of the toolchain being declared. */
@CanIgnoreReturnValue
Expand All @@ -77,6 +82,7 @@ public Builder addExecConstraints(Iterable<ConstraintValueInfo> constraints) {
}

/** Adds constraints describing the execution environment. */
@CanIgnoreReturnValue
public Builder addExecConstraints(ConstraintValueInfo... constraints) {
return addExecConstraints(ImmutableList.copyOf(constraints));
}
Expand All @@ -89,6 +95,7 @@ public Builder addTargetConstraints(Iterable<ConstraintValueInfo> constraints) {
}

/** Adds constraints describing the target environment. */
@CanIgnoreReturnValue
public Builder addTargetConstraints(ConstraintValueInfo... constraints) {
return addTargetConstraints(ImmutableList.copyOf(constraints));
}
Expand All @@ -99,10 +106,17 @@ public Builder addTargetSettings(Iterable<ConfigMatchingProvider> targetSettings
return this;
}

/** Sets the label of the {@code toolchain} target itself. */
@CanIgnoreReturnValue
public Builder targetLabel(Label targetLabel) {
this.targetLabel = targetLabel;
return this;
}

/** Sets the label of the toolchain to resolve for use in toolchain-aware rules. */
@CanIgnoreReturnValue
public Builder toolchainLabel(Label toolchainLabel) {
this.toolchainLabel = toolchainLabel;
public Builder resolvedToolchainLabel(Label resolvedToolchainLabel) {
this.resolvedToolchainLabel = resolvedToolchainLabel;
return this;
}

Expand Down Expand Up @@ -134,7 +148,8 @@ public DeclaredToolchainInfo build() throws DuplicateConstraintException {
execConstraints,
targetConstraints,
targetSettings.build(),
toolchainLabel);
targetLabel,
resolvedToolchainLabel);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleContext.getPrerequisites(ToolchainRule.TARGET_SETTING_ATTR).stream()
.map(target -> target.getProvider(ConfigMatchingProvider.class))
.collect(toImmutableList());
Label toolchainLabel =
Label resolvedToolchainLabel =
ruleContext.attributes().get(ToolchainRule.TOOLCHAIN_ATTR, BuildType.NODEP_LABEL);

DeclaredToolchainInfo registeredToolchain;
Expand All @@ -66,7 +66,8 @@ public ConfiguredTarget create(RuleContext ruleContext)
.addExecConstraints(execConstraints)
.addTargetConstraints(targetConstraints)
.addTargetSettings(targetSettings)
.toolchainLabel(toolchainLabel)
.resolvedToolchainLabel(resolvedToolchainLabel)
.targetLabel(ruleContext.getLabel())
.build();
} catch (DeclaredToolchainInfo.DuplicateConstraintException e) {
if (e.execConstraintsException() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,15 @@ public SkyValue compute(SkyKey skyKey, Environment env)
key.debug()
? message ->
rejectedToolchains.put(
toolchain.toolchainType().typeLabel(), toolchain.toolchainLabel(), message)
toolchain.toolchainType().typeLabel(), toolchain.targetLabel(), message)
: null;
if (ConfigMatchingUtil.validate(
toolchain.toolchainLabel(), toolchain.targetSettings(), errorHandler)) {
toolchain.targetLabel(), toolchain.targetSettings(), errorHandler)) {
validToolchains.add(toolchain);
}
} catch (InvalidConfigurationException e) {
throw new RegisteredToolchainsFunctionException(
new InvalidToolchainLabelException(toolchain.toolchainLabel(), e),
Transience.PERSISTENT);
new InvalidToolchainLabelException(toolchain.targetLabel(), e), Transience.PERSISTENT);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@
* A value which represents every toolchain known to Bazel and available for toolchain resolution.
*
* @param rejectedToolchains Any toolchains that were rejected, along with a reason. The row keys
* are the toolchain type labels, column keys are toolchain implementation labels, and cells are
* the reason. Only non-null if {@link RegisteredToolchainsValue.Key#debug} is {@code true}.
* are the toolchain type labels, column keys are toolchain target (not implementation) labels,
* and cells are the reason. Only non-null if {@link RegisteredToolchainsValue.Key#debug} is
* {@code true}.
*/
@AutoCodec
public record RegisteredToolchainsValue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static SingleToolchainResolutionDebugPrinter create(

void startToolchainResolution(Label toolchainType, Label targetPlatform);

void reportCompatibleTargetPlatform(Label toolchainLabel);
void reportCompatibleTargetPlatform(Label targetLabel, Label resolvedToolchainLabel);

void reportSkippedExecutionPlatformSeen(Label executionPlatform);

Expand All @@ -63,7 +63,8 @@ void reportMismatchedSettings(
ConstraintCollection toolchainConstraints,
boolean isTargetPlatform,
PlatformInfo platform,
Label toolchainLabel,
Label targetLabel,
Label resolvedToolchainLabel,
ImmutableSet<ConstraintSettingInfo> mismatchSettingsWithDefault);

void reportDone(Label toolchainType);
Expand All @@ -83,7 +84,7 @@ public void describeRejectedToolchains(ImmutableMap<Label, String> rejectedToolc
public void startToolchainResolution(Label toolchainType, Label targetPlatform) {}

@Override
public void reportCompatibleTargetPlatform(Label toolchainLabel) {}
public void reportCompatibleTargetPlatform(Label targetLabel, Label resolvedToolchainLabel) {}

@Override
public void reportSkippedExecutionPlatformSeen(Label executionPlatform) {}
Expand All @@ -106,7 +107,8 @@ public void reportMismatchedSettings(
ConstraintCollection toolchainConstraints,
boolean isTargetPlatform,
PlatformInfo platform,
Label toolchainLabel,
Label targetLabel,
Label resolvedToolchainLabel,
ImmutableSet<ConstraintSettingInfo> mismatchSettingsWithDefault) {}

@Override
Expand Down Expand Up @@ -173,11 +175,13 @@ public void startToolchainResolution(Label toolchainType, Label targetPlatform)
}

@Override
public void reportCompatibleTargetPlatform(Label toolchainLabel) {
public void reportCompatibleTargetPlatform(Label targetLabel, Label resolvedToolchainLabel) {
debugMessage(
IndentLevel.TOOLCHAIN_LEVEL,
"Toolchain %s is compatible with target platform, searching for execution platforms:",
toolchainLabel);
"Toolchain %s (resolves to %s) is compatible with target platform, searching for"
+ " execution platforms:",
targetLabel,
resolvedToolchainLabel);
}

@Override
Expand Down Expand Up @@ -225,11 +229,11 @@ public void reportResolvedToolchains(
toolchainType,
targetPlatform);
resolvedToolchains.forEach(
(executionPlatformKey, toolchainLabel) ->
(executionPlatformKey, resolvedToolchainLabel) ->
debugMessage(
IndentLevel.TOOLCHAIN_LEVEL,
"Selected %s to run on execution platform %s",
toolchainLabel,
resolvedToolchainLabel,
executionPlatformKey.getLabel()));
}
}
Expand All @@ -239,7 +243,8 @@ public void reportMismatchedSettings(
ConstraintCollection toolchainConstraints,
boolean isTargetPlatform,
PlatformInfo platform,
Label toolchainLabel,
Label targetLabel,
Label resolvedToolchainLabel,
ImmutableSet<ConstraintSettingInfo> mismatchSettingsWithDefault) {
if (!mismatchSettingsWithDefault.isEmpty()) {
String mismatchValues =
Expand All @@ -262,8 +267,9 @@ public void reportMismatchedSettings(
if (isTargetPlatform) {
debugMessage(
IndentLevel.TOOLCHAIN_LEVEL,
"Rejected toolchain %s%s",
toolchainLabel,
"Rejected toolchain %s (resolves to %s) %s",
targetLabel,
resolvedToolchainLabel,
mismatchValues + missingSettings);
} else {
debugMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,13 @@ private static SingleToolchainResolutionValue resolveConstraints(
toolchain.targetConstraints(),
/* isTargetPlatform= */ true,
targetPlatform,
toolchain.toolchainLabel())) {
toolchain.targetLabel(),
toolchain.resolvedToolchainLabel())) {
continue;
}

debugPrinter.reportCompatibleTargetPlatform(toolchain.toolchainLabel());
debugPrinter.reportCompatibleTargetPlatform(
toolchain.targetLabel(), toolchain.resolvedToolchainLabel());

boolean done = true;

Expand Down Expand Up @@ -201,14 +203,15 @@ private static SingleToolchainResolutionValue resolveConstraints(
toolchain.execConstraints(),
/* isTargetPlatform= */ false,
executionPlatform,
toolchain.toolchainLabel())) {
toolchain.targetLabel(),
toolchain.resolvedToolchainLabel())) {
// Keep looking for a valid toolchain for this exec platform
done = false;
continue;
}

debugPrinter.reportCompatibleExecutionPlatform(executionPlatformKey.getLabel());
builder.put(executionPlatformKey, toolchain.toolchainLabel());
builder.put(executionPlatformKey, toolchain.resolvedToolchainLabel());
platformKeysSeen.add(executionPlatformKey);
}

Expand All @@ -234,7 +237,8 @@ private static boolean checkConstraints(
ConstraintCollection toolchainConstraints,
boolean isTargetPlatform,
PlatformInfo platform,
Label toolchainLabel) {
Label targetLabel,
Label resolvedToolchainLabel) {

// Check every constraint_setting in either the toolchain or the platform.
ImmutableSet<ConstraintSettingInfo> mismatchSettings =
Expand All @@ -257,7 +261,8 @@ private static boolean checkConstraints(
toolchainConstraints,
isTargetPlatform,
platform,
toolchainLabel,
targetLabel,
resolvedToolchainLabel,
mismatchSettingsWithDefault);

return mismatchSettingsWithDefault.isEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ public void toolchainInfo_overlappingConstraintsError() throws Exception {
ConstraintValueInfo.create(setting2, Label.parseCanonicalUnchecked("//constraint:value5")));

DeclaredToolchainInfo.DuplicateConstraintException exception =
assertThrows(
DeclaredToolchainInfo.DuplicateConstraintException.class, () -> builder.build());
assertThrows(DeclaredToolchainInfo.DuplicateConstraintException.class, builder::build);
assertThat(exception.execConstraintsException()).isNotNull();
assertThat(exception.execConstraintsException())
.hasMessageThat()
Expand Down Expand Up @@ -86,14 +85,16 @@ public void toolchainInfo_equalsTester() throws Exception {
ToolchainTypeInfo.create(Label.parseCanonicalUnchecked("//toolchain:tc1")))
.addExecConstraints(ImmutableList.of(constraint1))
.addTargetConstraints(ImmutableList.of(constraint2))
.toolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.resolvedToolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain_def1"))
.targetLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.build(),
DeclaredToolchainInfo.builder()
.toolchainType(
ToolchainTypeInfo.create(Label.parseCanonicalUnchecked("//toolchain:tc1")))
.addExecConstraints(ImmutableList.of(constraint1))
.addTargetConstraints(ImmutableList.of(constraint2))
.toolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.resolvedToolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain_def1"))
.targetLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.build())
.addEqualityGroup(
// Different type.
Expand All @@ -102,7 +103,8 @@ public void toolchainInfo_equalsTester() throws Exception {
ToolchainTypeInfo.create(Label.parseCanonicalUnchecked("//toolchain:tc2")))
.addExecConstraints(ImmutableList.of(constraint1))
.addTargetConstraints(ImmutableList.of(constraint2))
.toolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.resolvedToolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain_def1"))
.targetLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.build())
.addEqualityGroup(
// Different constraints.
Expand All @@ -111,7 +113,8 @@ public void toolchainInfo_equalsTester() throws Exception {
ToolchainTypeInfo.create(Label.parseCanonicalUnchecked("//toolchain:tc1")))
.addExecConstraints(ImmutableList.of(constraint2))
.addTargetConstraints(ImmutableList.of(constraint1))
.toolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.resolvedToolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain_def1"))
.targetLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.build())
.addEqualityGroup(
// Different toolchain label.
Expand All @@ -120,7 +123,18 @@ public void toolchainInfo_equalsTester() throws Exception {
ToolchainTypeInfo.create(Label.parseCanonicalUnchecked("//toolchain:tc1")))
.addExecConstraints(ImmutableList.of(constraint1))
.addTargetConstraints(ImmutableList.of(constraint2))
.toolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain2"))
.resolvedToolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain_def2"))
.targetLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain1"))
.build())
.addEqualityGroup(
// Different target label.
DeclaredToolchainInfo.builder()
.toolchainType(
ToolchainTypeInfo.create(Label.parseCanonicalUnchecked("//toolchain:tc1")))
.addExecConstraints(ImmutableList.of(constraint1))
.addTargetConstraints(ImmutableList.of(constraint2))
.resolvedToolchainLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain_def1"))
.targetLabel(Label.parseCanonicalUnchecked("//toolchain:toolchain2"))
.build())
.testEquals();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void testMacOsToolchainSetup() throws Exception {
DeclaredToolchainInfo darwinToolchainInfo =
PlatformProviderUtils.declaredToolchainInfo(darwinToolchain);
assertThat(darwinToolchainInfo).isNotNull();
assertThat(darwinToolchainInfo.toolchainLabel())
assertThat(darwinToolchainInfo.resolvedToolchainLabel())
.isEqualTo(
Label.parseCanonicalUnchecked(
"//" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL_DIR + ":cc-compiler-darwin_x86_64"));
Expand All @@ -73,7 +73,7 @@ public void testIosDeviceToolchainSetup() throws Exception {
DeclaredToolchainInfo iosDeviceToolchainInfo =
PlatformProviderUtils.declaredToolchainInfo(iosDeviceToolchain);
assertThat(iosDeviceToolchainInfo).isNotNull();
assertThat(iosDeviceToolchainInfo.toolchainLabel())
assertThat(iosDeviceToolchainInfo.resolvedToolchainLabel())
.isEqualTo(
Label.parseCanonicalUnchecked(
"//" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL_DIR + ":cc-compiler-ios_arm64"));
Expand Down
Loading

0 comments on commit 4d29590

Please sign in to comment.