[8.0.1] Fix symlink planting for multiple aspects (#24741)

When Skymeld is enabled, symlinks for external repositories are planted lazily in response to an event sent in `BuildDriverFunction`. For top-level aspects, this event needs to be sent individually for each aspect as the transitive packages required can differ per aspect. `BuildDriverFunction` deduplicated all events, including the one for symlink planting, by type, which resulted in missing symlinks for actions registered by the second (and later) aspects. Fixes #24619 Closes #24676. PiperOrigin-RevId: 707458188 Change-Id: I9fe523bc746d29e6c6ddb6367e08ce2c6c48c69b Commit f4b071c Co-authored-by: Fabian Meumertzheim <[email protected]>
bazelbuild · Dec 19, 2024 · 9264ea1 · 9264ea1
1 parent e12aebc
commit 9264ea1
Showing 1 changed file with 12 additions and 4 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java
@@ -324,6 +324,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       ImmutableSet.Builder<Artifact> artifactsToBuild = ImmutableSet.builder();
       List<SkyKey> aspectCompletionKeys = new ArrayList<>();
 
+      // Do not trigger Skyframe restarts in this loop (see comments below).
       for (Map.Entry<AspectKey, AspectValue> entry :
           ((TopLevelAspectsValue) topLevelSkyValue).getTopLevelAspectsMap().entrySet()) {
         AspectKey aspectKey = entry.getKey();
@@ -339,13 +340,20 @@ public SkyValue compute(SkyKey skyKey, Environment env)
         NestedSet<Package> transitivePackagesForSymlinkPlanting =
             aspectValue.getTransitivePackages();
         if (transitivePackagesForSymlinkPlanting != null) {
-          postEventIfNecessary(
-              postedEventsTypes,
-              env,
-              TopLevelTargetReadyForSymlinkPlanting.create(transitivePackagesForSymlinkPlanting));
+          // This event should be sent out exactly once per aspect in this BuildDriverKey, even with
+          // resets. We achieve this by marking the event type as sent only after sending the event
+          // for all aspects, but must avoid triggering Skyframe restarts while doing so.
+          if (!postedEventsTypes.contains(
+              TopLevelStatusEvents.Type.TOP_LEVEL_TARGET_READY_FOR_SYMLINK_PLANTING)) {
+            env.getListener()
+                .post(
+                    TopLevelTargetReadyForSymlinkPlanting.create(
+                        transitivePackagesForSymlinkPlanting));
+          }
         }
         aspectCompletionKeys.add(AspectCompletionKey.create(aspectKey, topLevelArtifactContext));
       }
+      postedEventsTypes.add(TopLevelStatusEvents.Type.TOP_LEVEL_TARGET_READY_FOR_SYMLINK_PLANTING);
 
       if (!additionalPostAnalysisDepsRequestedAndAvailable.request(env, actionLookupKey)) {
         return null;