Skip to content

Commit

Permalink
Only inject symlink metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
coeuvre committed Nov 21, 2022
1 parent 9f3bbc2 commit 0349833
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 59 deletions.
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ java_library(
"LocalHostResourceManagerDarwin.java",
"LocalHostResourceFallback.java",
"MiddlemanType.java",
"RemoteFileStatus.java",
"ResourceManager.java",
"ResourceSet.java",
"ResourceSetOrBuilder.java",
Expand Down Expand Up @@ -263,6 +264,7 @@ java_library(
"FileStateType.java",
"FileStateValue.java",
"FileValue.java",
"RemoteFileStatus.java",
"cache/MetadataDigestUtils.java",
],
deps = [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.google.devtools.build.lib.actions;

import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
import com.google.devtools.build.lib.vfs.FileStatusWithDigest;

public interface RemoteFileStatus extends FileStatusWithDigest {
RemoteFileArtifactValue getRemoteMetadata();
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.actions.RemoteFileStatus;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.inmemoryfs.FileInfo;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryContentInfo;
Expand Down Expand Up @@ -128,46 +129,7 @@ void flush() throws IOException {
PathFragment path = execRoot.getRelative(entry.getKey());
Artifact output = entry.getValue();

if (maybeInjectMetadataForSymlink(path, output)) {
continue;
}

if (output.isTreeArtifact()) {
if (remoteOutputTree.exists(path)) {
SpecialArtifact parent = (SpecialArtifact) output;
TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent);

// TODO: Check directory content on the local fs to support mixed tree.
TreeArtifactValue.visitTree(
remoteOutputTree.getPath(path),
(parentRelativePath, type) -> {
if (type == Dirent.Type.DIRECTORY) {
return;
}
RemoteFileInfo remoteFile =
remoteOutputTree.getRemoteFileInfo(
path.getRelative(parentRelativePath), /* followSymlinks= */ true);
if (remoteFile != null) {
TreeFileArtifact child =
TreeFileArtifact.createTreeOutput(parent, parentRelativePath);
tree.putChild(child, createRemoteMetadata(remoteFile));
}
});

var metadata = tree.build();
// Don't inject empty tree metadata, in case that tree files are generated locally,
// skyframe has the chance to read metadata from local filesystem.
if (!metadata.getChildren().isEmpty()) {
metadataInjector.injectTree(parent, metadata);
}
}
} else {
RemoteFileInfo remoteFile =
remoteOutputTree.getRemoteFileInfo(path, /* followSymlinks= */ true);
if (remoteFile != null) {
metadataInjector.injectFile(output, createRemoteMetadata(remoteFile));
}
}
maybeInjectMetadataForSymlink(path, output);
}
}

Expand Down Expand Up @@ -337,36 +299,53 @@ protected boolean isExecutable(PathFragment path) throws IOException {
return m != null || super.isExecutable(path);
}

@FunctionalInterface
interface PathOp {
void run(Path path) throws IOException;
}

@Override
protected void setReadable(PathFragment path, boolean readable) throws IOException {
RemoteFileArtifactValue m = getRemoteMetadata(path);
if (m == null) {
super.setReadable(path, readable);
doOnRemoteAndLocal(path, p -> p.setReadable(readable));
}

private void doOnRemoteAndLocal(PathFragment path, PathOp pathOp) throws IOException {
FileNotFoundException remoteException = null;
try {
pathOp.run(remoteOutputTree.getPath(path));
} catch (FileNotFoundException e) {
remoteException = e;
}


FileNotFoundException localException = null;
try {
pathOp.run(delegateFs.getPath(path));
} catch (FileNotFoundException e) {
localException = e;
}

if (remoteException == null || localException == null) {
return;
}

localException.addSuppressed(remoteException);
throw localException;
}

@Override
public void setWritable(PathFragment path, boolean writable) throws IOException {
RemoteFileArtifactValue m = getRemoteMetadata(path);
if (m == null) {
super.setWritable(path, writable);
}
doOnRemoteAndLocal(path, p -> p.setWritable(writable));
}

@Override
protected void setExecutable(PathFragment path, boolean executable) throws IOException {
RemoteFileArtifactValue m = getRemoteMetadata(path);
if (m == null) {
super.setExecutable(path, executable);
}
doOnRemoteAndLocal(path, p -> p.setExecutable(executable));
}

@Override
protected void chmod(PathFragment path, int mode) throws IOException {
RemoteFileArtifactValue m = getRemoteMetadata(path);
if (m == null) {
super.chmod(path, mode);
}
doOnRemoteAndLocal(path, p -> p.chmod(mode));
}

// -------------------- Symlinks --------------------
Expand Down Expand Up @@ -472,7 +451,12 @@ protected FileStatus stat(PathFragment path, boolean followSymlinks) throws IOEx
}

private static FileStatus statFromRemoteMetadata(RemoteFileArtifactValue m) {
return new FileStatus() {
return new RemoteFileStatus() {
@Override
public byte[] getDigest() {
return m.getDigest();
}

@Override
public boolean isFile() {
return m.getType().isFile();
Expand Down Expand Up @@ -512,6 +496,11 @@ public long getLastChangeTime() {
public long getNodeId() {
throw new UnsupportedOperationException("Cannot get node id for " + m);
}

@Override
public RemoteFileArtifactValue getRemoteMetadata() {
return m;
}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.google.devtools.build.lib.vfs.FileStatusWithDigestAdapter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.actions.RemoteFileStatus;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.XattrProvider;
Expand Down Expand Up @@ -656,13 +657,19 @@ private static FileArtifactValue fileArtifactValueFromStat(
return FileArtifactValue.MISSING_FILE_MARKER;
}

if (stat.isDirectory()) {
return FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime());
}

if (stat instanceof RemoteFileStatus) {
return ((RemoteFileStatus) stat).getRemoteMetadata();
}

FileStateValue fileStateValue =
FileStateValue.createWithStatNoFollow(
rootedPath, stat, digestWillBeInjected, xattrProvider, tsgm);

return stat.isDirectory()
? FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime())
: FileArtifactValue.createForNormalFile(
return FileArtifactValue.createForNormalFile(
fileStateValue.getDigest(), fileStateValue.getContentsProxy(), stat.getSize());
}

Expand Down

0 comments on commit 0349833

Please sign in to comment.