diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index cc35b39c095974..b47645a653626a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -40,7 +40,6 @@ import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; -import com.google.devtools.build.lib.vfs.AbstractFileSystem; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.FileStatus; @@ -101,8 +100,7 @@ * sources, such as the same path existing in multiple underlying sources with different type or * contents. */ -public class RemoteActionFileSystem extends AbstractFileSystem - implements PathCanonicalizer.Resolver { +public class RemoteActionFileSystem extends FileSystem implements PathCanonicalizer.Resolver { private final PathFragment execRoot; private final PathFragment outputBase; private final InputMetadataProvider inputArtifactData; @@ -901,7 +899,7 @@ private Dirent maybeFollowSymlinkForDirent( @Override public void createFSDependentHardLink(PathFragment linkPath, PathFragment originalPath) throws IOException { - // Only called by the AbstractFileSystem#createHardLink base implementation, overridden below. + // Only called by the FileSystem#createHardLink base implementation, overridden below. throw new UnsupportedOperationException(); } diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java index ec865e27f82fe4..70574e6b64d495 100644 --- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java @@ -21,28 +21,26 @@ import com.google.devtools.build.lib.util.Blocker; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.StringEncoding; -import com.google.devtools.build.lib.vfs.AbstractFileSystem; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Dirent; +import com.google.devtools.build.lib.vfs.DiskBackedFileSystem; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileSymlinkLoopException; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.SymlinkTargetType; import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; import java.util.ArrayDeque; import java.util.Collection; import javax.annotation.Nullable; -/** This class implements the FileSystem interface using direct calls to the UNIX filesystem. */ +/** + * A disk-backed filesystem suitable for Unix systems, implemented using a mix of JNI and standard + * library calls. + */ @ThreadSafe -public class UnixFileSystem extends AbstractFileSystem { +public class UnixFileSystem extends DiskBackedFileSystem { protected final String hashAttributeName; public UnixFileSystem(DigestHashFunction hashFunction, String hashAttributeName) { @@ -441,79 +439,4 @@ public File getIoFile(PathFragment path) { public java.nio.file.Path getNioPath(PathFragment path) { return java.nio.file.Path.of(StringEncoding.internalToPlatform(path.getPathString())); } - - @Override - protected InputStream createFileInputStream(PathFragment path) throws IOException { - return new FileInputStream(StringEncoding.internalToPlatform(path.getPathString())); - } - - protected OutputStream createFileOutputStream(PathFragment path, boolean append) - throws FileNotFoundException { - return createFileOutputStream(path, append, /* internal= */ false); - } - - @Override - protected OutputStream createFileOutputStream(PathFragment path, boolean append, boolean internal) - throws FileNotFoundException { - String name = path.getPathString(); - if (!internal - && profiler.isActive() - && (profiler.isProfiling(ProfilerTask.VFS_WRITE) - || profiler.isProfiling(ProfilerTask.VFS_OPEN))) { - long startTime = Profiler.nanoTimeMaybe(); - var comp = Blocker.begin(); - try { - return new ProfiledFileOutputStream(name, /* append= */ append); - } finally { - Blocker.end(comp); - profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name); - } - } else { - var comp = Blocker.begin(); - try { - return new FileOutputStream(StringEncoding.internalToPlatform(name), /* append= */ append); - } finally { - Blocker.end(comp); - } - } - } - - private static final class ProfiledFileOutputStream extends FileOutputStream { - private final String name; - - private ProfiledFileOutputStream(String name, boolean append) throws FileNotFoundException { - super(StringEncoding.internalToPlatform(name), append); - this.name = name; - } - - @Override - public void write(int b) throws IOException { - long startTime = Profiler.nanoTimeMaybe(); - try { - super.write(b); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name); - } - } - - @Override - public void write(byte[] b) throws IOException { - long startTime = Profiler.nanoTimeMaybe(); - try { - super.write(b); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name); - } - } - - @Override - public void write(byte[] b, int off, int len) throws IOException { - long startTime = Profiler.nanoTimeMaybe(); - try { - super.write(b, off, len); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name); - } - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java deleted file mode 100644 index 573acf6d9c3f2c..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java +++ /dev/null @@ -1,203 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -package com.google.devtools.build.lib.vfs; - -import static com.google.common.base.Preconditions.checkNotNull; -import static java.nio.file.StandardOpenOption.CREATE; -import static java.nio.file.StandardOpenOption.READ; -import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING; -import static java.nio.file.StandardOpenOption.WRITE; - -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; -import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import com.google.devtools.build.lib.profiler.Profiler; -import com.google.devtools.build.lib.profiler.ProfilerTask; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.FilterInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.nio.channels.SeekableByteChannel; -import java.nio.file.Files; -import java.nio.file.StandardOpenOption; - -/** This class implements the FileSystem interface using direct calls to the UNIX filesystem. */ -@ThreadSafe -public abstract class AbstractFileSystem extends FileSystem { - - protected static final Profiler profiler = Profiler.instance(); - - public AbstractFileSystem(DigestHashFunction digestFunction) { - super(digestFunction); - } - - @Override - public InputStream getInputStream(PathFragment path) throws IOException { - try { - return createMaybeProfiledInputStream(path); - } catch (FileNotFoundException e) { - // FileInputStream throws FileNotFoundException if opening fails for any reason, including - // permissions. Fix it up here. TODO(tjgq): Migrate to java.nio. - if (e.getMessage().equals(path + ERR_PERMISSION_DENIED)) { - throw new FileAccessException(e.getMessage()); - } - throw e; - } - } - - /** Allows the mapping of PathFragment to InputStream to be overridden in subclasses. */ - protected InputStream createFileInputStream(PathFragment path) throws IOException { - return new FileInputStream(checkNotNull(getIoFile(path))); - } - - /** Returns either normal or profiled FileInputStream. */ - private InputStream createMaybeProfiledInputStream(PathFragment path) throws IOException { - final String name = path.toString(); - if (profiler.isActive() - && (profiler.isProfiling(ProfilerTask.VFS_READ) - || profiler.isProfiling(ProfilerTask.VFS_OPEN))) { - long startTime = Profiler.nanoTimeMaybe(); - try { - // Replace default FileInputStream instance with the custom one that does profiling. - return new ProfiledInputStream(createFileInputStream(path), name); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name); - } - } else { - // Use normal FileInputStream instance if profiler is not enabled. - return createFileInputStream(path); - } - } - - private static final ImmutableSet READ_WRITE_BYTE_CHANNEL_OPEN_OPTIONS = - Sets.immutableEnumSet(READ, WRITE, CREATE, TRUNCATE_EXISTING); - - @Override - public SeekableByteChannel createReadWriteByteChannel(PathFragment path) throws IOException { - boolean shouldProfile = profiler.isActive() && profiler.isProfiling(ProfilerTask.VFS_OPEN); - - long startTime = Profiler.nanoTimeMaybe(); - - try { - // Currently, we do not proxy SeekableByteChannel for profiling reads and writes. - return Files.newByteChannel(getNioPath(path), READ_WRITE_BYTE_CHANNEL_OPEN_OPTIONS); - } finally { - if (shouldProfile) { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, path.toString()); - } - } - } - - /** - * Returns either normal or profiled FileOutputStream. Should be used by subclasses to create - * default OutputStream instance. - */ - protected OutputStream createFileOutputStream(PathFragment path, boolean append, boolean internal) - throws FileNotFoundException { - if (!internal - && profiler.isActive() - && (profiler.isProfiling(ProfilerTask.VFS_WRITE) - || profiler.isProfiling(ProfilerTask.VFS_OPEN))) { - long startTime = Profiler.nanoTimeMaybe(); - try { - return new ProfiledFileOutputStream(getIoFile(path), append); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, path.toString()); - } - } else { - return new FileOutputStream(getIoFile(path), append); - } - } - - @Override - public OutputStream getOutputStream(PathFragment path, boolean append, boolean internal) - throws IOException { - try { - return createFileOutputStream(path, append, internal); - } catch (FileNotFoundException e) { - // FileOutputStream throws FileNotFoundException if opening fails for any reason, - // including permissions. Fix it up here. - // TODO(tjgq): Migrate to java.nio. - if (e.getMessage().equals(path + ERR_PERMISSION_DENIED)) { - throw new FileAccessException(e.getMessage()); - } - throw e; - } - } - - private static final class ProfiledInputStream extends FilterInputStream { - private final InputStream impl; - private final String name; - - public ProfiledInputStream(InputStream impl, String name) { - super(impl); - this.impl = impl; - this.name = name; - } - - @Override - public int read() throws IOException { - long startTime = Profiler.nanoTimeMaybe(); - try { - return impl.read(); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_READ, name); - } - } - - @Override - public int read(byte[] b) throws IOException { - return read(b, 0, b.length); - } - - @Override - public int read(byte[] b, int off, int len) throws IOException { - long startTime = Profiler.nanoTimeMaybe(); - try { - return impl.read(b, off, len); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_READ, name); - } - } - } - - private static final class ProfiledFileOutputStream extends FileOutputStream { - private final File file; - - public ProfiledFileOutputStream(File file, boolean append) throws FileNotFoundException { - super(file, append); - this.file = file; - } - - @Override - public void write(byte[] b) throws IOException { - write(b, 0, b.length); - } - - @Override - public void write(byte[] b, int off, int len) throws IOException { - long startTime = Profiler.nanoTimeMaybe(); - try { - super.write(b, off, len); - } finally { - profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, file.toString()); - } - } - } -} diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DiskBackedFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/DiskBackedFileSystem.java new file mode 100644 index 00000000000000..cdec542d5968e1 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/vfs/DiskBackedFileSystem.java @@ -0,0 +1,212 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +package com.google.devtools.build.lib.vfs; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.ProfilerTask; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.Files; +import java.nio.file.StandardOpenOption; + +/** + * This class extends {@link FileSystem} with default implementations providing access to files on + * disk through standard library APIs. + */ +@ThreadSafe +public abstract class DiskBackedFileSystem extends FileSystem { + private static final Profiler profiler = Profiler.instance(); + + private static final ImmutableSet READ_WRITE_BYTE_CHANNEL_OPEN_OPTIONS = + ImmutableSet.of( + StandardOpenOption.READ, + StandardOpenOption.WRITE, + StandardOpenOption.CREATE, + StandardOpenOption.TRUNCATE_EXISTING); + + protected DiskBackedFileSystem(DigestHashFunction hashFunction) { + super(hashFunction); + } + + // Force subclasses to override getIoFile and getNioPath, as the methods below require them. + + @Override + public abstract File getIoFile(PathFragment path); + + @Override + public abstract java.nio.file.Path getNioPath(PathFragment path); + + @Override + public InputStream getInputStream(PathFragment path) throws IOException { + File file = checkNotNull(getIoFile(path), "getIoFile() must not be null"); + + boolean profileOpen = profiler.isActive() && profiler.isProfiling(ProfilerTask.VFS_OPEN); + boolean profileRead = profiler.isActive() && profiler.isProfiling(ProfilerTask.VFS_READ); + + long startTime = profiler.nanoTimeMaybe(); + try { + return profileRead + ? new ProfiledFileInputStream(file, path.getPathString()) + : new FileInputStream(file); + } catch (FileNotFoundException e) { + // FileInputStream throws FileNotFoundException if opening fails for any reason, including + // permissions. Fix it up here. + if (e.getMessage().endsWith(ERR_PERMISSION_DENIED)) { + throw new FileAccessException(e.getMessage()); + } + throw e; + } finally { + if (profileOpen) { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, path.getPathString()); + } + } + } + + @Override + public OutputStream getOutputStream(PathFragment path, boolean append, boolean internal) + throws IOException { + File file = checkNotNull(getIoFile(path), "getIoFile() must not be null"); + + boolean profileOpen = + !internal && profiler.isActive() && profiler.isProfiling(ProfilerTask.VFS_OPEN); + boolean profileWrite = + !internal && profiler.isActive() && profiler.isProfiling(ProfilerTask.VFS_WRITE); + + long startTime = profiler.nanoTimeMaybe(); + try { + return profileWrite + ? new ProfiledFileOutputStream(file, append, path.getPathString()) + : new FileOutputStream(file, append); + } catch (FileNotFoundException e) { + // FileOutputStream throws FileNotFoundException if opening fails for any reason, including + // permissions. Fix it up here. + if (e.getMessage().endsWith(ERR_PERMISSION_DENIED)) { + throw new FileAccessException(e.getMessage()); + } + throw e; + } finally { + if (profileOpen) { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, path.getPathString()); + } + } + } + + @Override + public SeekableByteChannel createReadWriteByteChannel(PathFragment path) throws IOException { + java.nio.file.Path nioPath = checkNotNull(getNioPath(path), "getNioPath() must not be null"); + + boolean profileOpen = profiler.isActive() && profiler.isProfiling(ProfilerTask.VFS_OPEN); + + long startTime = Profiler.instance().nanoTimeMaybe(); + try { + // TODO: add profiling for read/write operations. + return Files.newByteChannel(nioPath, READ_WRITE_BYTE_CHANNEL_OPEN_OPTIONS); + } finally { + if (profileOpen) { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, path.toString()); + } + } + } + + /** + * A {@link FileInputStream} that adds profile traces around read operations. + * + *

Implementation note: this class extends {@link FileInputStream} instead of wrapping around + * it so that {@code instanceof FileInputStream} checks still work. + */ + private static class ProfiledFileInputStream extends FileInputStream { + private final String name; + + private ProfiledFileInputStream(File file, String name) throws IOException { + super(file); + this.name = name; + } + + @Override + public int read() throws IOException { + long startTime = profiler.nanoTimeMaybe(); + try { + return super.read(); + } finally { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_READ, name); + } + } + + @Override + public int read(byte[] b) throws IOException { + return read(b, 0, b.length); + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + long startTime = profiler.nanoTimeMaybe(); + try { + return super.read(b, off, len); + } finally { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_READ, name); + } + } + } + + /** + * A {@link FileOutputStream} that adds profile traces around write operations. + * + *

Implementation note: this class extends {@link FileOutputStream} instead of wrapping around + * it so that {@code instanceof FileOutputStream} checks still work. + */ + private static class ProfiledFileOutputStream extends FileOutputStream { + private final String name; + + private ProfiledFileOutputStream(File file, boolean append, String name) throws IOException { + super(file, append); + this.name = name; + } + + @Override + public void write(int b) throws IOException { + long startTime = profiler.nanoTimeMaybe(); + try { + super.write(b); + } finally { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name); + } + } + + @Override + public void write(byte[] b) throws IOException { + write(b, 0, b.length); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + long startTime = profiler.nanoTimeMaybe(); + try { + super.write(b, off, len); + } finally { + profiler.logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name); + } + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java index 369cef187754d6..6f4ceeb781dc6b 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java @@ -699,24 +699,18 @@ public void chmod(PathFragment path, int mode) throws IOException { } /** - * Creates an InputStream accessing the file denoted by the path. + * Creates an {@link InputStream}. * + * @param path the path to open * @throws FileNotFoundException if the file does not exist * @throws IOException if there was an error opening the file for reading */ public abstract InputStream getInputStream(PathFragment path) throws IOException; /** - * Returns a {@link SeekableByteChannel} for writing to a file at provided path. - * - *

Truncates the target file, therefore it cannot be used to read already existing files. - */ - public abstract SeekableByteChannel createReadWriteByteChannel(PathFragment path) - throws IOException; - - /** - * Creates an OutputStream accessing the file denoted by path. + * Creates an {@link OutputStream}. * + * @param path the path to open * @throws IOException if there was an error opening the file for writing */ protected final OutputStream getOutputStream(PathFragment path) throws IOException { @@ -724,9 +718,10 @@ protected final OutputStream getOutputStream(PathFragment path) throws IOExcepti } /** - * Creates an OutputStream accessing the file denoted by path. + * Creates an {@link OutputStream}. * - * @param append whether to open the output stream in append mode + * @param path the path to open + * @param append whether to open the file in append mode * @throws IOException if there was an error opening the file for writing */ protected final OutputStream getOutputStream(PathFragment path, boolean append) @@ -735,15 +730,25 @@ protected final OutputStream getOutputStream(PathFragment path, boolean append) } /** - * Creates an OutputStream accessing the file denoted by path. + * Creates an {@link OutputStream}. * - * @param append whether to open the output stream in append mode - * @param internal whether the file is a Bazel internal file + * @param path the path to open + * @param append whether to open the file in append mode + * @param internal whether the file is an internal file whose I/O should not be profiled * @throws IOException if there was an error opening the file for writing */ public abstract OutputStream getOutputStream(PathFragment path, boolean append, boolean internal) throws IOException; + /** + * Creates a {@link SeekableByteChannel}, truncating the file if it already exists. + * + * @param path the path to open + * @throws IOException if there was an error opening the file for reading + */ + public abstract SeekableByteChannel createReadWriteByteChannel(PathFragment path) + throws IOException; + /** * Renames the file denoted by "sourceNode" to the location "targetNode". See {@link * Path#renameTo} for specification. diff --git a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java index e274789e2f4114..dd7c5a8cff12f0 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java @@ -43,7 +43,7 @@ * system call - they all are associated with the VFS_STAT task. */ @ThreadSafe -public class JavaIoFileSystem extends AbstractFileSystem { +public class JavaIoFileSystem extends DiskBackedFileSystem { private static final LinkOption[] NO_LINK_OPTION = new LinkOption[0]; // This isn't generally safe; we rely on the file system APIs not modifying the array. private static final LinkOption[] NOFOLLOW_LINKS_OPTION = diff --git a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java index 8400ce11103017..9e1a8509074b66 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystem.java @@ -15,9 +15,10 @@ import java.io.IOException; import java.io.OutputStream; +import java.nio.channels.SeekableByteChannel; -/** Functionally like a read-only {@link AbstractFileSystem}. */ -public abstract class ReadonlyFileSystem extends AbstractFileSystem { +/** Functionally like a read-only {@link FileSystem}. */ +public abstract class ReadonlyFileSystem extends FileSystem { public ReadonlyFileSystem(DigestHashFunction hashFunction) { super(hashFunction); } @@ -35,6 +36,11 @@ public OutputStream getOutputStream(PathFragment path, boolean append, boolean i throw modificationException(); } + @Override + public SeekableByteChannel createReadWriteByteChannel(PathFragment path) throws IOException { + throw modificationException(); + } + @Override public void setReadable(PathFragment path, boolean readable) throws IOException { throw modificationException(); diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java index 8e1cbcd6814e36..64e98514f3cdaf 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java @@ -22,11 +22,11 @@ import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; -import com.google.devtools.build.lib.vfs.AbstractFileSystem; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileAccessException; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileSymlinkLoopException; +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.vfs.SymlinkTargetType; @@ -59,7 +59,7 @@ * bits are considered for the purpose of determining whether a file is accessible. */ @ThreadSafe -public class InMemoryFileSystem extends AbstractFileSystem { +public class InMemoryFileSystem extends FileSystem { protected final Clock clock; diff --git a/src/test/java/com/google/devtools/build/lib/unix/BUILD b/src/test/java/com/google/devtools/build/lib/unix/BUILD index afded4782a3e58..f74c19197431f5 100644 --- a/src/test/java/com/google/devtools/build/lib/unix/BUILD +++ b/src/test/java/com/google/devtools/build/lib/unix/BUILD @@ -27,6 +27,7 @@ java_library( "no_windows", ], deps = [ + "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/unix", "//src/main/java/com/google/devtools/build/lib/unix:procmeminfo_parser", "//src/main/java/com/google/devtools/build/lib/util:os", @@ -39,7 +40,9 @@ java_library( "//src/test/java/com/google/devtools/build/lib/vfs/util", "//third_party:guava-testlib", "//third_party:junit4", + "//third_party:mockito", "//third_party:truth", + "@maven//:com_google_testparameterinjector_test_parameter_injector", ], ) diff --git a/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java index 8b35e2d1faa459..5b4dabdca7d7ce 100644 --- a/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/unix/UnixFileSystemTest.java @@ -16,7 +16,13 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.ProfilerTask; +import com.google.devtools.build.lib.profiler.TraceProfilerService; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.FileAccessException; @@ -25,7 +31,12 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.SymlinkAwareFileSystemTest; import com.google.devtools.build.lib.vfs.Symlinks; +import com.google.testing.junit.testparameterinjector.TestParameter; +import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import org.junit.Test; /** Tests for the {@link com.google.devtools.build.lib.unix.UnixFileSystem} class. */ @@ -119,4 +130,42 @@ public void testReaddirPermissionError() throws Exception { assertThrows(FileAccessException.class, dir::getDirectoryEntries); assertThrows(FileAccessException.class, () -> dir.readdir(Symlinks.NOFOLLOW)); } + + @Test + public void testInputStreamIsFileInputStream(@TestParameter boolean profiling) throws Exception { + try (var m = new MaybeWithMockProfiler(profiling); + InputStream in = xFile.getInputStream()) { + assertThat(in).isInstanceOf(FileInputStream.class); + } + } + + @Test + public void testOutputStreamIsFileOutputStream(@TestParameter boolean profiling) + throws Exception { + try (var m = new MaybeWithMockProfiler(profiling); + OutputStream out = xFile.getOutputStream()) { + assertThat(out).isInstanceOf(FileOutputStream.class); + } + } + + private static class MaybeWithMockProfiler implements AutoCloseable { + private final boolean enabled; + + MaybeWithMockProfiler(boolean enabled) { + if (enabled) { + TraceProfilerService mock = mock(TraceProfilerService.class); + when(mock.isActive()).thenReturn(true); + when(mock.isProfiling(any(ProfilerTask.class))).thenReturn(true); + Profiler.setTraceProfilerServiceForTesting(mock); + } + this.enabled = enabled; + } + + @Override + public void close() { + if (enabled) { + Profiler.setTraceProfilerServiceForTesting(null); + } + } + } }