From dfccbf975358da93a5ef0fe0f9438873e2a06ee5 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 19 Oct 2022 12:15:58 -0700 Subject: [PATCH] Reclassify RemoteActionFileSystem as an in-memory filesystem, and rename the ActionFileSystemType enum values to better match reality. RemoteActionFileSystem, the action filesystem implementation used by Bazel, became an in-memory filesystem in revision 2876569. Its ActionFileSystemType must be recognized as such so that Skyframe creates output directories prior to local action execution. The correspondence between old -> new enum values is as follows: DISABLED -> DISABLED STAGE_REMOTE_FILES -> REMOTE_FILE_SYSTEM IN_MEMORY_FILE_SYSTEM -> IN_MEMORY_ONLY_FILE_SYSTEM IN_MEMORY_FILE_SYSTEM_AND_STAGE_REMOTE_FILES -> STAGE_REMOTE_FILES_FILE_SYSTEM Also fix a call site where a capability check was made by comparing with the enum value instead of calling the accessor. PiperOrigin-RevId: 482270459 Change-Id: I8ba6a0f32d8570e81281e1e1a29b232df2ac6c85 --- .../build/lib/buildtool/ExecutionTool.java | 4 +- .../build/lib/remote/RemoteOutputService.java | 2 +- .../lib/skyframe/ActionExecutionFunction.java | 5 +-- .../skyframe/ActionOutputDirectoryHelper.java | 10 ++--- .../lib/skyframe/SkyframeActionExecutor.java | 2 +- .../devtools/build/lib/vfs/OutputService.java | 37 +++++++++++-------- 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index 42ca3011ad28b0..fd4f0002fb803b 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java @@ -301,7 +301,7 @@ public void prepareForExecution(UUID buildId) startLocalOutputBuild(); } } - if (outputService == null || outputService.actionFileSystemType().supportLocalActions()) { + if (outputService == null || outputService.actionFileSystemType().supportsLocalActions()) { // Must be created after the output path is created above. createActionLogDirectory(); } @@ -388,7 +388,7 @@ void executeBuild( } } - if (outputService == null || outputService.actionFileSystemType().supportLocalActions()) { + if (outputService == null || outputService.actionFileSystemType().supportsLocalActions()) { // Must be created after the output path is created above. createActionLogDirectory(); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index ede863b922d4ba..bb110b63e8d31d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -51,7 +51,7 @@ void setActionInputFetcher(RemoteActionInputFetcher actionInputFetcher) { @Override public ActionFileSystemType actionFileSystemType() { return actionInputFetcher != null - ? ActionFileSystemType.STAGE_REMOTE_FILES + ? ActionFileSystemType.REMOTE_FILE_SYSTEM : ActionFileSystemType.DISABLED; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index fa22d9a12af593..952e2dd573c641 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -92,7 +92,6 @@ import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileSystem; -import com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.SkyFunction; @@ -254,9 +253,7 @@ private SkyValue computeInternal(SkyKey skyKey, Environment env) CheckInputResults checkedInputs = null; NestedSet allInputs = state.allInputs.getAllInputs( - !skyframeActionExecutor - .actionFileSystemType() - .equals(ActionFileSystemType.STAGE_REMOTE_FILES)); + skyframeActionExecutor.actionFileSystemType().supportsInputDiscovery()); if (!state.actionInputCollectedEventSent) { env.getListener() diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java index 8d2a745c2502bd..6fd2c88ab7cd21 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionOutputDirectoryHelper.java @@ -60,9 +60,9 @@ private enum DirectoryState { } /** - * Creates output directories for an {@link - * com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#IN_MEMORY_FILE_SYSTEM}. - * The action-local filesystem starts empty, so we expect the output directory creation to always + * Creates output directories for an in-memory action file system ({@link + * com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#inMemoryFileSystem}). The + * action-local filesystem starts empty, so we expect the output directory creation to always * succeed. There can be no interference from state left behind by prior builds or other actions * intra-build. */ @@ -96,8 +96,8 @@ void createActionFsOutputDirectories( * rewinding, actions that output tree artifacts need to recreate the directories because they are * deleted as part of the {@link com.google.devtools.build.lib.actions.Action#prepare} step. * - *

Note that this does not need to be called if using {@link - * com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#IN_MEMORY_FILE_SYSTEM}. + *

Note that this does not need to be called if using an in-memory action file system ({@link + * com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType#inMemoryFileSystem}). */ void invalidateTreeArtifactDirectoryCreation(ImmutableSet actionOutputs) { for (Artifact output : actionOutputs) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index a97944091724f7..644f289e193e5b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -1006,7 +1006,7 @@ public ActionStepOrResult run(Environment env) statusReporter.updateStatus(event); } env.getListener().post(event); - if (actionFileSystemType().supportLocalActions()) { + if (actionFileSystemType().supportsLocalActions()) { try (SilentCloseable d = profiler.profile(ProfilerTask.INFO, "action.prepare")) { // This call generally deletes any files at locations that are declared outputs of the // action, although some actions perform additional work, while others intentionally diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index e981614f7b1e06..4ec0c575dd56de 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -48,35 +48,42 @@ public interface OutputService { /** Properties of the action file system implementation provided by this output service. */ enum ActionFileSystemType { - /** Action file system is disabled */ + /** The action file system is disabled. */ DISABLED, /** - * The action file system implementation does not take over the output base but complements the - * file system by being able to stage remote outputs accessed as inputs by local actions, as - * used by Bazel. + * The action file system implementation is purely in-memory, taking full control of the output + * base. It's not able to stage remote outputs accessed as inputs by local actions, but is able + * to do input discovery. Used by Blaze. */ - STAGE_REMOTE_FILES, + IN_MEMORY_ONLY_FILE_SYSTEM, /** - * The action file system implementation is fully featured in-memory file system implementation - * and takes full control of the output base, as used by Blaze. + * The action file system implementation mixes an in-memory and a local file system. It uses the + * in-memory filesystem for in-process and remote actions, but is also aware of outputs from + * local actions. It's able to stage remote outputs accessed as inputs by local actions and to + * do input discovery. Used by Blaze. */ - IN_MEMORY_FILE_SYSTEM, + STAGE_REMOTE_FILES_FILE_SYSTEM, /** - * The action file system implementation mixes in-memory and local file system. It uses - * in-memory filesystem for in-process and remote actions but is also aware of outputs from - * local actions. It's able to stage remote outputs accessed as inputs by local actions. + * The action file system implementation mixes an in-memory and a local file system. It uses the + * in-memory filesystem for in-process and remote actions, but is also aware of outputs from + * local actions. It's able to stage remote outputs accessed as inputs by local actions, but + * unable to do input discovery. Used by Bazel. */ - IN_MEMORY_FILE_SYSTEM_AND_STAGE_REMOTE_FILES; + REMOTE_FILE_SYSTEM; public boolean inMemoryFileSystem() { - return this == IN_MEMORY_FILE_SYSTEM || this == IN_MEMORY_FILE_SYSTEM_AND_STAGE_REMOTE_FILES; + return this != DISABLED; + } + + public boolean supportsLocalActions() { + return this != IN_MEMORY_ONLY_FILE_SYSTEM; } - public boolean supportLocalActions() { - return this != IN_MEMORY_FILE_SYSTEM; + public boolean supportsInputDiscovery() { + return this != REMOTE_FILE_SYSTEM; } public boolean isEnabled() {