Propagate DefaultHashFunctionNotSetException past the filesystem
We want to discourage users to call getDefault() if they don't absolutely have to.
1) it shouldn't be called in static initialization, since the default will not be set, nor should it be called before initServer() is called for all BlazeModules (the later is less likely to happen, that's pretty early in Bazel startup, but still, one needs to be careful when modifying things that are set early in the Bazel server lifecycle.)
2) Calling it after initialization does not carry the DefaultNotSet risk but it may increase contention, since getDefault is synchronized.
Making the exception explicit at FileSystem initialization should serve as a warning for people to be careful. It also encourages those who can to pass in the known value instead of reading it from global state.
RELNOTES: None.
PiperOrigin-RevId: 209837703
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java
index db26f14..9f339d5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BazelFileSystemModule.java
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultAlreadySetException;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import com.google.devtools.build.lib.vfs.DigestHashFunction.DigestFunctionConverter;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
@@ -60,7 +61,8 @@
}
@Override
- public FileSystem getFileSystem(OptionsParsingResult startupOptions) {
+ public FileSystem getFileSystem(OptionsParsingResult startupOptions)
+ throws DefaultHashFunctionNotSetException {
if ("0".equals(System.getProperty("io.bazel.EnableJni"))) {
// Ignore UnixFileSystem, to be used for bootstrapping.
return OS.getCurrent() == OS.WINDOWS ? new WindowsFileSystem() : new JavaIoFileSystem();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index f2105fd..ba47faa 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.io.OutErr;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.OutputService;
import com.google.devtools.build.lib.vfs.Path;
@@ -81,7 +82,8 @@
*
* @param startupOptions the server's startup options
*/
- public FileSystem getFileSystem(OptionsParsingResult startupOptions) throws AbruptExitException {
+ public FileSystem getFileSystem(OptionsParsingResult startupOptions)
+ throws AbruptExitException, DefaultHashFunctionNotSetException {
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 3a7e9b5..a9df409 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -73,6 +73,7 @@
import com.google.devtools.build.lib.util.ProcessUtils;
import com.google.devtools.build.lib.util.ThreadUtils;
import com.google.devtools.build.lib.util.io.OutErr;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
import com.google.devtools.build.lib.vfs.Path;
@@ -965,7 +966,8 @@
}
}
- private static FileSystem defaultFileSystemImplementation() {
+ private static FileSystem defaultFileSystemImplementation()
+ throws DefaultHashFunctionNotSetException {
if ("0".equals(System.getProperty("io.bazel.EnableJni"))) {
// Ignore UnixFileSystem, to be used for bootstrapping.
return OS.getCurrent() == OS.WINDOWS ? new WindowsFileSystem() : new JavaIoFileSystem();
@@ -1063,18 +1065,22 @@
}
FileSystem fs = null;
- for (BlazeModule module : blazeModules) {
- FileSystem moduleFs = module.getFileSystem(options);
- if (moduleFs != null) {
- Preconditions.checkState(fs == null, "more than one module returns a file system");
- fs = moduleFs;
+ try {
+ for (BlazeModule module : blazeModules) {
+ FileSystem moduleFs = module.getFileSystem(options);
+ if (moduleFs != null) {
+ Preconditions.checkState(fs == null, "more than one module returns a file system");
+ fs = moduleFs;
+ }
}
- }
- if (fs == null) {
- fs = defaultFileSystemImplementation();
+ if (fs == null) {
+ fs = defaultFileSystemImplementation();
+ }
+ } catch (DefaultHashFunctionNotSetException e) {
+ throw new AbruptExitException(
+ "No module set the default hash function.", ExitCode.BLAZE_INTERNAL_ERROR, e);
}
-
Path.setFileSystemForSerialization(fs);
SubprocessBuilder.setSubprocessFactory(subprocessFactoryImplementation());
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 5d507c3..949be83 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
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.unix.NativePosixFiles.ReadTypes;
import com.google.devtools.build.lib.vfs.AbstractFileSystemWithCustomStat;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.Path;
@@ -38,8 +39,7 @@
@ThreadSafe
public class UnixFileSystem extends AbstractFileSystemWithCustomStat {
- public UnixFileSystem() {
- }
+ public UnixFileSystem() throws DefaultHashFunctionNotSetException {}
public UnixFileSystem(DigestHashFunction hashFunction) {
super(hashFunction);
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 096ca50..771e073 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
@@ -16,6 +16,7 @@
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 com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
@@ -30,7 +31,7 @@
protected static final String ERR_PERMISSION_DENIED = " (Permission denied)";
protected static final Profiler profiler = Profiler.instance();
- public AbstractFileSystem() {}
+ public AbstractFileSystem() throws DefaultHashFunctionNotSetException {}
public AbstractFileSystem(DigestHashFunction digestFunction) {
super(digestFunction);
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java
index 2a2b06b..3058de3 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystemWithCustomStat.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.vfs;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import java.io.IOException;
/**
@@ -22,7 +23,7 @@
*/
public abstract class AbstractFileSystemWithCustomStat extends AbstractFileSystem {
- public AbstractFileSystemWithCustomStat() {}
+ public AbstractFileSystemWithCustomStat() throws DefaultHashFunctionNotSetException {}
public AbstractFileSystemWithCustomStat(DigestHashFunction hashFunction) {
super(hashFunction);
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java b/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java
index 3f4b94f..b100940 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/DigestHashFunction.java
@@ -133,19 +133,20 @@
* {@link #setDefault(DigestHashFunction)}. Once this value is set, it's a constant, so to prevent
* blocking calls, users should cache this value if needed.
*
- * @throws DefaultNotSetException if the default has not yet been set by a previous call to {@link
- * #setDefault}.
+ * @throws DefaultHashFunctionNotSetException if the default has not yet been set by a previous
+ * call to {@link #setDefault}.
*/
- public static synchronized DigestHashFunction getDefault() throws DefaultNotSetException {
+ public static synchronized DigestHashFunction getDefault()
+ throws DefaultHashFunctionNotSetException {
if (!defaultHasBeenSet) {
- throw new DefaultNotSetException("DigestHashFunction default has not been set");
+ throw new DefaultHashFunctionNotSetException("DigestHashFunction default has not been set");
}
return defaultHash;
}
/** Indicates that the default has not been initialized. */
- public static final class DefaultNotSetException extends Exception {
- DefaultNotSetException(String message) {
+ public static final class DefaultHashFunctionNotSetException extends Exception {
+ DefaultHashFunctionNotSetException(String message) {
super(message);
}
}
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 048cd93..dc0020c 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
@@ -21,7 +21,7 @@
import com.google.common.io.ByteSource;
import com.google.common.io.CharStreams;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultNotSetException;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
@@ -39,19 +39,8 @@
private final DigestHashFunction digestFunction;
- public FileSystem() {
- DigestHashFunction defaultHash;
- try {
- defaultHash = DigestHashFunction.getDefault();
- } catch (DefaultNotSetException e) {
- // For now, be tolerant for cases where the default has not been set, and fallback to MD5, the
- // old default.
- // TODO(b/109764197): Remove this, third_party uses of this library should set their own
- // default, and tests should either set their own default or be able to be run with multiple
- // digest functions.
- defaultHash = DigestHashFunction.MD5;
- }
- digestFunction = defaultHash;
+ public FileSystem() throws DefaultHashFunctionNotSetException {
+ digestFunction = DigestHashFunction.getDefault();
}
public FileSystem(DigestHashFunction digestFunction) {
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 38c8d4c..de2698a 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
@@ -19,6 +19,7 @@
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 com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
@@ -52,7 +53,7 @@
protected static final String ERR_NO_SUCH_FILE_OR_DIR = " (No such file or directory)";
protected static final String ERR_NOT_A_DIRECTORY = " (Not a directory)";
- public JavaIoFileSystem() {
+ public JavaIoFileSystem() throws DefaultHashFunctionNotSetException {
this.clock = new JavaClock();
}
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 0aa71c8..7c85855 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
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.vfs;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import java.io.IOException;
import java.io.OutputStream;
@@ -36,8 +37,7 @@
*/
public abstract class ReadonlyFileSystem extends AbstractFileSystem {
- protected ReadonlyFileSystem() {
- }
+ protected ReadonlyFileSystem() throws DefaultHashFunctionNotSetException {}
public ReadonlyFileSystem(DigestHashFunction digestFunction) {
super(digestFunction);
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java
index ff320b5..0eb5895 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.vfs;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import java.io.IOException;
import java.io.OutputStream;
@@ -21,8 +22,7 @@
*/
public abstract class ReadonlyFileSystemWithCustomStat extends AbstractFileSystemWithCustomStat {
- protected ReadonlyFileSystemWithCustomStat() {
- }
+ protected ReadonlyFileSystemWithCustomStat() throws DefaultHashFunctionNotSetException {}
public ReadonlyFileSystemWithCustomStat(DigestHashFunction hashFunction) {
super(hashFunction);
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index 869ba85..d39173a 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
import com.google.devtools.build.lib.vfs.Path;
@@ -37,7 +38,7 @@
public static final LinkOption[] NO_OPTIONS = new LinkOption[0];
public static final LinkOption[] NO_FOLLOW = new LinkOption[] {LinkOption.NOFOLLOW_LINKS};
- public WindowsFileSystem() {}
+ public WindowsFileSystem() throws DefaultHashFunctionNotSetException {}
public WindowsFileSystem(DigestHashFunction hashFunction) {
super(hashFunction);
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionGlobalsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionGlobalsTest.java
index 77f7b62..a822da0 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionGlobalsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/DigestHashFunctionGlobalsTest.java
@@ -17,6 +17,7 @@
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import com.google.common.hash.Hashing;
+import com.google.devtools.build.lib.vfs.DigestHashFunction.DefaultHashFunctionNotSetException;
import com.google.devtools.build.lib.vfs.DigestHashFunction.DigestFunctionConverter;
import java.lang.reflect.Field;
import org.junit.Before;
@@ -88,8 +89,7 @@
@Test
public void unsetDefaultThrows() {
- assertThrows(
- DigestHashFunction.DefaultNotSetException.class, () -> DigestHashFunction.getDefault());
+ assertThrows(DefaultHashFunctionNotSetException.class, () -> DigestHashFunction.getDefault());
}
@Test
