Fail `Label` call referencing invalid repository with strict visibility
With Bzlmod's enforcement of strict visibility of repositories based on repository mapping, a `Label` created from a repo-absolute (but not canonical) label string such as `@repo//foo:bar` will be in an error state if there is no repository visible from the calling .bzl module under the apparent name `repo`.
Before this commit, such instances of `Label` could be freely constructed and used from Starlark, even though some of their fields return nonsensical values (for example, `workspace_root` will be `"external"` for `Label("@//foo:bar")` called from a repo without visibility into the main repository).
This commit ensures that instead the `Label` call (and also analogous calls to `relative`) fail with an error such as:
```
Invalid label string '@//foo:bar': no repository visible as '@' from repository '@current_repo'
```
Fixes #16528
Closes #16530.
PiperOrigin-RevId: 483373288
Change-Id: I4831e179a2d0363aa2f27ca4a25d79a634103f8f
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 0c11cec..b43e9d7 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
@@ -1004,7 +1004,15 @@
BazelModuleContext moduleContext =
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
try {
- return Label.parseWithRepoContext(labelString, moduleContext.packageContext());
+ Label label = Label.parseWithRepoContext(labelString, moduleContext.packageContext());
+ if (!label.getRepository().isVisible()) {
+ throw Starlark.errorf(
+ "Invalid label string '%s': no repository visible as '@%s' from %s",
+ labelString,
+ label.getRepository().getName(),
+ label.getRepository().getOwnerRepoDisplayString());
+ }
+ return label;
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Illegal absolute label syntax: %s", e.getMessage());
}
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index 42507f2..7105afc 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -37,8 +37,10 @@
import net.starlark.java.annot.Param;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
+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;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.StarlarkValue;
@@ -502,10 +504,21 @@
@Param(name = "relName", doc = "The label that will be resolved relative to this one.")
},
useStarlarkThread = true)
- public Label getRelative(String relName, StarlarkThread thread) throws LabelSyntaxException {
- return getRelativeWithRemapping(
- relName,
- BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread)).repoMapping());
+ public Label getRelative(String relName, StarlarkThread thread)
+ throws LabelSyntaxException, EvalException {
+ Label label =
+ getRelativeWithRemapping(
+ relName,
+ BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread))
+ .repoMapping());
+ if (!label.getRepository().isVisible()) {
+ throw Starlark.errorf(
+ "Invalid label string '%s': no repository visible as '@%s' from %s",
+ relName,
+ label.getRepository().getName(),
+ label.getRepository().getOwnerRepoDisplayString());
+ }
+ return label;
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
index 6bc7542..6077c23 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
@@ -121,6 +121,9 @@
@Before
public void setup() throws Exception {
workspaceRoot = scratch.dir("/ws");
+ String bazelToolsPath = "/ws/embedded_tools";
+ scratch.file(bazelToolsPath + "/MODULE.bazel", "module(name = 'bazel_tools')");
+ scratch.file(bazelToolsPath + "/WORKSPACE");
modulesRoot = scratch.dir("/modules");
differencer = new SequencedRecordingDifferencer();
evaluationContext =
@@ -178,7 +181,11 @@
externalFilesHelper))
.put(
SkyFunctions.MODULE_FILE,
- new ModuleFileFunction(registryFactory, workspaceRoot, ImmutableMap.of()))
+ new ModuleFileFunction(
+ registryFactory,
+ workspaceRoot,
+ // Required to load @_builtins.
+ ImmutableMap.of("bazel_tools", LocalPathOverride.create(bazelToolsPath))))
.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction())
.put(SkyFunctions.BZL_COMPILE, new BzlCompileFunction(packageFactory, hashFunction))
.put(
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
index 7bf6ac1..ce0d6ff 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
@@ -28,11 +28,16 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule;
+import com.google.devtools.build.lib.analysis.starlark.StarlarkModules;
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions.StarlarkRuleFunction;
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.analysis.util.TestAspects;
+import com.google.devtools.build.lib.cmdline.BazelModuleContext;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventKind;
@@ -64,6 +69,7 @@
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.util.FileTypeSet;
+import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Arrays;
import java.util.List;
import javax.annotation.Nullable;
@@ -75,8 +81,10 @@
import net.starlark.java.eval.StarlarkCallable;
import net.starlark.java.eval.StarlarkInt;
import net.starlark.java.eval.StarlarkList;
+import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.Structure;
import net.starlark.java.eval.Tuple;
+import net.starlark.java.syntax.FileOptions;
import net.starlark.java.syntax.ParserInput;
import net.starlark.java.syntax.Program;
import net.starlark.java.syntax.StarlarkFile;
@@ -2828,4 +2836,56 @@
ev.assertContainsError(
"Error in analysis_test: 'name' cannot be set or overridden in 'attr_values'");
}
+
+ private Object eval(Module module, String... lines) throws Exception {
+ ParserInput input = ParserInput.fromLines(lines);
+ return Starlark.eval(input, FileOptions.DEFAULT, module, ev.getStarlarkThread());
+ }
+
+ @Test
+ public void testLabelWithStrictVisibility() throws Exception {
+ ImmutableMap.Builder<String, Object> predeclared = ImmutableMap.builder();
+ StarlarkModules.addPredeclared(predeclared);
+ RepositoryName currentRepo = RepositoryName.createUnvalidated("module~1.2.3");
+ RepositoryName otherRepo = RepositoryName.createUnvalidated("dep~4.5");
+ Label bzlLabel =
+ Label.create(
+ PackageIdentifier.create(currentRepo, PathFragment.create("lib")), "label.bzl");
+ Object clientData =
+ BazelModuleContext.create(
+ bzlLabel,
+ RepositoryMapping.create(
+ ImmutableMap.of("my_module", currentRepo, "dep", otherRepo), currentRepo),
+ "lib/label.bzl",
+ /*loads=*/ ImmutableMap.of(),
+ /*bzlTransitiveDigest=*/ new byte[0]);
+ Module module =
+ Module.withPredeclaredAndData(
+ StarlarkSemantics.DEFAULT, predeclared.buildOrThrow(), clientData);
+
+ assertThat(eval(module, "Label('//foo:bar').workspace_root"))
+ .isEqualTo("external/module~1.2.3");
+ assertThat(eval(module, "Label('@my_module//foo:bar').workspace_root"))
+ .isEqualTo("external/module~1.2.3");
+ assertThat(eval(module, "Label('@@module~1.2.3//foo:bar').workspace_root"))
+ .isEqualTo("external/module~1.2.3");
+ assertThat(eval(module, "Label('@dep//foo:bar').workspace_root")).isEqualTo("external/dep~4.5");
+ assertThat(eval(module, "Label('@@dep~4.5//foo:bar').workspace_root"))
+ .isEqualTo("external/dep~4.5");
+ assertThat(eval(module, "Label('@@//foo:bar').workspace_root")).isEqualTo("");
+
+ assertThat(assertThrows(EvalException.class, () -> eval(module, "Label('@//foo:bar')")))
+ .hasMessageThat()
+ .contains(
+ "Invalid label string '@//foo:bar': no repository visible as '@' from repository "
+ + "'@module~1.2.3'");
+ assertThat(
+ assertThrows(
+ EvalException.class,
+ () -> eval(module, "Label('@@//foo:bar').relative('@not_dep//foo:bar')")))
+ .hasMessageThat()
+ .contains(
+ "Invalid label string '@not_dep//foo:bar': no repository visible as '@not_dep' "
+ + "from repository '@module~1.2.3'");
+ }
}
