From 1cd35886cf2233050e2ed14fb3c98e5ec1d756b5 Mon Sep 17 00:00:00 2001
From: Googler <leba@google.com>
Date: Fri, 5 May 2023 08:02:35 -0700
Subject: [PATCH] [Skymeld] Plant the convenience symlinks even in case of
 failure.

Before this CL, Skymeld only does so when the build succeeds.

PiperOrigin-RevId: 529717866
Change-Id: I8832106bc3e54127637c4dd07a2eb05b5080e52a
---
 .../build/lib/buildtool/BuildTool.java        |  8 ++++++--
 .../build/lib/buildtool/ExecutionTool.java    | 20 +++++++++++--------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index 67ed6ce296c566..eda3ce702efff2 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -325,7 +325,7 @@ private void buildTargetsWithMergedAnalysisExecution(
     Stopwatch executionTimer = Stopwatch.createUnstarted();
 
     // TODO(b/199053098): implement support for --nobuild.
-    AnalysisAndExecutionResult analysisAndExecutionResult;
+    AnalysisAndExecutionResult analysisAndExecutionResult = null;
     boolean buildCompleted = false;
     try {
       analysisAndExecutionResult =
@@ -360,7 +360,6 @@ public boolean forceExclusiveIfLocalTestsInParallel() {
       if (analysisAndExecutionResult == null) {
         return;
       }
-      executionTool.handleConvenienceSymlinks(analysisAndExecutionResult);
     } catch (InvalidConfigurationException
         | RepositoryMappingResolutionException
         | ViewCreationFailedException
@@ -374,6 +373,11 @@ public boolean forceExclusiveIfLocalTestsInParallel() {
       hasCatastrophe = true;
       throw e;
     } finally {
+      if (result.getBuildConfiguration() != null) {
+        // We still need to do this even in case of an exception.
+        executionTool.handleConvenienceSymlinks(
+            env.getBuildResultListener().getAnalyzedTargets(), result.getBuildConfiguration());
+      }
       executionTool.unconditionalExecutionPhaseFinalizations(
           executionTimer, env.getSkyframeExecutor());
 
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 f800e385ebcdb5..85bd58994e7dbf 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
@@ -396,7 +396,8 @@ void executeBuild(
       createActionLogDirectory();
     }
 
-    handleConvenienceSymlinks(analysisResult);
+    handleConvenienceSymlinks(
+        analysisResult.getTargetsToBuild(), analysisResult.getConfiguration());
 
     BuildRequestOptions options = request.getBuildOptions();
     ActionCache actionCache = null;
@@ -702,13 +703,15 @@ private static BuildConfigurationValue getConfiguration(
    * Otherwise, manage the convenience symlinks and then post a {@link
    * ConvenienceSymlinksIdentifiedEvent} build event.
    */
-  public void handleConvenienceSymlinks(AnalysisResult analysisResult) {
+  public void handleConvenienceSymlinks(
+      ImmutableSet<ConfiguredTarget> targetsToBuild, BuildConfigurationValue configuration) {
     try (SilentCloseable c =
         Profiler.instance().profile("ExecutionTool.handleConvenienceSymlinks")) {
       ImmutableList<ConvenienceSymlink> convenienceSymlinks = ImmutableList.of();
       if (request.getBuildOptions().experimentalConvenienceSymlinks
           != ConvenienceSymlinksMode.IGNORE) {
-        convenienceSymlinks = createConvenienceSymlinks(request.getBuildOptions(), analysisResult);
+        convenienceSymlinks =
+            createConvenienceSymlinks(request.getBuildOptions(), targetsToBuild, configuration);
       }
       if (request.getBuildOptions().experimentalConvenienceSymlinksBepEvent) {
         env.getEventBus().post(new ConvenienceSymlinksIdentifiedEvent(convenienceSymlinks));
@@ -730,22 +733,23 @@ public void handleConvenienceSymlinks(AnalysisResult analysisResult) {
    * in fact gets removed if it was already present from a previous invocation.
    */
   private ImmutableList<ConvenienceSymlink> createConvenienceSymlinks(
-      BuildRequestOptions buildRequestOptions, AnalysisResult analysisResult) {
+      BuildRequestOptions buildRequestOptions,
+      ImmutableSet<ConfiguredTarget> targetsToBuild,
+      BuildConfigurationValue configuration) {
     SkyframeExecutor executor = env.getSkyframeExecutor();
     Reporter reporter = env.getReporter();
 
     // Gather configurations to consider.
     Set<BuildConfigurationValue> targetConfigurations =
-        buildRequestOptions.useTopLevelTargetsForSymlinks()
-                && !analysisResult.getTargetsToBuild().isEmpty()
-            ? analysisResult.getTargetsToBuild().stream()
+        buildRequestOptions.useTopLevelTargetsForSymlinks() && !targetsToBuild.isEmpty()
+            ? targetsToBuild.stream()
                 .map(ConfiguredTarget::getActual)
                 .map(ConfiguredTarget::getConfigurationKey)
                 .filter(Objects::nonNull)
                 .distinct()
                 .map((key) -> executor.getConfiguration(reporter, key))
                 .collect(toImmutableSet())
-            : ImmutableSet.of(analysisResult.getConfiguration());
+            : ImmutableSet.of(configuration);
 
     String productName = runtime.getProductName();
     try (SilentCloseable c =