Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mark tool runfiles as such in expanded execution log #23702

Closed
wants to merge 1 commit into from
+81 −30
Closed

Mark tool runfiles as such in expanded execution log #23702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/exec/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/actions:runfiles_metadata",
"//src/main/java/com/google/devtools/build/lib/actions:runfiles_tree",
"//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry",
"//src/main/java/com/google/devtools/build/lib/analysis/platform:platform_utils",
Expand Down
23 changes: 21 additions & 2 deletions src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,20 @@
// limitations under the License.
package com.google.devtools.build.lib.exec;

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.FileArtifactValue.UnresolvedSymlinkArtifactValue;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.RunfilesArtifactValue;
import com.google.devtools.build.lib.actions.RunfilesTree;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.Spawns;
Expand Down Expand Up @@ -152,6 +158,15 @@ public void logSpawn(
builder.addAllEnvironmentVariables(getEnvironmentVariables(spawn));

ImmutableSet<? extends ActionInput> toolFiles = spawn.getToolFiles().toSet();
ImmutableList<PathFragment> toolRunfilesDirectories =
toolFiles.stream()
.filter(
actionInput ->
actionInput instanceof Artifact artifact && artifact.isMiddlemanArtifact())
.map(inputMetadataProvider::getRunfilesMetadata)
.map(RunfilesArtifactValue::getRunfilesTree)
.map(RunfilesTree::getExecPath)
.collect(toImmutableList());

try (SilentCloseable c1 = Profiler.instance().profile("logSpawn/inputs")) {
for (Map.Entry<PathFragment, ActionInput> e : inputMap.entrySet()) {
Expand All @@ -160,14 +175,18 @@ public void logSpawn(

if (input instanceof VirtualActionInput.EmptyActionInput) {
// Do not include a digest, as it's a waste of space.
builder.addInputsBuilder().setPath(displayPath.getPathString());
builder
.addInputsBuilder()
.setPath(displayPath.getPathString())
.setIsTool(toolRunfilesDirectories.stream().anyMatch(displayPath::startsWith));
continue;
}

boolean isTool =
toolFiles.contains(input)
|| input instanceof TreeFileArtifact treeFileArtifact
&& toolFiles.contains(treeFileArtifact.getParent());
&& toolFiles.contains(treeFileArtifact.getParent())
|| toolRunfilesDirectories.stream().anyMatch(displayPath::startsWith);

Path contentPath = fileSystem.getPath(execRoot.getRelative(input.getExecPathString()));

Expand Down
87 changes: 59 additions & 28 deletions src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ public void testUnresolvedSymlinkInput(@TestParameter InputsMode inputsMode) thr
}

@Test
public void testRunfilesFileInput() throws Exception {
public void testRunfilesFileInput(@TestParameter InputsMode inputsMode) throws Exception {
Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "data.txt");
Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles");

Expand All @@ -420,12 +420,15 @@ public void testRunfilesFileInput() throws Exception {
PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles");
RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput);

Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build();
SpawnBuilder spawnBuilder = defaultSpawnBuilder().withInput(runfilesMiddleman);
if (inputsMode.isTool()) {
spawnBuilder.withTool(runfilesMiddleman);
}

SpawnLogContext context = createSpawnLogContext();

context.logSpawn(
spawn,
spawnBuilder.build(),
createInputMetadataProvider(runfilesMiddleman, runfilesTree, runfilesInput),
createInputMap(runfilesTree),
fs,
Expand All @@ -442,12 +445,15 @@ public void testRunfilesFileInput() throws Exception {
+ "-out/k8-fastbuild/bin/foo.runfiles/"
+ WORKSPACE_NAME
+ "/data.txt")
.setDigest(getDigest("abc")))
.setDigest(getDigest("abc"))
.setIsTool(inputsMode.isTool()))
.build());
}

@Test
public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) throws Exception {
public void testRunfilesDirectoryInput(
@TestParameter DirContents dirContents, @TestParameter InputsMode inputsMode)
throws Exception {
Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles");
Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "dir");

Expand All @@ -459,12 +465,15 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t
PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles");
RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput);

Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build();
SpawnBuilder spawnBuilder = defaultSpawnBuilder().withInput(runfilesMiddleman);
if (inputsMode.isTool()) {
spawnBuilder.withTool(runfilesMiddleman);
}

SpawnLogContext context = createSpawnLogContext();

context.logSpawn(
spawn,
spawnBuilder.build(),
createInputMetadataProvider(runfilesMiddleman, runfilesTree, runfilesInput),
createInputMap(runfilesTree),
fs,
Expand All @@ -485,12 +494,13 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t
+ WORKSPACE_NAME
+ "/dir/data.txt")
.setDigest(getDigest("abc"))
.setIsTool(inputsMode.isTool())
.build()))
.build());
}

