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

Revert "Enabling Bazel to generate input symlinks as defined by RE AP… #7216

Closed
wants to merge 1 commit into from
Closed

Revert "Enabling Bazel to generate input symlinks as defined by RE AP… #7216

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
6 changes: 1 addition & 5 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,7 @@ 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,
options.incompatibleRemoteSymlinks);
new TreeNodeRepository(execRoot, context.getMetadataProvider(), digestUtil);
TreeNode inputRoot;
try (SilentCloseable c = Profiler.instance().profile("RemoteCache.computeMerkleDigests")) {
inputRoot = repository.buildFromActionInputs(inputMap);
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,7 @@ 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, remoteOptions.incompatibleRemoteSymlinks);
TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil);
SortedMap<PathFragment, ActionInput> inputMap = context.getInputMapping(true);
TreeNode inputRoot;
try (SilentCloseable c = Profiler.instance().profile("Remote.computeMerkleDigests")) {
Expand Down
6 changes: 0 additions & 6 deletions src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
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 @@ -34,7 +33,6 @@
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 @@ -80,10 +78,6 @@ 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
61 changes: 11 additions & 50 deletions src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
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 @@ -64,6 +63,9 @@
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 @@ -231,17 +233,11 @@ 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,
boolean uploadSymlinks) {
public TreeNodeRepository(Path execRoot, MetadataProvider inputFileCache, DigestUtil digestUtil) {
this.execRoot = execRoot;
this.inputFileCache = inputFileCache;
this.digestUtil = digestUtil;
this.uploadSymlinks = uploadSymlinks;
}

public MetadataProvider getInputFileCache() {
Expand Down Expand Up @@ -291,7 +287,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(Symlinks.NOFOLLOW));
List<Dirent> sortedDirent = new ArrayList<>(path.readdir(SYMLINK_POLICY));
sortedDirent.sort(Comparator.comparing(Dirent::getName));

List<TreeNode.ChildEntry> entries = new ArrayList<>(sortedDirent.size());
Expand All @@ -301,17 +297,6 @@ 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 @@ -340,25 +325,14 @@ private TreeNode buildParentNode(
Preconditions.checkArgument(
inputsStart == inputsEnd - 1, "Encountered two inputs with the same path.");
ActionInput input = inputs.get(inputsStart);
Path leafPath = ActionInputHelper.toInputPath(input, execRoot);
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 @@ -392,30 +366,17 @@ 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 digest = digestUtil.compute(virtualInput);
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);
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 = ActionInputHelper.toInputPath(input, execRoot);
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;
}
}
} else {
digest = DigestUtil.getFromInputCache(input, inputFileCache);
}
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
2 changes: 1 addition & 1 deletion src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public PathFragment getExecPath() {
public void testVirtualActionInputSupport() throws Exception {
GrpcRemoteCache client = newClient();
TreeNodeRepository treeNodeRepository =
new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL, true);
new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL);
PathFragment execPath = PathFragment.create("my/exec/path");
VirtualActionInput virtualActionInput = new StringVirtualActionInput("hello", execPath);
Digest digest = DIGEST_UTIL.compute(virtualActionInput.getBytes().toByteArray());
Expand Down
Loading