Skip to content

Commit

Permalink
Enforce file name format for MODULE.bazel includes
Browse files Browse the repository at this point in the history 
They must end in `.MODULE.bazel`.

Follow-up for bazelbuild#17880

Closes bazelbuild#22075.

PiperOrigin-RevId: 627136756
Change-Id: If9b1797f2e7ddc1aebd929646329e832288bfd8a
  • Loading branch information
@Wyverald
Wyverald authored and Copybara-Service committed Apr 22, 2024
1 parent b2bdf97 commit 8105bc0
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/CompiledModuleFile.java
View file
Expand Up @@ -142,7 +142,7 @@ public void visit(AssignmentStatement node) {
@Override
public void visit(DotExpression node) {
visit(node.getObject());
if (includeWasAssigned || !node.getField().getName().equals(INCLUDE_IDENTIFIER)) {
if (!node.getField().getName().equals(INCLUDE_IDENTIFIER)) {
// This is fine: `whatever.include`
// (so `include` can be used as a tag class name)
visit(node.getField());
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
View file
Expand Up @@ -355,6 +355,14 @@ private static ImmutableList<IncludeStatement> advanceHorizon(
includeStatement.includeLabel(),
includeStatement.location());
}
if (!includeStatement.includeLabel().endsWith(".MODULE.bazel")) {
throw errorf(
Code.BAD_MODULE,
"bad include label '%s' at %s: the file to be included must have a name ending in"
+ " '.MODULE.bazel'",
includeStatement.includeLabel(),
includeStatement.location());
}
try {
includeLabels.add(Label.parseCanonical(includeStatement.includeLabel()));
} catch (LabelSyntaxException e) {
Expand Down
5 changes: 4 additions & 1 deletion src/test/java/com/google/devtools/build/lib/bazel/bzlmod/CompiledModuleFileTest.java
View file
Expand Up @@ -91,13 +91,16 @@ public void checkSyntax_good_benignUsageOfInclude() throws Exception {
public void checkSyntax_good_includeIdentifierReassigned() throws Exception {
String program =
"""
include('world')
include = print
# from this point on, we no longer check anything about `include` usage.
include('hello')
str(include)
exclude = include
""";
assertThat(checkSyntax(program)).isEmpty();
assertThat(checkSyntax(program))
.containsExactly(
new IncludeStatement("world", Location.fromFileLineColumn("test file", 1, 1)));
}

@Test
Expand Down
50 changes: 33 additions & 17 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java
View file
Expand Up @@ -334,22 +334,22 @@ public void testRootModule_include_good() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"include('//java:MODULE.bazel.segment')",
"include('//java:java.MODULE.bazel')",
"bazel_dep(name='foo', version='1.0')",
"register_toolchains('//:whatever')",
"include('//python:MODULE.bazel.segment')");
"include('//python:python.MODULE.bazel')");
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
scratch.overwriteFile(
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
"bazel_dep(name='java-foo', version='1.0')");
scratch.overwriteFile(rootDirectory.getRelative("python/BUILD").getPathString());
scratch.overwriteFile(
rootDirectory.getRelative("python/MODULE.bazel.segment").getPathString(),
rootDirectory.getRelative("python/python.MODULE.bazel").getPathString(),
"bazel_dep(name='py-foo', version='1.0', repo_name='python-foo')",
"single_version_override(module_name='java-foo', version='2.0')",
"include('//python:toolchains/MODULE.bazel.segment')");
"include('//python:toolchains/toolchains.MODULE.bazel')");
scratch.overwriteFile(
rootDirectory.getRelative("python/toolchains/MODULE.bazel.segment").getPathString(),
rootDirectory.getRelative("python/toolchains/toolchains.MODULE.bazel").getPathString(),
"register_toolchains('//:python-whatever')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
Expand Down Expand Up @@ -387,7 +387,7 @@ public void testRootModule_include_bad_otherRepoLabel() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"include('@haha//java:MODULE.bazel.segment')");
"include('@haha//java:java.MODULE.bazel')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

Expand All @@ -403,7 +403,7 @@ public void testRootModule_include_bad_relativeLabel() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"include(':MODULE.bazel.segment')");
"include(':relative.MODULE.bazel')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

Expand All @@ -414,12 +414,28 @@ public void testRootModule_include_bad_relativeLabel() throws Exception {
assertThat(result.getError().toString()).contains("starting with double slashes");
}

@Test
public void testRootModule_include_bad_notEndingInModuleBazel() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"include('//:MODULE.bazel.segment')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

EvaluationResult<RootModuleFileValue> result =
evaluator.evaluate(
ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);
assertThat(result.hasError()).isTrue();
assertThat(result.getError().toString()).contains("have a name ending in '.MODULE.bazel'");
}

@Test
public void testRootModule_include_bad_badLabelSyntax() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"include('//haha/:::')");
"include('//haha/:::.MODULE.bazel')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

Expand All @@ -435,10 +451,10 @@ public void testRootModule_include_bad_badLabelSyntax() throws Exception {
public void testRootModule_include_bad_moduleAfterInclude() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"include('//java:MODULE.bazel.segment')");
"include('//java:java.MODULE.bazel')");
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
scratch.overwriteFile(
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
"module(name='bet-you-didnt-expect-this-didya')",
"bazel_dep(name='java-foo', version='1.0', repo_name='foo')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
Expand All @@ -457,15 +473,15 @@ public void testRootModule_include_bad_repoNameCollision() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"include('//java:MODULE.bazel.segment')",
"include('//python:MODULE.bazel.segment')");
"include('//java:java.MODULE.bazel')",
"include('//python:python.MODULE.bazel')");
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
scratch.overwriteFile(
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
"bazel_dep(name='java-foo', version='1.0', repo_name='foo')");
scratch.overwriteFile(rootDirectory.getRelative("python/BUILD").getPathString());
scratch.overwriteFile(
rootDirectory.getRelative("python/MODULE.bazel.segment").getPathString(),
rootDirectory.getRelative("python/python.MODULE.bazel").getPathString(),
"bazel_dep(name='python-foo', version='1.0', repo_name='foo')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
Expand All @@ -484,10 +500,10 @@ public void testRootModule_include_bad_tryingToLeakBindings() throws Exception {
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"FOO_NAME = 'foo'",
"include('//java:MODULE.bazel.segment')");
"include('//java:java.MODULE.bazel')");
scratch.overwriteFile(rootDirectory.getRelative("java/BUILD").getPathString());
scratch.overwriteFile(
rootDirectory.getRelative("java/MODULE.bazel.segment").getPathString(),
rootDirectory.getRelative("java/java.MODULE.bazel").getPathString(),
"bazel_dep(name=FOO_NAME, version='1.0')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
Expand Down
4 changes: 2 additions & 2 deletions src/test/py/bazel/bzlmod/bazel_module_test.py
View file
Expand Up @@ -911,12 +911,12 @@ def testInclude(self):
[
'module(name="foo")',
'bazel_dep(name="bbb", version="1.0")',
'include("//java:MODULE.bazel.segment")',
'include("//java:java.MODULE.bazel")',
],
)
self.ScratchFile('java/BUILD')
self.ScratchFile(
'java/MODULE.bazel.segment',
'java/java.MODULE.bazel',
[
'bazel_dep(name="aaa", version="1.0", repo_name="lol")',
],
Expand Down

0 comments on commit 8105bc0

Please sign in to comment.