Add a new `FileSystem` method to get a `SeekableByteChannel` for writing files.
Add `FileSystem::createSeekableChannel` which allows us to create a `SeekableByteChannel` for file writes. The channel is opened in truncated mode, meaning that it is not suitable for reading existing files.
PiperOrigin-RevId: 393833983
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
index 8a4e53d..1ff1435 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java
@@ -14,8 +14,13 @@
//
package com.google.devtools.build.lib.vfs;
+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;
@@ -27,8 +32,11 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
import java.nio.file.Files;
-import java.util.EnumSet;
+import java.nio.file.OpenOption;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
/** This class implements the FileSystem interface using direct calls to the UNIX filesystem. */
@ThreadSafe
@@ -83,21 +91,42 @@
}
}
+ private static final ImmutableSet<StandardOpenOption> READABLE_BYTE_CHANNEL_OPEN_OPTIONS =
+ Sets.immutableEnumSet(READ);
+ private static final ImmutableSet<ProfilerTask> READABLE_BYTE_CHANNEL_PROFILER_TASKS =
+ Sets.immutableEnumSet(ProfilerTask.VFS_OPEN, ProfilerTask.VFS_READ);
+ private static final ImmutableSet<StandardOpenOption> READ_WRITE_BYTE_CHANNEL_OPEN_OPTIONS =
+ Sets.immutableEnumSet(READ, WRITE, CREATE, TRUNCATE_EXISTING);
+ private static final ImmutableSet<ProfilerTask> READ_WRITE_BYTE_CHANNEL_PROFILER_TASKS =
+ Sets.immutableEnumSet(ProfilerTask.VFS_OPEN, ProfilerTask.VFS_READ, ProfilerTask.VFS_WRITE);
+
@Override
protected ReadableByteChannel createReadableByteChannel(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 {
- // Currently, we do not proxy ReadableByteChannel for profiling.
- return Files.newByteChannel(java.nio.file.Paths.get(name), EnumSet.of(READ));
- } finally {
- profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name);
- }
- } else {
- return Files.newByteChannel(java.nio.file.Paths.get(name));
+ return createSeekableByteChannelInternal(
+ path, READABLE_BYTE_CHANNEL_OPEN_OPTIONS, READABLE_BYTE_CHANNEL_PROFILER_TASKS);
+ }
+
+ @Override
+ protected SeekableByteChannel createReadWriteByteChannel(PathFragment path) throws IOException {
+ return createSeekableByteChannelInternal(
+ path, READ_WRITE_BYTE_CHANNEL_OPEN_OPTIONS, READ_WRITE_BYTE_CHANNEL_PROFILER_TASKS);
+ }
+
+ private static SeekableByteChannel createSeekableByteChannelInternal(
+ PathFragment path,
+ ImmutableSet<? extends OpenOption> options,
+ ImmutableSet<ProfilerTask> profilerTasks)
+ throws IOException {
+ String name = path.getPathString();
+ if (!profiler.isActive() || profilerTasks.stream().noneMatch(profiler::isProfiling)) {
+ return Files.newByteChannel(Paths.get(name), options);
+ }
+ long startTime = Profiler.nanoTimeMaybe();
+ try {
+ // Currently, we do not proxy SeekableByteChannel for profiling.
+ return Files.newByteChannel(Paths.get(name), options);
+ } finally {
+ profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/BUILD b/src/main/java/com/google/devtools/build/lib/vfs/BUILD
index bcd1791..2f1c648 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/vfs/BUILD
@@ -59,6 +59,7 @@
"//third_party:caffeine",
"//third_party:guava",
"//third_party:jsr305",
+ "//third_party/protobuf:protobuf_java",
],
)
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 283e87b..67a225b 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
@@ -28,6 +28,7 @@
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
import java.nio.file.FileAlreadyExistsException;
import java.util.Collection;
import java.util.Iterator;
@@ -720,6 +721,16 @@
}
/**
+ * Returns a {@link SeekableByteChannel} for writing to a file at provided path.
+ *
+ * <p>Truncates the target file, therefore it cannot be used to read already existing files.
+ * Please use {@link #createReadableByteChannel} to get a {@linkplain ReadableByteChannel channel}
+ * for reads instead.
+ */
+ protected abstract SeekableByteChannel createReadWriteByteChannel(PathFragment path)
+ throws IOException;
+
+ /**
* Creates an OutputStream accessing the file denoted by path.
*
* @throws IOException if there was an error opening the file for writing
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
index 71d21c6..bfc4161 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
@@ -32,6 +32,7 @@
import java.io.OutputStream;
import java.io.Serializable;
import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
import java.util.ArrayList;
import java.util.Collection;
import javax.annotation.Nullable;
@@ -790,10 +791,21 @@
}
/**
+ * Opens the file denoted by this path, following symbolic links, for reading and writing and
+ * returns a file channel for it.
+ *
+ * <p>Truncates the file, therefore it cannot be used to read already existing files. Please use
+ * {@link #createReadableByteChannel} to get a {@linkplain ReadableByteChannel channel} for reads
+ * instead.
+ */
+ public SeekableByteChannel createReadWriteByteChannel() throws IOException {
+ return fileSystem.createReadWriteByteChannel(asFragment());
+ }
+
+ /**
* Returns a java.io.File representation of this path.
*
- * <p>Caveat: the result may be useless if this path's getFileSystem() is not
- * the UNIX filesystem.
+ * <p>Caveat: the result may be useless if this path's getFileSystem() is not the UNIX filesystem.
*/
public File getPathFile() {
return new File(getPathString());
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java
index 3070239..063ecc6 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java
@@ -19,6 +19,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
import java.util.Collection;
/**
@@ -178,6 +179,11 @@
}
@Override
+ protected SeekableByteChannel createReadWriteByteChannel(PathFragment path) throws IOException {
+ return delegateFs.createReadWriteByteChannel(toDelegatePath(path));
+ }
+
+ @Override
protected OutputStream getOutputStream(PathFragment path, boolean append) throws IOException {
return delegateFs.getOutputStream(toDelegatePath(path), append);
}
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 097f7fa..39eb14f 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
@@ -35,6 +35,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
@@ -611,6 +612,12 @@
}
@Override
+ protected synchronized SeekableByteChannel createReadWriteByteChannel(PathFragment path) {
+ // It's feasible to implement, but so far it is not needed.
+ throw new UnsupportedOperationException("Not implemented");
+ }
+
+ @Override
protected synchronized byte[] getFastDigest(PathFragment path) throws IOException {
return statFile(path).getFastDigest();
}
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/BUILD b/src/test/java/com/google/devtools/build/lib/vfs/BUILD
index 49defcd..662b976 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/vfs/BUILD
@@ -66,10 +66,8 @@
"//src/test/java/com/google/devtools/build/lib/testutil:TestThread",
"//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
"//src/test/java/com/google/devtools/build/lib/vfs/util",
- "//third_party:caffeine",
"//third_party:guava",
"//third_party:guava-testlib",
- "//third_party:jsr305",
"//third_party:junit4",
"//third_party:mockito",
"//third_party:truth",
@@ -90,6 +88,7 @@
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
+ "@com_google_testparameterinjector//:testparameterinjector",
],
)
@@ -147,5 +146,6 @@
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
+ "@com_google_testparameterinjector//:testparameterinjector",
],
)
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
index b4f6f60..09fc304 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
@@ -14,7 +14,9 @@
//
package com.google.devtools.build.lib.vfs;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;
@@ -26,6 +28,9 @@
import com.google.devtools.build.lib.unix.FileStatus;
import com.google.devtools.build.lib.unix.NativePosixFiles;
import com.google.devtools.build.lib.util.Fingerprint;
+import com.google.testing.junit.testparameterinjector.TestParameter;
+import com.google.testing.junit.testparameterinjector.TestParameter.TestParameterValuesProvider;
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
@@ -33,9 +38,10 @@
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.channels.ReadableByteChannel;
+import java.nio.channels.SeekableByteChannel;
+import java.nio.channels.WritableByteChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileAlreadyExistsException;
-import java.util.Collection;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Matcher;
@@ -44,17 +50,13 @@
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
/**
* This class handles the generic tests that any filesystem must pass.
*
- * <p>Each filesystem-test should inherit from this class, thereby obtaining
- * all the tests.
+ * <p>Each filesystem-test should inherit from this class, thereby obtaining all the tests.
*/
-@RunWith(Parameterized.class)
+@RunWith(TestParameterInjector.class)
public abstract class FileSystemTest {
private long savedTime;
@@ -69,18 +71,15 @@
protected Path xNonEmptyDirectoryFoo;
protected Path xEmptyDirectory;
- @Parameters(name = "{index}: digestHashFunction={0}")
- public static Collection<DigestHashFunction[]> hashFunctions() {
- // TODO(b/112537387): Remove the array-ification and return Collection<DigestHashFunction>. This
- // is possible in Junit4.12, but 4.11 requires the array. Bazel 0.18 will have Junit4.12, so
- // this can change then.
- return DigestHashFunction.getPossibleHashFunctions()
- .stream()
- .map(dhf -> new DigestHashFunction[] {dhf})
- .collect(ImmutableList.toImmutableList());
- }
+ @TestParameter(valuesProvider = DigestHashFunctionsProvider.class)
+ public DigestHashFunction digestHashFunction;
- @Parameter public DigestHashFunction digestHashFunction;
+ private static final class DigestHashFunctionsProvider implements TestParameterValuesProvider {
+ @Override
+ public ImmutableList<?> provideValues() {
+ return DigestHashFunction.getPossibleHashFunctions().asList();
+ }
+ }
@Before
public final void createDirectories() throws Exception {
@@ -1321,6 +1320,74 @@
}
}
+ @Test
+ public void testCreateReadWriteByteChannelWrite(@TestParameter boolean overwrite)
+ throws Exception {
+ String text = "hello";
+ Path file = overwrite ? xFile : xNothing;
+ try (SeekableByteChannel channel = file.createReadWriteByteChannel()) {
+ writeToChannelAsLatin1(channel, text);
+ assertThat(channel.position()).isEqualTo(text.length());
+ }
+
+ assertThat(FileSystemUtils.readContent(file, ISO_8859_1)).isEqualTo("hello");
+ }
+
+ @Test
+ public void testCreateReadWriteByteChannelWriteAfterSeek() throws Exception {
+ try (SeekableByteChannel channel = xNothing.createReadWriteByteChannel()) {
+ writeToChannelAsLatin1(channel, "01234567890");
+ channel.position(5);
+ writeToChannelAsLatin1(channel, "hello!");
+ assertThat(channel.position()).isEqualTo(5 + "hello!".length());
+ }
+
+ assertThat(FileSystemUtils.readContent(xNothing, ISO_8859_1)).isEqualTo("01234hello!");
+ }
+
+ @Test
+ public void testCreateReadWriteByteChannelRead(@TestParameter({"0", "5", "12"}) int seekPosition)
+ throws Exception {
+ String text = "hello there!";
+ try (SeekableByteChannel channel = xNothing.createReadWriteByteChannel()) {
+ writeToChannelAsLatin1(channel, text);
+ channel.position(seekPosition);
+
+ String read = readAllAsString(channel, text.length() - seekPosition);
+ assertThat(channel.position()).isEqualTo(text.length());
+ assertThat(read).isEqualTo(text.substring(seekPosition));
+ }
+ }
+
+ private static void writeToChannelAsLatin1(WritableByteChannel channel, String text)
+ throws IOException {
+ byte[] bytes = text.getBytes(ISO_8859_1);
+ ByteBuffer buffer = ByteBuffer.wrap(bytes);
+ int toWrite = bytes.length;
+ while (toWrite > 0) {
+ toWrite -= channel.write(buffer);
+ }
+ assertThat(toWrite).isEqualTo(0);
+ assertThat(buffer.remaining()).isEqualTo(0);
+ }
+
+ private static String readAllAsString(ReadableByteChannel channel, int expectedSize)
+ throws IOException {
+ checkArgument(expectedSize >= 0, "negative expected size: %s", expectedSize);
+ // +1 to make sure we can observe EOF -- Channel::read will always return 0 for a full buffer.
+ ByteBuffer buffer = ByteBuffer.allocate(expectedSize + 1);
+ int totalRead = 0;
+ for (; ; ) {
+ int read = channel.read(buffer);
+ if (read == -1) {
+ assertThat(totalRead).isEqualTo(expectedSize);
+ return new String(buffer.array(), 0, expectedSize, ISO_8859_1);
+ }
+ totalRead += read;
+ assertThat(buffer.position()).isEqualTo(totalRead);
+ }
+ }
+
private static int readFromChannel(ReadableByteChannel channel, ByteBuffer buffer, int expected)
throws IOException {
int numRead = 0;
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java
index e1654a0..827e3f2 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystemTest.java
@@ -26,6 +26,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.SymlinkAwareFileSystemTest;
+import com.google.testing.junit.testparameterinjector.TestParameter;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
@@ -64,8 +65,8 @@
}
/**
- * Tests concurrent creation of a substantial tree hierarchy including
- * files, directories, symlinks, file contents, and permissions.
+ * Tests concurrent creation of a substantial tree hierarchy including files, directories,
+ * symlinks, file contents, and permissions.
*/
@Test
public void testConcurrentTreeConstruction() throws Exception {
@@ -410,4 +411,19 @@
assertThat(symlink.getxattr("some.xattr")).isNull();
}
+
+ // createSeekableByteChannel is not supported by InMemoryFileSystem.
+ @Test
+ @Override
+ public void testCreateReadWriteByteChannelWrite(@TestParameter boolean overwrite) {}
+
+ // createSeekableByteChannel is not supported by InMemoryFileSystem.
+ @Test
+ @Override
+ public void testCreateReadWriteByteChannelWriteAfterSeek() {}
+
+ // createSeekableByteChannel is not supported by InMemoryFileSystem.
+ @Test
+ @Override
+ public void testCreateReadWriteByteChannelRead(@TestParameter({"0"}) int seekPosition) {}
}
