Skip to content

Commit

Permalink
Rename RemoteFileStatus to FileStatusWithMetadata and make it able to…
Browse files Browse the repository at this point in the history
… return any file metadata rather than remote.

Also make RemoteFileInfo store the metadata directly so we don't need to convert back and forth.

PiperOrigin-RevId: 529665424
Change-Id: I038ef07b6b328f3fc641e73d2d7b1f5ab3c255b6
  • Loading branch information
coeuvre authored and copybara-github committed May 5, 2023
1 parent 2dca982 commit f1af869
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 39 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ java_library(
"ResourceManager.java",
"ResourceSet.java",
"ResourceSetOrBuilder.java",
"RemoteFileStatus.java",
"FileStatusWithMetadata.java",
"SharedActionEvent.java",
"PackageRootResolver.java",
"PackageRoots.java",
Expand Down Expand Up @@ -306,9 +306,9 @@ java_library(
"FileContentsProxy.java",
"FileStateType.java",
"FileStateValue.java",
"FileStatusWithMetadata.java",
"FileValue.java",
"RemoteArtifactChecker.java",
"RemoteFileStatus.java",
"cache/MetadataDigestUtils.java",
],
deps = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;

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

/**
* A FileStatus that exists remotely and provides remote metadata.
* A FileStatus that provides metadata.
*
* <p>Filesystem may return implementation of this interface if the files are stored remotely. When
* checking outputs of actions, Skyframe will inject the metadata returned by {@link
* #getRemoteMetadata()} if the output file has {@link RemoteFileStatus}.
* <p>Filesystem may return implementation of this interface if the files are stored in the memory.
* When checking outputs of actions, Skyframe will inject the metadata returned by {@link
* #getMetadata()} instead of construct a new metadata from the status.
*/
public interface RemoteFileStatus extends FileStatusWithDigest {
RemoteFileArtifactValue getRemoteMetadata();
public interface FileStatusWithMetadata extends FileStatusWithDigest {
FileArtifactValue getMetadata();
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
import com.google.devtools.build.lib.actions.FileStatusWithMetadata;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.RemoteFileStatus;
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
Expand Down Expand Up @@ -247,14 +247,6 @@ private void maybeInjectMetadataForSymlinkOrDownload(PathFragment linkPath, Arti
}
}

private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) {
return RemoteFileArtifactValue.create(
remoteFile.getFastDigest(),
remoteFile.getSize(),
/* locationIndex= */ 1,
remoteFile.getExpireAtEpochMilli());
}

@Override
public String getFileSystemType(PathFragment path) {
return "remoteActionFS";
Expand Down Expand Up @@ -520,7 +512,7 @@ protected FileStatus stat(PathFragment path, boolean followSymlinks) throws IOEx
}

private static FileStatus statFromRemoteMetadata(RemoteFileArtifactValue m) {
return new RemoteFileStatus() {
return new FileStatusWithMetadata() {
@Override
public byte[] getDigest() {
return m.getDigest();
Expand Down Expand Up @@ -567,7 +559,7 @@ public long getNodeId() {
}

@Override
public RemoteFileArtifactValue getRemoteMetadata() {
public RemoteFileArtifactValue getMetadata() {
return m;
}
};
Expand Down Expand Up @@ -596,7 +588,7 @@ public FileArtifactValue getOutputMetadataForTopLevelArtifactDownloader(ActionIn
remoteOutputTree.getRemoteFileInfo(
execRoot.getRelative(input.getExecPath()), /* followSymlinks= */ true);
if (remoteFile != null) {
return createRemoteMetadata(remoteFile);
return remoteFile.getMetadata();
}

// TODO(tjgq): This should not work.
Expand Down Expand Up @@ -627,7 +619,7 @@ private FileArtifactValue getMetadataByExecPath(PathFragment execPath) {
remoteOutputTree.getRemoteFileInfo(
execRoot.getRelative(execPath), /* followSymlinks= */ true);
if (remoteFile != null) {
return createRemoteMetadata(remoteFile);
return remoteFile.getMetadata();
}

return null;
Expand Down Expand Up @@ -846,7 +838,10 @@ void injectRemoteFile(PathFragment path, byte[] digest, long size, long expireAt
}

RemoteFileInfo remoteFileInfo = (RemoteFileInfo) node;
remoteFileInfo.set(digest, size, expireAtEpochMilli);

var metadata =
RemoteFileArtifactValue.create(digest, size, /* locationIndex= */ 1, expireAtEpochMilli);
remoteFileInfo.set(metadata);
}

@Nullable
Expand All @@ -859,21 +854,15 @@ RemoteFileInfo getRemoteFileInfo(PathFragment path, boolean followSymlinks) {
}
}

static class RemoteFileInfo extends FileInfo {

private byte[] digest;
private long size;

private long expireAtEpochMilli;
static class RemoteFileInfo extends FileInfo implements FileStatusWithMetadata {
private RemoteFileArtifactValue metadata;

RemoteFileInfo(Clock clock) {
super(clock);
}

private void set(byte[] digest, long size, long expireAtEpochMilli) {
this.digest = digest;
this.size = size;
this.expireAtEpochMilli = expireAtEpochMilli;
private void set(RemoteFileArtifactValue metadata) {
this.metadata = metadata;
}

@Override
Expand All @@ -893,16 +882,26 @@ public byte[] getxattr(String name) throws IOException {

@Override
public byte[] getFastDigest() {
return digest;
return metadata.getDigest();
}

@Override
public byte[] getDigest() throws IOException {
return metadata.getDigest();
}

@Override
public long getSize() {
return size;
return metadata.getSize();
}

public long getExpireAtEpochMilli() {
return expireAtEpochMilli;
return metadata.getExpireAtEpochMilli();
}

@Override
public RemoteFileArtifactValue getMetadata() {
return metadata;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileStateType;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FileStatusWithMetadata;
import com.google.devtools.build.lib.actions.FilesetManifest;
import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.RemoteFileStatus;
import com.google.devtools.build.lib.actions.cache.OutputMetadataStore;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.DigestUtils;
Expand Down Expand Up @@ -682,8 +682,8 @@ private static FileArtifactValue fileArtifactValueFromStat(
return FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime());
}

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

FileStateValue fileStateValue =
Expand Down

0 comments on commit f1af869

Please sign in to comment.