Skip to content

Commit

Permalink
Enabling Bazel to generate input symlinks as defined by RE API V2.
Browse files Browse the repository at this point in the history
Design doc: https://docs.google.com/document/d/1zqXCrQk1gF6kRvHXxpMiEfZGNBJ7rlVN8PeBl195_Zc/edit
This change is guarded by an --incompatible_remote_symlinks flag.

Fixes #6631.

SKIP_CI=it passed, but took longer than usual.
PiperOrigin-RevId: 221313450
  • Loading branch information
olaola authored and Copybara-Service committed Nov 13, 2018
1 parent b57c2a4 commit baa1786
Show file tree
Hide file tree
Showing 8 changed files with 410 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(true);
// Temporary hack: the TreeNodeRepository should be created and maintained upstream!
TreeNodeRepository repository =
new TreeNodeRepository(execRoot, context.getMetadataProvider(), digestUtil);
new TreeNodeRepository(
execRoot,
context.getMetadataProvider(),
digestUtil,
options.incompatibleRemoteSymlinks);
TreeNode inputRoot;
try (SilentCloseable c = Profiler.instance().profile("RemoteCache.computeMerkleDigests")) {
inputRoot = repository.buildFromActionInputs(inputMap);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
context.report(ProgressStatus.EXECUTING, getName());
// Temporary hack: the TreeNodeRepository should be created and maintained upstream!
MetadataProvider inputFileCache = context.getMetadataProvider();
TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil);
TreeNodeRepository repository =
new TreeNodeRepository(
execRoot, inputFileCache, digestUtil, remoteOptions.incompatibleRemoteSymlinks);
SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(true);
TreeNode inputRoot;
try (SilentCloseable c = Profiler.instance().profile("Remote.computeMerkleDigests")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import build.bazel.remote.execution.v2.Directory;
import build.bazel.remote.execution.v2.DirectoryNode;
import build.bazel.remote.execution.v2.FileNode;
import build.bazel.remote.execution.v2.SymlinkNode;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
Expand All @@ -37,6 +38,7 @@
import com.google.devtools.build.lib.remote.util.DigestUtil.ActionKey;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.protobuf.ByteString;
import com.google.protobuf.InvalidProtocolBufferException;
import java.io.ByteArrayInputStream;
Expand Down Expand Up @@ -97,6 +99,10 @@ public void downloadTree(Digest rootDigest, Path rootLocation)
for (DirectoryNode child : directory.getDirectoriesList()) {
downloadTree(child.getDigest(), rootLocation.getRelative(child.getName()));
}
for (SymlinkNode symlink : directory.getSymlinksList()) {
PathFragment targetPath = PathFragment.create(symlink.getTarget());
rootLocation.getRelative(symlink.getName()).createSymbolicLink(targetPath);
}
}

