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.

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

Document and enforce filename restrictions for module file segments #24575

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public class ModuleFileFunction implements SkyFunction {
# For more details, please check https://github.com/bazelbuild/bazel/issues/18958
###############################################################################
""";
private static final String INCLUDE_FILENAME_SUFFIX = ".MODULE.bazel";

/**
* @param builtinModules A list of "built-in" modules that are treated as implicit dependencies of
Expand Down Expand Up @@ -328,16 +329,9 @@ 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());
}
Label includeLabel;
try {
includeLabels.add(Label.parseCanonical(includeStatement.includeLabel()));
includeLabel = Label.parseCanonical(includeStatement.includeLabel());
} catch (LabelSyntaxException e) {
throw errorf(
Code.BAD_MODULE,
Expand All @@ -346,6 +340,26 @@ private static ImmutableList<IncludeStatement> advanceHorizon(
includeStatement.location(),
e.getMessage());
}
String basename =
includeLabel.getName().substring(includeLabel.getName().lastIndexOf('/') + 1);
if (!basename.endsWith(INCLUDE_FILENAME_SUFFIX)) {
throw errorf(
Code.BAD_MODULE,
"bad include label '%s' at %s: the file to be included must have a name ending in"
+ " '%s'",
includeStatement.includeLabel(),
includeStatement.location(),
INCLUDE_FILENAME_SUFFIX);
}
if (basename.startsWith(".")) {
throw errorf(
Code.BAD_MODULE,
"bad include label '%s' at %s: the name of the file to be included must not start"
+ " with '.'",
includeStatement.includeLabel(),
includeStatement.location());
}
includeLabels.add(includeLabel);
}
SkyframeLookupResult result =
env.getValuesAndExceptions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,8 @@ public void call(
doc =
"The label pointing to the file to include. The label must point to a file in the"
+ " main repo; in other words, it <strong>must<strong> start with double"
+ " slashes (<code>//</code>)."),
+ " slashes (<code>//</code>). The name of the file must end with"
+ " <code>.MODULE.bazel</code> and must not start with <code>.</code>."),
},
useStarlarkThread = true)
public void include(String label, StarlarkThread thread)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,23 @@ public void testRootModule_include_bad_notEndingInModuleBazel() throws Exception
assertThat(result.getError().toString()).contains("have a name ending in '.MODULE.bazel'");
}

@Test
public void testRootModule_include_bad_startsWithPeriod() throws Exception {
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"module(name='aaa')",
"include('//pkg:dir/.MODULE.bazel')");
FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableSet.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("the name of the file to be included must not start with '.'");
}

@Test
public void testRootModule_include_bad_badLabelSyntax() throws Exception {
scratch.overwriteFile(
Expand Down
Loading