@Test
public void testRunfilesEmptyInput() throws Exception {
public void testRunfilesEmptyInput(@TestParameter InputsMode inputsMode) throws Exception {
Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles");

Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "sub/dir/script.py");
Expand Down Expand Up @@ -518,12 +528,15 @@ public void testRunfilesEmptyInput() throws Exception {
createRunfilesTree(
runfilesRoot, runfilesInput, externalGenArtifact, externalSourceArtifact);

Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build();
SpawnBuilder spawnBuilder = defaultSpawnBuilder().withInput(runfilesMiddleman);
if (inputsMode.isTool()) {
spawnBuilder.withTool(runfilesMiddleman);
}

SpawnLogContext context = createSpawnLogContext();

context.logSpawn(
spawn,
spawnBuilder.build(),
createInputMetadataProvider(
runfilesMiddleman,
runfilesTree,
Expand All @@ -540,59 +553,69 @@ public void testRunfilesEmptyInput() throws Exception {
defaultSpawnExecBuilder()
.addInputs(
File.newBuilder()
.setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/__init__.py"))
.setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/__init__.py")
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/foo.runfiles/"
+ WORKSPACE_NAME
+ "/sub/__init__.py"))
+ "/sub/__init__.py")
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/foo.runfiles/"
+ WORKSPACE_NAME
+ "/sub/dir/__init__.py"))
+ "/sub/dir/__init__.py")
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/foo.runfiles/"
+ WORKSPACE_NAME
+ "/sub/dir/script.py")
.setDigest(getDigest("abc")))
.setDigest(getDigest("abc"))
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/__init__.py"))
PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/__init__.py")
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/__init__.py"))
+ "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/__init__.py")
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/__init__.py"))
+ "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/__init__.py")
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/gen.py")
.setDigest(getDigest("external_gen")))
.setDigest(getDigest("external_gen"))
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/__init__.py"))
+ "-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/__init__.py")
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/lib.py")
.setDigest(getDigest("external_source")))
.setDigest(getDigest("external_source"))
.setIsTool(inputsMode.isTool()))
.build());
}

Expand Down Expand Up @@ -1291,7 +1314,8 @@ public void testRunfilesPostOrderCollision(@TestParameter boolean nestBoth) thro
}

@Test
public void testRunfilesSymlinkTargets(@TestParameter boolean rootSymlinks) throws Exception {
public void testRunfilesSymlinkTargets(
@TestParameter boolean rootSymlinks, @TestParameter InputsMode inputsMode) throws Exception {
Artifact sourceFile = ActionsTestUtil.createArtifact(rootDir, "pkg/file.txt");
writeFile(sourceFile, "source");
Artifact sourceDir = ActionsTestUtil.createArtifact(rootDir, "pkg/source_dir");
Expand Down Expand Up @@ -1329,12 +1353,15 @@ public void testRunfilesSymlinkTargets(@TestParameter boolean rootSymlinks) thro
: ImmutableMap.of(),
/* legacyExternalRunfiles= */ false);

Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build();
var spawnBuilder = defaultSpawnBuilder().withInput(runfilesMiddleman);
if (inputsMode.isTool()) {
spawnBuilder.withTool(runfilesMiddleman);
}

SpawnLogContext context = createSpawnLogContext();

context.logSpawn(
spawn,
spawnBuilder.build(),
createInputMetadataProvider(
runfilesMiddleman, runfilesTree, sourceFile, sourceDir, genDir, symlink),
createInputMap(runfilesTree),
Expand All @@ -1352,31 +1379,35 @@ public void testRunfilesSymlinkTargets(@TestParameter boolean rootSymlinks) thro
+ "-out/k8-fastbuild/bin/tools/foo.runfiles/"
+ WORKSPACE_NAME
+ "/file")
.setDigest(getDigest("source")))
.setDigest(getDigest("source"))
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/tools/foo.runfiles/"
+ WORKSPACE_NAME
+ "/gen_dir/other_file")
.setDigest(getDigest("gen_dir_file")))
.setDigest(getDigest("gen_dir_file"))
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/tools/foo.runfiles/"
+ WORKSPACE_NAME
+ "/source_dir/some_file")
.setDigest(getDigest("source_dir_file")))
.setDigest(getDigest("source_dir_file"))
.setIsTool(inputsMode.isTool()))
.addInputs(
File.newBuilder()
.setPath(
PRODUCT_NAME
+ "-out/k8-fastbuild/bin/tools/foo.runfiles/"
+ WORKSPACE_NAME
+ "/symlink")
.setSymlinkTargetPath("/some/path"))
.setSymlinkTargetPath("/some/path")
.setIsTool(inputsMode.isTool()))
.build());
}

Expand Down Expand Up @@ -1407,7 +1438,7 @@ public void testRunfileSymlinkFileWithDirectoryContents(
spawn,
createInputMetadataProvider(runfilesMiddleman, runfilesTree, genFile),
createInputMap(runfilesTree),
fs,
outputsMode.getActionFileSystem(fs),
defaultTimeout(),
defaultSpawnResult());

Expand Down
Loading