Inject digest hash function directly, instead of behind the back using global state, in BazelFileSystemModule. PiperOrigin-RevId: 330773261
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 5c4eaff..bc44b50 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
@@ -13,7 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.runtime; -import com.google.common.base.Preconditions; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.base.Strings; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.Filesystem; @@ -24,8 +25,6 @@ import com.google.devtools.build.lib.util.ExitCode; 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.JavaIoFileSystem; import com.google.devtools.build.lib.vfs.PathFragment; @@ -38,69 +37,46 @@ * {@code SHA256} as the default hash function, or else what's specified by {@code * -Dbazel.DigestFunction}. * - * <p>For legacy reasons we can't make the {@link com.google.devtools.build.lib.vfs.FileSystem} - * class use {@code SHA256} by default. + * <p>Because of Blaze/Bazel divergence we can't make the {@link + * com.google.devtools.build.lib.vfs.FileSystem} class use {@code SHA256} by default. */ public class BazelFileSystemModule extends BlazeModule { - - @Override - public void globalInit(OptionsParsingResult startupOptionsProvider) throws AbruptExitException { - BlazeServerStartupOptions startupOptions = - Preconditions.checkNotNull( - startupOptionsProvider.getOptions(BlazeServerStartupOptions.class)); - DigestHashFunction commandLineHashFunction = startupOptions.digestHashFunction; - try { - if (commandLineHashFunction != null) { - DigestHashFunction.setDefault(commandLineHashFunction); - } else { - String value = System.getProperty("bazel.DigestFunction", "SHA256"); - DigestHashFunction jvmPropertyHash; - try { - jvmPropertyHash = new DigestFunctionConverter().convert(value); - } catch (OptionsParsingException e) { - throw new AbruptExitException( - DetailedExitCode.of( - ExitCode.COMMAND_LINE_ERROR, - FailureDetail.newBuilder() - .setMessage(Strings.nullToEmpty(e.getMessage())) - .setFilesystem( - Filesystem.newBuilder() - .setCode(Code.DEFAULT_DIGEST_HASH_FUNCTION_INVALID_VALUE)) - .build()), - e); - } - DigestHashFunction.setDefault(jvmPropertyHash); - } - } catch (DefaultAlreadySetException e) { - throw new AbruptExitException( - DetailedExitCode.of( - ExitCode.BLAZE_INTERNAL_ERROR, - FailureDetail.newBuilder() - .setMessage(Strings.nullToEmpty(e.getMessage())) - .setFilesystem( - Filesystem.newBuilder().setCode(Code.DEFAULT_DIGEST_HASH_FUNCTION_CHANGED)) - .build()), - e); - } - } - @Override public ModuleFileSystem getFileSystem( OptionsParsingResult startupOptions, PathFragment realExecRootBase) - throws DefaultHashFunctionNotSetException { - BlazeServerStartupOptions options = startupOptions.getOptions(BlazeServerStartupOptions.class); - boolean enableSymLinks = options != null && options.enableWindowsSymlinks; + throws AbruptExitException { + BlazeServerStartupOptions options = + checkNotNull(startupOptions.getOptions(BlazeServerStartupOptions.class)); + DigestHashFunction digestHashFunction = options.digestHashFunction; + if (digestHashFunction == null) { + String value = System.getProperty("bazel.DigestFunction", "SHA256"); + try { + digestHashFunction = new DigestFunctionConverter().convert(value); + } catch (OptionsParsingException e) { + throw new AbruptExitException( + DetailedExitCode.of( + ExitCode.COMMAND_LINE_ERROR, + FailureDetail.newBuilder() + .setMessage(Strings.nullToEmpty(e.getMessage())) + .setFilesystem( + Filesystem.newBuilder() + .setCode(Code.DEFAULT_DIGEST_HASH_FUNCTION_INVALID_VALUE)) + .build()), + e); + } + } + boolean enableSymLinks = options.enableWindowsSymlinks; if ("0".equals(System.getProperty("io.bazel.EnableJni"))) { // Ignore UnixFileSystem, to be used for bootstrapping. return ModuleFileSystem.create( OS.getCurrent() == OS.WINDOWS - ? new WindowsFileSystem(enableSymLinks) - : new JavaIoFileSystem()); + ? new WindowsFileSystem(digestHashFunction, enableSymLinks) + : new JavaIoFileSystem(digestHashFunction)); } // The JNI-based UnixFileSystem is faster, but on Windows it is not available. return ModuleFileSystem.create( OS.getCurrent() == OS.WINDOWS - ? new WindowsFileSystem(enableSymLinks) - : new UnixFileSystem()); + ? new WindowsFileSystem(digestHashFunction, enableSymLinks) + : new UnixFileSystem(digestHashFunction)); } }
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 f75b3b8..003cc87 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
@@ -37,7 +37,6 @@ import com.google.devtools.build.lib.skyframe.TopDownActionCache; 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; @@ -91,7 +90,7 @@ */ public ModuleFileSystem getFileSystem( OptionsParsingResult startupOptions, PathFragment realExecRootBase) - throws AbruptExitException, DefaultHashFunctionNotSetException { + throws AbruptExitException { 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 b52e565..8cfc1f3 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
@@ -97,7 +97,6 @@ import com.google.devtools.build.lib.util.TestType; 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.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -1216,24 +1215,16 @@ FileSystem fs = null; Path execRootBasePath = null; - try { - for (BlazeModule module : blazeModules) { - BlazeModule.ModuleFileSystem moduleFs = - module.getFileSystem(options, outputBase.getRelative(ServerDirectories.EXECROOT)); - if (moduleFs != null) { - execRootBasePath = moduleFs.virtualExecRootBase(); - Preconditions.checkState(fs == null, "more than one module returns a file system"); - fs = moduleFs.fileSystem(); - } + for (BlazeModule module : blazeModules) { + BlazeModule.ModuleFileSystem moduleFs = + module.getFileSystem(options, outputBase.getRelative(ServerDirectories.EXECROOT)); + if (moduleFs != null) { + execRootBasePath = moduleFs.virtualExecRootBase(); + Preconditions.checkState(fs == null, "more than one module returns a file system"); + fs = moduleFs.fileSystem(); } - - } catch (DefaultHashFunctionNotSetException e) { - throw createFilesystemExitException( - "No module set the default hash function.", - ExitCode.BLAZE_INTERNAL_ERROR, - Filesystem.Code.DEFAULT_DIGEST_HASH_FUNCTION_NOT_SET, - e); } + Preconditions.checkNotNull(fs, "No module set the file system"); SubscriberExceptionHandler currentHandlerValue = null;
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 8e11fa0..e60bd5e 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,7 +18,6 @@ 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; @@ -40,10 +39,6 @@ private final boolean createSymbolicLinks; - public WindowsFileSystem(boolean createSymbolicLinks) throws DefaultHashFunctionNotSetException { - this.createSymbolicLinks = createSymbolicLinks; - } - public WindowsFileSystem(DigestHashFunction hashFunction, boolean createSymbolicLinks) { super(hashFunction); this.createSymbolicLinks = createSymbolicLinks;