Skip to content

Commit

Permalink
Remove default method variants from interfaces (#7223)
Browse files Browse the repository at this point in the history
it is difficult to understand what each implementation actually does.

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

AOSP: c575fe5b9d9dd881b2925d06e1cca0219a67c2c8

Co-authored-by: Googler <[email protected]>
  • Loading branch information
LeFrosch and Googler authored Jan 17, 2025
1 parent 65ecc64 commit c5640d9
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@
import com.google.idea.blaze.base.command.buildresult.LocalFileArtifact;
import com.google.idea.blaze.base.command.buildresult.ParsedBepOutput;
import com.google.idea.blaze.base.model.primitives.Label;
import com.google.idea.blaze.common.Interners;
import com.google.idea.blaze.common.artifact.OutputArtifact;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -55,7 +57,7 @@ public AndroidDeployInfo readDeployInfoProtoForTarget(
if (outputArtifacts.isEmpty()) {
Logger log = Logger.getInstance(BlazeApkDeployInfoProtoHelper.class.getName());
try {
ParsedBepOutput bepOutput = buildResultHelper.getBuildOutput();
ParsedBepOutput bepOutput = buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING);
log.warn("Local execroot: " + bepOutput.getLocalExecRoot());
log.warn("All output artifacts:");
for (OutputArtifact outputArtifact : bepOutput.getAllOutputArtifacts(path -> true)) {
Expand Down
2 changes: 2 additions & 0 deletions base/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ java_library(
":base",
"//shared:artifact",
"//shared:exception",
"//shared/java/com/google/idea/blaze/common",
"//testing:lib",
"//third_party/bazel/src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//third_party/java/auto_value",
Expand Down Expand Up @@ -544,6 +545,7 @@ intellij_unit_test_suite(
"//querysync/javatests/com/google/idea/blaze/qsync/testdata",
"//shared",
"//shared:artifact",
"//shared/java/com/google/idea/blaze/common",
"//shared/javatests/com/google/idea/blaze/common:test_utils",
"//shared/javatests/com/google/idea/blaze/common/artifact:test_utils",
"//testing:lib",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.google.idea.blaze.base.model.primitives.WorkspaceRoot
import com.google.idea.blaze.base.scope.BlazeContext
import com.google.idea.blaze.base.sync.aspects.BlazeBuildOutputs
import com.google.idea.blaze.base.sync.aspects.BuildResult
import com.google.idea.blaze.common.Interners
import com.google.idea.blaze.common.PrintOutput
import com.google.protobuf.CodedInputStream
import com.intellij.execution.configurations.PtyCommandLine
Expand All @@ -27,6 +28,7 @@ import com.intellij.util.ui.EDT
import kotlinx.coroutines.*
import java.io.BufferedInputStream
import java.io.FileInputStream
import java.util.Optional
import kotlin.io.path.pathString

private val LOG: Logger = Logger.getInstance(BazelExecService::class.java)
Expand Down Expand Up @@ -185,7 +187,10 @@ class BazelExecService(private val project: Project) : Disposable {
if (result.status == BuildResult.Status.FATAL_ERROR) {
BlazeBuildOutputs.noOutputs(result)
} else {
BlazeBuildOutputs.fromParsedBepOutput(result, provider.getBuildOutput())
BlazeBuildOutputs.fromParsedBepOutput(
result,
provider.getBuildOutput(Optional.empty(), Interners.STRING),
)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public BlazeBuildOutputs run(
Optional.ofNullable(context.getScope(SharedStringPoolScope.class))
.map(SharedStringPoolScope::getStringInterner)
.orElse(null);
ParsedBepOutput buildOutput = buildResultHelper.getBuildOutput(stringInterner);
ParsedBepOutput buildOutput = buildResultHelper.getBuildOutput(Optional.empty(), stringInterner);
context.output(PrintOutput.log("BEP outputs retrieved (%s).", StringUtilRt.formatFileSize(buildOutput.getBepBytesConsumed())));
return BlazeBuildOutputs.fromParsedBepOutput(buildResult, buildOutput);
} catch (GetArtifactsException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
Expand All @@ -46,46 +47,9 @@ public interface BuildResultHelper extends AutoCloseable {
* getBuildOutput may restrict parallelism for cases in which many builds are executed in parallel
* (e.g. remote builds).
*/
default ParsedBepOutput getBuildOutput() throws GetArtifactsException {
return getBuildOutput(Optional.empty());
}

/**
* Parses the BEP output data and returns the corresponding {@link ParsedBepOutput}. May only be
* called once, after the build is complete.
*
* <p>As BEP retrieval can be memory-intensive for large projects, implementations of
* getBuildOutput may restrict parallelism for cases in which many builds are executed in parallel
* (e.g. remote builds).
*/
default ParsedBepOutput getBuildOutput(Interner<String> stringInterner)
throws GetArtifactsException {
return getBuildOutput(Optional.empty(), stringInterner);
}

/**
* Parses the BEP output data and returns the corresponding {@link ParsedBepOutput}. May only be
* called once, after the build is complete.
*
* <p>As BEP retrieval can be memory-intensive for large projects, implementations of
* getBuildOutput may restrict parallelism for cases in which many builds are executed in parallel
* (e.g. remote builds).
*/
default ParsedBepOutput getBuildOutput(
ParsedBepOutput getBuildOutput(
Optional<String> completionBuildId, Interner<String> stringInterner)
throws GetArtifactsException {
return getBuildOutput(completionBuildId);
}

/**
* Retrieves BEP build events according to given id, parses them and returns the corresponding
* {@link ParsedBepOutput}. May only be called once, after the build is complete.
*
* <p>As BEP retrieval can be memory-intensive for large projects, implementations of
* getBuildOutput may restrict parallelism for cases in which many builds are executed in parallel
* (e.g. remote builds).
*/
ParsedBepOutput getBuildOutput(Optional<String> completedBuildId) throws GetArtifactsException;
throws GetArtifactsException;

/**
* Retrieves test results, parses them and returns the corresponding {@link BlazeTestResults}. May
Expand Down Expand Up @@ -146,7 +110,7 @@ default BuildFlags getBlazeFlags() throws GetFlagsException {
*/
default ImmutableList<OutputArtifact> getAllOutputArtifacts(Predicate<String> pathFilter)
throws GetArtifactsException {
return getBuildOutput().getAllOutputArtifacts(pathFilter).asList();
return getBuildOutput(Optional.empty(), Interners.STRING).getAllOutputArtifacts(pathFilter).asList();
}

/**
Expand All @@ -157,7 +121,7 @@ default ImmutableList<OutputArtifact> getAllOutputArtifacts(Predicate<String> pa
*/
default ImmutableList<OutputArtifact> getBuildArtifactsForTarget(
Label target, Predicate<String> pathFilter) throws GetArtifactsException {
return getBuildOutput().getDirectArtifactsForTarget(target, pathFilter).asList();
return getBuildOutput(Optional.empty(), Interners.STRING).getDirectArtifactsForTarget(target, pathFilter).asList();
}

/**
Expand All @@ -166,7 +130,7 @@ default ImmutableList<OutputArtifact> getBuildArtifactsForTarget(
*/
default ImmutableList<OutputArtifact> getArtifactsForOutputGroup(
String outputGroup, Predicate<String> pathFilter) throws GetArtifactsException {
return getBuildOutput().getOutputGroupArtifacts(outputGroup, pathFilter);
return getBuildOutput(Optional.empty(), Interners.STRING).getOutputGroupArtifacts(outputGroup, pathFilter);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.google.idea.blaze.base.command.buildresult;

import com.google.common.collect.Interner;
import com.google.idea.blaze.base.command.buildresult.BuildEventStreamProvider.BuildEventStreamException;
import com.google.idea.blaze.base.io.InputStreamProvider;
import com.google.idea.blaze.base.run.testlogs.BlazeTestResults;
Expand Down Expand Up @@ -55,7 +56,7 @@ public List<String> getBuildFlags() {
}

@Override
public ParsedBepOutput getBuildOutput(Optional<String> completedBuildId)
public ParsedBepOutput getBuildOutput(Optional<String> completedBuildId, Interner<String> stringInterner)
throws GetArtifactsException {
try (InputStream inputStream = new BufferedInputStream(new FileInputStream(outputFile))) {
return ParsedBepOutput.parseBepArtifacts(inputStream);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.base.sync.aspects.BlazeBuildOutputs;
import com.google.idea.blaze.base.sync.aspects.BuildResult;
import com.google.idea.blaze.common.Interners;
import com.google.idea.blaze.exception.BuildException;
import com.intellij.openapi.project.Project;

import java.io.InputStream;
import java.util.Map;
import java.util.Optional;

/**
* A fake for {@link BlazeCommandRunner} that doesn't execute the build, but returns results from
Expand All @@ -52,7 +54,7 @@ public FakeBlazeCommandRunner() {
this(
buildResultHelper ->
BlazeBuildOutputs.fromParsedBepOutput(
BuildResult.SUCCESS, buildResultHelper.getBuildOutput()));
BuildResult.SUCCESS, buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING)));
}

public FakeBlazeCommandRunner(BuildFunction buildFunction) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.idea.blaze.base.bazel;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
import com.google.idea.blaze.base.command.buildresult.BuildFlags;
import com.google.idea.blaze.base.command.buildresult.BuildResultHelper;
import com.google.idea.blaze.base.command.buildresult.ParsedBepOutput;
Expand Down Expand Up @@ -49,7 +50,7 @@ public List<String> getBuildFlags() {
}

@Override
public ParsedBepOutput getBuildOutput(Optional<String> completedBuildId)
public ParsedBepOutput getBuildOutput(Optional<String> completedBuildId, Interner<String> stringInterner)
throws GetArtifactsException {
if (parsedBepOutput == null) {
throw new GetArtifactsException("Could not get artifacts from null bep");
Expand Down
1 change: 1 addition & 0 deletions python/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ java_library(
"//intellij_platform_sdk:plugin_api",
"//intellij_platform_sdk:jsr305",
"//shared",
"//shared/java/com/google/idea/blaze/common",
"//third_party/python",
],
)
Expand Down
3 changes: 1 addition & 2 deletions shared/java/com/google/idea/blaze/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ java_library(
name = "common",
srcs = glob(["*.java"]),
visibility = [
"//querysync/javatests/com/google/idea/blaze/qsync:__subpackages__",
"//shared:__subpackages__",
"//visibility:public",
],
deps = [
"@jsr305_annotations//jar",
Expand Down

0 comments on commit c5640d9

Please sign in to comment.