Skip to content

Commit c6fc42a

Browse files
zhengwei143copybara-github
authored andcommitted
Evaluate the RepositoryMapping in StarlarkMapActionTemplate and re-wrap it in a new supplier to be passed to the StarlarkTemplateContext.
This prevents `AnalysisEnvironment` (specifically `CachingAnalysisEnvironment`) from being captured by the supplier and hence require that it be serialized. Additionally, add serialization checking to `StarlarkMapActionTemplateTest`. PiperOrigin-RevId: 825479304 Change-Id: I8c3c1fd04f7ddd107951b5314fbd9577ed3b4dce
1 parent fc4f5ed commit c6fc42a

File tree

3 files changed

+21
-4
lines changed

3 files changed

+21
-4
lines changed

src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkMapActionTemplate.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public final class StarlarkMapActionTemplate extends ActionKeyComputer
102102
private final ImmutableMap<String, String> executionRequirements;
103103
private final OutputPathsMode outputPathsMode;
104104
private final ActionEnvironment env;
105-
private final InterruptibleSupplier<RepositoryMapping> repoMappingSupplier;
105+
private final RepositoryMapping repoMapping;
106106
private final String expandedActionsMnemonic;
107107
private final StarlarkFunction implementation;
108108
private final StarlarkSemantics semantics;
@@ -124,7 +124,7 @@ public StarlarkMapActionTemplate(
124124
StarlarkFunction implementation,
125125
StarlarkSemantics semantics,
126126
SymbolGenerator<?> symbolGenerator)
127-
throws EvalException {
127+
throws EvalException, InterruptedException {
128128
NestedSetBuilder<Artifact> allInputsNsBuilder = NestedSetBuilder.<Artifact>stableOrder();
129129
NestedSetBuilder<Artifact> toolsNsBuilder = NestedSetBuilder.<Artifact>stableOrder();
130130
this.actionOwner = actionOwner;
@@ -149,7 +149,7 @@ public StarlarkMapActionTemplate(
149149
this.executionRequirements = executionRequirements;
150150
this.outputPathsMode = outputPathsMode;
151151
this.env = env;
152-
this.repoMappingSupplier = repoMappingSupplier;
152+
this.repoMapping = repoMappingSupplier.get();
153153
this.expandedActionsMnemonic = expandedActionsMnemonic;
154154
this.implementation = implementation;
155155
this.semantics = semantics;
@@ -224,7 +224,7 @@ public ImmutableList<SpawnAction> generateActionsForInputArtifacts(
224224
actionOwner,
225225
artifactOwner,
226226
spawnActionBuilder,
227-
repoMappingSupplier,
227+
() -> repoMapping,
228228
ImmutableSet.copyOf(outputDirectories.values()));
229229

230230
ImmutableMap.Builder<String, ExpandedDirectory> expandedDirectories = ImmutableMap.builder();

src/test/java/com/google/devtools/build/lib/starlark/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ java_test(
420420
"//src/main/java/com/google/devtools/build/lib/util/io:out-err",
421421
"//src/main/java/com/google/devtools/build/lib/vfs",
422422
"//src/test/java/com/google/devtools/build/lib/buildtool/util",
423+
"//src/test/java/com/google/devtools/build/lib/testutil:SkyframeExecutorTestHelper",
423424
"//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
424425
"//third_party:guava",
425426
"//third_party:junit4",

src/test/java/com/google/devtools/build/lib/starlark/StarlarkMapActionTemplateTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase;
2828
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue;
2929
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
30+
import com.google.devtools.build.lib.testutil.SkyframeExecutorTestHelper;
3031
import com.google.devtools.build.lib.testutil.TestConstants;
3132
import com.google.devtools.build.lib.util.io.RecordingOutErr;
3233
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -171,6 +172,7 @@ def split_directory_impl(
171172

172173
@Test
173174
public void doSimpleMappingWithAdditionalInputsAndParams() throws Exception {
175+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
174176
write(
175177
"test/rule_def.bzl",
176178
"""
@@ -208,6 +210,7 @@ def rule_impl(ctx):
208210

209211
@Test
210212
public void multipleInputDirectories() throws Exception {
213+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
211214
write(
212215
"test/rule_def.bzl",
213216
"""
@@ -241,6 +244,7 @@ def rule_impl(ctx):
241244

242245
@Test
243246
public void multipleOutputDirectories() throws Exception {
247+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
244248
write(
245249
"test/rule_def.bzl",
246250
"""
@@ -276,6 +280,7 @@ def rule_impl(ctx):
276280

277281
@Test
278282
public void outputDirectoriesCanBeChainedToSubsequentMapDirectoryCalls() throws Exception {
283+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
279284
write(
280285
"test/rule_def.bzl",
281286
"""
@@ -337,6 +342,7 @@ def rule_impl(ctx):
337342

338343
@Test
339344
public void executionRequirementsPropagatedToExpandedActions() throws Exception {
345+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
340346
write(
341347
"test/rule_def.bzl",
342348
"""
@@ -377,6 +383,7 @@ def rule_impl(ctx):
377383

378384
@Test
379385
public void actionEnvironmentPropagatedToExpandedActions() throws Exception {
386+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
380387
write(
381388
"test/rule_def.bzl",
382389
"""
@@ -462,6 +469,7 @@ def rule_impl(ctx):
462469
@TestParameters("{value: 'set()', errorType: 'set'}")
463470
// Only boolean integer and strings are allowed in additional_params.
464471
public void disallowedAdditionalParams(String value, String errorType) throws Exception {
472+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
465473
write(
466474
"test/rule_def.bzl",
467475
String.format(
@@ -504,6 +512,7 @@ def rule_impl(ctx):
504512
@TestParameters("{inputs: '{}', outputs: '{\"output_dir\": output_dir}', errorType: 'input'}")
505513
public void emptyInputOrOutputDirectoriesNotAllowed(
506514
String inputs, String outputs, String errorType) throws Exception {
515+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
507516
write(
508517
"test/rule_def.bzl",
509518
String.format(
@@ -533,6 +542,7 @@ def rule_impl(ctx):
533542

534543
@Test
535544
public void failingImplementation() throws Exception {
545+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
536546
write(
537547
"test/rule_def.bzl",
538548
"""
@@ -566,6 +576,7 @@ def rule_impl(ctx):
566576

567577
@Test
568578
public void cannotDeclareFileInNonOutputDirectory() throws Exception {
579+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
569580
write(
570581
"test/rule_def.bzl",
571582
"""
@@ -601,6 +612,8 @@ def rule_impl(ctx):
601612

602613
@Test
603614
public void actionConflicts_conflictingOutputsInSameDirectory() throws Exception {
615+
// Don't check serialization here, since the action conflict only occurs during execution,
616+
// but serialization checks end up throwing (due to action conflicts) before we get there.
604617
write(
605618
"test/rule_def.bzl",
606619
"""
@@ -651,6 +664,7 @@ def rule_impl(ctx):
651664
@TestParameters("{output: 'some_file', path: 'test/some_file'}")
652665
public void actionConflicts_conflictingOutputsFromOtherContext(String output, String path)
653666
throws Exception {
667+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
654668
write(
655669
"test/rule_def.bzl",
656670
String.format(
@@ -717,6 +731,7 @@ def rule_impl(ctx):
717731
@TestParameters("{value: '(1, 2)', repr: '\\(1, 2\\)'}")
718732
public void implementationWithNonNoneReturnValueDisallowed(String value, String repr)
719733
throws Exception {
734+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
720735
write(
721736
"test/rule_def.bzl",
722737
String.format(
@@ -760,6 +775,7 @@ def rule_impl(ctx):
760775
public void nonTopLevelImplementationsDisallowed(
761776
@TestParameter({"non_top_level_impl", "lambda_impl"}) String implementation)
762777
throws Exception {
778+
SkyframeExecutorTestHelper.process(getSkyframeExecutor());
763779
write(
764780
"test/rule_def.bzl",
765781
String.format(

0 commit comments

Comments
 (0)