Skip to content

Commit

Permalink
Remove default helper methods in BuildResultHelper
Browse files Browse the repository at this point in the history
Utility methods are supposed to be static in preferably in utility
classes or as Kotlin extension methods.

This is to enable safe and bigger refactoring of this area of code.

Bug: n/a
Test: n/a
Change-Id: I9dd7c5515c3289b05042299c6aea3d5c70c70180

AOSP: ccf72a5c77927d76c9e7cf55378ea1fc6aec6334
  • Loading branch information
Googler authored and LeFrosch committed Jan 17, 2025
1 parent 0d97ae9 commit ffee687
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ public AndroidDeployInfo readDeployInfoProtoForTarget(
throws GetDeployInfoException {
ImmutableList<OutputArtifact> outputArtifacts;
try {
outputArtifacts = buildResultHelper.getBuildArtifactsForTarget(target, pathFilter);
outputArtifacts =
buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING).getDirectArtifactsForTarget(target, pathFilter).asList();
} catch (GetArtifactsException e) {
throw new GetDeployInfoException(e.getMessage());
}
Expand All @@ -65,7 +66,7 @@ public AndroidDeployInfo readDeployInfoProtoForTarget(
}
log.warn("All local artifacts for " + target + ":");
List<OutputArtifact> allBuildArtifacts =
buildResultHelper.getBuildArtifactsForTarget(target, path -> true);
buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING).getDirectArtifactsForTarget(target, path1 -> true).asList();
List<File> allLocalFiles = LocalFileArtifact.getLocalFiles(allBuildArtifacts);
for (File file : allLocalFiles) {
String path = file.getPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,14 @@
*/
package com.google.idea.blaze.base.command.buildresult;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
import com.google.idea.blaze.base.model.primitives.Label;
import com.google.idea.blaze.base.run.testlogs.BlazeTestResults;
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.common.Interners;
import com.google.idea.blaze.common.artifact.OutputArtifact;
import com.google.idea.blaze.exception.BuildException;
import java.io.InputStream;
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Predicate;

/** Assists in getting build artifacts from a build operation. */
public interface BuildResultHelper extends AutoCloseable {
Expand Down Expand Up @@ -95,44 +90,6 @@ default InputStream getStderr(String completedBuildId) throws BuildException {
return InputStream.nullInputStream();
}

/**
* Parses the BEP output data to collect all build flags used. Return all flags that pass filters
*/
default BuildFlags getBlazeFlags() throws GetFlagsException {
return getBlazeFlags(Optional.empty());
}

/**
* Returns the build result. May only be called once, after the build is complete, or no artifacts
* will be returned.
*
* @return The build artifacts from the build operation.
*/
default ImmutableList<OutputArtifact> getAllOutputArtifacts(Predicate<String> pathFilter)
throws GetArtifactsException {
return getBuildOutput(Optional.empty(), Interners.STRING).getAllOutputArtifacts(pathFilter).asList();
}

/**
* Returns the build artifacts, filtering out all artifacts not directly produced by the specified
* target.
*
* <p>May only be called once, after the build is complete, or no artifacts will be returned.
*/
default ImmutableList<OutputArtifact> getBuildArtifactsForTarget(
Label target, Predicate<String> pathFilter) throws GetArtifactsException {
return getBuildOutput(Optional.empty(), Interners.STRING).getDirectArtifactsForTarget(target, pathFilter).asList();
}

/**
* Returns all build artifacts belonging to the given output groups. May only be called once,
* after the build is complete, or no artifacts will be returned.
*/
default ImmutableList<OutputArtifact> getArtifactsForOutputGroup(
String outputGroup, Predicate<String> pathFilter) throws GetArtifactsException {
return getBuildOutput(Optional.empty(), Interners.STRING).getOutputGroupArtifacts(outputGroup, pathFilter);
}

@Override
void close();

Expand Down
1 change: 1 addition & 0 deletions clwb/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ java_library(
"//intellij_platform_sdk:plugin_api",
"//sdkcompat",
"//clwb/sdkcompat",
"//shared",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.idea.blaze.base.run.confighandler.BlazeCommandRunConfigurationRunner;
import com.google.idea.blaze.base.sync.aspects.BuildResult;
import com.google.idea.blaze.base.util.SaveUtil;
import com.google.idea.blaze.common.Interners;
import com.intellij.execution.ExecutionException;
import com.intellij.execution.Executor;
import com.intellij.execution.RunCanceledByUserException;
Expand All @@ -46,6 +47,7 @@
import com.jetbrains.cidr.execution.CidrCommandLineState;
import com.jetbrains.cidr.lang.workspace.compiler.ClangCompilerKind;

import java.util.Optional;
import javax.annotation.Nullable;
import java.io.File;
import java.util.List;
Expand Down Expand Up @@ -167,7 +169,8 @@ private File getExecutableToDebug(ExecutionEnvironment env) throws ExecutionExce
try {
candidateFiles =
LocalFileArtifact.getLocalFiles(
buildResultHelper.getBuildArtifactsForTarget(target, file -> true))
buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING)
.getDirectArtifactsForTarget(target, file -> true))
.stream()
.filter(File::canExecute)
.collect(Collectors.toList());
Expand Down
1 change: 1 addition & 0 deletions golang/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ java_library(
"//intellij_platform_sdk:plugin_api",
"//proto:proto_deps",
"//sdkcompat",
"//shared",
"//third_party/go",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import com.google.idea.blaze.base.sync.data.BlazeDataStorage;
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.google.idea.blaze.base.util.SaveUtil;
import com.google.idea.blaze.common.Interners;
import com.google.idea.common.experiments.BoolExperiment;
import com.intellij.execution.ExecutionException;
import com.intellij.execution.ExecutionResult;
Expand Down Expand Up @@ -370,7 +371,8 @@ private static ExecutableInfo getExecutableToDebug(ExecutionEnvironment env)
try {
candidateFiles =
LocalFileArtifact.getLocalFiles(
buildResultHelper.getBuildArtifactsForTarget(label, file -> true))
buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING)
.getDirectArtifactsForTarget(label, file -> true))
.stream()
.filter(File::canExecute)
.collect(Collectors.toList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import com.google.idea.blaze.base.settings.BuildSystemName;
import com.google.idea.blaze.base.sync.aspects.BuildResult;
import com.google.idea.blaze.base.sync.aspects.BuildResult.Status;
import com.google.idea.blaze.common.Interners;
import com.google.idea.blaze.java.AndroidBlazeRules;
import com.google.idea.blaze.java.JavaBlazeRules;
import com.google.idea.blaze.java.fastbuild.FastBuildChangedFilesService.ChangedSources;
Expand All @@ -70,6 +71,7 @@
import java.io.IOException;
import java.nio.file.Files;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -322,17 +324,19 @@ private FastBuildState.BuildOutput buildDeployJar(
try {
ImmutableList<File> deployJarArtifacts =
LocalFileArtifact.getLocalFiles(
resultHelper.getBuildArtifactsForTarget(
deployJarStrategy.deployJarOwnerLabel(label, blazeVersionData), jarPredicate));
resultHelper.getBuildOutput(Optional.empty(), Interners.STRING)
.getDirectArtifactsForTarget(
deployJarStrategy.deployJarOwnerLabel(label, blazeVersionData), jarPredicate));
checkState(deployJarArtifacts.size() == 1);
File deployJar = deployJarArtifacts.get(0);

Predicate<String> filePredicate =
file -> aspectStrategy.getAspectOutputFilePredicate().test(file);
ImmutableList<File> ideInfoFiles =
LocalFileArtifact.getLocalFiles(
resultHelper.getArtifactsForOutputGroup(
aspectStrategy.getAspectOutputGroup(), filePredicate));
resultHelper.getBuildOutput(Optional.empty(), Interners.STRING)
.getOutputGroupArtifacts(
aspectStrategy.getAspectOutputGroup(), filePredicate));

// if targets are built with multiple configurations, just take the first one
// TODO(brendandouglas): choose a consistent configuration instead
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@
import com.google.idea.blaze.base.sync.aspects.BuildResult;
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.google.idea.blaze.base.util.SaveUtil;
import com.google.idea.blaze.common.Interners;
import com.intellij.debugger.impl.HotSwapProgress;
import com.intellij.execution.ExecutionException;
import com.intellij.execution.RunCanceledByUserException;
import com.intellij.execution.runners.ExecutionEnvironment;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Key;
import java.io.File;
import java.util.Optional;
import java.util.concurrent.CancellationException;
import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -133,8 +135,8 @@ public static ClassFileManifest.Diff buildManifest(
try {
jars =
LocalFileArtifact.getLocalFiles(
buildResultHelper.getArtifactsForOutputGroup(
JavaClasspathAspectStrategy.OUTPUT_GROUP, file -> true))
buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING)
.getOutputGroupArtifacts(JavaClasspathAspectStrategy.OUTPUT_GROUP, file -> true))
.stream()
.filter(f -> f.getName().endsWith(".jar"))
.collect(toImmutableList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.google.idea.blaze.base.util.ProcessGroupUtil;
import com.google.idea.blaze.base.util.SaveUtil;
import com.google.idea.blaze.common.Interners;
import com.google.idea.blaze.python.PySdkUtils;
import com.intellij.execution.ExecutionException;
import com.intellij.execution.ExecutionResult;
Expand Down Expand Up @@ -79,6 +80,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CancellationException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -342,7 +344,8 @@ private static PyExecutionInfo getExecutableToDebug(ExecutionEnvironment env)
try {
candidateFiles =
LocalFileArtifact.getLocalFiles(
buildResultHelper.getBuildArtifactsForTarget(target, file -> true))
buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING)
.getDirectArtifactsForTarget(target, file -> true).asList())
.stream()
.filter(File::canExecute)
.collect(Collectors.toList());
Expand Down
1 change: 1 addition & 0 deletions scala/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ java_library(
"//java",
"//proto:proto_deps",
"//sdkcompat",
"//shared",
"//third_party/scala",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.google.idea.blaze.base.settings.BlazeUserSettings;
import com.google.idea.blaze.base.sync.aspects.BuildResult;
import com.google.idea.blaze.base.util.SaveUtil;
import com.google.idea.blaze.common.Interners;
import com.intellij.execution.BeforeRunTask;
import com.intellij.execution.BeforeRunTaskProvider;
import com.intellij.execution.ExecutionException;
Expand All @@ -59,6 +60,7 @@
import icons.BlazeIcons;
import java.io.File;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CancellationException;
import javax.annotation.Nullable;
import javax.swing.Icon;
Expand Down Expand Up @@ -187,8 +189,9 @@ private static File getDeployableJar(

List<File> outputs =
LocalFileArtifact.getLocalFiles(
buildResultHelper.getBuildArtifactsForTarget(
target.withTargetName(target.targetName() + "_deploy.jar"), file -> true));
buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING)
.getDirectArtifactsForTarget(
target.withTargetName(target.targetName() + "_deploy.jar"), file -> true));
if (outputs.isEmpty()) {
throw new ExecutionException(
String.format("Failed to find deployable jar when building %s", target));
Expand Down

0 comments on commit ffee687

Please sign in to comment.