Skip to content

Commit cf3f48c

Browse files
fmeumcopybara-github
authored andcommitted
Relax Label repo visibility validation
Failing the build eagerly when `Label` is passed a label string referencing an unknown apparent repository name turned out to be too strict and causes failures on invalid, but unused labels: https://buildkite.com/bazel/bazel-bazel-with-bzlmod/builds/667#01841861-3d3a-4b65-91b7-b7083ee6b42b Instead, fail when a method that requires a valid repository name is called on an invalid `Label` instance. Closes bazelbuild#16578. PiperOrigin-RevId: 484232254 Change-Id: Ic64ea5db691f39a20dda47e60e2d6f5436ebca1e
1 parent 07c5c1a commit cf3f48c

File tree

4 files changed

+30
-38
lines changed

4 files changed

+30
-38
lines changed

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

+1-9
Original file line numberDiff line numberDiff line change
@@ -1004,15 +1004,7 @@ public Label label(String labelString, StarlarkThread thread) throws EvalExcepti
10041004
BazelModuleContext moduleContext =
10051005
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
10061006
try {
1007-
Label label = Label.parseWithRepoContext(labelString, moduleContext.packageContext());
1008-
if (!label.getRepository().isVisible()) {
1009-
throw Starlark.errorf(
1010-
"Invalid label string '%s': no repository visible as '@%s' from %s",
1011-
labelString,
1012-
label.getRepository().getName(),
1013-
label.getRepository().getOwnerRepoDisplayString());
1014-
}
1015-
return label;
1007+
return Label.parseWithRepoContext(labelString, moduleContext.packageContext());
10161008
} catch (LabelSyntaxException e) {
10171009
throw Starlark.errorf("Illegal absolute label syntax: %s", e.getMessage());
10181010
}

src/main/java/com/google/devtools/build/lib/cmdline/Label.java

+14-17
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,8 @@ public String getPackageName() {
338338
+ " \"external/repo\"</pre>",
339339
useStarlarkSemantics = true)
340340
@Deprecated
341-
public String getWorkspaceRootForStarlarkOnly(StarlarkSemantics semantics) {
341+
public String getWorkspaceRootForStarlarkOnly(StarlarkSemantics semantics) throws EvalException {
342+
checkRepoVisibilityForStarlark("workspace_root");
342343
return packageIdentifier
343344
.getRepository()
344345
.getExecPath(semantics.getBool(BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT))
@@ -431,7 +432,8 @@ public String getUnambiguousCanonicalForm() {
431432
"The repository part of this label. For instance, "
432433
+ "<pre class=language-python>Label(\"@foo//bar:baz\").workspace_name"
433434
+ " == \"foo\"</pre>")
434-
public String getWorkspaceName() {
435+
public String getWorkspaceName() throws EvalException {
436+
checkRepoVisibilityForStarlark("workspace_name");
435437
return packageIdentifier.getRepository().getName();
436438
}
437439

@@ -504,21 +506,10 @@ public Label getLocalTargetLabel(String targetName) throws LabelSyntaxException
504506
@Param(name = "relName", doc = "The label that will be resolved relative to this one.")
505507
},
506508
useStarlarkThread = true)
507-
public Label getRelative(String relName, StarlarkThread thread)
508-
throws LabelSyntaxException, EvalException {
509-
Label label =
510-
getRelativeWithRemapping(
511-
relName,
512-
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread))
513-
.repoMapping());
514-
if (!label.getRepository().isVisible()) {
515-
throw Starlark.errorf(
516-
"Invalid label string '%s': no repository visible as '@%s' from %s",
517-
relName,
518-
label.getRepository().getName(),
519-
label.getRepository().getOwnerRepoDisplayString());
520-
}
521-
return label;
509+
public Label getRelative(String relName, StarlarkThread thread) throws LabelSyntaxException {
510+
return getRelativeWithRemapping(
511+
relName,
512+
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread)).repoMapping());
522513
}
523514

