diff --git a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
index 93eab45bb15538..dbfc6bdc4767f5 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java
@@ -70,7 +70,7 @@ private interface ExecLogEntrySupplier {
private final XattrProvider xattrProvider;
// Maps a key identifying an entry into its ID.
- // Each key is either a NestedSet.Node or the String path of a file or directory.
+ // Each key is either a NestedSet.Node or the String path of a file, directory or symlink.
// Only entries that are likely to be referenced by future entries are stored.
// Use a specialized map for minimal memory footprint.
@GuardedBy("this")
@@ -143,17 +143,21 @@ public void logSpawn(
for (ActionInput output : spawn.getOutputFiles()) {
Path path = fileSystem.getPath(execRoot.getRelative(output.getExecPath()));
- if (!path.exists()) {
+ if (!output.isDirectory() && !output.isSymlink() && path.isFile()) {
builder.addOutputs(
- ExecLogEntry.Output.newBuilder().setMissingPath(output.getExecPathString()));
- } else if (path.isDirectory()) {
+ ExecLogEntry.Output.newBuilder()
+ .setFileId(logFile(output, path, inputMetadataProvider)));
+ } else if (output.isDirectory() && path.isDirectory()) {
builder.addOutputs(
ExecLogEntry.Output.newBuilder()
.setDirectoryId(logDirectory(output, path, inputMetadataProvider)));
- } else {
+ } else if (output.isSymlink() && path.isSymbolicLink()) {
builder.addOutputs(
ExecLogEntry.Output.newBuilder()
- .setFileId(logFile(output, path, inputMetadataProvider)));
+ .setUnresolvedSymlinkId(logUnresolvedSymlink(output, path)));
+ } else {
+ builder.addOutputs(
+ ExecLogEntry.Output.newBuilder().setInvalidOutputPath(output.getExecPathString()));
}
}
@@ -335,12 +339,12 @@ private int logFile(ActionInput input, Path path, InputMetadataProvider inputMet
/**
* Logs a directory.
*
- * This may be either a source directory, a fileset or an output directory. An output directory
- * is not guaranteed to be a tree artifact; we must support the case where a directory is produced
- * when a file was expected. For runfiles, {@link #logRunfilesDirectory} must be used instead.
+ *
This may be either a source directory, a fileset or an output directory. For runfiles,
+ * {@link #logRunfilesDirectory} must be used instead.
*
- * @param input the input representing the directory
- * @param root the path to the directory
+ * @param input the input representing the directory.
+ * @param root the path to the directory, which must have already been verified to be of the
+ * correct type.
* @return the entry ID of the {@link ExecLogEntry.Directory} describing the directory.
*/
private int logDirectory(
@@ -455,6 +459,26 @@ private ImmutableList expandDirectory(
return builder.build();
}
+ /**
+ * Logs an unresolved symlink.
+ *
+ * @param input the input representing the unresolved symlink.
+ * @param path the path to the unresolved symlink, which must have already been verified to be of
+ * the correct type.
+ * @return the entry ID of the {@link ExecLogEntry.UnresolvedSymlink} describing the unresolved
+ * symlink.
+ */
+ private int logUnresolvedSymlink(ActionInput input, Path path) throws IOException {
+ return logEntry(
+ input.getExecPathString(),
+ () ->
+ ExecLogEntry.newBuilder()
+ .setUnresolvedSymlink(
+ ExecLogEntry.UnresolvedSymlink.newBuilder()
+ .setPath(input.getExecPathString())
+ .setTargetPath(path.readSymbolicLink().getPathString())));
+ }
+
/**
* Ensures an entry is written to the log and returns its assigned ID.
*
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java
index be90501d3496d9..6054e5d7f00669 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.actions.ActionInput;
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.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -56,7 +57,6 @@
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
-import java.util.TreeMap;
import java.util.function.Consumer;
import javax.annotation.Nullable;
@@ -144,7 +144,6 @@ public void logSpawn(
SpawnResult result)
throws IOException, ExecException {
try (SilentCloseable c = Profiler.instance().profile("logSpawn")) {
- SortedMap existingOutputs = listExistingOutputs(spawn, fileSystem);
SpawnExec.Builder builder = SpawnExec.newBuilder();
builder.addAllCommandArgs(spawn.getArguments());
builder.addAllEnvironmentVariables(getEnvironmentVariables(spawn));
@@ -175,6 +174,16 @@ public void logSpawn(
continue;
}
+ if (input.isSymlink()) {
+ UnresolvedSymlinkArtifactValue metadata =
+ (UnresolvedSymlinkArtifactValue) inputMetadataProvider.getInputMetadata(input);
+ builder
+ .addInputsBuilder()
+ .setPath(displayPath.getPathString())
+ .setSymlinkTargetPath(metadata.getSymlinkTarget());
+ continue;
+ }
+
Digest digest =
computeDigest(
input,
@@ -201,29 +210,33 @@ public void logSpawn(
Collections.sort(outputPaths);
builder.addAllListedOutputs(outputPaths);
try {
- for (Map.Entry e : existingOutputs.entrySet()) {
- Path path = e.getKey();
- ActionInput output = e.getValue();
- if (path.isDirectory()) {
+ for (ActionInput output : spawn.getOutputFiles()) {
+ Path path = fileSystem.getPath(execRoot.getRelative(output.getExecPathString()));
+ if (!output.isDirectory() && !output.isSymlink() && path.isFile()) {
+ builder
+ .addActualOutputsBuilder()
+ .setPath(output.getExecPathString())
+ .setDigest(
+ computeDigest(
+ output,
+ path,
+ inputMetadataProvider,
+ xattrProvider,
+ digestHashFunction,
+ /* includeHashFunctionName= */ true));
+ } else if (output.isDirectory() && path.isDirectory()) {
listDirectoryContents(
output.getExecPath(),
path,
builder::addActualOutputs,
inputMetadataProvider,
/* isTool= */ false);
- continue;
+ } else if (output.isSymlink() && path.isSymbolicLink()) {
+ builder
+ .addActualOutputsBuilder()
+ .setPath(output.getExecPathString())
+ .setSymlinkTargetPath(path.readSymbolicLink().getPathString());
}
- builder
- .addActualOutputsBuilder()
- .setPath(output.getExecPathString())
- .setDigest(
- computeDigest(
- output,
- path,
- inputMetadataProvider,
- xattrProvider,
- digestHashFunction,
- /* includeHashFunctionName= */ true));
}
} catch (IOException ex) {
logger.atWarning().withCause(ex).log("Error computing spawn output properties");
@@ -292,18 +305,6 @@ public void close() throws IOException {
}
}
- private SortedMap listExistingOutputs(Spawn spawn, FileSystem fileSystem) {
- TreeMap result = new TreeMap<>();
- for (ActionInput output : spawn.getOutputFiles()) {
- Path outputPath = fileSystem.getPath(execRoot.getRelative(output.getExecPathString()));
- // TODO(olaola): once symlink API proposal is implemented, report symlinks here.
- if (outputPath.exists()) {
- result.put(outputPath, output);
- }
- }
- return result;
- }
-
/**
* Expands a directory into its contents.
*
@@ -318,7 +319,6 @@ private void listDirectoryContents(
InputMetadataProvider inputMetadataProvider,
boolean isTool)
throws IOException {
- // TODO(olaola): once symlink API proposal is implemented, report symlinks here.
List sortedDirent = new ArrayList<>(contentPath.readdir(Symlinks.NOFOLLOW));
sortedDirent.sort(Comparator.comparing(Dirent::getName));
diff --git a/src/main/protobuf/spawn.proto b/src/main/protobuf/spawn.proto
index d5c3d21c9b04aa..fe01830f6f06fc 100644
--- a/src/main/protobuf/spawn.proto
+++ b/src/main/protobuf/spawn.proto
@@ -42,8 +42,12 @@ message File {
// Path to the file relative to the execution root.
string path = 1;
+ // Symlink target path.
+ // Only set for unresolved symlinks.
+ string symlink_target_path = 4;
+
// File digest.
- // May be omitted for empty files.
+ // Always omitted for unresolved symlinks. May be omitted for empty files.
Digest digest = 2;
// Whether the file is a tool.
@@ -147,6 +151,8 @@ message SpawnExec {
string mnemonic = 10;
// The outputs generated by the execution.
+ // In order for one of the listed_outputs to appear here, it must have been
+ // produced and have the expected type (file, directory or symlink).
repeated File actual_outputs = 11;
// If the spawn did not hit a disk or remote cache, this will be the name of
@@ -225,6 +231,7 @@ message ExecLogEntry {
}
// An input or output directory.
+ // May be a source directory, a runfiles or fileset tree, or a tree artifact.
message Directory {
// The directory path.
string path = 1;
@@ -232,6 +239,14 @@ message ExecLogEntry {
repeated File files = 2;
}
+ // An unresolved symlink.
+ message UnresolvedSymlink {
+ // The symlink path.
+ string path = 1;
+ // The path the symlink points to.
+ string target_path = 2;
+ }
+
// A set of spawn inputs.
// The contents of the set are the directly referenced files and directories
// in addition to the contents of all transitively referenced sets. Sets are
@@ -249,12 +264,15 @@ message ExecLogEntry {
// A spawn output.
message Output {
oneof type {
- // An output file.
+ // An output file, i.e., ctx.actions.declare_file.
int32 file_id = 1;
- // An output directory.
+ // An output directory, i.e., ctx.actions.declare_directory.
int32 directory_id = 2;
- // A declared output that was not produced.
- string missing_path = 3;
+ // An output unresolved symlink, i.e., ctx.actions.declare_symlink.
+ int32 unresolved_symlink_id = 3;
+ // A declared output that is either missing or has the wrong type
+ // (e.g., a file where a directory was expected).
+ string invalid_output_path = 4;
}
}
@@ -325,7 +343,8 @@ message ExecLogEntry {
Invocation invocation = 2;
File file = 3;
Directory directory = 4;
- InputSet input_set = 5;
- Spawn spawn = 6;
+ UnresolvedSymlink unresolved_symlink = 5;
+ InputSet input_set = 6;
+ Spawn spawn = 7;
}
}
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 a60379acd31ed7..69dd8dfeb168ed 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
@@ -198,8 +198,14 @@ private SpawnExec reconstructSpawnExec(
builder.addActualOutputs(reconstructFile(dir, dirFile, hashFunctionName));
}
break;
- case MISSING_PATH:
- builder.addListedOutputs(output.getMissingPath());
+ case UNRESOLVED_SYMLINK_ID:
+ ExecLogEntry.UnresolvedSymlink symlink =
+ checkNotNull(entryMap.get(output.getUnresolvedSymlinkId())).getUnresolvedSymlink();
+ builder.addListedOutputs(symlink.getPath());
+ builder.addActualOutputs(reconstructSymlink(symlink));
+ break;
+ case INVALID_OUTPUT_PATH:
+ builder.addListedOutputs(output.getInvalidOutputPath());
break;
case TYPE_NOT_SET:
throw new AssertionError("malformed log entry");
@@ -252,4 +258,11 @@ private File reconstructFile(
return builder.build();
}
+
+ private File reconstructSymlink(ExecLogEntry.UnresolvedSymlink symlink) {
+ return File.newBuilder()
+ .setPath(symlink.getPath())
+ .setSymlinkTargetPath(symlink.getTargetPath())
+ .build();
+ }
}
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 bf414f1cbabb5c..3524301a02dd66 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
@@ -124,6 +124,16 @@ boolean isEmpty() {
}
}
+ /** Test parameter determining whether an output is indirected through a symlink. */
+ enum OutputIndirection {
+ DIRECT,
+ INDIRECT;
+
+ boolean viaSymlink() {
+ return this == INDIRECT;
+ }
+ }
+
@Test
public void testFileInput(@TestParameter InputsMode inputsMode) throws Exception {
Artifact fileInput = ActionsTestUtil.createArtifact(rootDir, "file");
@@ -384,10 +394,22 @@ public void testFilesetInput(@TestParameter DirContents dirContents) throws Exce
}
@Test
- public void testFileOutput(@TestParameter OutputsMode outputsMode) throws Exception {
+ public void testFileOutput(
+ @TestParameter OutputsMode outputsMode, @TestParameter OutputIndirection indirection)
+ throws Exception {
Artifact fileOutput = ActionsTestUtil.createArtifact(outputDir, "file");
- writeFile(fileOutput, "abc");
+ Path actualPath =
+ indirection.viaSymlink()
+ ? outputDir.getRoot().asPath().getChild("actual")
+ : fileOutput.getPath();
+
+ if (indirection.viaSymlink()) {
+ fileOutput.getPath().getParentDirectory().createDirectoryAndParents();
+ fileOutput.getPath().createSymbolicLink(actualPath);
+ }
+
+ writeFile(actualPath, "abc");
Spawn spawn = defaultSpawnBuilder().withOutputs(fileOutput).build();
@@ -410,17 +432,13 @@ public void testFileOutput(@TestParameter OutputsMode outputsMode) throws Except
}
@Test
- public void testDirectoryOutput(
- @TestParameter OutputsMode outputsMode, @TestParameter DirContents dirContents)
+ public void testFileOutputWithInvalidType(@TestParameter OutputsMode outputsMode)
throws Exception {
- Artifact dirOutput = ActionsTestUtil.createArtifact(outputDir, "dir");
+ Artifact fileOutput = ActionsTestUtil.createArtifact(outputDir, "file");
- dirOutput.getPath().createDirectoryAndParents();
- if (!dirContents.isEmpty()) {
- writeFile(dirOutput.getPath().getChild("file"), "abc");
- }
+ fileOutput.getPath().createDirectoryAndParents();
- SpawnBuilder spawn = defaultSpawnBuilder().withOutputs(dirOutput);
+ SpawnBuilder spawn = defaultSpawnBuilder().withOutputs(fileOutput);
SpawnLogContext context = createSpawnLogContext();
@@ -432,31 +450,31 @@ public void testDirectoryOutput(
defaultTimeout(),
defaultSpawnResult());
- closeAndAssertLog(
- context,
- defaultSpawnExecBuilder()
- .addListedOutputs("out/dir")
- .addAllActualOutputs(
- dirContents.isEmpty()
- ? ImmutableList.of()
- : ImmutableList.of(
- File.newBuilder()
- .setPath("out/dir/file")
- .setDigest(getDigest("abc"))
- .build()))
- .build());
+ closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/file").build());
}
@Test
public void testTreeOutput(
- @TestParameter OutputsMode outputsMode, @TestParameter DirContents dirContents)
+ @TestParameter OutputsMode outputsMode,
+ @TestParameter DirContents dirContents,
+ @TestParameter OutputIndirection indirection)
throws Exception {
SpecialArtifact treeOutput =
ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputDir, "tree");
- treeOutput.getPath().createDirectoryAndParents();
+ Path actualPath =
+ indirection.viaSymlink()
+ ? outputDir.getRoot().asPath().getChild("actual")
+ : treeOutput.getPath();
+
+ if (indirection.viaSymlink()) {
+ treeOutput.getPath().getParentDirectory().createDirectoryAndParents();
+ treeOutput.getPath().createSymbolicLink(actualPath);
+ }
+
+ actualPath.createDirectoryAndParents();
if (!dirContents.isEmpty()) {
- writeFile(treeOutput.getPath().getChild("child"), "abc");
+ writeFile(actualPath.getChild("child"), "abc");
}
Spawn spawn = defaultSpawnBuilder().withOutputs(treeOutput).build();
@@ -486,6 +504,78 @@ public void testTreeOutput(
.build());
}
+ @Test
+ public void testTreeOutputWithInvalidType(@TestParameter OutputsMode outputsMode)
+ throws Exception {
+ Artifact treeOutput = ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputDir, "tree");
+
+ writeFile(treeOutput, "abc");
+
+ SpawnBuilder spawn = defaultSpawnBuilder().withOutputs(treeOutput);
+
+ SpawnLogContext context = createSpawnLogContext();
+
+ context.logSpawn(
+ spawn.build(),
+ createInputMetadataProvider(),
+ createInputMap(),
+ outputsMode.getActionFileSystem(fs),
+ defaultTimeout(),
+ defaultSpawnResult());
+
+ closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/tree").build());
+ }
+
+ @Test
+ public void testUnresolvedSymlinkOutput(@TestParameter OutputsMode outputsMode) throws Exception {
+ Artifact symlinkOutput = ActionsTestUtil.createUnresolvedSymlinkArtifact(outputDir, "symlink");
+
+ symlinkOutput.getPath().getParentDirectory().createDirectoryAndParents();
+ symlinkOutput.getPath().createSymbolicLink(PathFragment.create("/some/path"));
+
+ SpawnBuilder spawn = defaultSpawnBuilder().withOutputs(symlinkOutput);
+
+ SpawnLogContext context = createSpawnLogContext();
+
+ context.logSpawn(
+ spawn.build(),
+ createInputMetadataProvider(),
+ createInputMap(),
+ outputsMode.getActionFileSystem(fs),
+ defaultTimeout(),
+ defaultSpawnResult());
+
+ closeAndAssertLog(
+ context,
+ defaultSpawnExecBuilder()
+ .addListedOutputs("out/symlink")
+ .addActualOutputs(
+ File.newBuilder().setPath("out/symlink").setSymlinkTargetPath("/some/path"))
+ .build());
+ }
+
+ @Test
+ public void testUnresolvedSymlinkOutputWithInvalidType(@TestParameter OutputsMode outputsMode)
+ throws Exception {
+ Artifact symlinkOutput = ActionsTestUtil.createUnresolvedSymlinkArtifact(outputDir, "symlink");
+
+ writeFile(symlinkOutput, "abc");
+
+ SpawnBuilder spawn = defaultSpawnBuilder().withOutputs(symlinkOutput);
+
+ SpawnLogContext context = createSpawnLogContext();
+
+ context.logSpawn(
+ spawn.build(),
+ createInputMetadataProvider(),
+ createInputMap(),
+ outputsMode.getActionFileSystem(fs),
+ defaultTimeout(),
+ defaultSpawnResult());
+
+ closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/symlink").build());
+ }
+
@Test
public void testMissingOutput(@TestParameter OutputsMode outputsMode) throws Exception {
Artifact missingOutput = ActionsTestUtil.createArtifact(outputDir, "missing");