Skip to content

Commit

Permalink
Separate BEP fetching and parsing
Browse files Browse the repository at this point in the history
This is to enable independent changes to both processes.

1. Introduce a `BuildResultHelper.getBepStream()` method that knows
   how to get a BEP stream. Each `BuildResultHelper` gets its own
   implementation.

2. Re-implement other methods like `getBuildOutput()` to operate on
   externally supplied `BuildEventStreamProvider`. These methods should
   not eventually depend on a specific implementation of
   `BuildResultHelper` and will be moved out soon.

3. Update tests accordingly.

Bug: n/a
Test: existing
Change-Id: I48f6cf5d19889e60c6795c1b097243cd7006c686

AOSP: 47436ebddcad73d6a5d4bf1d8ab8a8ccbe1c2235
  • Loading branch information
Googler authored and LeFrosch committed Jan 22, 2025
1 parent 79a0262 commit d50505a
Show file tree
Hide file tree
Showing 21 changed files with 200 additions and 138 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ public AndroidDeployInfo readDeployInfoProtoForTarget(
throws GetDeployInfoException {
ImmutableList<OutputArtifact> outputArtifacts;
ParsedBepOutput bepOutput;
try {
bepOutput = buildResultHelper.getBuildOutput(Optional.empty(), Interners.STRING);
try (final var bepStream = buildResultHelper.getBepStream(Optional.empty())) {
bepOutput = buildResultHelper.getBuildOutput(bepStream, Interners.STRING);
outputArtifacts = bepOutput.getDirectArtifactsForTarget(target, pathFilter).asList();
} catch (GetArtifactsException e) {
throw new GetDeployInfoException(e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,10 @@ public void run(@NotNull BlazeLaunchContext launchContext)
if (retVal != 0) {
context.setHasError();
} else {
testResultsHolder.setTestResults(
buildResultHelper.getTestResults(Optional.empty()));
try (final var bepStream = buildResultHelper.getBepStream(Optional.empty())) {
testResultsHolder.setTestResults(
buildResultHelper.getTestResults(bepStream));
}
}
ListenableFuture<Void> unusedFuture =
FileCaches.refresh(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class BazelExecService(private val project: Project) : Disposable {
LOG.assertTrue(cmdBuilder.name == BlazeCommandName.BUILD)

return executionScope(ctx) { provider ->
cmdBuilder.addBlazeFlags(provider.getBuildFlags())
cmdBuilder.addBlazeFlags(provider.buildFlags)

val parseJob = parseEvents(ctx, provider)

Expand All @@ -185,11 +185,13 @@ class BazelExecService(private val project: Project) : Disposable {
parseJob.cancelAndJoin()

if (result.status == BuildResult.Status.FATAL_ERROR) {
BlazeBuildOutputs.noOutputs(result)
} else {
return@executionScope BlazeBuildOutputs.noOutputs(result)
}

provider.getBepStream(Optional.empty()).use { bepStream ->
BlazeBuildOutputs.fromParsedBepOutput(
result,
provider.getBuildOutput(Optional.empty(), Interners.STRING),
provider.getBuildOutput(bepStream, Interners.STRING),
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ public BlazeBuildOutputs run(
Optional.ofNullable(context.getScope(SharedStringPoolScope.class))
.map(SharedStringPoolScope::getStringInterner)
.orElse(null);
ParsedBepOutput buildOutput = buildResultHelper.getBuildOutput(Optional.empty(), stringInterner);
ParsedBepOutput buildOutput;
try (final var bepStream = buildResultHelper.getBepStream(Optional.empty())) {
buildOutput = buildResultHelper.getBuildOutput(bepStream, stringInterner);
}
context.output(PrintOutput.log("BEP outputs retrieved (%s).", StringUtilRt.formatFileSize(buildOutput.getBepBytesConsumed())));
return BlazeBuildOutputs.fromParsedBepOutput(buildResult, buildOutput);
} catch (GetArtifactsException e) {
Expand Down Expand Up @@ -128,8 +131,8 @@ public BlazeTestResults runTest(
return BlazeTestResults.NO_RESULTS;
}
context.output(PrintOutput.log("Build command finished. Retrieving BEP outputs..."));
try {
return buildResultHelper.getTestResults(Optional.empty());
try(final var bepStream = buildResultHelper.getBepStream(Optional.empty())) {
return buildResultHelper.getTestResults(bepStream);
} catch (GetArtifactsException e) {
context.output(PrintOutput.log("Failed to get build outputs: " + e.getMessage()));
context.setHasError();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@
public final class BuildEventProtocolOutputReader {

private BuildEventProtocolOutputReader() {}
/** Returns all test results from a BEP-formatted {@link InputStream}. */
public static BlazeTestResults parseTestResults(InputStream inputStream)
throws BuildEventStreamException {
return parseTestResults(BuildEventStreamProvider.fromInputStream(inputStream));
}
/**
* Returns all test results from {@link BuildEventStreamProvider}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@
import com.google.idea.blaze.exception.BuildException;
import java.io.IOException;
import java.io.InputStream;
import java.util.Optional;
import javax.annotation.Nullable;

/** Provides {@link BuildEventStreamProtos.BuildEvent} */
public interface BuildEventStreamProvider {
public interface BuildEventStreamProvider extends AutoCloseable {

/** An exception parsing a stream of build events. */
class BuildEventStreamException extends BuildException {
Expand All @@ -47,9 +48,29 @@ static BuildEventStreamProtos.BuildEvent parseNextEventFromStream(InputStream st
}
}

/**
* Creates a {@link BuildEventStreamProvider} from the given {@code stream}.
*
* <p>Note: takes ownership of the {@code stream} and closes it when is being closed.
*/
static BuildEventStreamProvider fromInputStream(InputStream stream) {
CountingInputStream countingStream = new CountingInputStream(stream);
return new BuildEventStreamProvider() {
@Override
public void close() {
try {
stream.close();
}
catch (IOException e) {
throw new RuntimeException(e);
}
}

@Override
public Object getId() {
return Optional.empty();
}

@Nullable
@Override
public BuildEvent getNext() throws BuildEventStreamException {
Expand All @@ -63,9 +84,17 @@ public long getBytesConsumed() {
};
}

/**
* @return an object that represents the identity of the build to the user.
*/
Object getId();

/** Returns the next build event in the stream, or null if there are none remaining. */
@Nullable
BuildEventStreamProtos.BuildEvent getNext() throws BuildEventStreamException;

long getBytesConsumed();

@Override
void close();
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.idea.blaze.base.command.buildresult;

import com.google.common.collect.Interner;
import com.google.errorprone.annotations.MustBeClosed;
import com.google.idea.blaze.base.run.testlogs.BlazeTestResults;
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.exception.BuildException;
Expand All @@ -34,31 +35,38 @@ public interface BuildResultHelper extends AutoCloseable {
*/
List<String> getBuildFlags();


/**
* Gets the BEP stream for the build. May only be called once. May only be called after the build is complete.
*/
@MustBeClosed
BuildEventStreamProvider getBepStream(Optional<String> completionBuildId) throws GetArtifactsException;

/**
* Parses the BEP output data and returns the corresponding {@link ParsedBepOutput}. May only be
* called once, after the build is complete.
* Parses the BEP stream and returns the corresponding {@link ParsedBepOutput}. May only be
* called once on a given stream.
*
* <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> completionBuildId, Interner<String> stringInterner)
BuildEventStreamProvider bepStream, Interner<String> stringInterner)
throws GetArtifactsException;

/**
* Retrieves test results, parses them and returns the corresponding {@link BlazeTestResults}. May
* only be called once, after the build is complete.
* Parses BEP stream and returns the corresponding {@link BlazeTestResults}. May
* only be called once on a given stream.
*/
BlazeTestResults getTestResults(Optional<String> completedBuildId) throws GetArtifactsException;
BlazeTestResults getTestResults(BuildEventStreamProvider bepStream) throws GetArtifactsException;

/** Deletes the local BEP output file associated with the test results */
default void deleteTemporaryOutputFiles() {}

/**
* Parses the BEP output data to collect all build flags used. Return all flags that pass filters
* Parses the BEP stream and collects all build flags used. Return all flags that pass filters
*/
BuildFlags getBlazeFlags(Optional<String> completedBuildId) throws GetFlagsException;
BuildFlags getBlazeFlags(BuildEventStreamProvider bepStream) throws GetFlagsException;

/**
* Parses the BEP output data to collect message on stdout.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@

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;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.FileNotFoundException;
import java.util.List;
import java.util.Optional;
import org.jetbrains.annotations.VisibleForTesting;
Expand Down Expand Up @@ -56,22 +54,32 @@ public List<String> getBuildFlags() {
}

@Override
public ParsedBepOutput getBuildOutput(Optional<String> completedBuildId, Interner<String> stringInterner)
public BuildEventStreamProvider getBepStream(Optional<String> completionBuildId) throws GetArtifactsException {
try {
return BuildEventStreamProvider.fromInputStream(new BufferedInputStream(new FileInputStream(outputFile)));
}
catch (FileNotFoundException e) {
logger.error(e);
throw new GetArtifactsException(e.getMessage());
}
}

@Override
public ParsedBepOutput getBuildOutput(BuildEventStreamProvider bepStream, Interner<String> stringInterner)
throws GetArtifactsException {
try (InputStream inputStream = new BufferedInputStream(new FileInputStream(outputFile))) {
return ParsedBepOutput.parseBepArtifacts(inputStream);
} catch (IOException | BuildEventStreamException e) {
try {
return ParsedBepOutput.parseBepArtifacts(bepStream, stringInterner);
} catch (BuildEventStreamException e) {
logger.error(e);
throw new GetArtifactsException(e.getMessage());
}
}

@Override
public BlazeTestResults getTestResults(Optional<String> completedBuildId) {
try (InputStream inputStream =
new BufferedInputStream(InputStreamProvider.getInstance().forFile(outputFile))) {
return BuildEventProtocolOutputReader.parseTestResults(inputStream);
} catch (IOException | BuildEventStreamException e) {
public BlazeTestResults getTestResults(BuildEventStreamProvider bepStream) {
try {
return BuildEventProtocolOutputReader.parseTestResults(bepStream);
} catch (BuildEventStreamException e) {
logger.warn(e);
return BlazeTestResults.NO_RESULTS;
}
Expand All @@ -85,10 +93,10 @@ public void deleteTemporaryOutputFiles() {
}

@Override
public BuildFlags getBlazeFlags(Optional<String> completedBuildId) throws GetFlagsException {
try (InputStream inputStream = new BufferedInputStream(new FileInputStream(outputFile))) {
return BuildFlags.parseBep(inputStream);
} catch (IOException | BuildEventStreamException e) {
public BuildFlags getBlazeFlags(BuildEventStreamProvider bepStream) throws GetFlagsException {
try {
return BuildFlags.parseBep(bepStream);
} catch (BuildEventStreamException e) {
throw new GetFlagsException(e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,6 @@ public void end() {
0,
ImmutableSet.of());

/** Parses BEP events into {@link ParsedBepOutput} */
public static ParsedBepOutput parseBepArtifacts(InputStream bepStream)
throws BuildEventStreamException {
return parseBepArtifacts(BuildEventStreamProvider.fromInputStream(bepStream));
}

/** Parses BEP events into {@link ParsedBepOutput} */
public static ParsedBepOutput parseBepArtifacts(BuildEventStreamProvider stream)
throws BuildEventStreamException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ public LocalBuildEventProtocolTestFinderStrategy(BuildResultHelper buildResultHe

@Override
public BlazeTestResults findTestResults() throws GetArtifactsException {
return buildResultHelper.getTestResults(Optional.empty());
try (final var bepStream = buildResultHelper.getBepStream(Optional.empty())) {
return buildResultHelper.getTestResults(bepStream);
}
}

@Override
Expand Down
Loading

0 comments on commit d50505a

Please sign in to comment.