From 39974a43abdd32e3a1acbc7da945b08da9983e4e Mon Sep 17 00:00:00 2001 From: felly Date: Thu, 9 Aug 2018 14:28:14 -0700 Subject: Allow skyframe-aware actions to pass partial results through ActionExecutionContext. Remove FilesetProvider. PiperOrigin-RevId: 208111251 --- .../lib/actions/ExecutableSymlinkActionTest.java | 1 + .../build/lib/actions/util/ActionsTestUtil.java | 6 +- .../analysis/actions/FileWriteActionTestCase.java | 1 + .../analysis/actions/ParamFileWriteActionTest.java | 3 +- .../lib/analysis/actions/SymlinkActionTest.java | 3 +- .../actions/TemplateExpansionActionTest.java | 3 +- .../build/lib/analysis/util/BuildViewTestCase.java | 3 +- .../lib/rules/cpp/CreateIncSymlinkActionTest.java | 3 +- .../build/lib/rules/cpp/LtoBackendActionTest.java | 3 +- .../lib/rules/objc/BazelJ2ObjcLibraryTest.java | 2 + .../lib/skyframe/FilesetEntryFunctionTest.java | 184 +-------------------- .../lib/skyframe/SkyframeAwareActionTest.java | 6 +- .../standalone/StandaloneSpawnStrategyTest.java | 3 +- 13 files changed, 30 insertions(+), 191 deletions(-) (limited to 'src/test') diff --git a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java index 9f233b3dd4..5f8bf79f1c 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java @@ -66,6 +66,7 @@ public class ExecutableSymlinkActionTest { ImmutableMap.of(), ImmutableMap.of(), null, + null, null); } diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 80b8e7e403..1aa4022b34 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -147,7 +147,8 @@ public final class ActionsTestUtil { actionGraph == null ? createDummyArtifactExpander() : ActionInputHelper.actionGraphArtifactExpander(actionGraph), - /*actionFileSystem=*/ null); + /*actionFileSystem=*/ null, + /*skyframeDepsResult=*/ null); } public static ActionExecutionContext createContextForInputDiscovery( @@ -182,7 +183,8 @@ public final class ActionsTestUtil { ImmutableMap.of(), ImmutableMap.of(), createDummyArtifactExpander(), - /*actionFileSystem=*/ null); + /*actionFileSystem=*/ null, + /*skyframeDepsResult=*/ null); } private static ArtifactExpander createDummyArtifactExpander() { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java index 12cbfb4ffe..fe4b0a2805 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java @@ -68,6 +68,7 @@ public abstract class FileWriteActionTestCase extends BuildViewTestCase { ImmutableMap.of(), ImmutableMap.of(), null, + null, null); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java index 59364a3328..b736576d57 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java @@ -203,7 +203,8 @@ public class ParamFileWriteActionTest extends BuildViewTestCase { ImmutableMap.of(), ImmutableMap.of(), artifactExpander, - /*actionFileSystem=*/ null); + /*actionFileSystem=*/ null, + /*skyframeDepsResult=*/ null); } private enum KeyAttributes { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java index c3e25614fd..ae213a4616 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java @@ -90,7 +90,8 @@ public class SymlinkActionTest extends BuildViewTestCase { ImmutableMap.of(), ImmutableMap.of(), null, - /*actionFileSystem=*/ null)); + /*actionFileSystem=*/ null, + /*skyframeDepsResult=*/ null)); assertThat(actionResult.spawnResults()).isEmpty(); assertThat(output.isSymbolicLink()).isTrue(); assertThat(output.resolveSymbolicLinks()).isEqualTo(input); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java index 48c169af4d..b7fc6c755e 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java @@ -192,7 +192,8 @@ public class TemplateExpansionActionTest extends FoundationTestCase { ImmutableMap.of(), ImmutableMap.of(), null, - /*actionFileSystem=*/ null); + /*actionFileSystem=*/ null, + /*skyframeDepsResult=*/ null); } private void executeTemplateExpansion(String expected) throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index c3bea4f72c..9a1816864f 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -2168,7 +2168,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase { clientEnv, ImmutableMap.of(), artifactExpander, - /*actionFileSystem=*/ null); + /*actionFileSystem=*/ null, + /*skyframeDepsResult*/ null); } } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java index c447f67450..e84c832e47 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java @@ -116,7 +116,8 @@ public class CreateIncSymlinkActionTest extends FoundationTestCase { private ActionExecutionContext makeDummyContext() { DummyExecutor executor = new DummyExecutor(fileSystem, rootDirectory); return new ActionExecutionContext( - executor, null, null, null, null, null, ImmutableMap.of(), ImmutableMap.of(), null, null); + executor, null, null, null, null, null, ImmutableMap.of(), ImmutableMap.of(), null, null, + null); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java index 25c7e0d444..f8881ee628 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java @@ -90,7 +90,8 @@ public class LtoBackendActionTest extends BuildViewTestCase { ImmutableMap.of(), ImmutableMap.of(), null, - /*actionFileSystem=*/ null); + /*actionFileSystem=*/ null, + /*skyframeDepsResult=*/ null); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java index f617673885..c74e14000d 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java @@ -794,6 +794,7 @@ public class BazelJ2ObjcLibraryTest extends J2ObjcLibraryTest { ImmutableMap.of(), ImmutableMap.of(), DUMMY_ARTIFACT_EXPANDER, + null, null); ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream(); ByteArrayOutputStream umbrellaHeaderStream = new ByteArrayOutputStream(); @@ -845,6 +846,7 @@ public class BazelJ2ObjcLibraryTest extends J2ObjcLibraryTest { ImmutableMap.of(), ImmutableMap.of(), DUMMY_ARTIFACT_EXPANDER, + null, null); ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java index e972020051..5e5ff1420c 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java @@ -592,147 +592,6 @@ public final class FilesetEntryFunctionTest extends FoundationTestCase { assertRecursiveTraversalForPackage(SymlinkBehavior.DEREFERENCE, REPORT_ERROR); } - @Test - public void testNestedFileFilesetTraversal() throws Exception { - Artifact path1 = getSourceArtifact("foo/bar.file"); - createFile(path1, "blah"); - Artifact path2 = getSourceArtifact("foo/baz.file"); - createFile(path2, "what"); - FilesetTraversalParams inner1 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//foo"), - /*fileToTraverse=*/ path1, - PathFragment.create("inner-out1"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - FilesetTraversalParams inner2 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//foo"), - /*fileToTraverse=*/ path2, - PathFragment.create("inner-out2"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - FilesetTraversalParams outer = - FilesetTraversalParamsFactory.nestedTraversal( - /*ownerLabel=*/ label("//foo:bar"), - /*nested=*/ ImmutableList.of(inner1, inner2), - PathFragment.create("outer-out"), - /*excludes=*/ null); - assertSymlinksCreatedInOrder( - outer, - symlink("outer-out/inner-out1", rootedPath(path1)), - symlink("outer-out/inner-out2", rootedPath(path2))); - } - - @Test - public void testMultiLevelNesting() throws Exception { - Artifact path1 = getSourceArtifact("foo/bar.file"); - createFile(path1, "blah"); - Artifact path2 = getSourceArtifact("foo/baz.file"); - createFile(path2, "what"); - Artifact path3 = getSourceArtifact("foo/hw.file"); - createFile(path3, "hello"); - FilesetTraversalParams inner1 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//foo"), - /*fileToTraverse=*/ path1, - PathFragment.create("inner-out1"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - FilesetTraversalParams inner2 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//foo"), - /*fileToTraverse=*/ path2, - PathFragment.create("inner-out2"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - FilesetTraversalParams middle1 = - FilesetTraversalParamsFactory.nestedTraversal( - /*ownerLabel=*/ label("//foo:middle1"), - /*nested=*/ ImmutableList.of(inner1, inner2), - PathFragment.create("middle-out1"), - /*excludes=*/ null); - - FilesetTraversalParams inner3 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//foo:inner3"), - /*fileToTraverse=*/ path3, - PathFragment.create("inner-out3"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - FilesetTraversalParams middle2 = - FilesetTraversalParamsFactory.nestedTraversal( - /*ownerLabel=*/ label("//foo:middle2"), - /*nested=*/ ImmutableList.of(inner3), - PathFragment.create("middle-out2"), - /*excludes=*/ null); - - FilesetTraversalParams outer = - FilesetTraversalParamsFactory.nestedTraversal( - /*ownerLabel=*/ label("//foo:bar"), - /*nested=*/ ImmutableList.of(middle1, middle2), - PathFragment.create("outer-out"), - /*excludes=*/ null); - assertSymlinksCreatedInOrder( - outer, - symlink("outer-out/middle-out1/inner-out1", rootedPath(path1)), - symlink("outer-out/middle-out1/inner-out2", rootedPath(path2)), - symlink("outer-out/middle-out2/inner-out3", rootedPath(path3))); - } - - private void assertNestedRecursiveFilesetTraversal(boolean useInnerDir) throws Exception { - Artifact dir = getSourceArtifact("foo/dir"); - RootedPath fileA = createFile(childOf(dir, "file.a"), "hello"); - RootedPath fileB = createFile(childOf(dir, "file.b"), "hello"); - RootedPath fileC = createFile(childOf(dir, "sub/file.c"), "world"); - - FilesetTraversalParams inner = - FilesetTraversalParamsFactory.recursiveTraversalOfDirectory( - /*ownerLabel=*/ label("//foo"), - /*directoryToTraverse=*/ dir, - PathFragment.create(useInnerDir ? "inner-dir" : ""), - /*excludes=*/ null, - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - FilesetTraversalParams outer = - FilesetTraversalParamsFactory.nestedTraversal( - /*ownerLabel=*/ label("//foo"), - /*nested=*/ ImmutableList.of(inner), - PathFragment.create("outer-dir"), - /*excludes=*/ ImmutableSet.of("file.a", "sub/file.c")); - - if (useInnerDir) { - assertSymlinksCreatedInOrder( - outer, - // no file is excluded, since no files from "inner" are top-level in the outer Fileset - symlink("outer-dir/inner-dir/file.a", fileA), - symlink("outer-dir/inner-dir/file.b", fileB), - symlink("outer-dir/inner-dir/sub/file.c", fileC)); // only top-level files are excluded - } else { - assertSymlinksCreatedInOrder( - outer, - // file.a can be excluded because it's top-level (there's no output directory for "inner") - symlink("outer-dir/file.b", fileB), - symlink("outer-dir/sub/file.c", fileC)); // only top-level files could be excluded - } - } - - @Test - public void testNestedRecursiveFilesetTraversalWithInnerDestDir() throws Exception { - assertNestedRecursiveFilesetTraversal(true); - } - - @Test - public void testNestedRecursiveFilesetTraversalWithoutInnerDestDir() throws Exception { - assertNestedRecursiveFilesetTraversal(false); - } - @Test public void testFileTraversalForDanglingSymlink() throws Exception { Artifact linkName = getSourceArtifact("foo/dangling.sym"); @@ -1071,48 +930,13 @@ public final class FilesetEntryFunctionTest extends FoundationTestCase { @Test public void testFingerprintOfNestedTraversal() throws Exception { - FilesetTraversalParams n1 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//blah"), - /*fileToTraverse=*/ getSourceArtifact("blah/file.a"), - PathFragment.create("output-name"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - - FilesetTraversalParams n2 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//blah"), - /*fileToTraverse=*/ getSourceArtifact("meow/file.b"), - PathFragment.create("output-name"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - ImmutableList nested1 = ImmutableList.of(n1, n2); - - FilesetTraversalParams n3 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//blah"), - /*fileToTraverse=*/ getSourceArtifact("brrr/file.c"), - PathFragment.create("output-name"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - - FilesetTraversalParams n4 = - FilesetTraversalParamsFactory.fileTraversal( - /*ownerLabel=*/ label("//blah"), - /*fileToTraverse=*/ getSourceArtifact("hurr/file.d"), - PathFragment.create("output-name"), - /*symlinkBehaviorMode=*/ SymlinkBehavior.COPY, - /*pkgBoundaryMode=*/ DONT_CROSS, - /*strictFilesetOutput=*/ false); - ImmutableList nested2 = ImmutableList.of(n3, n4); + Artifact nested1 = getSourceArtifact("a/b"); + Artifact nested2 = getSourceArtifact("a/c"); new FingerprintTester( ImmutableMap.of( "ownerLabel", notPartOfFingerprint("//foo", "//bar"), - "nested", partOfFingerprint(nested1, nested2), + "nestedArtifact", partOfFingerprint(nested1, nested2), "destDir", partOfFingerprint("out1", "out2"), "excludes", partOfFingerprint(ImmutableSet.of(), ImmutableSet.of("x")))) { @@ -1121,7 +945,7 @@ public final class FilesetEntryFunctionTest extends FoundationTestCase { FilesetTraversalParams create(Map kwArgs) throws Exception { return FilesetTraversalParamsFactory.nestedTraversal( label((String) kwArgs.get("ownerLabel")), - (ImmutableList) kwArgs.get("nested"), + (Artifact) kwArgs.get("nestedArtifact"), PathFragment.create((String) kwArgs.get("destDir")), (Set) kwArgs.get("excludes")); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java index 3060a08d3e..18f117f43a 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java @@ -249,11 +249,12 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase { } @Override - public void establishSkyframeDependencies(Environment env) + public Object establishSkyframeDependencies(Environment env) throws ExceptionBase, InterruptedException { // Establish some Skyframe dependency. A real action would then use this to compute and // cache data for the execute(...) method. env.getValue(actionDepKey); + return null; } } @@ -797,8 +798,9 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase { registerAction( new SingleOutputSkyframeAwareAction(genFile1, genFile2) { @Override - public void establishSkyframeDependencies(Environment env) throws ExceptionBase { + public Object establishSkyframeDependencies(Environment env) throws ExceptionBase { assertThat(env.valuesMissing()).isFalse(); + return null; } @Override diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java index da31f120d2..ccccc612e1 100644 --- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java @@ -194,7 +194,8 @@ public class StandaloneSpawnStrategyTest { ImmutableMap.of(), ImmutableMap.of(), SIMPLE_ARTIFACT_EXPANDER, - /*actionFileSystem=*/ null); + /*actionFileSystem=*/ null, + /*skyframeDepsResult=*/ null); } @Test -- cgit v1.2.3