Move FDO support to the analysis phase by wrapping FdoSupport in its own SkyFunction.
This removes one of the two reasons for the existence of BuildConfiguration#prepareToBuild() which makes implementing dynamic configurations impossible and also makes FDO support halfway sane; now FDO is exactly as ugly as remote repositories, that is to say, reasonably okay.
Ideally, we'd implement the zip extraction as an Action and make it a TreeArtifact, but support for TreeArtifacts is not mature yet enough, so it's not possible at the moment.
--
MOS_MIGRATED_REVID=119150223
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java
index 393110d..4830d8f 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/ZipFileSystem.java
@@ -15,11 +15,15 @@
import com.google.common.base.Predicate;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.util.Preconditions;
+import java.io.Closeable;
import java.io.File;
import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -31,9 +35,11 @@
* Inherits the constraints imposed by ReadonlyFileSystem.
*/
@ThreadSafe
-public class ZipFileSystem extends ReadonlyFileSystem {
+public class ZipFileSystem extends ReadonlyFileSystem implements Closeable {
+ private final File tempFile; // In case this needs to be written to the file system
private final ZipFile zipFile;
+ private boolean open;
/**
* The sole purpose of this field is to hold a strong reference to all leaf
@@ -53,14 +59,25 @@
* Constructs a ZipFileSystem from a zip file identified with a given path.
*/
public ZipFileSystem(Path zipPath) throws IOException {
- // Throw some more specific exceptions than ZipFile does.
- // We do this using File instead of Path, in case zipPath points to an
- // InMemoryFileSystem. This case is not really supported but
- // can occur in tests.
- File file = zipPath.getPathFile();
- if (!file.exists()) {
+ if (!zipPath.exists()) {
throw new FileNotFoundException(String.format("File '%s' does not exist", zipPath));
}
+
+ File file = zipPath.getPathFile();
+ if (!file.exists()) {
+ // If the File says that it does not exist but the Path says that it does, we are probably
+ // dealing with a FileSystem that does not represent the actual file system we are running
+ // under. Then we copy the Path into a temporary File.
+ tempFile = File.createTempFile("bazel.test.", ".tmp");
+ file = tempFile;
+ byte[] contents = FileSystemUtils.readContent(zipPath);
+ try (OutputStream os = new FileOutputStream(tempFile)) {
+ os.write(contents);
+ }
+ } else {
+ tempFile = null;
+ }
+
if (!file.isFile()) {
throw new IOException(String.format("'%s' is not a file", zipPath));
}
@@ -70,6 +87,7 @@
this.zipFile = new ZipFile(file);
this.paths = populatePathTree();
+ this.open = true;
}
// ZipPath extends Path with a set-once ZipEntry field.
@@ -160,12 +178,14 @@
@Override
protected InputStream getInputStream(Path path) throws IOException {
+ Preconditions.checkState(open);
return zipFile.getInputStream(zipEntryNonNull(path));
}
@Override
protected Collection<Path> getDirectoryEntries(Path path)
throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
final Collection<Path> result = new ArrayList<>();
((ZipPath) path).applyToChildren(new Predicate<Path>() {
@@ -182,46 +202,54 @@
@Override
protected boolean exists(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
return zipEntry(path) != null;
}
@Override
protected boolean isDirectory(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
ZipEntry entry = zipEntry(path);
return entry != null && entry.isDirectory();
}
@Override
protected boolean isFile(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
ZipEntry entry = zipEntry(path);
return entry != null && !entry.isDirectory();
}
@Override
protected boolean isSpecialFile(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
return false;
}
@Override
protected boolean isReadable(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
return true;
}
@Override
protected boolean isWritable(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
return false;
}
@Override
protected boolean isExecutable(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
return false;
}
@Override
protected PathFragment readSymbolicLink(Path path) throws IOException {
+ Preconditions.checkState(open);
zipEntryNonNull(path);
throw new NotASymlinkException(path);
}
@@ -229,22 +257,26 @@
@Override
protected long getFileSize(Path path, boolean followSymlinks)
throws IOException {
+ Preconditions.checkState(open);
return zipEntryNonNull(path).getSize();
}
@Override
protected long getLastModifiedTime(Path path, boolean followSymlinks)
throws FileNotFoundException {
+ Preconditions.checkState(open);
return zipEntryNonNull(path).getTime();
}
@Override
protected boolean isSymbolicLink(Path path) {
+ Preconditions.checkState(open);
return false;
}
@Override
protected FileStatus statIfFound(Path path, boolean followSymlinks) {
+ Preconditions.checkState(open);
try {
return stat(path, followSymlinks);
} catch (FileNotFoundException e) {
@@ -255,4 +287,14 @@
}
}
+ @Override
+ public void close() {
+ if (open) {
+ close();
+ if (tempFile != null) {
+ tempFile.delete();
+ }
+ open = false;
+ }
+ }
}
