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.

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

[7.1.0] Correctly handle unresolved symlinks when they appear in the inputs. #21181

Merged
merged 1 commit into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.nio.charset.StandardCharsets.ISO_8859_1;

Expand Down Expand Up @@ -282,6 +283,11 @@ public static FileArtifactValue createForVirtualActionInput(byte[] digest, long
return new RegularFileArtifactValue(digest, /* proxy= */ null, size);
}

public static FileArtifactValue createForUnresolvedSymlink(Artifact artifact) throws IOException {
checkArgument(artifact.isSymlink());
return createForUnresolvedSymlink(artifact.getPath());
}

public static FileArtifactValue createForUnresolvedSymlink(Path symlink) throws IOException {
return new UnresolvedSymlinkArtifactValue(symlink);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,8 @@ private int logNestedSet(
Path path = fileSystem.getPath(execRoot.getRelative(input.getExecPath()));
if (isInputDirectory(input, inputMetadataProvider)) {
builder.addDirectoryIds(logDirectory(input, path, inputMetadataProvider));
} else if (input.isSymlink()) {
builder.addUnresolvedSymlinkIds(logUnresolvedSymlink(input, path));
} else {
builder.addFileIds(logFile(input, path, inputMetadataProvider));
}
Expand All @@ -306,7 +308,8 @@ private int logNestedSet(
* Logs a file.
*
* @param input the input representing the file.
* @param path the path to the file
* @param path the path to the file, which must have already been verified to be of the correct
* type.
* @return the entry ID of the {@link ExecLogEntry.File} describing the file.
*/
private int logFile(ActionInput input, Path path, InputMetadataProvider inputMetadataProvider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ public void logSpawn(
builder
.addInputsBuilder()
.setPath(displayPath.getPathString())
.setSymlinkTargetPath(metadata.getSymlinkTarget());
.setSymlinkTargetPath(metadata.getSymlinkTarget())
.setIsTool(isTool);
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ static boolean isInputDirectory(ActionInput input, InputMetadataProvider inputMe
if (input.isDirectory()) {
return true;
}
if (!(input instanceof SourceArtifact)) {
if (input.isSymlink() || !(input instanceof SourceArtifact)) {
return false;
}
// A source artifact may be a directory in spite of claiming to be a file. Avoid unnecessary I/O
Expand Down
12 changes: 7 additions & 5 deletions src/main/protobuf/spawn.proto
Original file line number Diff line number Diff line change
Expand Up @@ -248,17 +248,19 @@ message ExecLogEntry {
}

// 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
// not canonical: two sets with different structure may yield the same
// contents.
// The contents of the set are the directly referenced files, directories and
// symlinks in addition to the contents of all transitively referenced sets.
// Sets are not canonical: two sets with different structure may yield the
// same contents.
message InputSet {
// Entry IDs of files belonging to this set.
repeated int32 file_ids = 1;
// Entry IDs of directories belonging to this set.
repeated int32 directory_ids = 2;
// Entry IDs of unresolved symlinks belonging to this set.
repeated int32 unresolved_symlink_ids = 3;
// Entry IDs of other sets contained in this set.
repeated int32 transitive_set_ids = 3;
repeated int32 transitive_set_ids = 4;
}

// A spawn output.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ private SortedMap<String, File> reconstructInputs(
inputs.put(dirFile.getPath(), reconstructFile(dir, dirFile, hashFunctionName));
}
}
for (int symlinkId : set.getUnresolvedSymlinkIdsList()) {
ExecLogEntry.UnresolvedSymlink symlink =
checkNotNull(entryMap.get(symlinkId)).getUnresolvedSymlink();
inputs.put(symlink.getPath(), reconstructSymlink(symlink));
}
setsToVisit.addAll(set.getTransitiveSetIdsList());
}
return inputs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,39 @@ public void testTreeInput(
.build());
}

@Test
public void testUnresolvedSymlinkInput(@TestParameter InputsMode inputsMode) throws Exception {
Artifact symlinkInput = ActionsTestUtil.createUnresolvedSymlinkArtifact(outputDir, "symlink");

symlinkInput.getPath().getParentDirectory().createDirectoryAndParents();
symlinkInput.getPath().createSymbolicLink(PathFragment.create("/some/path"));

SpawnBuilder spawn = defaultSpawnBuilder().withInputs(symlinkInput);
if (inputsMode.isTool()) {
spawn.withTools(symlinkInput);
}

SpawnLogContext context = createSpawnLogContext();

context.logSpawn(
spawn.build(),
createInputMetadataProvider(symlinkInput),
createInputMap(symlinkInput),
fs,
defaultTimeout(),
defaultSpawnResult());

closeAndAssertLog(
context,
defaultSpawnExecBuilder()
.addInputs(
File.newBuilder()
.setPath("out/symlink")
.setSymlinkTargetPath("/some/path")
.setIsTool(inputsMode.isTool()))
.build());
}

@Test
public void testRunfilesFileInput() throws Exception {
Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "data.txt");
Expand Down Expand Up @@ -862,6 +895,8 @@ protected static InputMetadataProvider createInputMetadataProvider(Artifact... a
treeMetadata.getChildValues().entrySet()) {
builder.put(entry.getKey(), entry.getValue());
}
} else if (artifact.isSymlink()) {
builder.put(artifact, FileArtifactValue.createForUnresolvedSymlink(artifact));
} else {
builder.put(artifact, FileArtifactValue.createForTesting(artifact));
}
Expand Down
Loading