sandbox should create regular empty files, not symlink to /dev/null.

Adds a test based on our Python rules that makes sure that this actually
fixes the issue. Thanks to @duggelz for the suggestion. It seems like
our Python rules are the only place that actually provides an
EmptyFilesSupplier to Runfiles, so there's probably no simpler way to
test this behavior in an integration test.

Fix #1458. Fix #2394.

--
PiperOrigin-RevId: 148656193
MOS_MIGRATED_REVID=148656193

4 files changed, 40 insertions, 21 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
index 44a05f055c..8d464ea171 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
@@ -292,13 +292,20 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
     }
   }
 
-  private ImmutableMap<PathFragment, Path> finalizeLinks(Map<PathFragment, Path> unfinalized)
+  private Map<PathFragment, Path> finalizeLinks(Map<PathFragment, Path> unfinalized)
       throws IOException {
-    ImmutableMap.Builder<PathFragment, Path> finalizedLinks = new ImmutableMap.Builder<>();
+    HashMap<PathFragment, Path> finalizedLinks = new HashMap<>();
     for (Map.Entry<PathFragment, Path> mount : unfinalized.entrySet()) {
       PathFragment target = mount.getKey();
       Path source = mount.getValue();
 
+      // If the source is null, the target is supposed to be an empty file. In this case we don't
+      // have to deal with finalizing the link.
+      if (source == null) {
+        finalizedLinks.put(target, source);
+        continue;
+      }
+
       FileStatus stat = source.statNullable(Symlinks.NOFOLLOW);
 
       if (stat != null && stat.isDirectory()) {
@@ -311,14 +318,11 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
         finalizeLinksPath(finalizedLinks, target, source, stat);
       }
     }
-    return finalizedLinks.build();
+    return finalizedLinks;
   }
 
   private void finalizeLinksPath(
-      ImmutableMap.Builder<PathFragment, Path> finalizedMounts,
-      PathFragment target,
-      Path source,
-      FileStatus stat)
+      Map<PathFragment, Path> finalizedMounts, PathFragment target, Path source, FileStatus stat)
       throws IOException {
     // The source must exist.
     Preconditions.checkArgument(stat != null, "%s does not exist", source.toString());
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/HardlinkedExecRoot.java b/src/main/java/com/google/devtools/build/lib/sandbox/HardlinkedExecRoot.java
index 7ddb401bfd..5bf7c4fd92 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/HardlinkedExecRoot.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/HardlinkedExecRoot.java
@@ -58,11 +58,11 @@ public class HardlinkedExecRoot implements SandboxExecRoot {
       if (errWriter != null) {
         errWriter.printf("createdir: %s\n", createDir.getPathString());
       }
-      FileSystemUtils.createDirectoryAndParents(createDir);
+      FileSystemUtils.createDirectoryAndParentsWithCache(createdDirs, createDir);
     }
 
     // Link all the inputs.
-    linkInputs(inputs);
+    linkInputs(inputs, createdDirs);
   }
 
   private void createDirectoriesForOutputs(Collection<PathFragment> outputs, Set<Path> createdDirs)
