commit 1a80bb87fef46017b1c2e440085d0741dadb7328
author ulfjack <ulfjack@google.com> Tue Nov 26 07:52:35 2019 -0800
committer Copybara-Service <copybara-worker@google.com> Tue Nov 26 07:53:38 2019 -0800
tree f11537a7dadf385ab4be4428ff517740805cd2ac
parent 5be24a81ff374ab7690ef08a933501f6eaf05efe

OutputService: simplify interface

Originally, OutputService.createSymlinkTree had to read the MANIFEST file
in order to get the list of symlinks. This was problematic, because the
file is not round-trip safe for paths containing whitespace characters.

In addition, it was responsible for creating a symlink from the output
manifest location to the input manifest location, which all subclasses
implemented in the same way.

Instead, it now receives a map representation of the symlinks, and the
creation of the output manifest symlink is the responsibility of the
action.

The current change is primarily a cleanup to reduce code duplication.

However, this change also makes it possible to globally change the
contents of the output manifest. By making it a non-symlink (e.g.,
a hash of the symlink map), we can decouple the symlink tree action
from the manifest writing action. We could run both in parallel, or
even skip writing the input manifest entirely (though note that the
input manifest is currently still necessary in some cases because
we use an external subprocess to create the symlink tree).

PiperOrigin-RevId: 282564612

src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
index b81ad7a..b355035 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
@@ -16,7 +16,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -34,7 +34,6 @@
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.vfs.OutputService;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Map;
 import org.junit.Test;
@@ -48,7 +47,8 @@
   @Test
   public void testArtifactToPathConversion() {
     Artifact artifact = getBinArtifactWithNoOwner("dir/foo");
-    assertThat(SymlinkTreeStrategy.TO_PATH.apply(artifact)).isEqualTo(artifact.getPath());
+    assertThat(SymlinkTreeStrategy.TO_PATH.apply(artifact))
+        .isEqualTo(artifact.getPath().asFragment());
     assertThat(SymlinkTreeStrategy.TO_PATH.apply(null)).isEqualTo(null);
   }
 
@@ -68,6 +68,13 @@
     Artifact inputManifest = getBinArtifactWithNoOwner("dir/manifest.in");
     Artifact outputManifest = getBinArtifactWithNoOwner("dir/MANIFEST");
     Artifact runfile = getBinArtifactWithNoOwner("dir/runfile");
+    doAnswer(
+            (i) -> {
+              outputManifest.getPath().getParentDirectory().createDirectoryAndParents();
+              return null;
+            })
+        .when(outputService)
+        .createSymlinkTree(any(), any());
 
     Runfiles runfiles =
         new Runfiles.Builder("TESTING", false)
@@ -87,13 +94,12 @@
     action.execute(context);
 
     @SuppressWarnings("unchecked")
-    ArgumentCaptor<Map<PathFragment, Path>> capture = ArgumentCaptor.forClass(Map.class);
-    verify(outputService, times(1))
-        .createSymlinkTree(any(), capture.capture(), any(), anyBoolean(), any());
+    ArgumentCaptor<Map<PathFragment, PathFragment>> capture = ArgumentCaptor.forClass(Map.class);
+    verify(outputService, times(1)).createSymlinkTree(capture.capture(), any());
     assertThat(capture.getValue())
         .containsExactly(
             PathFragment.create("TESTING/dir/runfile"),
-            runfile.getPath(),
+            runfile.getPath().asFragment(),
             PathFragment.create("TESTING/dir/empty"),
             null);
   }