Refactor and cleanup the sandboxing code.

- Remove Optional<> where it's not needed. It's nice for return values, but IMHO it was overused in this code (e.g. Optional<List<X>> is an anti-pattern, as the list itself can already signal that it is empty).
- Use Bazel's own Path class when dealing with paths, not String or java.io.File.
- Move LinuxSandboxUtil into the "sandbox" package.
- Remove dead code and unused fields.
- Migrate deprecated VFS method calls to their replacements.
- Fix a bug in ExecutionStatistics where a FileInputStream was not closed.

Closes #4868.

PiperOrigin-RevId: 190217476

1 files changed, 18 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
index b10a62f6ec..c9575d0679 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -48,7 +48,6 @@ import java.time.Duration;
 import java.util.EnumMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -249,7 +248,7 @@ public class LocalSpawnRunner implements SpawnRunner {
       setState(State.LOCAL_ACTION_RUNNING);
 
       Path tmpDir = createActionTemp(execRoot);
-      Optional<String> statisticsPath = Optional.empty();
+      Path statisticsPath = null;
       try {
         Command cmd;
         OutputStream stdOut;
@@ -274,8 +273,8 @@ public class LocalSpawnRunner implements SpawnRunner {
                   .setTimeout(policy.getTimeout())
                   .setKillDelay(Duration.ofSeconds(localExecutionOptions.localSigkillGraceSeconds));
           if (localExecutionOptions.collectLocalExecutionStatistics) {
-            statisticsPath = Optional.of(tmpDir.getRelative("stats.out").getPathString());
-            commandLineBuilder.setStatisticsPath(statisticsPath.get());
+            statisticsPath = tmpDir.getRelative("stats.out");
+            commandLineBuilder.setStatisticsPath(statisticsPath);
           }
           List<String> cmdLine = commandLineBuilder.build();
           cmd =
@@ -343,19 +342,19 @@ public class LocalSpawnRunner implements SpawnRunner {
                 .setExitCode(exitCode)
                 .setExecutorHostname(hostName)
                 .setWallTime(wallTime);
-        if (statisticsPath.isPresent()) {
-          Optional<ExecutionStatistics.ResourceUsage> resourceUsage =
-              ExecutionStatistics.getResourceUsage(statisticsPath.get());
-          if (resourceUsage.isPresent()) {
-            spawnResultBuilder.setUserTime(resourceUsage.get().getUserExecutionTime());
-            spawnResultBuilder.setSystemTime(resourceUsage.get().getSystemExecutionTime());
-            spawnResultBuilder.setNumBlockOutputOperations(
-                resourceUsage.get().getBlockOutputOperations());
-            spawnResultBuilder.setNumBlockInputOperations(
-                resourceUsage.get().getBlockInputOperations());
-            spawnResultBuilder.setNumInvoluntaryContextSwitches(
-                resourceUsage.get().getInvoluntaryContextSwitches());
-          }
+        if (statisticsPath != null) {
+          ExecutionStatistics.getResourceUsage(statisticsPath)
+              .ifPresent(
+                  resourceUsage -> {
+                    spawnResultBuilder.setUserTime(resourceUsage.getUserExecutionTime());
+                    spawnResultBuilder.setSystemTime(resourceUsage.getSystemExecutionTime());
+                    spawnResultBuilder.setNumBlockOutputOperations(
+                        resourceUsage.getBlockOutputOperations());
+                    spawnResultBuilder.setNumBlockInputOperations(
+                        resourceUsage.getBlockInputOperations());
+                    spawnResultBuilder.setNumInvoluntaryContextSwitches(
+                        resourceUsage.getInvoluntaryContextSwitches());
+                  });
         }
         return spawnResultBuilder.build();
       } finally {
@@ -378,8 +377,8 @@ public class LocalSpawnRunner implements SpawnRunner {
       }
     }
 
-    private String getPathOrDevNull(Path path) {
-      return path == null ? "/dev/null" : path.getPathString();
+    private Path getPathOrDevNull(Path path) {
+      return path == null ? execRoot.getRelative("/dev/null") : path;
     }
 
     private boolean wasTimeout(Duration timeout, Duration wallTime) {