Skip to content

Commit

Permalink
Add no_match_error attribute to toolchain_type
Browse files Browse the repository at this point in the history
Fixes #12318

RELNOTES: The new `no_match_error` attribute on `toolchain_type` can be used to show a custom message when no matching toolchain is found for that type, but one is required.

Closes #25134.

PiperOrigin-RevId: 722733008
Change-Id: I2ff51f7524325be1b70314bb2db24c829602f5fc
  • Loading branch information
fmeum authored and copybara-github committed Feb 3, 2025
1 parent 8bcfb06 commit d514705
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:hash_codes",
"//src/main/java/com/google/devtools/build/lib/util:string",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/eval",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@

package com.google.devtools.build.lib.analysis.platform;

import com.google.common.annotations.VisibleForTesting;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.NativeInfo;
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainTypeInfoApi;
import com.google.devtools.build.lib.util.HashCodes;
import java.util.Objects;
import javax.annotation.Nullable;
import net.starlark.java.eval.Printer;

/** A provider that supplies information about a specific toolchain type. */
Expand All @@ -33,13 +36,20 @@ public class ToolchainTypeInfo extends NativeInfo implements ToolchainTypeInfoAp
new BuiltinProvider<ToolchainTypeInfo>(STARLARK_NAME, ToolchainTypeInfo.class) {};

private final Label typeLabel;
@Nullable private final String noneFoundError;

public static ToolchainTypeInfo create(Label typeLabel, @Nullable String noneFoundError) {
return new ToolchainTypeInfo(typeLabel, noneFoundError);
}

@VisibleForTesting
public static ToolchainTypeInfo create(Label typeLabel) {
return new ToolchainTypeInfo(typeLabel);
return new ToolchainTypeInfo(typeLabel, /* noneFoundError= */ null);
}