524515
/**
@@ -676,4 +667,10 @@ public String expandToCommandLine() {
676667
// TODO(wyv): Consider using StarlarkSemantics here too for optional unambiguity.
677668
return getCanonicalForm();
678669
}
670+
671+
private void checkRepoVisibilityForStarlark(String method) throws EvalException {
672+
if (!getRepository().isVisible()) {
673+
throw Starlark.errorf("'%s' is not allowed on invalid Label %s", method, this);
674+
}
675+
}
679676
}

src/main/java/com/google/devtools/build/skydoc/SkydocMain.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public static void main(String[] args)
163163
providerInfoMap,
164164
userDefinedFunctions,
165165
aspectInfoMap);
166-
} catch (StarlarkEvaluationException exception) {
166+
} catch (StarlarkEvaluationException | EvalException exception) {
167167
exception.printStackTrace();
168168
System.err.println("Stardoc documentation generation failed: " + exception.getMessage());
169169
System.exit(1);
@@ -386,7 +386,8 @@ private Module recursiveEval(
386386
List<RuleInfoWrapper> ruleInfoList,
387387
List<ProviderInfoWrapper> providerInfoList,
388388
List<AspectInfoWrapper> aspectInfoList)
389-
throws InterruptedException, IOException, LabelSyntaxException, StarlarkEvaluationException {
389+
throws InterruptedException, IOException, LabelSyntaxException, StarlarkEvaluationException,
390+
EvalException {
390391
Path path = pathOfLabel(label, semantics);
391392

392393
if (pending.contains(path)) {
@@ -473,7 +474,7 @@ path, load, pathOfLabel(relativeLabel, semantics), depRoots),
473474
return module;
474475
}
475476

476-
private Path pathOfLabel(Label label, StarlarkSemantics semantics) {
477+
private Path pathOfLabel(Label label, StarlarkSemantics semantics) throws EvalException {
477478
String workspacePrefix = "";
478479
if (!label.getWorkspaceRootForStarlarkOnly(semantics).isEmpty()
479480
&& !label.getWorkspaceName().equals(workspaceName)) {

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

+11-9
Original file line numberDiff line numberDiff line change
@@ -2874,18 +2874,20 @@ public void testLabelWithStrictVisibility() throws Exception {
28742874
.isEqualTo("external/dep~4.5");
28752875
assertThat(eval(module, "Label('@@//foo:bar').workspace_root")).isEqualTo("");
28762876

2877-
assertThat(assertThrows(EvalException.class, () -> eval(module, "Label('@//foo:bar')")))
2877+
assertThat(eval(module, "str(Label('@//foo:bar'))")).isEqualTo("@//foo:bar");
2878+
assertThat(
2879+
assertThrows(
2880+
EvalException.class, () -> eval(module, "Label('@//foo:bar').workspace_name")))
28782881
.hasMessageThat()
2879-
.contains(
2880-
"Invalid label string '@//foo:bar': no repository visible as '@' from repository "
2881-
+ "'@module~1.2.3'");
2882+
.isEqualTo(
2883+
"'workspace_name' is not allowed on invalid Label @[unknown repo '' requested from"
2884+
+ " @module~1.2.3]//foo:bar");
28822885
assertThat(
28832886
assertThrows(
2884-
EvalException.class,
2885-
() -> eval(module, "Label('@@//foo:bar').relative('@not_dep//foo:bar')")))
2887+
EvalException.class, () -> eval(module, "Label('@//foo:bar').workspace_root")))
28862888
.hasMessageThat()
2887-
.contains(
2888-
"Invalid label string '@not_dep//foo:bar': no repository visible as '@not_dep' "
2889-
+ "from repository '@module~1.2.3'");
2889+
.isEqualTo(
2890+
"'workspace_root' is not allowed on invalid Label @[unknown repo '' requested from"
2891+
+ " @module~1.2.3]//foo:bar");
28902892
}
28912893
}

0 commit comments

Comments
 (0)