From 07668e6861dcb13b8575b896964e32c6f0112a3e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 7 Oct 2024 11:28:12 -0700 Subject: [PATCH] Add execlog test with nested runfiles middleman Verifies that runfiles middlemen artifacts in non-top-level nested sets are handled. Also makes the tests more realistic by ensuring that tools are always a subset of inputs. Related to #23884 Closes #23890. PiperOrigin-RevId: 683259163 Change-Id: Ic71377c069970038f20c0bd27bb7f719f3daef70 --- .../lib/exec/CompactSpawnLogContextTest.java | 4 +- .../lib/exec/SpawnLogContextTestBase.java | 93 ++++++++++++++----- 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java index 7267bd4cc783a6..8bb508bf2d2851 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java @@ -185,14 +185,14 @@ public void testRunfilesTreeReusedForTool() throws Exception { context.logSpawn( firstSpawn, - createInputMetadataProvider(toolRunfilesMiddleman, runfilesTree, firstInput), + createInputMetadataProvider(runfilesTree, toolRunfilesMiddleman, firstInput), createInputMap(runfilesTree, firstInput), fs, defaultTimeout(), defaultSpawnResult()); context.logSpawn( secondSpawn, - createInputMetadataProvider(toolRunfilesMiddleman, runfilesTree, secondInput), + createInputMetadataProvider(runfilesTree, toolRunfilesMiddleman, secondInput), createInputMap(runfilesTree, secondInput), fs, defaultTimeout(), diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java index 76b68d70b44b38..24f8f6967a8521 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java @@ -26,7 +26,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; -import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionEnvironment; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; @@ -430,7 +429,7 @@ public void testRunfilesFileInput(@TestParameter InputsMode inputsMode) throws E context.logSpawn( spawnBuilder.build(), - createInputMetadataProvider(runfilesMiddleman, runfilesTree, runfilesInput), + createInputMetadataProvider(runfilesTree, runfilesMiddleman, runfilesInput), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -451,6 +450,60 @@ public void testRunfilesFileInput(@TestParameter InputsMode inputsMode) throws E .build()); } + @Test + public void testRunfilesNestedMiddleman() throws Exception { + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "data.txt"); + writeFile(runfilesInput, "abc"); + Artifact toolFile1 = ActionsTestUtil.createArtifact(rootDir, "tool1"); + writeFile(toolFile1, "def"); + Artifact toolFile2 = ActionsTestUtil.createArtifact(rootDir, "tool2"); + writeFile(toolFile2, "ghi"); + + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); + RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput); + + NestedSet tools = + NestedSetBuilder.stableOrder() + .add(toolFile1) + .addTransitive( + NestedSetBuilder.stableOrder() + .add(runfilesMiddleman) + .add(toolFile2) + .build()) + .build(); + Spawn spawn = defaultSpawnBuilder().withInputs(tools).withTools(tools).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesTree, runfilesMiddleman, runfilesInput, toolFile1, toolFile2), + createInputMap(runfilesTree, toolFile1, toolFile2), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/data.txt") + .setDigest(getDigest("abc")) + .setIsTool(true)) + .addInputs( + File.newBuilder().setPath("tool1").setDigest(getDigest("def")).setIsTool(true)) + .addInputs( + File.newBuilder().setPath("tool2").setDigest(getDigest("ghi")).setIsTool(true)) + .build()); + } + @Test public void testRunfilesDirectoryInput( @TestParameter DirContents dirContents, @TestParameter InputsMode inputsMode) @@ -475,7 +528,7 @@ public void testRunfilesDirectoryInput( context.logSpawn( spawnBuilder.build(), - createInputMetadataProvider(runfilesMiddleman, runfilesTree, runfilesInput), + createInputMetadataProvider(runfilesTree, runfilesMiddleman, runfilesInput), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -539,8 +592,8 @@ public void testRunfilesEmptyInput(@TestParameter InputsMode inputsMode) throws context.logSpawn( spawnBuilder.build(), createInputMetadataProvider( - runfilesMiddleman, runfilesTree, + runfilesMiddleman, runfilesInput, externalGenArtifact, externalSourceArtifact), @@ -685,8 +738,8 @@ public void testRunfilesMixedRoots(@TestParameter boolean legacyExternalRunfiles context.logSpawn( spawn, createInputMetadataProvider( - runfilesMiddleman, runfilesTree, + runfilesMiddleman, sourceArtifact, genArtifact, externalSourceArtifact, @@ -830,8 +883,8 @@ public void testRunfilesExternalOnly( context.logSpawn( spawn, createInputMetadataProvider( - runfilesMiddleman, runfilesTree, + runfilesMiddleman, externalSourceArtifact, externalGenArtifact, symlinkTarget, @@ -963,8 +1016,8 @@ public void testRunfilesFilesCollide(@TestParameter boolean legacyExternalRunfil context.logSpawn( spawn, createInputMetadataProvider( - runfilesMiddleman, runfilesTree, + runfilesMiddleman, sourceArtifact, genArtifact, externalSourceArtifact, @@ -1069,8 +1122,8 @@ public void testRunfilesFilesAndSymlinksCollide(@TestParameter boolean legacyExt context.logSpawn( spawn, createInputMetadataProvider( - runfilesMiddleman, runfilesTree, + runfilesMiddleman, sourceArtifact, genArtifact, externalSourceArtifact, @@ -1162,7 +1215,7 @@ public void testRunfilesFileAndRootSymlinkCollide() throws Exception { context.logSpawn( spawn, createInputMetadataProvider( - runfilesMiddleman, runfilesTree, sourceArtifact, symlinkSourceArtifact), + runfilesTree, runfilesMiddleman, sourceArtifact, symlinkSourceArtifact), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -1209,7 +1262,7 @@ public void testRunfilesCrossTypeCollision(@TestParameter boolean symlinkFirst) context.logSpawn( spawn, - createInputMetadataProvider(runfilesMiddleman, runfilesTree, file, symlink), + createInputMetadataProvider(runfilesTree, runfilesMiddleman, file, symlink), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -1286,7 +1339,7 @@ public void testRunfilesPostOrderCollision(@TestParameter boolean nestBoth) thro context.logSpawn( spawn, createInputMetadataProvider( - runfilesMiddleman, runfilesTree, sourceFile, genFile, otherSourceFile, otherGenFile), + runfilesTree, runfilesMiddleman, sourceFile, genFile, otherSourceFile, otherGenFile), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -1364,7 +1417,7 @@ public void testRunfilesSymlinkTargets( context.logSpawn( spawnBuilder.build(), createInputMetadataProvider( - runfilesMiddleman, runfilesTree, sourceFile, sourceDir, genDir, symlink), + runfilesTree, runfilesMiddleman, sourceFile, sourceDir, genDir, symlink), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -1437,7 +1490,7 @@ public void testRunfileSymlinkFileWithDirectoryContents( context.logSpawn( spawn, - createInputMetadataProvider(runfilesMiddleman, runfilesTree, sourceFile), + createInputMetadataProvider(runfilesTree, runfilesMiddleman, sourceFile), createInputMap(runfilesTree), outputsMode.getActionFileSystem(fs), defaultTimeout(), @@ -2059,19 +2112,13 @@ protected static RunfilesTree createRunfilesTree( protected static InputMetadataProvider createInputMetadataProvider(Artifact... artifacts) throws Exception { - return createInputMetadataProvider(null, null, artifacts); + return createInputMetadataProvider(null, artifacts); } protected static InputMetadataProvider createInputMetadataProvider( - Artifact runfilesMiddleman, RunfilesTree runfilesTree, Artifact... artifacts) - throws Exception { - Iterable allArtifacts = Arrays.asList(artifacts); + RunfilesTree runfilesTree, Artifact... artifacts) throws Exception { FakeActionInputFileCache builder = new FakeActionInputFileCache(); - if (runfilesMiddleman != null) { - allArtifacts = Iterables.concat(allArtifacts, runfilesTree.getArtifacts().toList()); - builder.putRunfilesTree(runfilesMiddleman, runfilesTree); - } - for (Artifact artifact : allArtifacts) { + for (Artifact artifact : artifacts) { if (artifact.isTreeArtifact()) { // Emulate ActionInputMap: add both tree and children. TreeArtifactValue treeMetadata = createTreeArtifactValue(artifact); @@ -2082,6 +2129,8 @@ protected static InputMetadataProvider createInputMetadataProvider( } } else if (artifact.isSymlink()) { builder.put(artifact, FileArtifactValue.createForUnresolvedSymlink(artifact)); + } else if (artifact.isMiddlemanArtifact()) { + builder.putRunfilesTree(artifact, runfilesTree); } else { builder.put(artifact, FileArtifactValue.createForTesting(artifact)); }