Skip to content

Commit

Permalink
Fix symlink planting for multiple aspects
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fmeum authored and copybara-github committed Dec 18, 2024
1 parent 2864f2a commit f4b071c
Showing 1 changed file with 12 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down

0 comments on commit f4b071c

Please sign in to comment.