@@ -89,7 +89,8 @@ public class HardlinkedExecRoot implements SandboxExecRoot {
    * names (by following solib symlinks back) to modify the paths to the shared libraries in
    * cc_binaries.
    */
-  private void linkInputs(Map<PathFragment, Path> inputs) throws IOException {
+  private void linkInputs(Map<PathFragment, Path> inputs, Set<Path> createdDirs)
+      throws IOException {
     // Create directory for input files.
     Path inputsDir = sandboxPath.getRelative("inputs");
     if (!inputsDir.exists()) {
@@ -97,6 +98,16 @@ public class HardlinkedExecRoot implements SandboxExecRoot {
     }
 
     for (ImmutableMap.Entry<PathFragment, Path> entry : inputs.entrySet()) {
+      Path targetName = sandboxExecRoot.getRelative(entry.getKey());
+      FileSystemUtils.createDirectoryAndParentsWithCache(
+          createdDirs, targetName.getParentDirectory());
+
+      // The target is supposed to be an empty file.
+      if (entry.getValue() == null) {
+        FileSystemUtils.createEmptyFile(targetName);
+        continue;
+      }
+
       // Hardlink, resolve symlink here instead in finalizeLinks.
       Path target = entry.getValue().resolveSymbolicLinks();
       Path hardlinkName =
@@ -115,12 +126,10 @@ public class HardlinkedExecRoot implements SandboxExecRoot {
       }
 
       // symlink
-      Path symlinkName = sandboxExecRoot.getRelative(entry.getKey());
       if (errWriter != null) {
-        errWriter.printf("symlink: %s -> %s\n", symlinkName, hardlinkName);
+        errWriter.printf("symlink: %s -> %s\n", targetName, hardlinkName);
       }
-      FileSystemUtils.createDirectoryAndParents(symlinkName.getParentDirectory());
-      symlinkName.createSymbolicLink(hardlinkName);
+      targetName.createSymbolicLink(hardlinkName);
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
index 8e3c26f93d..12b3fdc3e6 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
@@ -127,7 +127,7 @@ public final class SpawnHelpers {
       Path source;
       switch (fields.length) {
         case 1:
-          source = fs.getPath("/dev/null");
+          source = null;
           break;
         case 2:
           source = fs.getPath(fields[1]);
@@ -152,12 +152,12 @@ public final class SpawnHelpers {
       }
       for (Map.Entry<PathFragment, Artifact> mapping : rootAndMappings.getValue().entrySet()) {
         Artifact sourceArtifact = mapping.getValue();
-        PathFragment source =
-            (sourceArtifact != null) ? sourceArtifact.getExecPath() : new PathFragment("/dev/null");
+        Path source =
+            (sourceArtifact != null) ? execRoot.getRelative(sourceArtifact.getExecPath()) : null;
 
         Preconditions.checkArgument(!mapping.getKey().isAbsolute());
         PathFragment target = root.getRelative(mapping.getKey());
-        mounts.put(target, execRoot.getRelative(source));
+        mounts.put(target, source);
       }
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java
index bc60eae523..173f1cefa6 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SymlinkedExecRoot.java
@@ -48,7 +48,7 @@ public final class SymlinkedExecRoot implements SandboxExecRoot {
     cleanFileSystem(inputs.keySet());
     FileSystemUtils.createDirectoryAndParentsWithCache(createdDirs, sandboxExecRoot);
     createParentDirectoriesForInputs(createdDirs, inputs.keySet());
-    createSymlinksForInputs(inputs);
+    createInputs(inputs);
     createWritableDirectories(createdDirs, writableDirs);
     createDirectoriesForOutputs(createdDirs, outputs);
   }
@@ -97,19 +97,25 @@ public final class SymlinkedExecRoot implements SandboxExecRoot {
     }
   }
 
-  private void createSymlinksForInputs(Map<PathFragment, Path> inputs) throws IOException {
+  private void createInputs(Map<PathFragment, Path> inputs) throws IOException {
     // All input files are relative to the execroot.
     for (Entry<PathFragment, Path> entry : inputs.entrySet()) {
       Path key = sandboxExecRoot.getRelative(entry.getKey());
       FileStatus keyStat = key.statNullable(Symlinks.NOFOLLOW);
       if (keyStat != null) {
         if (keyStat.isSymbolicLink()
+            && entry.getValue() != null
             && key.readSymbolicLink().equals(entry.getValue().asFragment())) {
           continue;
         }
         key.delete();
       }
-      key.createSymbolicLink(entry.getValue());
+      // A null value means that we're supposed to create an empty file as the input.
+      if (entry.getValue() != null) {
+        key.createSymbolicLink(entry.getValue());
+      } else {
+        FileSystemUtils.createEmptyFile(key);
+      }
     }
   }