Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enforce file name format for MODULE.bazel includes #22075

Closed
wants to merge 1 commit into from
Closed

Enforce file name format for MODULE.bazel includes #22075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/CompiledModuleFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Loading