private Digest uploadFileContents(Path file) throws IOException, InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
Expand All @@ -63,9 +64,6 @@
public final class TreeNodeRepository {
private static final BaseEncoding LOWER_CASE_HEX = BaseEncoding.base16().lowerCase();

// In this implementation, symlinks are NOT followed when expanding directory artifacts
public static final Symlinks SYMLINK_POLICY = Symlinks.NOFOLLOW;

private final Traverser<TreeNode> traverser =
Traverser.forTree((TreeNode node) -> children(node));

Expand Down Expand Up @@ -233,11 +231,17 @@ public String toDebugString() {
private final Map<VirtualActionInput, Digest> virtualInputDigestCache = new HashMap<>();
private final Map<Digest, VirtualActionInput> digestVirtualInputCache = new HashMap<>();
private final DigestUtil digestUtil;
private final boolean uploadSymlinks;

public TreeNodeRepository(Path execRoot, MetadataProvider inputFileCache, DigestUtil digestUtil) {
public TreeNodeRepository(
Path execRoot,
MetadataProvider inputFileCache,
DigestUtil digestUtil,
boolean uploadSymlinks) {
this.execRoot = execRoot;
this.inputFileCache = inputFileCache;
this.digestUtil = digestUtil;
this.uploadSymlinks = uploadSymlinks;
}

public MetadataProvider getInputFileCache() {
Expand Down Expand Up @@ -287,7 +291,7 @@ public TreeNode buildFromActionInputs(SortedMap<PathFragment, ActionInput> sorte

// Expand the descendant of an artifact (input) directory
private List<TreeNode.ChildEntry> buildInputDirectoryEntries(Path path) throws IOException {
List<Dirent> sortedDirent = new ArrayList<>(path.readdir(SYMLINK_POLICY));
List<Dirent> sortedDirent = new ArrayList<>(path.readdir(Symlinks.NOFOLLOW));
sortedDirent.sort(Comparator.comparing(Dirent::getName));

List<TreeNode.ChildEntry> entries = new ArrayList<>(sortedDirent.size());
Expand All @@ -297,6 +301,17 @@ private List<TreeNode.ChildEntry> buildInputDirectoryEntries(Path path) throws I
TreeNode childNode;
if (dirent.getType() == Dirent.Type.DIRECTORY) {
childNode = interner.intern(new TreeNode(buildInputDirectoryEntries(child), null));
} else if (dirent.getType() == Dirent.Type.SYMLINK) {
PathFragment target = child.readSymbolicLink();
// Need to resolve the symbolic link to know what to expand it to, file or directory.
FileStatus statFollow = child.statIfFound(Symlinks.FOLLOW);
Preconditions.checkNotNull(statFollow, "Dangling symbolic link %s to %s", child, target);
boolean uploadSymlinkAsDirectory = !uploadSymlinks || target.isAbsolute();
if (statFollow.isDirectory() && uploadSymlinkAsDirectory) {
childNode = interner.intern(new TreeNode(buildInputDirectoryEntries(child), null));
} else {
childNode = interner.intern(new TreeNode(ActionInputHelper.fromPath(child.asFragment())));
}
} else {
childNode = interner.intern(new TreeNode(ActionInputHelper.fromPath(child.asFragment())));
}
Expand Down Expand Up @@ -325,14 +340,25 @@ private TreeNode buildParentNode(
Preconditions.checkArgument(
inputsStart == inputsEnd - 1, "Encountered two inputs with the same path.");
ActionInput input = inputs.get(inputsStart);
Path leafPath = execRoot.getRelative(input.getExecPathString());
if (!(input instanceof VirtualActionInput) && uploadSymlinks) {
FileStatus stat = leafPath.stat(Symlinks.NOFOLLOW);
if (stat.isSymbolicLink()) {
PathFragment target = leafPath.readSymbolicLink();
FileStatus statFollow = leafPath.statIfFound(Symlinks.FOLLOW);
Preconditions.checkNotNull(
statFollow, "Action input %s is a dangling symbolic link to %s ", leafPath, target);
if (!target.isAbsolute()) {
return interner.intern(new TreeNode(input));
}
}
}
try {
if (!(input instanceof VirtualActionInput)
&& getInputMetadata(input).getType().isDirectory()) {
Path leafPath = execRoot.getRelative(input.getExecPathString());
return interner.intern(new TreeNode(buildInputDirectoryEntries(leafPath), input));
}
} catch (DigestOfDirectoryException e) {
Path leafPath = execRoot.getRelative(input.getExecPathString());
return interner.intern(new TreeNode(buildInputDirectoryEntries(leafPath), input));
}
return interner.intern(new TreeNode(input));
Expand Down Expand Up @@ -366,17 +392,30 @@ private synchronized Directory getOrComputeDirectory(TreeNode node) throws IOExc
TreeNode child = entry.getChild();
if (child.isLeaf()) {
ActionInput input = child.getActionInput();
final Digest digest;
if (input instanceof VirtualActionInput) {
VirtualActionInput virtualInput = (VirtualActionInput) input;
digest = digestUtil.compute(virtualInput);
Digest digest = digestUtil.compute(virtualInput);
virtualInputDigestCache.put(virtualInput, digest);
// There may be multiple inputs with the same digest. In that case, we don't care which
// one we get back from the digestVirtualInputCache later.
digestVirtualInputCache.put(digest, virtualInput);
} else {
digest = DigestUtil.getFromInputCache(input, inputFileCache);
b.addFilesBuilder().setName(entry.getSegment()).setDigest(digest).setIsExecutable(true);
continue;
}
if (uploadSymlinks) {
// We need to stat the input to check whether it is a symlink.
// getInputMetadata only gives target metadata.
Path inputPath = execRoot.getRelative(input.getExecPath());
FileStatus stat = inputPath.stat(Symlinks.NOFOLLOW);
if (stat.isSymbolicLink()) {
PathFragment target = inputPath.readSymbolicLink();
if (!target.isAbsolute()) {
b.addSymlinksBuilder().setName(entry.getSegment()).setTarget(target.toString());
continue;
}
}
}
Digest digest = DigestUtil.getFromInputCache(input, inputFileCache);
b.addFilesBuilder().setName(entry.getSegment()).setDigest(digest).setIsExecutable(true);
} else {
Digest childDigest = Preconditions.checkNotNull(treeNodeDigestCache.get(child));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public PathFragment getExecPath() {
public void testVirtualActionInputSupport() throws Exception {
GrpcRemoteCache client = newClient();
TreeNodeRepository treeNodeRepository =
new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL);
new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL, true);
PathFragment execPath = PathFragment.create("my/exec/path");
VirtualActionInput virtualActionInput = new StringVirtualActionInput("hello", execPath);
Digest digest = DIGEST_UTIL.compute(virtualActionInput.getBytes().toByteArray());
Expand Down
Loading

0 comments on commit baa1786

Please sign in to comment.