Use top-level targets' configurations for convenience symlinks.

This adds a new flag, --use_top_level_targets_for_symlinks.

Configuration-specific convenience symlinks (i.e., (prefix)bin,
(prefix)genfiles, (prefix)testlogs) previously pointed at the one top-level
target configuration. If there were multiple top-level target configurations,
which could only happen when --experimental_multi_cpu was used, then no symlinks
would be created, though they would not be deleted either.

With the flag on, the output directories of the configurations actually used by
the top-level targets (after rule class transitions etc.) are compared. If all
targets with non-null configurations have the same configuration (or, more
specifically, if all targets with non-null configurations have configurations
with the same output directory), then a symlink is created pointing at it.
Otherwise, the symlink is deleted, to avoid confusion with stale symlinks from
old builds.

Known changes in behavior WITHOUT the flag turned on:
* In the --experimental_multi_cpu case, non-configuration-specific
  convenience symlinks (i.e., (prefix)(workspace) and (prefix)out) will always
  be created, even though they previously wouldn't have been created at all in
  this case. (should be harmless)
* In the --experimental_multi_cpu case, configuration-specific convenience
  symlinks will be created if all configs have the same output directory.
  (should be safe, or specifically, should never happen because multi-cpu
  would necessarily change the output path, but shouldn't hurt anything even if
  it did)
* In the --experimental_multi_cpu case, configuration-based convenience
  symlinks will be deleted if some configs have a different output directory.
  (slightly more noticeable since that will be every build using
  experimental_multi_cpu, but then, it is experimental)

RELNOTES: None.
PiperOrigin-RevId: 168720843

3 files changed, 119 insertions, 30 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index d020dfa6e8..fc12d55597 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -387,6 +387,37 @@ public class BuildRequest implements OptionsClassProvider {
       return symlinkPrefix == null ? productName + "-" : symlinkPrefix;
     }
 
