Simplify logic around moving inconsistent workspace out of the way

gradle · Mar 19, 2024 · fb0cc7e · fb0cc7e
1 parent 33046d5
commit fb0cc7e
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 23 deletions.
diff --git a/...ution/src/main/java/org/gradle/internal/execution/steps/AssignImmutableWorkspaceStep.java b/...ution/src/main/java/org/gradle/internal/execution/steps/AssignImmutableWorkspaceStep.java
@@ -146,11 +146,10 @@ private Optional<WorkspaceResult> loadImmutableWorkspaceIfConsistent(UnitOfWork
         ImmutableListMultimap<String, HashCode> outputHashes = calculateOutputHashes(outputSnapshots);
         ImmutableWorkspaceMetadata metadata = workspaceMetadataStore.loadWorkspaceMetadata(immutableLocation);
         if (!metadata.getOutputPropertyHashes().equals(outputHashes)) {
-            boolean moveSuccessful = workspace.withTemporaryWorkspace(temporaryWorkspace -> moveInconsistentImmutableWorkspaceToTemporaryLocation(immutableLocation, temporaryWorkspace, outputSnapshots));
-            if (moveSuccessful) {
+            return workspace.withTemporaryWorkspace(temporaryWorkspace -> {
+                moveInconsistentImmutableWorkspaceToTemporaryLocation(immutableLocation, temporaryWorkspace, outputSnapshots);
                 return Optional.empty();
-            }
-            // We couldn't move the inconsistent workspace out of the way, falling back to reusing it
+            });
         }
 
         return Optional.of(loadImmutableWorkspace(work, immutableLocation, metadata, outputSnapshots));
@@ -168,38 +167,32 @@ private static WorkspaceResult loadImmutableWorkspace(UnitOfWork work, File immu
                 originMetadata),
             immutableLocation);
     }
-    private boolean moveInconsistentImmutableWorkspaceToTemporaryLocation(File immutableLocation, File failedWorkspaceLocation, ImmutableSortedMap<String, FileSystemSnapshot> outputSnapshots) {
-        boolean moveSuccessful;
-        String moveResult;
+
+    private void moveInconsistentImmutableWorkspaceToTemporaryLocation(File immutableLocation, File failedWorkspaceLocation, ImmutableSortedMap<String, FileSystemSnapshot> outputSnapshots) {
         fileSystemAccess.invalidate(ImmutableList.of(immutableLocation.getAbsolutePath()));
         String outputHashes = outputSnapshots.entrySet().stream()
             .map(entry -> entry.getKey() + ":\n" + entry.getValue().roots()
                 .map(AssignImmutableWorkspaceStep::describeSnapshot)
                 .collect(Collectors.joining("\n")))
             .collect(Collectors.joining("\n"));
-        try {
-            // We move the inconsistent workspace to a "temporary" location as a way to atomically move it out of the permanent workspace.
-            // Deleting it in-place is not an option, as we can't do that atomically.
-            // By moving the inconsistent workspace we also preserve it for later inspection.
-            Files.move(immutableLocation.toPath(), failedWorkspaceLocation.toPath(), StandardCopyOption.ATOMIC_MOVE);
-            moveResult = String.format("The inconsistent workspace has been moved to '%s', and will be recreated.", failedWorkspaceLocation.getAbsolutePath());
-            moveSuccessful = true;
-        } catch (IOException e) {
-            LOGGER.warn("Could not move inconsistent immutable workspace {} to temporary location {}",
-                immutableLocation.getAbsolutePath(), failedWorkspaceLocation.getAbsolutePath(), e);
-            moveResult = "The inconsistent workspace could not be moved, attempting to reuse.";
-            moveSuccessful = false;
-        }
         DeprecationLogger.deprecateBehaviour(String.format("The contents of the immutable workspace '%s' have been modified.", immutableLocation.getAbsolutePath()))
             .withContext("These workspace directories are not supposed to be modified once they are created. " +
                 "The modification might have been caused by an external process, or could be the result of disk corruption. " +
-                moveResult +
+                String.format("The inconsistent workspace will be moved to '%s', and will be recreated.", failedWorkspaceLocation.getAbsolutePath()) +
                 "\n" +
                 outputHashes)
             .willBecomeAnErrorInGradle9()
             .undocumented()
             .nagUser();
-        return moveSuccessful;
+        try {
+            // We move the inconsistent workspace to a "temporary" location as a way to atomically move it out of the permanent workspace.
+            // Deleting it in-place is not an option, as we can't do that atomically.
+            // By moving the inconsistent workspace we also preserve it for later inspection.
+            Files.move(immutableLocation.toPath(), failedWorkspaceLocation.toPath(), StandardCopyOption.ATOMIC_MOVE);
+        } catch (IOException e) {
+            throw new UncheckedIOException(String.format("Could not move inconsistent immutable workspace (%s) to temporary location (%s)",
+                immutableLocation.getAbsolutePath(), failedWorkspaceLocation.getAbsolutePath()), e);
+        }
     }
 
     private WorkspaceResult executeInTemporaryWorkspace(UnitOfWork work, C context, ImmutableWorkspace workspace) {

diff --git a/...vy/org/gradle/integtests/resolve/transform/ArtifactTransformCachingIntegrationTest.groovy b/...vy/org/gradle/integtests/resolve/transform/ArtifactTransformCachingIntegrationTest.groovy
@@ -1346,7 +1346,7 @@ class ArtifactTransformCachingIntegrationTest extends AbstractHttpDependencyReso
         def outputDir = immutableOutputDir("lib1.jar", "0/lib1-green.jar")
         def workspaceDir = outputDir.parentFile
         outputDir.file("tamper-tamper.txt").text = "Making a mess"
-        executer.expectDeprecationWarningWithMultilinePattern("""The contents of the immutable workspace '.*' have been modified. This behavior has been deprecated. This will fail with an error in Gradle 9.0. These workspace directories are not supposed to be modified once they are created. The modification might have been caused by an external process, or could be the result of disk corruption. The inconsistent workspace has been moved to '.*', and will be recreated.
+        executer.expectDeprecationWarningWithMultilinePattern("""The contents of the immutable workspace '.*' have been modified. This behavior has been deprecated. This will fail with an error in Gradle 9.0. These workspace directories are not supposed to be modified once they are created. The modification might have been caused by an external process, or could be the result of disk corruption. The inconsistent workspace will be moved to '.*', and will be recreated.
 outputDirectory:
  - transformed \\(Directory, [0-9a-f]+\\)
    - 0 \\(Directory, [0-9a-f]+\\)