Create convenience symlinks before the build so that they are available for intra-build debugging.

As a drive-by fix, create the configuration-independent symlinks even when we have >1 target configurations. Fix broken windows!

--
MOS_MIGRATED_REVID=112764311

2 files changed, 37 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 213a9730ce..b4dd117718 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
@@ -351,6 +351,18 @@ public class ExecutionTool {
       startLocalOutputBuild(); // TODO(bazel-team): this could be just another OutputService
     }
 
+    if (getWorkspace().getFileSystem().supportsSymbolicLinks()) {
+      List<BuildConfiguration> targetConfigurations = configurations.getTargetConfigurations();
+      BuildConfiguration targetConfiguration = targetConfigurations.size() == 1
+          ? targetConfigurations.get(0) : null;
+      if (targetConfigurations.size() == 1) {
+        OutputDirectoryLinksUtils.createOutputDirectoryLinks(
+            runtime.getWorkspaceName(), getWorkspace(), getExecRoot(),
+            runtime.getOutputPath(), getReporter(), targetConfiguration,
+            request.getBuildOptions().getSymlinkPrefix());
+      }
+    }
+
     ActionCache actionCache = getActionCache();
     SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor();
     Builder builder =
@@ -435,21 +447,6 @@ public class ExecutionTool {
       }
       if (buildCompleted) {
         getReporter().handle(Event.progress("Building complete."));
-
-        // Create symlinks only after we've actually built something so that we don't create
-        // dangling symlinks.
-        if (getWorkspace().getFileSystem().supportsSymbolicLinks()) {
-          List<BuildConfiguration> targetConfigurations = configurations.getTargetConfigurations();
-          // TODO(bazel-team): This is not optimal - we retain backwards compatibility in the case
-          // where there's only a single configuration, but we don't create any symlinks in the
-          // multi-config case. Can we do better? [multi-config]
-          if (targetConfigurations.size() == 1) {
-            OutputDirectoryLinksUtils.createOutputDirectoryLinks(
-                runtime.getWorkspaceName(), getWorkspace(), getExecRoot(),
-                runtime.getOutputPath(), getReporter(), targetConfigurations.get(0),
-                request.getBuildOptions().getSymlinkPrefix());
-          }
-        }
       }
 
       env.getEventBus().post(new ExecutionFinishedEvent(ImmutableMap.<String, Long> of(), 0L,
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java b/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java
index 9af444cb57..afa4356157 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/OutputDirectoryLinksUtils.java
@@ -27,6 +27,8 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+import javax.annotation.Nullable;
+
 /**
  * Static utilities for managing output directory symlinks.
  */
@@ -49,7 +51,7 @@ public class OutputDirectoryLinksUtils {
    */
   public static void createOutputDirectoryLinks(String workspaceName,
       Path workspace, Path execRoot, Path outputPath,
-      EventHandler eventHandler, BuildConfiguration targetConfig, String symlinkPrefix) {
+      EventHandler eventHandler, @Nullable BuildConfiguration targetConfig, String symlinkPrefix) {
     if (NO_CREATE_SYMLINKS_PREFIX.equals(symlinkPrefix)) {
       return;
     }
@@ -62,12 +64,16 @@ public class OutputDirectoryLinksUtils {
 
     // Points to execroot
     createLink(workspace, execRootSymlink(workspaceName), execRoot, failures);
-    createLink(workspace, symlinkPrefix + "bin", targetConfig.getBinDirectory().getPath(),
-        failures);
-    createLink(workspace, symlinkPrefix + "testlogs", targetConfig.getTestLogsDirectory().getPath(),
-        failures);
-    createLink(workspace, symlinkPrefix + "genfiles", targetConfig.getGenfilesDirectory().getPath(),
-        failures);
+
+    if (targetConfig != null) {
+      createLink(workspace, symlinkPrefix + "bin",
+          targetConfig.getBinDirectory().getPath(), failures);
+      createLink(workspace, symlinkPrefix + "testlogs",
+          targetConfig.getTestLogsDirectory().getPath(), failures);
+      createLink(workspace, symlinkPrefix + "genfiles",
+          targetConfig.getGenfilesDirectory().getPath(), failures);
+    }
+
     if (!failures.isEmpty()) {
       eventHandler.handle(Event.warn(String.format(
           "failed to create one or more convenience symlinks for prefix '%s':\n  %s",
@@ -157,15 +163,21 @@ public class OutputDirectoryLinksUtils {
    */
   private static boolean createLink(Path base, String name, Path target, List<String> failures) {
     try {
-      if (target.exists()) {
-        // Do not create a symlink to non-existent directories
-        FileSystemUtils.ensureSymbolicLink(base.getRelative(name), target);
-      }
-      return true;
+      FileSystemUtils.createDirectoryAndParents(target);
     } catch (IOException e) {
-      failures.add(String.format("%s -> %s:  %s", name, target.getPathString(), e.getMessage()));
+      failures.add(String.format("cannot create directory %s: %s",
+          target.getPathString(), e.getMessage()));
       return false;
     }
+    try {
+      FileSystemUtils.ensureSymbolicLink(base.getRelative(name), target);
+    } catch (IOException e) {
+      failures.add(String.format("cannot create symbolic link %s -> %s:  %s",
+          name, target.getPathString(), e.getMessage()));
+      return false;
+    }
+
+    return true;
   }
 
   /**