private ToolchainTypeInfo(Label typeLabel) {
private ToolchainTypeInfo(Label typeLabel, String noneFoundError) {
this.typeLabel = typeLabel;
this.noneFoundError = noneFoundError;
}

@Override
Expand All @@ -52,14 +62,19 @@ public Label typeLabel() {
return typeLabel;
}

@Nullable
public String noneFoundError() {
return noneFoundError;
}

@Override
public void repr(Printer printer) {
printer.append(String.format("ToolchainTypeInfo(%s)", typeLabel));
}

@Override
public int hashCode() {
return Objects.hashCode(typeLabel);
return HashCodes.hashObjects(typeLabel, noneFoundError);
}

@Override
Expand All @@ -68,6 +83,7 @@ public boolean equals(Object other) {
return false;
}

return Objects.equals(typeLabel, otherToolchainTypeInfo.typeLabel);
return Objects.equals(typeLabel, otherToolchainTypeInfo.typeLabel)
&& Objects.equals(noneFoundError, otherToolchainTypeInfo.noneFoundError);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.rules;

import static com.google.devtools.build.lib.packages.Attribute.attr;

import com.google.devtools.build.lib.actions.ActionConflictException;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
Expand All @@ -27,6 +29,7 @@
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode;
import com.google.devtools.build.lib.packages.Type;
import javax.annotation.Nullable;

/**
Expand All @@ -39,7 +42,10 @@ public class ToolchainType implements RuleConfiguredTargetFactory {
public ConfiguredTarget create(RuleContext ruleContext)
throws ActionConflictException, InterruptedException {

ToolchainTypeInfo toolchainTypeInfo = ToolchainTypeInfo.create(ruleContext.getLabel());
String noMatchError = ruleContext.attributes().get("no_match_error", Type.STRING);
ToolchainTypeInfo toolchainTypeInfo =
ToolchainTypeInfo.create(
ruleContext.getLabel(), noMatchError.isEmpty() ? null : noMatchError);

return new RuleConfiguredTargetBuilder(ruleContext)
.addProvider(RunfilesProvider.simple(Runfiles.EMPTY))
Expand All @@ -58,6 +64,12 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
.removeAttribute("licenses")
.removeAttribute("distribs")
.removeAttribute(":action_listener")
/*<!-- #BLAZE_RULE(toolchain_type).ATTRIBUTE(no_match_error) -->
A custom error message to display when no matching toolchain is found for this type.
<!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
.add(
attr("no_match_error", Type.STRING)
.nonconfigurable("low-level attribute, used in platform configuration"))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.SequencedSet;
import java.util.Set;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -225,7 +227,7 @@ private static void determineToolchainImplementations(
// Determine the potential set of toolchains.
Table<ConfiguredTargetKey, ToolchainTypeInfo, Label> resolvedToolchains =
HashBasedTable.create();
List<Label> missingMandatoryToolchains = new ArrayList<>();
SequencedSet<ToolchainTypeInfo> missingMandatoryToolchains = new LinkedHashSet<>();
for (SingleToolchainResolutionKey key : registeredToolchainKeys) {
SingleToolchainResolutionValue singleToolchainResolutionValue =
(SingleToolchainResolutionValue)
Expand All @@ -244,7 +246,7 @@ private static void determineToolchainImplementations(
findPlatformsAndLabels(requiredToolchainType, singleToolchainResolutionValue));
} else if (key.toolchainType().mandatory()) {
// Save the missing type and continue looping to check for more.
missingMandatoryToolchains.add(key.toolchainType().toolchainType());
missingMandatoryToolchains.add(key.toolchainTypeInfo());
}
// TODO(katre): track missing optional toolchains?
}
Expand Down Expand Up @@ -377,7 +379,7 @@ static final class ValueMissingException extends Exception {

/** Exception used when a toolchain type is required but no matching toolchain is found. */
static final class UnresolvedToolchainsException extends ToolchainException {
UnresolvedToolchainsException(List<Label> missingToolchainTypes) {
UnresolvedToolchainsException(SequencedSet<ToolchainTypeInfo> missingToolchainTypes) {
super(getMessage(missingToolchainTypes));
}

Expand All @@ -386,15 +388,29 @@ protected Code getDetailedCode() {
return Code.NO_MATCHING_TOOLCHAIN;
}

private static String getMessage(List<Label> missingToolchainTypes) {
private static String getMessage(SequencedSet<ToolchainTypeInfo> missingToolchainTypes) {
ImmutableList<String> labelStrings =
missingToolchainTypes.stream().map(Label::toString).collect(toImmutableList());
missingToolchainTypes.stream()
.map(ToolchainTypeInfo::typeLabel)
.map(Label::toString)
.collect(toImmutableList());
ImmutableList<String> missingToolchainRows =
missingToolchainTypes.stream()
.map(
type ->
String.format(
" %s%s",
type.typeLabel(),
type.noneFoundError() != null ? ": " + type.noneFoundError() : ""))
.collect(toImmutableList());
return String.format(
"No matching toolchains found for types %s."
+ "\nTo debug, rerun with --toolchain_resolution_debug='%s'"
+ "\nFor more information on platforms or toolchains see "
+ "https://bazel.build/concepts/platforms-intro.",
String.join(", ", labelStrings), String.join("|", labelStrings));
"""
No matching toolchains found for types:
%s
To debug, rerun with --toolchain_resolution_debug='%s'
For more information on platforms or toolchains see https://bazel.build/concepts/platforms-intro.\
""",
String.join("\n", missingToolchainRows), String.join("|", labelStrings));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,13 @@ public void resolve_mandatory_missing() throws Exception {
.hasErrorEntryForKeyThat(key)
.hasExceptionThat()
.hasMessageThat()
.contains("No matching toolchains found for types //toolchain:test_toolchain");
.isEqualTo(
"""
No matching toolchains found for types:
//toolchain:test_toolchain
To debug, rerun with --toolchain_resolution_debug='//toolchain:test_toolchain'
For more information on platforms or toolchains see https://bazel.build/concepts/platforms-intro.\
""");
}

@Test
Expand Down
12 changes: 8 additions & 4 deletions src/test/shell/bazel/bazel_java_test_defaults.sh
Original file line number Diff line number Diff line change
Expand Up @@ -427,11 +427,13 @@ EOF

bazel build --platforms=//pkg:exotic_platform --java_runtime_version=local_jdk \
//pkg:foo &>"$TEST_log" && fail "Build should fail"
expect_log "While resolving toolchains for target //pkg:foo ([0-9a-f]*): No matching toolchains found for types @@bazel_tools//tools/jdk:runtime_toolchain_type"
expect_log "While resolving toolchains for target //pkg:foo ([0-9a-f]*): No matching toolchains found for types:$"
expect_log "^ @@bazel_tools//tools/jdk:runtime_toolchain_type$"

bazel build --platforms=//pkg:exotic_platform --java_runtime_version=local_jdk \
//pkg:foo_deploy.jar &>"$TEST_log" && fail "Build should fail"
expect_log "While resolving toolchains for target //pkg:foo_deploy.jar ([0-9a-f]*): No matching toolchains found for types @@bazel_tools//tools/jdk:runtime_toolchain_type"
expect_log "While resolving toolchains for target //pkg:foo_deploy.jar ([0-9a-f]*): No matching toolchains found for types:$"
expect_log "^ @@bazel_tools//tools/jdk:runtime_toolchain_type$"
}

function test_executable_java_binary_fails_without_runtime_with_remote_jdk() {
Expand All @@ -456,11 +458,13 @@ EOF

bazel build --platforms=//pkg:exotic_platform --java_runtime_version=remotejdk_11 \
//pkg:foo &>"$TEST_log" && fail "Build should fail"
expect_log "While resolving toolchains for target //pkg:foo ([0-9a-f]*): No matching toolchains found for types @@bazel_tools//tools/jdk:runtime_toolchain_type"
expect_log "While resolving toolchains for target //pkg:foo ([0-9a-f]*): No matching toolchains found for types:$"
expect_log "^ @@bazel_tools//tools/jdk:runtime_toolchain_type$"

bazel build --platforms=//pkg:exotic_platform --java_runtime_version=remotejdk_11 \
//pkg:foo_deploy.jar &>"$TEST_log" && fail "Build should fail"
expect_log "While resolving toolchains for target //pkg:foo_deploy.jar ([0-9a-f]*): No matching toolchains found for types @@bazel_tools//tools/jdk:runtime_toolchain_type"
expect_log "While resolving toolchains for target //pkg:foo_deploy.jar ([0-9a-f]*): No matching toolchains found for types:$"
expect_log "^ @@bazel_tools//tools/jdk:runtime_toolchain_type$"
}

run_suite "Java toolchains tests, configured using flags or the default_java_toolchain macro."
38 changes: 35 additions & 3 deletions src/test/shell/integration/toolchain_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,37 @@ use_toolchain(
EOF

bazel build "//${pkg}/demo:use" &> $TEST_log && fail "Build failure expected"
expect_log "While resolving toolchains for target //${pkg}/demo:use[^:]*: No matching toolchains found for types //${pkg}/toolchain:test_toolchain."
expect_log "While resolving toolchains for target //${pkg}/demo:use[^:]*: No matching toolchains found for types:$"
expect_log " //${pkg}/toolchain:test_toolchain$"
}

function test_toolchain_use_in_rule_missing_with_custom_error {
local -r pkg="${FUNCNAME[0]}"
write_test_toolchain "${pkg}" test_toolchain_with_message
cat > "${pkg}/toolchain/BUILD" <<EOF
toolchain_type(
name = 'test_toolchain_with_message',
no_match_error = 'Go register a toolchain!',
)
EOF
write_test_rule "${pkg}" use_toolchain test_toolchain_with_message
# Do not register test_toolchain_with_message to trigger the error.

mkdir -p "${pkg}/demo"
cat > "${pkg}/demo/BUILD" <<EOF
load('//${pkg}/toolchain:rule_use_toolchain.bzl', 'use_toolchain')
package(default_visibility = ["//visibility:public"])
# Use the toolchain.
use_toolchain(
name = 'use',
message = 'this is the rule')
EOF

bazel build "//${pkg}/demo:use" &> $TEST_log && fail "Build failure expected"
expect_log "While resolving toolchains for target //${pkg}/demo:use[^:]*: No matching toolchains found for types:$"
expect_log "^ //${pkg}/toolchain:test_toolchain_with_message: Go register a toolchain!$"
}

function test_multiple_toolchain_use_in_rule {
Expand Down Expand Up @@ -621,7 +651,8 @@ use_toolchains(
EOF

bazel build "//${pkg}/demo:use" &> $TEST_log && fail "Build failure expected"
expect_log "While resolving toolchains for target //${pkg}/demo:use[^:]*: No matching toolchains found for types //${pkg}/toolchain:test_toolchain_2."
expect_log "While resolving toolchains for target //${pkg}/demo:use[^:]*: No matching toolchains found for types:$"
expect_log "^ //${pkg}/toolchain:test_toolchain_2$"
}

function test_toolchain_use_in_rule_non_required_toolchain {
Expand Down Expand Up @@ -961,7 +992,8 @@ EOF
--host_platform="//${pkg}:platform1" \
--platforms="//${pkg}:platform1" \
"//${pkg}/demo:use" &> $TEST_log && fail "Build failure expected"
expect_log "While resolving toolchains for target //${pkg}/demo:use[^:]*: No matching toolchains found for types //${pkg}/toolchain:test_toolchain."
expect_log "While resolving toolchains for target //${pkg}/demo:use[^:]*: No matching toolchains found for types:$"
expect_log "^ //${pkg}/toolchain:test_toolchain$"
expect_not_log 'Using toolchain: rule message:'
}

Expand Down

0 comments on commit d514705

Please sign in to comment.