Skip to content

Commit 33523f3

Browse files
benjaminpbenjaminp
committed
Perform builtins injection for WORKSPACE-loaded bzl files.
bazelbuild#17713
1 parent 6e319b3 commit 33523f3

File tree

5 files changed

+78
-30
lines changed

5 files changed

+78
-30
lines changed

‎src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,14 @@ public final class BazelStarlarkEnvironment {
5050
private final ImmutableMap<String, Object> uninjectedBuildBzlNativeBindings;
5151
/** The "native" module fields for a WORKSPACE-loaded bzl module. */
5252
private final ImmutableMap<String, Object> workspaceBzlNativeBindings;
53-
/** The top-level predeclared symbols for a BUILD-loaded bzl module, before builtins injection. */
53+
/** The top-level predeclared symbols for a BUILD-loaded bzl module before builtins injection. */
5454
private final ImmutableMap<String, Object> uninjectedBuildBzlEnv;
5555
/** The top-level predeclared symbols for BUILD files, before builtins injection and prelude. */
5656
private final ImmutableMap<String, Object> uninjectedBuildEnv;
57-
/** The top-level predeclared symbols for a WORKSPACE-loaded bzl module. */
58-
private final ImmutableMap<String, Object> workspaceBzlEnv;
57+
/**
58+
* The top-level predeclared symbols for a WORKSPACE-loaded bzl module before builtins injection.
59+
*/
60+
private final ImmutableMap<String, Object> uninjectedWorkspaceBzlEnv;
5961
/** The top-level predeclared symbols for a bzl module in the {@code @_builtins} pseudo-repo. */
6062
private final ImmutableMap<String, Object> builtinsBzlEnv;
6163
/** The top-level predeclared symbols for a bzl module in the Bzlmod system. */
@@ -75,7 +77,8 @@ public final class BazelStarlarkEnvironment {
7577
this.workspaceBzlNativeBindings = createWorkspaceBzlNativeBindings(ruleClassProvider, version);
7678
this.uninjectedBuildBzlEnv =
7779
createUninjectedBuildBzlEnv(ruleClassProvider, uninjectedBuildBzlNativeBindings);
78-
this.workspaceBzlEnv = createWorkspaceBzlEnv(ruleClassProvider, workspaceBzlNativeBindings);
80+
this.uninjectedWorkspaceBzlEnv =
81+
createWorkspaceBzlEnv(ruleClassProvider, workspaceBzlNativeBindings);
7982
// TODO(pcloudy): this should be a bzlmod specific environment, but keep using the workspace
8083
// envirnment until we implement module rules.
8184
this.bzlmodBzlEnv = createWorkspaceBzlEnv(ruleClassProvider, workspaceBzlNativeBindings);
@@ -122,9 +125,9 @@ public ImmutableMap<String, Object> getUninjectedBuildEnv() {
122125
return uninjectedBuildEnv;
123126
}
124127

125-
/** Returns the environment for WORKSPACE-loaded bzl files. */
126-
public ImmutableMap<String, Object> getWorkspaceBzlEnv() {
127-
return workspaceBzlEnv;
128+
/** Returns the environment for WORKSPACE-loaded bzl files before builtins injection. */
129+
public ImmutableMap<String, Object> getUninjectedWorkspaceBzlEnv() {
130+
return uninjectedWorkspaceBzlEnv;
128131
}
129132

130133
/** Returns the environment for bzl files in the {@code @_builtins} pseudo-repository. */
@@ -338,17 +341,57 @@ private static boolean injectionApplies(String key, Map<String, Boolean> overrid
338341
* documentation for {@code --experimental_builtins_injection_override}. Non-injected symbols must
339342
* still obey the above constraints.
340343
*
341-
* @see StarlarkBuiltinsFunction
344+
* @see com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction
342345
*/
343346
public ImmutableMap<String, Object> createBuildBzlEnvUsingInjection(
344347
Map<String, Object> exportedToplevels,
345348
Map<String, Object> exportedRules,
346349
List<String> overridesList)
347350
throws InjectionException {
351+
return createBuildBzlEnvUsingInjection(
352+
exportedToplevels, exportedRules, overridesList, uninjectedBuildBzlNativeBindings);
353+
}
354+
355+
/**
356+
* Constructs an environment for a WORKSPACE-loaded bzl file based on the default environment, the
357+
* maps corresponding to the {@code exported_toplevels} and {@code exported_rules} dicts, and the
358+
* value of {@code --experimental_builtins_injection_override}.
359+
*
360+
* @see com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction
361+
*/
362+
public ImmutableMap<String, Object> createWorkspaceBzlEnvUsingInjection(
363+
Map<String, Object> exportedToplevels,
364+
Map<String, Object> exportedRules,
365+
List<String> overridesList)
366+
throws InjectionException {
367+
return createBuildBzlEnvUsingInjection(
368+
exportedToplevels, exportedRules, overridesList, workspaceBzlNativeBindings);
369+
}
370+
371+
private ImmutableMap<String, Object> createBuildBzlEnvUsingInjection(
372+
Map<String, Object> exportedToplevels,
373+
Map<String, Object> exportedRules,
374+
List<String> overridesList,
375+
Map<String, Object> nativeBase)
376+
throws InjectionException {
348377
Map<String, Boolean> overridesMap = parseInjectionOverridesList(overridesList);
349378

379+
Map<String, Object> env = new HashMap<>(ruleClassProvider.getEnvironment());
380+
381+
// Determine "native" bindings.
382+
// TODO(#11954): See above comment in createUninjectedBuildBzlEnv.
383+
Map<String, Object> nativeBindings = new HashMap<>(nativeBase);
384+
for (Map.Entry<String, Object> entry : exportedRules.entrySet()) {
385+
String key = entry.getKey();
386+
String name = getKeySuffix(key);
387+
validateSymbolIsInjectable(name, nativeBindings.keySet(), ruleFunctions.keySet(), "rule");
388+
if (injectionApplies(key, overridesMap)) {
389+
nativeBindings.put(name, entry.getValue());
390+
}
391+
}
392+
env.put("native", createNativeModule(nativeBindings));
393+
350394
// Determine top-level symbols.
351-
Map<String, Object> env = new HashMap<>(uninjectedBuildBzlEnv);
352395
for (Map.Entry<String, Object> entry : exportedToplevels.entrySet()) {
353396
String key = entry.getKey();
354397
String name = getKeySuffix(key);
@@ -362,19 +405,6 @@ public ImmutableMap<String, Object> createBuildBzlEnvUsingInjection(
362405
}
363406
}
364407

365-
// Determine "native" bindings.
366-
// TODO(#11954): See above comment in createUninjectedBuildBzlEnv.
367-
Map<String, Object> nativeBindings = new HashMap<>(uninjectedBuildBzlNativeBindings);
368-
for (Map.Entry<String, Object> entry : exportedRules.entrySet()) {
369-
String key = entry.getKey();
370-
String name = getKeySuffix(key);
371-
validateSymbolIsInjectable(name, nativeBindings.keySet(), ruleFunctions.keySet(), "rule");
372-
if (injectionApplies(key, overridesMap)) {
373-
nativeBindings.put(name, entry.getValue());
374-
}
375-
}
376-
377-
env.put("native", createNativeModule(nativeBindings));
378408
return ImmutableMap.copyOf(env);
379409
}
380410

‎src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,8 @@ private BzlLoadValue computeInternal(
586586
private StarlarkBuiltinsValue getBuiltins(
587587
BzlLoadValue.Key key, Environment env, @Nullable InliningState inliningState)
588588
throws BzlLoadFailedException, InterruptedException {
589-
if (!(key instanceof BzlLoadValue.KeyForBuild)) {
589+
if (!(key instanceof BzlLoadValue.KeyForBuild)
590+
&& !(key instanceof BzlLoadValue.KeyForWorkspace)) {
590591
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
591592
if (starlarkSemantics == null) {
592593
return null;
@@ -1185,7 +1186,7 @@ private ImmutableMap<String, Object> getAndDigestPredeclaredEnvironment(
11851186
fp.addBytes(builtins.transitiveDigest);
11861187
return builtins.predeclaredForBuildBzl;
11871188
} else if (key instanceof BzlLoadValue.KeyForWorkspace) {
1188-
return starlarkEnv.getWorkspaceBzlEnv();
1189+
return builtins.predeclaredForWorkspaceBzl;
11891190
} else if (key instanceof BzlLoadValue.KeyForBzlmod) {
11901191
return starlarkEnv.getBzlmodBzlEnv();
11911192
} else if (key instanceof BzlLoadValue.KeyForBuiltins) {

‎src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,18 @@ private static StarlarkBuiltinsValue computeInternal(
174174
exportedToplevels,
175175
exportedRules,
176176
starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE));
177+
ImmutableMap<String, Object> predeclaredForWorkspaceBzl =
178+
starlarkEnv.createWorkspaceBzlEnvUsingInjection(
179+
exportedToplevels,
180+
exportedRules,
181+
starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE));
177182
ImmutableMap<String, Object> predeclaredForBuild =
178183
starlarkEnv.createBuildEnvUsingInjection(
179184
exportedRules,
180185
starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE));
181186
return StarlarkBuiltinsValue.create(
182187
predeclaredForBuildBzl,
188+
predeclaredForWorkspaceBzl,
183189
predeclaredForBuild,
184190
exportedToJava,
185191
transitiveDigest,

‎src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ static boolean isBuiltinsRepo(RepositoryName repo) {
6464
*/
6565
public final ImmutableMap<String, Object> predeclaredForBuildBzl;
6666

67+
/**
68+
* Top-level predeclared symbols for a .bzl file loaded on behalf of a WORKSPACE file after
69+
* builtins injection has been applied.
70+
*/
71+
public final ImmutableMap<String, Object> predeclaredForWorkspaceBzl;
72+
6773
/**
6874
* Top-level predeclared symbols for a BUILD file, after builtins injection but before any prelude
6975
* file has been applied.
@@ -81,11 +87,13 @@ static boolean isBuiltinsRepo(RepositoryName repo) {
8187

8288
private StarlarkBuiltinsValue(
8389
ImmutableMap<String, Object> predeclaredForBuildBzl,
90+
ImmutableMap<String, Object> predeclaredForWorkspaceBzl,
8491
ImmutableMap<String, Object> predeclaredForBuild,
8592
ImmutableMap<String, Object> exportedToJava,
8693
byte[] transitiveDigest,
8794
StarlarkSemantics starlarkSemantics) {
8895
this.predeclaredForBuildBzl = predeclaredForBuildBzl;
96+
this.predeclaredForWorkspaceBzl = predeclaredForWorkspaceBzl;
8997
this.predeclaredForBuild = predeclaredForBuild;
9098
this.exportedToJava = exportedToJava;
9199
this.transitiveDigest = transitiveDigest;
@@ -94,12 +102,14 @@ private StarlarkBuiltinsValue(
94102

95103
public static StarlarkBuiltinsValue create(
96104
ImmutableMap<String, Object> predeclaredForBuildBzl,
105+
ImmutableMap<String, Object> predeclaredForWorkspaceBzl,
97106
ImmutableMap<String, Object> predeclaredForBuild,
98107
ImmutableMap<String, Object> exportedToJava,
99108
byte[] transitiveDigest,
100109
StarlarkSemantics starlarkSemantics) {
101110
return new StarlarkBuiltinsValue(
102111
predeclaredForBuildBzl,
112+
predeclaredForWorkspaceBzl,
103113
predeclaredForBuild,
104114
exportedToJava,
105115
transitiveDigest,
@@ -116,10 +126,11 @@ public static StarlarkBuiltinsValue create(
116126
*/
117127
public static StarlarkBuiltinsValue createEmpty(StarlarkSemantics starlarkSemantics) {
118128
return new StarlarkBuiltinsValue(
119-
/*predeclaredForBuildBzl=*/ ImmutableMap.of(),
120-
/*predeclaredForBuild=*/ ImmutableMap.of(),
121-
/*exportedToJava=*/ ImmutableMap.of(),
122-
/*transitiveDigest=*/ new byte[] {},
129+
/* predeclaredForBuildBzl= */ ImmutableMap.of(),
130+
/* predeclaredForWorkspaceBzl= */ ImmutableMap.of(),
131+
/* predeclaredForBuild= */ ImmutableMap.of(),
132+
/* exportedToJava= */ ImmutableMap.of(),
133+
/* transitiveDigest= */ new byte[] {},
123134
starlarkSemantics);
124135
}
125136

‎src/test/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironmentTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ protected ConfiguredRuleClassProvider createRuleClassProvider() {
6161
public void buildAndWorkspaceBzlEnvsDeclareSameNames() throws Exception {
6262
BazelStarlarkEnvironment starlarkEnv = pkgFactory.getBazelStarlarkEnvironment();
6363
Set<String> buildBzlNames = starlarkEnv.getUninjectedBuildBzlEnv().keySet();
64-
Set<String> workspaceBzlNames = starlarkEnv.getWorkspaceBzlEnv().keySet();
64+
Set<String> workspaceBzlNames = starlarkEnv.getUninjectedWorkspaceBzlEnv().keySet();
6565
assertThat(buildBzlNames).isEqualTo(workspaceBzlNames);
6666
}
6767

@@ -72,7 +72,7 @@ public void buildAndWorkspaceBzlEnvsAreSameExceptForNative() throws Exception {
7272
buildBzlEnv.putAll(starlarkEnv.getUninjectedBuildBzlEnv());
7373
buildBzlEnv.remove("native");
7474
Map<String, Object> workspaceBzlEnv = new HashMap<>();
75-
workspaceBzlEnv.putAll(starlarkEnv.getWorkspaceBzlEnv());
75+
workspaceBzlEnv.putAll(starlarkEnv.getUninjectedWorkspaceBzlEnv());
7676
workspaceBzlEnv.remove("native");
7777
assertThat(buildBzlEnv).isEqualTo(workspaceBzlEnv);
7878
}

0 commit comments

Comments
 (0)