Skip to content

Commit 8c82249

Browse files
fmeumfmeum
committed
Validate AttributeValues
1 parent 7685fc2 commit 8c82249

File tree

5 files changed

+34
-27
lines changed

5 files changed

+34
-27
lines changed

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,9 @@ java_library(
279279
name = "repo_rule_creator",
280280
srcs = ["BzlmodRepoRuleCreator.java"],
281281
deps = [
282+
":common",
283+
":module_extension",
282284
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
283-
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension",
284285
"//src/main/java/com/google/devtools/build/lib/cmdline",
285286
"//src/main/java/com/google/devtools/build/lib/events",
286287
"//src/main/java/com/google/devtools/build/lib/packages",

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,11 @@ public static Rule createRule(
100100
return rule;
101101
}
102102

103-
public static void validateLabelAttrs(Rule rule, ModuleExtensionId owningExtension)
103+
public static void validateLabelAttrs(
104+
AttributeValues attributes, ModuleExtensionId owningExtension, String what)
104105
throws EvalException {
105-
for (var attribute : rule.getRuleClassObject().getAttributes()) {
106-
Object value = rule.getAttr(attribute.getName());
106+
for (var entry : attributes.attributes().entrySet()) {
107+
Object value = entry.getValue();
107108
Collection<?> toValidate =
108109
switch (value) {
109110
case List<?> list -> list;
@@ -117,8 +118,7 @@ public static void validateLabelAttrs(Rule rule, ModuleExtensionId owningExtensi
117118
if (label.getRepository().isVisible()) {
118119
continue;
119120
}
120-
String unprefixedRepoName =
121-
label.getRepository().getName().substring(label.getName().lastIndexOf('~') + 1);
121+
String repoName = label.getRepository().getName();
122122
RepositoryName owningModuleRepoName = owningExtension.getBzlFileLabel().getRepository();
123123
String owningModule;
124124
if (owningModuleRepoName.isMain()) {
@@ -133,17 +133,16 @@ public static void validateLabelAttrs(Rule rule, ModuleExtensionId owningExtensi
133133
}
134134
throw Starlark.errorf(
135135
"no repository visible as '@%s', but referenced by label '@%s//%s:%s' in attribute %s"
136-
+ " of %s '%s'. Only repositories visible to the %s can be referenced"
136+
+ " of %s. Only repositories visible to the %s can be referenced"
137137
+ " here, are you missing a bazel_dep or use_repo(..., \"%s\")?.",
138-
label.getRepository().getName(),
139-
label.getRepository().getName(),
138+
repoName,
139+
repoName,
140140
label.getPackageName(),
141141
label.getName(),
142-
attribute.getName(),
143-
rule.getRuleClass(),
144-
unprefixedRepoName,
142+
entry.getKey(),
143+
what,
145144
owningModule,
146-
unprefixedRepoName);
145+
repoName);
147146
}
148147
}
149148
}

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,19 @@ public void createRepo(StarlarkThread thread, Dict<String, Object> kwargs, RuleC
112112
"RepositoryRuleFunction.createRule",
113113
ruleClass,
114114
Maps.transformEntries(kwargs, (k, v) -> k.equals("name") ? prefixedName : v));
115-
BzlmodRepoRuleCreator.validateLabelAttrs(rule, owningExtension);
116115

117116
Map<String, Object> attributes =
118117
Maps.filterKeys(
119118
Maps.transformEntries(kwargs, (k, v) -> rule.getAttr(k)), k -> !k.equals("name"));
120119
String bzlFile = ruleClass.getRuleDefinitionEnvironmentLabel().getUnambiguousCanonicalForm();
120+
var attributesValue = AttributeValues.create(attributes);
121+
BzlmodRepoRuleCreator.validateLabelAttrs(
122+
attributesValue, owningExtension, String.format("%s '%s'", rule.getRuleClass(), name));
121123
RepoSpec repoSpec =
122124
RepoSpec.builder()
123125
.setBzlFile(bzlFile)
124126
.setRuleClassName(ruleClass.getName())
125-
.setAttributes(AttributeValues.create(attributes))
127+
.setAttributes(attributesValue)
126128
.build();
127129

128130
generatedRepos.put(name, RepoSpecAndLocation.create(repoSpec, thread.getCallerLocation()));

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,7 @@ public RunModuleExtensionResult run(
731731
String name = (String) kwargs.get("name");
732732
String prefixedName = usagesValue.getExtensionUniqueName() + "~" + name;
733733
Rule ruleInstance;
734+
AttributeValues attributesValue;
734735
try {
735736
ruleInstance =
736737
BzlmodRepoRuleCreator.createRule(
@@ -742,7 +743,15 @@ public RunModuleExtensionResult run(
742743
"SingleExtensionEval.createInnateExtensionRepoRule",
743744
repoRule.getRuleClass(),
744745
Maps.transformEntries(kwargs, (k, v) -> k.equals("name") ? prefixedName : v));
745-
BzlmodRepoRuleCreator.validateLabelAttrs(ruleInstance, extensionId);
746+
attributesValue =
747+
AttributeValues.create(
748+
Maps.filterKeys(
749+
Maps.transformEntries(kwargs, (k, v) -> ruleInstance.getAttr(k)),
750+
k -> !k.equals("name")));
751+
BzlmodRepoRuleCreator.validateLabelAttrs(
752+
attributesValue,
753+
extensionId,
754+
String.format("%s '%s'", ruleInstance.getRuleClass(), name));
746755
} catch (InvalidRuleException | NoSuchPackageException | EvalException e) {
747756
throw new SingleExtensionEvalFunctionException(
748757
ExternalDepsException.withCauseAndMessage(
@@ -761,11 +770,7 @@ public RunModuleExtensionResult run(
761770
.getRuleDefinitionEnvironmentLabel()
762771
.getUnambiguousCanonicalForm())
763772
.setRuleClassName(repoRule.getRuleClass().getName())
764-
.setAttributes(
765-
AttributeValues.create(
766-
Maps.filterKeys(
767-
Maps.transformEntries(kwargs, (k, v) -> ruleInstance.getAttr(k)),
768-
k -> !k.equals("name"))))
773+
.setAttributes(attributesValue)
769774
.build();
770775
generatedRepoSpecs.put(name, repoSpec);
771776
}

‎src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,7 +1372,7 @@ public void badRepoNameInExtensionImplFunction() throws Exception {
13721372
}
13731373

13741374
@Test
1375-
public void nonVisibleLabelInLabelAttr() throws Exception {
1375+
public void nonVisibleLabelInLabelAttrNonRootModule() throws Exception {
13761376
registry.addModule(
13771377
createModuleKey("ext_module", "1.0"), "module(name='ext_module',version='1.0')");
13781378
scratch.file(modulesRoot.getRelative("ext_module~v1.0/WORKSPACE").getPathString());
@@ -1405,13 +1405,13 @@ public void nonVisibleLabelInLabelAttr() throws Exception {
14051405
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
14061406
assertContainsEvent(
14071407
"Error in repository_rule: no repository visible as '@other_repo', but referenced by label"
1408-
+ " '@other_repo//:foo' in attribute data of data_repo 'other_repo'. Only repositories"
1408+
+ " '@other_repo//:foo' in attribute data of data_repo 'ext'. Only repositories"
14091409
+ " visible to the module 'ext_module' can be referenced here, are you missing a"
14101410
+ " bazel_dep or use_repo(..., \"other_repo\")?");
14111411
}
14121412

14131413
@Test
1414-
public void nonVisibleLabelInLabelAttrInRootModule() throws Exception {
1414+
public void nonVisibleLabelInLabelAttr() throws Exception {
14151415
scratch.file(
14161416
workspaceRoot.getRelative("MODULE.bazel").getPathString(),
14171417
"ext = use_extension('//:defs.bzl','ext')",
@@ -1439,7 +1439,7 @@ public void nonVisibleLabelInLabelAttrInRootModule() throws Exception {
14391439
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
14401440
assertContainsEvent(
14411441
"Error in repository_rule: no repository visible as '@other_repo', but referenced by label"
1442-
+ " '@other_repo//:foo' in attribute data of data_repo 'other_repo'. Only repositories"
1442+
+ " '@other_repo//:foo' in attribute data of data_repo 'ext'. Only repositories"
14431443
+ " visible to the root module can be referenced here, are you missing a bazel_dep or"
14441444
+ " use_repo(..., \"other_repo\")?");
14451445
}
@@ -1473,7 +1473,7 @@ public void nonVisibleLabelInLabelListAttr() throws Exception {
14731473
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
14741474
assertContainsEvent(
14751475
"Error in repository_rule: no repository visible as '@other_repo', but referenced by label"
1476-
+ " '@other_repo//:foo' in attribute data of data_repo 'other_repo'. Only repositories"
1476+
+ " '@other_repo//:foo' in attribute data of data_repo 'ext'. Only repositories"
14771477
+ " visible to the root module can be referenced here, are you missing a bazel_dep or"
14781478
+ " use_repo(..., \"other_repo\")?");
14791479
}
@@ -1507,7 +1507,7 @@ public void nonVisibleLabelInLabelKeyedStringDictAttr() throws Exception {
15071507
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
15081508
assertContainsEvent(
15091509
"Error in repository_rule: no repository visible as '@other_repo', but referenced by label"
1510-
+ " '@other_repo//:foo' in attribute data of data_repo 'other_repo'. Only repositories"
1510+
+ " '@other_repo//:foo' in attribute data of data_repo 'ext'. Only repositories"
15111511
+ " visible to the root module can be referenced here, are you missing a bazel_dep or"
15121512
+ " use_repo(..., \"other_repo\")?");
15131513
}

0 commit comments

Comments
 (0)