Skip to content

Commit

Permalink
Only report rejected toolchains of the correct type.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 681117450
Change-Id: Ic90c873ca847686f0bc0c0c2a64c7b3ec6050a47
  • Loading branch information
katre authored and copybara-github committed Oct 1, 2024
1 parent fd67506 commit 15de2aa
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import static java.util.stream.Collectors.joining;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Table;
import com.google.devtools.build.lib.actions.ActionLookupKey;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetValue;
Expand Down Expand Up @@ -156,8 +158,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)

// Check which toolchains are valid according to their configuration.
ImmutableList.Builder<DeclaredToolchainInfo> validToolchains = new ImmutableList.Builder<>();
ImmutableMap.Builder<Label, String> rejectedToolchains =
key.debug() ? new ImmutableMap.Builder<>() : null;
// Some toolchains end up with repeated reasons, so use a HashBasedTable to handle duplicates.
Table<Label, Label, String> rejectedToolchains = key.debug() ? HashBasedTable.create() : null;
for (DeclaredToolchainInfo toolchain : registeredToolchains) {
try {
validate(toolchain, validToolchains, rejectedToolchains);
Expand All @@ -170,7 +172,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)

return RegisteredToolchainsValue.create(
validToolchains.build(),
rejectedToolchains != null ? rejectedToolchains.buildKeepingLast() : null);
rejectedToolchains != null ? ImmutableTable.copyOf(rejectedToolchains) : null);
}

/**
Expand Down Expand Up @@ -282,7 +284,7 @@ private static ImmutableList<DeclaredToolchainInfo> configureRegisteredToolchain
private static void validate(
DeclaredToolchainInfo declaredToolchainInfo,
ImmutableList.Builder<DeclaredToolchainInfo> validToolchains,
ImmutableMap.Builder<Label, String> rejectedToolchains)
Table<Label, Label, String> rejectedToolchains)
throws InvalidConfigurationException {
// Make sure the target setting matches but watch out for resolution errors.
AccumulateResults accumulateResults =
Expand All @@ -308,9 +310,15 @@ private static void validate(
validToolchains.add(declaredToolchainInfo);
} else if (!accumulateResults.nonMatching().isEmpty() && rejectedToolchains != null) {
String nonMatchingList =
accumulateResults.nonMatching().stream().map(Label::getName).collect(joining(", "));
accumulateResults.nonMatching().stream()
.distinct()
.map(Label::getName)
.collect(joining(", "));
String message = String.format("mismatching config settings: %s", nonMatchingList);
rejectedToolchains.put(declaredToolchainInfo.toolchainLabel(), message);
rejectedToolchains.put(
declaredToolchainInfo.toolchainType().typeLabel(),
declaredToolchainInfo.toolchainLabel(),
message);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableTable;
import com.google.devtools.build.lib.analysis.platform.DeclaredToolchainInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
Expand Down Expand Up @@ -108,16 +108,17 @@ public final SkyKeyInterner<Key> getSkyKeyInterner() {

public static RegisteredToolchainsValue create(
ImmutableList<DeclaredToolchainInfo> registeredToolchains,
@Nullable ImmutableMap<Label, String> rejectedToolchains) {
@Nullable ImmutableTable<Label, Label, String> rejectedToolchains) {
return new AutoValue_RegisteredToolchainsValue(registeredToolchains, rejectedToolchains);
}

public abstract ImmutableList<DeclaredToolchainInfo> registeredToolchains();

/**
* Any toolchains that were rejected, along with a reason. Only non-null if {@link
* RegisteredToolchainsValue.Key#debug} is {@code true}.
* 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}.
*/
@Nullable
public abstract ImmutableMap<Label, String> rejectedToolchains();
public abstract ImmutableTable<Label, Label, String> rejectedToolchains();
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (debug
&& toolchains.rejectedToolchains() != null
&& !toolchains.rejectedToolchains().isEmpty()) {
for (Map.Entry<Label, String> entry : toolchains.rejectedToolchains().entrySet()) {
ImmutableMap<Label, String> rejectedToolchainImplementations =
toolchains.rejectedToolchains().row(key.toolchainType().toolchainType());
for (Map.Entry<Label, String> entry : rejectedToolchainImplementations.entrySet()) {
Label toolchainLabel = entry.getKey();
String message = entry.getValue();
debugMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,8 @@ public void testRegisteredToolchains_targetSetting_debug() throws Exception {
RegisteredToolchainsValue registeredToolchainsValue = result.get(toolchainsKey);
assertThat(registeredToolchainsValue.rejectedToolchains()).isNotNull();
assertThat(registeredToolchainsValue.rejectedToolchains())
.containsEntry(
.containsCell(
testToolchainTypeLabel,
Label.parseCanonicalUnchecked("//extra:extra_toolchain_impl"),
"mismatching config settings: optimized");
}
Expand Down

0 comments on commit 15de2aa

Please sign in to comment.