+    // Transitional flag for safely rolling out new convenience symlink behavior.
+    // To be made a no-op and deleted once new symlink behavior is battle-tested.
+    @Option(
+      name = "use_top_level_targets_for_symlinks",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+      help =
+          "If enabled, the symlinks are based on the configurations of the top-level targets "
+              + " rather than the top-level target configuration. If this would be ambiguous, "
+              + " the symlinks will be deleted to avoid confusion."
+    )
+    public boolean useTopLevelTargetsForSymlinks;
+
+    /**
+     * Returns whether to use the output directories used by the top-level targets for convenience
+     * symlinks.
+     *
+     * <p>If true, then symlinks use the actual output directories of the top-level targets.
+     * The symlinks will be created iff all top-level targets share the same output directory.
+     * Otherwise, any stale symlinks from previous invocations will be deleted to avoid ambiguity.
+     *
+     * <p>If false, then symlinks use the output directory implied by command-line flags, regardless
+     * of whether top-level targets have transitions which change them (or even have any output
+     * directories at all, as in the case of a build with no targets or one which only builds source
+     * files).
+     */
+    public boolean useTopLevelTargetsForSymlinks() {
+      return useTopLevelTargetsForSymlinks;
+    }
+
     @Option(
       name = "use_action_cache",
       defaultValue = "true",
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 83d259c04f..128596140f 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
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.buildtool;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 import com.google.common.base.Joiner;
@@ -118,7 +119,7 @@ import java.util.logging.Logger;
  *
  * <p>This is only intended for use by {@link BuildTool}.
  *
- * <p>This class contains an ActionCache, and refers to the BlazeRuntime's BuildView and
+ * <p>This class contains an ActionCache, and refers to the Blaze Runtime's BuildView and
  * PackageCache.
  *
  * @see BuildTool
@@ -357,17 +358,26 @@ public class ExecutionTool {
     // Must be created after the output path is created above.
     createActionLogDirectory();
 
-    List<BuildConfiguration> targetConfigurations = configurations.getTargetConfigurations();
-    BuildConfiguration targetConfiguration = targetConfigurations.size() == 1
-        ? targetConfigurations.get(0) : null;
-    if (targetConfigurations.size() == 1) {
-      String productName = runtime.getProductName();
-      String workspaceName = env.getWorkspaceName();
-      OutputDirectoryLinksUtils.createOutputDirectoryLinks(
-          workspaceName, env.getWorkspace(), env.getDirectories().getExecRoot(workspaceName),
-          env.getDirectories().getOutputPath(workspaceName), getReporter(), targetConfiguration,
-          request.getBuildOptions().getSymlinkPrefix(productName), productName);
-    }
+    // Create convenience symlinks from the configurations actually used by the requested targets.
+    // Symlinks will be created if all such configurations would point the symlink to the same path;
+    // if this does not hold, stale symlinks (if present from a previous invocation) will be
+    // deleted instead.
+    Set<BuildConfiguration> targetConfigurations =
+        request.getBuildOptions().useTopLevelTargetsForSymlinks()
+        ? analysisResult
+            .getTargetsToBuild()
+            .stream()
+            .map(ConfiguredTarget::getConfiguration)
+            .filter(configuration -> configuration != null)
+            .distinct()
+            .collect(toImmutableSet())
+        : ImmutableSet.copyOf(configurations.getTargetConfigurations());
+    String productName = runtime.getProductName();
+    String workspaceName = env.getWorkspaceName();
+    OutputDirectoryLinksUtils.createOutputDirectoryLinks(
+        workspaceName, env.getWorkspace(), env.getDirectories().getExecRoot(workspaceName),
+        env.getDirectories().getOutputPath(workspaceName), getReporter(), targetConfigurations,
+        request.getBuildOptions().getSymlinkPrefix(productName), productName);
 
     ActionCache actionCache = getActionCache();
     SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor();
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 e4b2b68a83..7211b916db 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
@@ -14,9 +14,11 @@
 package com.google.devtools.build.lib.buildtool;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
@@ -24,11 +26,10 @@ import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.Symlinks;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import javax.annotation.Nullable;
+import java.util.Set;
 
 /**
  * Static utilities for managing output directory symlinks.
@@ -54,16 +55,27 @@ public class OutputDirectoryLinksUtils {
   private static String execRootSymlink(String symlinkPrefix, String workspaceName) {
     return symlinkPrefix + workspaceName;
   }
+
   /**
-   * Attempts to create convenience symlinks in the workspaceDirectory and in
-   * execRoot to the output area and to the configuration-specific output
-   * directories. Issues a warning if it fails, e.g. because workspaceDirectory
-   * is readonly.
+   * Attempts to create convenience symlinks in the workspaceDirectory and in execRoot to the output
+   * area and to the configuration-specific output directories. Issues a warning if it fails, e.g.
+   * because workspaceDirectory is readonly.
+   *
+   * <p>Configuration-specific output symlinks will be created or updated if and only if the set of
+   * {@code targetConfigs} contains only configurations whose output directories match. Otherwise -
+   * i.e., if there are multiple configurations with distinct output directories or there were no
+   * targets with non-null configurations in the build - any stale symlinks left over from previous
+   * invocations will be removed.
    */
-  static void createOutputDirectoryLinks(String workspaceName,
-      Path workspace, Path execRoot, Path outputPath,
-      EventHandler eventHandler, @Nullable BuildConfiguration targetConfig,
-      String symlinkPrefix, String productName) {
+  static void createOutputDirectoryLinks(
+      String workspaceName,
+      Path workspace,
+      Path execRoot,
+      Path outputPath,
+      EventHandler eventHandler,
+      Set<BuildConfiguration> targetConfigs,
+      String symlinkPrefix,
+      String productName) {
     if (NO_CREATE_SYMLINKS_PREFIX.equals(symlinkPrefix)) {
       return;
     }
@@ -80,13 +92,49 @@ public class OutputDirectoryLinksUtils {
     createLink(workspace, execRootSymlink(
         symlinkPrefix, workspace.getBaseName()), execRoot, failures);
     RepositoryName repositoryName = RepositoryName.createFromValidStrippedName(workspaceName);
-    if (targetConfig != null) {
-      createLink(workspace, symlinkPrefix + "bin",
-          targetConfig.getBinDirectory(repositoryName).getPath(), failures);
-      createLink(workspace, symlinkPrefix + "testlogs",
-          targetConfig.getTestLogsDirectory(repositoryName).getPath(), failures);
-      createLink(workspace, symlinkPrefix + "genfiles",
-          targetConfig.getGenfilesDirectory(repositoryName).getPath(), failures);
+
+    // Set up convenience symlinks iff there's only one useful target for them to point to;
+    // otherwise, remove stale symlinks from previous invocations at that path to avoid confusion
+    Set<Path> binPaths =
+        targetConfigs
+            .stream()
+            .map(targetConfig -> targetConfig.getBinDirectory(repositoryName).getPath())
+            .distinct()
+            .collect(toImmutableSet());
+    if (binPaths.size() == 1) {
+      createLink(workspace, symlinkPrefix + "bin", Iterables.getOnlyElement(binPaths), failures);
+    } else {
+      removeLink(workspace, symlinkPrefix + "bin", failures);
+    }
+    Set<Path> testLogsPaths =
+        targetConfigs
+            .stream()
+            .map(targetConfig -> targetConfig.getTestLogsDirectory(repositoryName).getPath())
+            .distinct()
+            .collect(toImmutableSet());
+    if (testLogsPaths.size() == 1) {
+      createLink(
+          workspace,
+          symlinkPrefix + "testlogs",
+          Iterables.getOnlyElement(testLogsPaths),
+          failures);
+    } else {
+      removeLink(workspace, symlinkPrefix + "testlogs", failures);
+    }
+    Set<Path> genfilesPaths =
+        targetConfigs
+            .stream()
+            .map(targetConfig -> targetConfig.getGenfilesDirectory(repositoryName).getPath())
+            .distinct()
+            .collect(toImmutableSet());
+    if (genfilesPaths.size() == 1) {
+      createLink(
+          workspace,
+          symlinkPrefix + "genfiles",
+          Iterables.getOnlyElement(genfilesPaths),
+          failures);
+    } else {
+      removeLink(workspace, symlinkPrefix + "genfiles", failures);
     }
 
     if (!failures.isEmpty()) {
@@ -208,7 +256,7 @@ public class OutputDirectoryLinksUtils {
   private static boolean removeLink(Path base, String name, List<String> failures) {
     Path link = base.getRelative(name);
     try {
-      if (link.exists(Symlinks.NOFOLLOW)) {
+      if (link.isSymbolicLink()) {
         ExecutionTool.logger.finest("Removing " + link);
         link.delete();
       }