diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java index 438863e9167daa..b2b39ff6a62f93 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java @@ -113,6 +113,10 @@ public ArtifactsInOutputGroup getArtifacts(String outputGroup) { return artifactOutputGroups.get(outputGroup); } + public ImmutableMap getOutputs() { + return artifactOutputGroups; + } + public CompletionContext getCompletionContext() { return completionContext; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java index 52316f713302d9..d87e1c892cb075 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java @@ -102,6 +102,14 @@ public Path getRunfilesDirectory() { return null; } + @Nullable + public Runfiles getRunfiles() { + if (runfilesSupport != null) { + return runfilesSupport.getRunfiles(); + } + return null; + } + @Nullable public Artifact getExecutable() { return executable; @@ -298,6 +306,10 @@ public ArtifactsInOutputGroup getOutputGroup(String outputGroup) { return outputs.get(outputGroup); } + public ImmutableMap getOutputs() { + return outputs; + } + // TODO(aehlig): remove as soon as we managed to get rid of the deprecated "important_output" // field. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java index 07e94efde35fc3..d295bb33f1c151 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java @@ -38,8 +38,11 @@ import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.InputFileErrorException; import com.google.devtools.build.lib.actions.RemoteArtifactChecker; +import com.google.devtools.build.lib.analysis.AspectCompleteEvent; import com.google.devtools.build.lib.analysis.ConfiguredObjectValue; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.Runfiles; +import com.google.devtools.build.lib.analysis.TargetCompleteEvent; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper; import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup; @@ -69,6 +72,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -326,13 +330,12 @@ public SkyValue compute(SkyKey skyKey, Environment env) return null; } - ensureToplevelArtifacts(env, allArtifacts, inputMap); - ExtendedEventHandler.Postable postable = completor.createSucceeded(key, value, ctx, artifactsToBuild, env); if (postable == null) { return null; } + ensureToplevelArtifacts(env, postable, inputMap); env.getListener().post(postable); topLevelArtifactsMetric.mergeIn(currentConsumer); @@ -340,7 +343,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) } private void ensureToplevelArtifacts( - Environment env, ImmutableCollection artifacts, ActionInputMap inputMap) + Environment env, ExtendedEventHandler.Postable postable, ActionInputMap inputMap) throws CompletionFunctionException, InterruptedException { // For skymeld, a non-toplevel target might become a toplevel after it has been executed. This // is the last chance to download the missing toplevel outputs in this case before sending out @@ -364,86 +367,33 @@ private void ensureToplevelArtifacts( return; } + ImmutableMap allOutputGroups; + Runfiles runfiles = null; + if (postable instanceof TargetCompleteEvent targetCompleteEvent) { + allOutputGroups = targetCompleteEvent.getOutputs(); + runfiles = targetCompleteEvent.getExecutableTargetData().getRunfiles(); + } else if (postable instanceof AspectCompleteEvent aspectCompleteEvent) { + allOutputGroups = aspectCompleteEvent.getOutputs(); + } else { + return; + } + var futures = new ArrayList>(); - for (var artifact : artifacts) { - if (!(artifact instanceof DerivedArtifact derivedArtifact)) { + for (var outputGroup : allOutputGroups.values()) { + if (!outputGroup.areImportant()) { continue; } - // Metadata can be null during error bubbling, only download outputs that are already - // generated. b/342188273 - if (artifact.isTreeArtifact()) { - var treeMetadata = inputMap.getTreeMetadata(artifact.getExecPath()); - if (treeMetadata == null) { - continue; - } - - var filesToDownload = new ArrayList(treeMetadata.getChildValues().size()); - for (var child : treeMetadata.getChildValues().entrySet()) { - var treeFile = child.getKey(); - var metadata = child.getValue(); - if (metadata.isRemote() - && !remoteArtifactChecker.shouldTrustRemoteArtifact( - treeFile, (RemoteFileArtifactValue) metadata)) { - filesToDownload.add(treeFile); - } - } - if (!filesToDownload.isEmpty()) { - var action = - ActionUtils.getActionForLookupData(env, derivedArtifact.getGeneratingActionKey()); - var future = - actionInputPrefetcher.prefetchFiles( - action, filesToDownload, inputMap::getInputMetadata, Priority.LOW); - futures.add( - Futures.catchingAsync( - future, - Throwable.class, - e -> - Futures.immediateFailedFuture( - new ActionExecutionException( - e, - action, - true, - DetailedExitCode.of( - FailureDetail.newBuilder().setMessage(e.getMessage()).build()))), - directExecutor())); - } - } else { - var metadata = inputMap.getInputMetadata(artifact); - if (metadata == null) { - continue; - } + for (var artifact : outputGroup.getArtifacts().toList()) { + downloadArtifact( + env, remoteArtifactChecker, actionInputPrefetcher, inputMap, artifact, futures); + } + } - if (metadata.isRemote() - && !remoteArtifactChecker.shouldTrustRemoteArtifact( - artifact, (RemoteFileArtifactValue) metadata)) { - var action = - ActionUtils.getActionForLookupData(env, derivedArtifact.getGeneratingActionKey()); - var future = - actionInputPrefetcher.prefetchFiles( - action, ImmutableList.of(artifact), inputMap::getInputMetadata, Priority.LOW); - futures.add( - Futures.catchingAsync( - future, - Throwable.class, - e -> - Futures.immediateFailedFuture( - new ActionExecutionException( - e, - action, - true, - DetailedExitCode.of( - FailureDetail.newBuilder() - .setMessage(e.getMessage()) - .setRemoteExecution( - RemoteExecution.newBuilder() - .setCode( - RemoteExecution.Code - .TOPLEVEL_OUTPUTS_DOWNLOAD_FAILURE) - .build()) - .build()))), - directExecutor())); - } + if (runfiles != null) { + for (var artifact : runfiles.getAllArtifacts().toList()) { + downloadArtifact( + env, remoteArtifactChecker, actionInputPrefetcher, inputMap, artifact, futures); } } @@ -458,6 +408,95 @@ private void ensureToplevelArtifacts( } } + private void downloadArtifact( + Environment env, + RemoteArtifactChecker remoteArtifactChecker, + ActionInputPrefetcher actionInputPrefetcher, + ActionInputMap inputMap, + Artifact artifact, + List> futures + ) throws InterruptedException { + if (!(artifact instanceof DerivedArtifact derivedArtifact)) { + return; + } + + // Metadata can be null during error bubbling, only download outputs that are already + // generated. b/342188273 + if (artifact.isTreeArtifact()) { + var treeMetadata = inputMap.getTreeMetadata(artifact.getExecPath()); + if (treeMetadata == null) { + return; + } + + var filesToDownload = new ArrayList(treeMetadata.getChildValues().size()); + for (var child : treeMetadata.getChildValues().entrySet()) { + var treeFile = child.getKey(); + var metadata = child.getValue(); + if (metadata.isRemote() + && !remoteArtifactChecker.shouldTrustRemoteArtifact( + treeFile, (RemoteFileArtifactValue) metadata)) { + filesToDownload.add(treeFile); + } + } + if (!filesToDownload.isEmpty()) { + var action = + ActionUtils.getActionForLookupData(env, derivedArtifact.getGeneratingActionKey()); + var future = + actionInputPrefetcher.prefetchFiles( + action, filesToDownload, inputMap::getInputMetadata, Priority.LOW); + futures.add( + Futures.catchingAsync( + future, + Throwable.class, + e -> + Futures.immediateFailedFuture( + new ActionExecutionException( + e, + action, + true, + DetailedExitCode.of( + FailureDetail.newBuilder().setMessage(e.getMessage()).build()))), + directExecutor())); + } + } else { + var metadata = inputMap.getInputMetadata(artifact); + if (metadata == null) { + return; + } + + if (metadata.isRemote() + && !remoteArtifactChecker.shouldTrustRemoteArtifact( + artifact, (RemoteFileArtifactValue) metadata)) { + var action = + ActionUtils.getActionForLookupData(env, derivedArtifact.getGeneratingActionKey()); + var future = + actionInputPrefetcher.prefetchFiles( + action, ImmutableList.of(artifact), inputMap::getInputMetadata, Priority.LOW); + futures.add( + Futures.catchingAsync( + future, + Throwable.class, + e -> + Futures.immediateFailedFuture( + new ActionExecutionException( + e, + action, + true, + DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(e.getMessage()) + .setRemoteExecution( + RemoteExecution.newBuilder() + .setCode( + RemoteExecution.Code + .TOPLEVEL_OUTPUTS_DOWNLOAD_FAILURE) + .build()) + .build()))), + directExecutor())); + } + } + } + private void handleSourceFileError( Artifact input, DetailedExitCode detailedExitCode,