Skip to content

Commit d251cc3

Browse files
committed
Add a new include() directive to MODULE.bazel files
This new directive allows the root module to divide its `MODULE.bazel` into multiple segments. This directive can only be used by root modules; only files in the main repo may be included; variable bindings are only visible in the file they occur in, not in any included or including files. See the docs for `include()` (in `ModuleFileGlobals.java`) for more details. In follow-ups, we'll need to address: 1. Enforcing the loaded files to have some sort of naming format (tentatively `foo.MODULE.bazel` where `foo` is anything) 2. Making `bazel mod tidy` work with included files RELNOTES: Added a new `include()` directive to `MODULE.bazel` files. Fixes #17880. Closes #21855. PiperOrigin-RevId: 627034184 Change-Id: Ifc2f616cf0791445daeeac9ca5ec4478e83382aa
1 parent d448e11 commit d251cc3

File tree

13 files changed

+940
-98
lines changed

13 files changed

+940
-98
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ java_library(
123123
"BazelModuleResolutionEvent.java",
124124
"BazelModuleResolutionValue.java",
125125
"BzlmodFlagsAndEnvVars.java",
126+
"CompiledModuleFile.java",
126127
"GitOverride.java",
127128
"InterimModule.java",
128129
"LocalPathOverride.java",
@@ -149,6 +150,7 @@ java_library(
149150
],
150151
deps = [
151152
":common",
153+
":exception",
152154
":inspection",
153155
":module_extension",
154156
":module_extension_metadata",
@@ -169,6 +171,7 @@ java_library(
169171
"//src/main/java/net/starlark/java/annot",
170172
"//src/main/java/net/starlark/java/eval",
171173
"//src/main/java/net/starlark/java/syntax",
174+
"//src/main/protobuf:failure_details_java_proto",
172175
"//third_party:auto_value",
173176
"//third_party:gson",
174177
"//third_party:guava",
@@ -231,6 +234,8 @@ java_library(
231234
"//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_value",
232235
"//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_function",
233236
"//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_value",
237+
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
238+
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
234239
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
235240
"//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
236241
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import com.google.common.collect.ImmutableMap;
2525
import com.google.common.collect.ImmutableSet;
26+
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
2627
import com.google.devtools.build.lib.cmdline.Label;
2728
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
2829
import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -50,6 +51,11 @@ public class BazelModTidyFunction implements SkyFunction {
5051
@Nullable
5152
public SkyValue compute(SkyKey skyKey, Environment env)
5253
throws InterruptedException, SkyFunctionException {
54+
RootModuleFileValue rootModuleFileValue =
55+
(RootModuleFileValue) env.getValue(ModuleFileValue.KEY_FOR_ROOT_MODULE);
56+
if (rootModuleFileValue == null) {
57+
return null;
58+
}
5359
BazelDepGraphValue depGraphValue = (BazelDepGraphValue) env.getValue(BazelDepGraphValue.KEY);
5460
if (depGraphValue == null) {
5561
return null;
@@ -112,6 +118,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
112118

113119
return BazelModTidyValue.create(
114120
buildozer.asPath(),
121+
rootModuleFileValue.getIncludeLabelToCompiledModuleFile(),
115122
MODULE_OVERRIDES.get(env),
116123
IGNORE_DEV_DEPS.get(env),
117124
LOCKFILE_MODE.get(env),

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ public abstract class BazelModTidyValue implements SkyValue {
3535
/** The path of the buildozer binary provided by the "buildozer" module. */
3636
public abstract Path buildozer();
3737

38+
public abstract ImmutableMap<String, CompiledModuleFile> includeLabelToCompiledModuleFile();
39+
3840
/** The value of {@link ModuleFileFunction#MODULE_OVERRIDES}. */
3941
public abstract ImmutableMap<String, ModuleOverride> moduleOverrides();
4042

@@ -52,12 +54,14 @@ public abstract class BazelModTidyValue implements SkyValue {
5254

5355
static BazelModTidyValue create(
5456
Path buildozer,
57+
ImmutableMap<String, CompiledModuleFile> includeLabelToCompiledModuleFile,
5558
Map<String, ModuleOverride> moduleOverrides,
5659
boolean ignoreDevDeps,
5760
LockfileMode lockfileMode,
5861
StarlarkSemantics starlarkSemantics) {
5962
return new AutoValue_BazelModTidyValue(
6063
buildozer,
64+
includeLabelToCompiledModuleFile,
6165
ImmutableMap.copyOf(moduleOverrides),
6266
ignoreDevDeps,
6367
lockfileMode,
Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
// Copyright 2024 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
//
15+
package com.google.devtools.build.lib.bazel.bzlmod;
16+
17+
import com.google.common.annotations.VisibleForTesting;
18+
import com.google.common.collect.ImmutableList;
19+
import com.google.devtools.build.lib.events.Event;
20+
import com.google.devtools.build.lib.events.ExtendedEventHandler;
21+
import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment;
22+
import com.google.devtools.build.lib.packages.DotBazelFileSyntaxChecker;
23+
import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
24+
import net.starlark.java.eval.EvalException;
25+
import net.starlark.java.eval.Module;
26+
import net.starlark.java.eval.Starlark;
27+
import net.starlark.java.eval.StarlarkSemantics;
28+
import net.starlark.java.eval.StarlarkThread;
29+
import net.starlark.java.syntax.Argument;
30+
import net.starlark.java.syntax.AssignmentStatement;
31+
import net.starlark.java.syntax.CallExpression;
32+
import net.starlark.java.syntax.DotExpression;
33+
import net.starlark.java.syntax.ExpressionStatement;
34+
import net.starlark.java.syntax.Identifier;
35+
import net.starlark.java.syntax.Location;
36+
import net.starlark.java.syntax.ParserInput;
37+
import net.starlark.java.syntax.Program;
38+
import net.starlark.java.syntax.StarlarkFile;
39+
import net.starlark.java.syntax.StringLiteral;
40+
import net.starlark.java.syntax.SyntaxError;
41+
42+
/**
43+
* Represents a compiled MODULE.bazel file, ready to be executed on a {@link StarlarkThread}. It's
44+
* been successfully checked for syntax errors.
45+
*
46+
* <p>Use the {@link #parseAndCompile} factory method instead of directly instantiating this record.
47+
*/
48+
public record CompiledModuleFile(
49+
ModuleFile moduleFile,
50+
Program program,
51+
Module predeclaredEnv,
52+
ImmutableList<IncludeStatement> includeStatements) {
53+
public static final String INCLUDE_IDENTIFIER = "include";
54+
55+
record IncludeStatement(String includeLabel, Location location) {}
56+
57+
/** Parses and compiles a given module file, checking it for syntax errors. */
58+
public static CompiledModuleFile parseAndCompile(
59+
ModuleFile moduleFile,
60+
ModuleKey moduleKey,
61+
StarlarkSemantics starlarkSemantics,
62+
BazelStarlarkEnvironment starlarkEnv,
63+
ExtendedEventHandler eventHandler)
64+
throws ExternalDepsException {
65+
StarlarkFile starlarkFile =
66+
StarlarkFile.parse(ParserInput.fromUTF8(moduleFile.getContent(), moduleFile.getLocation()));
67+
if (!starlarkFile.ok()) {
68+
Event.replayEventsOn(eventHandler, starlarkFile.errors());
69+
throw ExternalDepsException.withMessage(
70+
Code.BAD_MODULE, "error parsing MODULE.bazel file for %s", moduleKey);
71+
}
72+
try {
73+
ImmutableList<IncludeStatement> includeStatements = checkModuleFileSyntax(starlarkFile);
74+
Module predeclaredEnv =
75+
Module.withPredeclared(starlarkSemantics, starlarkEnv.getModuleBazelEnv());
76+
Program program = Program.compileFile(starlarkFile, predeclaredEnv);
77+
return new CompiledModuleFile(moduleFile, program, predeclaredEnv, includeStatements);
78+
} catch (SyntaxError.Exception e) {
79+
Event.replayEventsOn(eventHandler, e.errors());
80+
throw ExternalDepsException.withMessage(
81+
Code.BAD_MODULE, "syntax error in MODULE.bazel file for %s", moduleKey);
82+
}
83+
}
84+
85+
/**
86+
* Checks the given `starlarkFile` for module file syntax, and returns the list of `include`
87+
* statements it contains. This is a somewhat crude sweep over the AST; we loudly complain about
88+
* any usage of `include` that is not in a top-level function call statement with one single
89+
* string literal positional argument, *except* that we don't do this check once `include` is
90+
* assigned to, due to backwards compatibility concerns.
91+
*/
92+
@VisibleForTesting
93+
static ImmutableList<IncludeStatement> checkModuleFileSyntax(StarlarkFile starlarkFile)
94+
throws SyntaxError.Exception {
95+
var includeStatements = ImmutableList.<IncludeStatement>builder();
96+
new DotBazelFileSyntaxChecker("MODULE.bazel files", /* canLoadBzl= */ false) {
97+
// Once `include` the identifier is assigned to, we no longer care about its appearance
98+
// anywhere. This allows `include` to be used as a module extension proxy (and technically
99+
// any other variable binding).
100+
private boolean includeWasAssigned = false;
101+
102+
@Override
103+
public void visit(ExpressionStatement node) {
104+
// We can assume this statement isn't nested in any block, since we don't allow
105+
// `if`/`def`/`for` in MODULE.bazel.
106+
if (!includeWasAssigned
107+
&& node.getExpression() instanceof CallExpression call
108+
&& call.getFunction() instanceof Identifier id
109+
&& id.getName().equals(INCLUDE_IDENTIFIER)) {
110+
// Found a top-level call to `include`!
111+
if (call.getArguments().size() == 1
112+
&& call.getArguments().getFirst() instanceof Argument.Positional pos
113+
&& pos.getValue() instanceof StringLiteral str) {
114+
includeStatements.add(new IncludeStatement(str.getValue(), call.getStartLocation()));
115+
// Nothing else to check, we can stop visiting sub-nodes now.
116+
return;
117+
}
118+
error(
119+
node.getStartLocation(),
120+
"the `include` directive MUST be called with exactly one positional argument that "
121+
+ "is a string literal");
122+
return;
123+
}
124+
super.visit(node);
125+
}
126+
127+
@Override
128+
public void visit(AssignmentStatement node) {
129+
visit(node.getRHS());
130+
if (!includeWasAssigned
131+
&& node.getLHS() instanceof Identifier id
132+
&& id.getName().equals(INCLUDE_IDENTIFIER)) {
133+
includeWasAssigned = true;
134+
// Technically someone could do something like
135+
// (include, myvar) = (print, 3)
136+
// and work around our check, but at that point IDGAF.
137+
} else {
138+
visit(node.getLHS());
139+
}
140+
}
141+
142+
@Override
143+
public void visit(DotExpression node) {
144+
visit(node.getObject());
145+
if (includeWasAssigned || !node.getField().getName().equals(INCLUDE_IDENTIFIER)) {
146+
// This is fine: `whatever.include`
147+
// (so `include` can be used as a tag class name)
148+
visit(node.getField());
149+
}
150+
}
151+
152+
@Override
153+
public void visit(Identifier node) {
154+
if (!includeWasAssigned && node.getName().equals(INCLUDE_IDENTIFIER)) {
155+
// If we somehow reach the `include` identifier but NOT as the other allowed cases above,
156+
// cry foul.
157+
error(
158+
node.getStartLocation(),
159+
"the `include` directive MUST be called directly at the top-level");
160+
}
161+
super.visit(node);
162+
}
163+
}.check(starlarkFile);
164+
return includeStatements.build();
165+
}
166+
167+
public void runOnThread(StarlarkThread thread) throws EvalException, InterruptedException {
168+
Starlark.execFileProgram(program, predeclaredEnv, thread);
169+
}
170+
}

0 commit comments

Comments
 (0)