Skip to content

Commit

Permalink
Correctly log paths for runfiles and filesets.
Browse files Browse the repository at this point in the history 
The input mapping keys are canonical wrt the filesystem location of an input file as observed by the spawn. However, in the case of a runfiles or fileset tree, the actual contents are found at the filesystem location of the input mapping values.

PiperOrigin-RevId: 588801774
Change-Id: Iea111802b006f5750655eb4fa228abdd52567812
  • Loading branch information
@tjgq @copybara-github
tjgq authored and copybara-github committed Dec 7, 2023
1 parent dbd5c6e commit 773daaf
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 50 deletions.
106 changes: 60 additions & 46 deletions src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,30 +157,32 @@ public void logSpawn(

try (SilentCloseable c = Profiler.instance().profile("logSpawn/inputs")) {
for (Map.Entry<PathFragment, ActionInput> e : inputMap.entrySet()) {
PathFragment displayPath = e.getKey();
ActionInput input = e.getValue();
if (input instanceof VirtualActionInput.EmptyActionInput) {
continue;
}
Path inputPath = fileSystem.getPath(execRoot.getRelative(input.getExecPathString()));
if (inputPath.isDirectory()) {
listDirectoryContents(inputPath, builder::addInputs, inputMetadataProvider);
Path contentPath = fileSystem.getPath(execRoot.getRelative(input.getExecPathString()));
if (contentPath.isDirectory()) {
listDirectoryContents(
displayPath, contentPath, builder::addInputs, inputMetadataProvider);
continue;
}
Digest digest =
computeDigest(
input, inputPath, inputMetadataProvider, xattrProvider, digestHashFunction);
input, contentPath, inputMetadataProvider, xattrProvider, digestHashFunction);
boolean isTool =
toolFiles.contains(input)
|| (input instanceof TreeFileArtifact
&& toolFiles.contains(((TreeFileArtifact) input).getParent()));
builder
.addInputsBuilder()
.setPath(input.getExecPathString())
.setPath(displayPath.getPathString())
.setDigest(digest)
.setIsTool(isTool);
}
} catch (IOException e) {
logger.atWarning().withCause(e).log("Error computing spawn inputs");
logger.atWarning().withCause(e).log("Error computing spawn input properties");
}
try (SilentCloseable c = Profiler.instance().profile("logSpawn/outputs")) {
ArrayList<String> outputPaths = new ArrayList<>();
Expand All @@ -189,21 +191,24 @@ public void logSpawn(
}
Collections.sort(outputPaths);
builder.addAllListedOutputs(outputPaths);
for (Map.Entry<Path, ActionInput> e : existingOutputs.entrySet()) {
Path path = e.getKey();
if (path.isDirectory()) {
listDirectoryContents(path, builder::addActualOutputs, inputMetadataProvider);
} else {
File.Builder outputBuilder = builder.addActualOutputsBuilder();
outputBuilder.setPath(path.relativeTo(fileSystem.getPath(execRoot)).toString());
try {
outputBuilder.setDigest(
computeDigest(
e.getValue(), path, inputMetadataProvider, xattrProvider, digestHashFunction));
} catch (IOException ex) {
logger.atWarning().withCause(ex).log("Error computing spawn event output properties");
try {
for (Map.Entry<Path, ActionInput> e : existingOutputs.entrySet()) {
Path path = e.getKey();
ActionInput output = e.getValue();
if (path.isDirectory()) {
listDirectoryContents(
output.getExecPath(), path, builder::addActualOutputs, inputMetadataProvider);
continue;
}
builder
.addActualOutputsBuilder()
.setPath(output.getExecPathString())
.setDigest(
computeDigest(
output, path, inputMetadataProvider, xattrProvider, digestHashFunction));
}
} catch (IOException ex) {
logger.atWarning().withCause(ex).log("Error computing spawn output properties");
}
}
builder.setRemotable(Spawns.mayBeExecutedRemotely(spawn));
Expand Down Expand Up @@ -288,35 +293,44 @@ private SortedMap<Path, ActionInput> listExistingOutputs(Spawn spawn, FileSystem
return result;
}

/**
* Expands a directory into its contents.
*
* <p>Note the difference between {@code displayPath} and {@code contentPath}: the first is where
* the spawn can find the directory, while the second is where Bazel can find it. They're not the
* same for a directory appearing in a runfiles or fileset tree.
*/
private void listDirectoryContents(
Path path, Consumer<File> addFile, InputMetadataProvider inputMetadataProvider) {
try {
// TODO(olaola): once symlink API proposal is implemented, report symlinks here.
List<Dirent> sortedDirent = new ArrayList<>(path.readdir(Symlinks.NOFOLLOW));
sortedDirent.sort(Comparator.comparing(Dirent::getName));
for (Dirent dirent : sortedDirent) {
String name = dirent.getName();
Path child = path.getRelative(name);
if (dirent.getType() == Dirent.Type.DIRECTORY) {
listDirectoryContents(child, addFile, inputMetadataProvider);
} else {
String pathString;
if (child.startsWith(execRoot)) {
pathString = child.asFragment().relativeTo(execRoot).getPathString();
} else {
pathString = child.getPathString();
}
addFile.accept(
File.newBuilder()
.setPath(pathString)
.setDigest(
computeDigest(
null, child, inputMetadataProvider, xattrProvider, digestHashFunction))
.build());
}
PathFragment displayPath,
Path contentPath,
Consumer<File> addFile,
InputMetadataProvider inputMetadataProvider)
throws IOException {
// TODO(olaola): once symlink API proposal is implemented, report symlinks here.
List<Dirent> sortedDirent = new ArrayList<>(contentPath.readdir(Symlinks.NOFOLLOW));
sortedDirent.sort(Comparator.comparing(Dirent::getName));

for (Dirent dirent : sortedDirent) {
String name = dirent.getName();
PathFragment childDisplayPath = displayPath.getChild(name);
Path childContentPath = contentPath.getChild(name);

if (dirent.getType() == Dirent.Type.DIRECTORY) {
listDirectoryContents(childDisplayPath, childContentPath, addFile, inputMetadataProvider);
continue;
}
} catch (IOException e) {
logger.atWarning().withCause(e).log("Error computing spawn event file properties");

addFile.accept(
File.newBuilder()
.setPath(childDisplayPath.getPathString())
.setDigest(
computeDigest(
null,
childContentPath,
inputMetadataProvider,
xattrProvider,
digestHashFunction))
.build());
}
}
}
45 changes: 41 additions & 4 deletions src/test/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,15 +245,16 @@ public void testTreeInput(
}

@Test
public void testRunfilesInput() throws Exception {
public void testRunfilesFileInput() throws Exception {
Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "data.txt");

writeFile(runfilesInput, "abc");

SpawnLogContext context = createSpawnLogContext();

context.logSpawn(
// In reality, the spawn would have a RunfilesSupplier and a runfiles middleman input.
// In reality, the spawn would have a RunfilesSupplier and a runfiles middleman input, but
// these don't affect the operation of the spawn logger.
defaultSpawn(),
createInputMetadataProvider(runfilesInput),
/* inputMap= */ ImmutableSortedMap.of(
Expand All @@ -262,11 +263,47 @@ public void testRunfilesInput() throws Exception {
defaultTimeout(),
defaultSpawnResult());

// TODO(tjgq): The path should be foo.runfiles/data.txt.
closeAndAssertLog(
context,
defaultSpawnExecBuilder()
.addInputs(File.newBuilder().setPath("data.txt").setDigest(getDigest("abc")))
.addInputs(
File.newBuilder().setPath("out/foo.runfiles/data.txt").setDigest(getDigest("abc")))
.build());
}

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

runfilesInput.getPath().createDirectoryAndParents();
if (!dirContents.isEmpty()) {
writeFile(runfilesInput.getPath().getChild("data.txt"), "abc");
}

SpawnLogContext context = createSpawnLogContext();

context.logSpawn(
// In reality, the spawn would have a RunfilesSupplier and a runfiles middleman input, but
// these don't affect the operation of the spawn logger.
defaultSpawn(),
createInputMetadataProvider(runfilesInput),
/* inputMap= */ ImmutableSortedMap.of(
outputDir.getExecPath().getRelative("foo.runfiles/dir"), runfilesInput),
fs,
defaultTimeout(),
defaultSpawnResult());

closeAndAssertLog(
context,
defaultSpawnExecBuilder()
.addAllInputs(
dirContents.isEmpty()
? ImmutableList.of()
: ImmutableList.of(
File.newBuilder()
.setPath("out/foo.runfiles/dir/data.txt")
.setDigest(getDigest("abc"))
.build()))
.build());
}

Expand Down

0 comments on commit 773daaf

Please sign in to comment.