Fix relative label parsing with Starlark toolchain APIs.
- Use a LabelConversionContext in rule.toolchains to improve parsing.
- Update StarlarkToolchainContext to properly resolve labels using LabelConversionContext.
- Also move label conversion into the LabelConversionContext so it can be shared more widely.
PiperOrigin-RevId: 368044826
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 4932b10..4a3d8c1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -2244,6 +2244,7 @@
":resolved_toolchain_context",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
"//src/main/java/net/starlark/java/eval",
"//third_party:auto_value",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
index a6e7bec..8bafe6c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
@@ -46,7 +46,6 @@
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute;
@@ -57,6 +56,7 @@
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.BuildSetting;
import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.BuildType.LabelConversionContext;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.packages.ExecGroup;
import com.google.devtools.build.lib.packages.FunctionSplitTransitionAllowlist;
@@ -477,19 +477,22 @@
* Parses a sequence of label strings with a repo mapping.
*
* @param inputs sequence of input strings
- * @param mapping repository mapping
+ * @param thread repository mapping
* @param adjective describes the purpose of the label; used for errors
* @throws EvalException if the label can't be parsed
*/
private static ImmutableList<Label> parseLabels(
- Iterable<String> inputs,
- ImmutableMap<RepositoryName, RepositoryName> mapping,
- String adjective)
- throws EvalException {
+ Iterable<String> inputs, StarlarkThread thread, String adjective) throws EvalException {
ImmutableList.Builder<Label> parsedLabels = new ImmutableList.Builder<>();
+ BazelStarlarkContext bazelStarlarkContext = BazelStarlarkContext.from(thread);
+ LabelConversionContext context =
+ new LabelConversionContext(
+ BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread)).label(),
+ bazelStarlarkContext.getRepoMapping(),
+ bazelStarlarkContext.getConvertedLabelsInPackage());
for (String input : inputs) {
try {
- Label label = Label.parseAbsolute(input, mapping);
+ Label label = context.convert(input);
parsedLabels.add(label);
} catch (LabelSyntaxException e) {
throw Starlark.errorf(
@@ -501,18 +504,13 @@
private static ImmutableList<Label> parseToolchains(Sequence<?> inputs, StarlarkThread thread)
throws EvalException {
- return parseLabels(
- Sequence.cast(inputs, String.class, "toolchains"),
- BazelStarlarkContext.from(thread).getRepoMapping(),
- "toolchain");
+ return parseLabels(Sequence.cast(inputs, String.class, "toolchains"), thread, "toolchain");
}
private static ImmutableList<Label> parseExecCompatibleWith(
Sequence<?> inputs, StarlarkThread thread) throws EvalException {
return parseLabels(
- Sequence.cast(inputs, String.class, "exec_compatible_with"),
- BazelStarlarkContext.from(thread).getRepoMapping(),
- "constraint");
+ Sequence.cast(inputs, String.class, "exec_compatible_with"), thread, "constraint");
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java
index d195acc..a75e6f3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkToolchainContext.java
@@ -21,9 +21,13 @@
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.packages.BazelModuleContext;
+import com.google.devtools.build.lib.packages.BazelStarlarkContext;
+import com.google.devtools.build.lib.packages.BuildType.LabelConversionContext;
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Module;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkSemantics;
@@ -84,9 +88,15 @@
return ((ToolchainTypeInfo) key).typeLabel();
} else if (key instanceof String) {
try {
- // TODO(jcater): Use LabelConversionContext and StarlarkThread to convert this instead.
- // Also remove repoMapping from ResolvedToolchainContext.
- return Label.parseAbsolute((String) key, toolchainContext().repoMapping());
+ BazelStarlarkContext bazelStarlarkContext = BazelStarlarkContext.from(starlarkThread);
+ LabelConversionContext context =
+ new LabelConversionContext(
+ BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(starlarkThread))
+ .label(),
+ bazelStarlarkContext.getRepoMapping(),
+ bazelStarlarkContext.getConvertedLabelsInPackage());
+
+ return context.convert((String) key);
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Unable to parse toolchain label '%s': %s", key, e.getMessage());
}
diff --git a/src/test/shell/bazel/toolchain_test.sh b/src/test/shell/bazel/toolchain_test.sh
index d7ef1e3..2a21543 100755
--- a/src/test/shell/bazel/toolchain_test.sh
+++ b/src/test/shell/bazel/toolchain_test.sh
@@ -2135,6 +2135,104 @@
expect_log 'Inner toolchain //inner:impl'
}
+# Test that toolchain type labels are correctly resolved relative to the
+# enclosing file, regardless of the repository name.
+# See http://b/183060658 for details.
+function test_repository_relative_toolchain_type() {
+ # Create a repository that defines a toolchain type and simple rule.
+ # The toolchain type used in the repository is relative to the repository.
+ mkdir -p external/rules_foo
+ touch external/rules_foo/WORKSPACE
+ mkdir -p external/rules_foo/rule
+ touch external/rules_foo/rule/BUILD
+ cat > external/rules_foo/rule/rule.bzl <<EOF
+def _foo_impl(ctx):
+ print(ctx.toolchains["//toolchain:foo_toolchain_type"])
+ return []
+
+foo_rule = rule(
+ implementation = _foo_impl,
+ toolchains = ["//toolchain:foo_toolchain_type"],
+)
+EOF
+ mkdir -p external/rules_foo/toolchain/
+ cat > external/rules_foo/toolchain/BUILD <<EOF
+load(":toolchain.bzl", "foo_toolchain")
+
+toolchain_type(
+ name = "foo_toolchain_type",
+ visibility = ["//visibility:public"],
+)
+
+foo_toolchain(
+ name = "foo_toolchain",
+ visibility = ["//visibility:public"],
+)
+
+toolchain(
+ name = "foo_default_toolchain",
+ toolchain = ":foo_toolchain",
+ toolchain_type = ":foo_toolchain_type",
+)
+EOF
+ cat > external/rules_foo/toolchain/toolchain.bzl <<EOF
+_ATTRS = dict(
+ foo_tool = attr.label(
+ allow_files = True,
+ default = "//foo_tools:foo_tool",
+ ),
+)
+
+def _impl(ctx):
+ return [platform_common.ToolchainInfo(
+ **{name: getattr(ctx.attr, name) for name in _ATTRS.keys()}
+ )]
+
+foo_toolchain = rule(
+ implementation = _impl,
+ attrs = _ATTRS,
+)
+EOF
+ mkdir -p external/rules_foo/foo_tools
+ cat > external/rules_foo/foo_tools/BUILD <<EOF
+sh_binary(
+ name = "foo_tool",
+ srcs = ["foo_tool.sh"],
+ visibility = ["//visibility:public"],
+)
+EOF
+ cat > external/rules_foo/foo_tools/foo_tool.sh <<EOF
+echo creating \$1
+touch \$1
+EOF
+ chmod +x external/rules_foo/foo_tools/foo_tool.sh
+
+ # Create a target that uses the rule.
+ mkdir -p demo
+ cat > demo/BUILD <<EOF
+load("@rules_foo//rule:rule.bzl", "foo_rule")
+
+foo_rule(name = "demo")
+EOF
+
+ # Set up the WORKSPACE.
+ cat >> WORKSPACE <<EOF
+local_repository(
+ name = "rules_foo",
+ path = "external/rules_foo",
+)
+
+register_toolchains(
+ "@rules_foo//toolchain:foo_default_toolchain",
+)
+EOF
+
+ # Test the build.
+ bazel build \
+ //demo:demo &> $TEST_log || fail "Build failed"
+ expect_log "foo_tool = <target @rules_foo//foo_tools:foo_tool>"
+}
+
# TODO(katre): Test using toolchain-provided make variables from a genrule.
run_suite "toolchain tests"
