Skip to content

Commit

Permalink
Avoid writing D8 info and error messages directly to the Bazel console
Browse files Browse the repository at this point in the history 
This pull request provides a custom `DiagnosticsHandler` implementation that redirects info and warning messages to the original `System#err` stream so that the noisy D8 messages are written to the worker log file instead of directly to the console.

In addition this PR also removes the custom system stream redirection, and instead uses the baked in `WorkerIO` implementation that automatically wraps the system stream for us that was added here #14201.

#17647

Closes #17648.

PiperOrigin-RevId: 560600558
Change-Id: I9d2d162b62fe5dfd00e8614ba73caf1cac5f02d7
  • Loading branch information
@Bencodes @copybara-github
Bencodes authored and copybara-github committed Aug 28, 2023
1 parent 81d12db commit 788d00e
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 64 deletions.
90 changes: 62 additions & 28 deletions src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@
import com.android.tools.r8.D8;
import com.android.tools.r8.D8Command;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.DiagnosticsHandler;
import com.android.tools.r8.SyntheticInfoConsumer;
import com.android.tools.r8.SyntheticInfoConsumerData;
import com.android.tools.r8.errors.DexFileOverflowDiagnostic;
import com.android.tools.r8.origin.ArchiveEntryOrigin;
import com.android.tools.r8.origin.PathOrigin;
import com.android.tools.r8.references.ClassReference;
import com.android.tools.r8.utils.StringDiagnostic;
import com.google.auto.value.AutoValue;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
Expand All @@ -39,7 +42,6 @@
import com.google.devtools.build.lib.worker.WorkRequestHandler;
import com.google.devtools.common.options.OptionsParsingException;
import java.io.BufferedOutputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintStream;
Expand Down Expand Up @@ -68,6 +70,7 @@
* <p>D8 version of DexBuilder.
*/
public class CompatDexBuilder {

private static final long ONE_MEG = 1024 * 1024;

private static class ContextConsumer implements SyntheticInfoConsumer {
Expand Down Expand Up @@ -142,15 +145,8 @@ public static void main(String[] args)
throws IOException, InterruptedException, ExecutionException, OptionsParsingException {
CompatDexBuilder compatDexBuilder = new CompatDexBuilder();
if (ImmutableSet.copyOf(args).contains("--persistent_worker")) {
ByteArrayOutputStream buf = new ByteArrayOutputStream();
PrintStream ps = new PrintStream(buf, true);
PrintStream realStdOut = System.out;
PrintStream realStdErr = System.err;

// Redirect all stdout and stderr output for logging.
System.setOut(ps);
System.setErr(ps);

// Set up dexer cache
Cache<DexingKeyR8, byte[]> dexCache =
CacheBuilder.newBuilder()
Expand All @@ -167,56 +163,51 @@ public int weigh(DexingKeyR8 key, byte[] value) {
})
.build();
try {
DexDiagnosticsHandler diagnosticsHandler = new DexDiagnosticsHandler(System.err);
WorkRequestHandler workerHandler =
new WorkRequestHandler.WorkRequestHandlerBuilder(
new WorkRequestHandler.WorkRequestCallback(
(request, pw) ->
compatDexBuilder.processRequest(
dexCache, request.getArgumentsList(), pw, buf)),
dexCache, diagnosticsHandler, request.getArgumentsList(), pw)),
realStdErr,
new ProtoWorkerMessageProcessor(System.in, realStdOut))
new ProtoWorkerMessageProcessor(System.in, System.out))
.setCpuUsageBeforeGc(Duration.ofSeconds(10))
.build();
workerHandler.processRequests();
} catch (IOException e) {
realStdErr.println(e.getMessage());
System.exit(1);
} finally {
System.setOut(realStdOut);
System.setErr(realStdErr);
}
} else {
// Dex cache has no value in non-persistent mode, so pass it as null.
compatDexBuilder.dexEntries(/* dexCache= */ null, Arrays.asList(args));
compatDexBuilder.dexEntries(
/* dexCache= */ null, Arrays.asList(args), new DexDiagnosticsHandler(System.err));
}
}

private int processRequest(
@Nullable Cache<DexingKeyR8, byte[]> dexCache,
DiagnosticsHandler diagnosticsHandler,
List<String> args,
PrintWriter pw,
ByteArrayOutputStream buf) {
PrintWriter pw) {
try {
dexEntries(dexCache, args);
dexEntries(dexCache, args, diagnosticsHandler);
return 0;
} catch (OptionsParsingException e) {
pw.println("CompatDexBuilder raised OptionsParsingException: " + e.getMessage());
return 1;
} catch (IOException | InterruptedException | ExecutionException e) {
e.printStackTrace();
return 1;
} finally {
// Write the captured buffer to the work response
synchronized (buf) {
String captured = buf.toString(UTF_8).trim();
buf.reset();
pw.print(captured);
}
}
}

@SuppressWarnings("JdkObsolete")
private void dexEntries(@Nullable Cache<DexingKeyR8, byte[]> dexCache, List<String> args)
private void dexEntries(
@Nullable Cache<DexingKeyR8, byte[]> dexCache,
List<String> args,
DiagnosticsHandler dexDiagnosticsHandler)
throws IOException, InterruptedException, ExecutionException, OptionsParsingException {
List<String> flags = new ArrayList<>();
String input = null;
Expand Down Expand Up @@ -313,7 +304,8 @@ private void dexEntries(@Nullable Cache<DexingKeyR8, byte[]> dexCache, List<Stri
classEntry,
compilationMode,
minSdkVersion,
executor)));
executor,
dexDiagnosticsHandler)));
}
StringBuilder contextMappingBuilder = new StringBuilder();
for (int i = 0; i < futures.size(); i++) {
Expand Down Expand Up @@ -345,10 +337,11 @@ private DexConsumer dexEntry(
ZipEntry classEntry,
CompilationMode mode,
int minSdkVersion,
ExecutorService executor)
ExecutorService executor,
DiagnosticsHandler dexDiagnosticsHandler)
throws IOException, CompilationFailedException {
DexConsumer consumer = new DexConsumer();
D8Command.Builder builder = D8Command.builder();
D8Command.Builder builder = D8Command.builder(dexDiagnosticsHandler);
builder
.setProgramConsumer(consumer)
.setSyntheticInfoConsumer(consumer.getContextConsumer())
Expand Down Expand Up @@ -402,5 +395,46 @@ public static DexingKeyR8 create(
@SuppressWarnings("mutable")
public abstract byte[] classfileContent();
}

/**
* Custom implementation of DiagnosticsHandler that writes the info/warning diagnostics messages
* to original System#err stream instead of the WorkerResponse output. This keeps the Bazel
* console clean and concise.
*
* <p>Errors will continue to be written to the work response and can be seen directly in the
* console output.
*/
private static class DexDiagnosticsHandler implements DiagnosticsHandler {

private final PrintStream stream;

public DexDiagnosticsHandler(PrintStream stream) {
this.stream = stream;
}

@Override
public void warning(Diagnostic warning) {
DiagnosticsHandler.printDiagnosticToStream(warning, "Warning", stream);
}

@Override
public void info(Diagnostic info) {
DiagnosticsHandler.printDiagnosticToStream(info, "Info", stream);
}

@Override
public void error(Diagnostic error) {
if (error instanceof DexFileOverflowDiagnostic) {
DexFileOverflowDiagnostic overflowDiagnostic = (DexFileOverflowDiagnostic) error;
if (!overflowDiagnostic.hasMainDexSpecification()) {
DiagnosticsHandler.super.error(
new StringDiagnostic(
overflowDiagnostic.getDiagnosticMessage() + ". Try supplying a main-dex list"));
return;
}
}
DiagnosticsHandler.super.error(error);
}
}
}

80 changes: 44 additions & 36 deletions src/tools/android/java/com/google/devtools/build/android/r8/Desugar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import static com.google.common.base.Preconditions.checkArgument;
import static java.lang.Math.max;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;

import com.android.tools.r8.ArchiveClassFileProvider;
Expand All @@ -26,6 +25,7 @@
import com.android.tools.r8.D8Command;
import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.DiagnosticsHandler;
import com.android.tools.r8.errors.DexFileOverflowDiagnostic;
import com.android.tools.r8.errors.InterfaceDesugarMissingTypeDiagnostic;
import com.android.tools.r8.utils.StringDiagnostic;
import com.google.common.collect.ImmutableList;
Expand All @@ -42,7 +42,6 @@
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.ShellQuotedParamsFilePreProcessor;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.PrintStream;
import java.io.PrintWriter;
Expand Down Expand Up @@ -369,9 +368,11 @@ public static class DesugarOptions extends OptionsBase {
}

private final DesugarOptions options;
private final PrintStream diagnosticsHandlerPrintStream;

private Desugar(DesugarOptions options) {
private Desugar(DesugarOptions options, PrintStream diagnosticsHandlerPrintStream) {
this.options = options;
this.diagnosticsHandlerPrintStream = diagnosticsHandlerPrintStream;
}

private static DesugarOptions parseCommandLineOptions(String[] args) {
Expand Down Expand Up @@ -412,12 +413,14 @@ private DependencyCollector createDependencyCollector() {
}
}

private class DesugarDiagnosticsHandler implements DiagnosticsHandler {
private static class DesugarDiagnosticsHandler implements DiagnosticsHandler {

OutputConsumer outputConsumer;
private final OutputConsumer outputConsumer;
private final PrintStream stream;

private DesugarDiagnosticsHandler(OutputConsumer outputConsumer) {
private DesugarDiagnosticsHandler(OutputConsumer outputConsumer, PrintStream stream) {
this.outputConsumer = outputConsumer;
this.stream = stream;
}

@Override
Expand All @@ -440,7 +443,26 @@ public void warning(Diagnostic warning) {
// Ignore.
return;
}
DiagnosticsHandler.super.warning(warning);
DiagnosticsHandler.printDiagnosticToStream(warning, "Warning", stream);
}

@Override
public void info(Diagnostic info) {
DiagnosticsHandler.printDiagnosticToStream(info, "Info", stream);
}

@Override
public void error(Diagnostic error) {
if (error instanceof DexFileOverflowDiagnostic) {
DexFileOverflowDiagnostic overflowDiagnostic = (DexFileOverflowDiagnostic) error;
if (!overflowDiagnostic.hasMainDexSpecification()) {
DiagnosticsHandler.super.error(
new StringDiagnostic(
overflowDiagnostic.getDiagnosticMessage() + ". Try supplying a main-dex list"));
return;
}
}
DiagnosticsHandler.super.error(error);
}
}

Expand All @@ -449,7 +471,8 @@ private void desugar(
ClassFileResourceProvider classpath,
Path input,
Path output,
Path desugaredLibConfig)
Path desugaredLibConfig,
PrintStream diagnosticsHandlerPrintStream)
throws CompilationFailedException, IOException {
checkArgument(!Files.isDirectory(input), "Input must be a jar (%s is a directory)", input);
DependencyCollector dependencyCollector = createDependencyCollector();
Expand Down Expand Up @@ -483,7 +506,7 @@ private void desugar(
ArchiveProgramResourceProvider.includeClassFileOrDexEntries(p)
&& isProgramClassForShard(numberOfShards, currentShard, p));
D8Command.Builder builder =
D8Command.builder(new DesugarDiagnosticsHandler(consumer))
D8Command.builder(new DesugarDiagnosticsHandler(consumer, diagnosticsHandlerPrintStream))
.addClasspathResourceProvider(orderedClassFileResourceProvider)
.addProgramResourceProvider(programProvider)
.setIntermediate(true)
Expand All @@ -497,7 +520,7 @@ private void desugar(
}
}

private void desugar() throws CompilationFailedException, IOException {
public void desugar() throws CompilationFailedException, IOException {
// Prepare bootclasspath and classpath. Some jars on the classpath are considered to be
// bootclasspath, and are moved there.
ImmutableList.Builder<ClassFileResourceProvider> bootclasspathProvidersBuilder =
Expand Down Expand Up @@ -529,7 +552,8 @@ private void desugar() throws CompilationFailedException, IOException {
classpathProvider,
options.inputJars.get(i),
options.outputJars.get(i),
options.desugarCoreLibs ? options.desugaredLibConfig.get(0) : null);
options.desugarCoreLibs ? options.desugaredLibConfig.get(0) : null,
diagnosticsHandlerPrintStream);
}
}

Expand Down Expand Up @@ -644,60 +668,44 @@ private static void validateOptions(DesugarOptions options) {
options.outputJars.size());
}

private static int processRequest(List<String> args) throws Exception {
private static int processRequest(List<String> args, PrintStream diagnosticsHandlerPrintStream)
throws Exception {
DesugarOptions options = parseCommandLineOptions(args.toArray(new String[0]));
validateOptions(options);
new Desugar(options).desugar();
new Desugar(options, diagnosticsHandlerPrintStream).desugar();
return 0;
}

private static int processRequest(List<String> args, PrintWriter pw, ByteArrayOutputStream buf) {
private static int processRequest(
List<String> args, PrintWriter pw, PrintStream diagnosticsHandlerPrintStream) {
int exitCode;
try {
// Process the actual request and grab the exit code
exitCode = processRequest(args);
exitCode = processRequest(args, diagnosticsHandlerPrintStream);
} catch (Exception e) {
e.printStackTrace(pw);
exitCode = 1;
} finally {
// Write the captured buffer to the work response. We synchronize to avoid race conditions
// while reading from and calling reset on the shared ByteArrayOutputStream.
String captured;
synchronized (buf) {
captured = buf.toString(UTF_8).trim();
buf.reset();
}
pw.print(captured);
}
return exitCode;
}

private static int runPersistentWorker() {
ByteArrayOutputStream buf = new ByteArrayOutputStream();
PrintStream ps = new PrintStream(buf, true);
PrintStream realStdOut = System.out;
PrintStream realStdErr = System.err;

// Redirect all stdout and stderr output for logging.
System.setOut(ps);
System.setErr(ps);
try {
WorkRequestHandler workerHandler =
new WorkRequestHandler.WorkRequestHandlerBuilder(
new WorkRequestHandler.WorkRequestCallback(
(request, pw) -> processRequest(request.getArgumentsList(), pw, buf)),
(request, pw) -> processRequest(request.getArgumentsList(), pw, realStdErr)),
realStdErr,
new ProtoWorkerMessageProcessor(System.in, realStdOut))
new ProtoWorkerMessageProcessor(System.in, System.out))
.setCpuUsageBeforeGc(Duration.ofSeconds(10))
.build();
workerHandler.processRequests();
} catch (IOException e) {
logger.severe(e.getMessage());
e.printStackTrace(realStdErr);
return 1;
} finally {
System.setOut(realStdOut);
System.setErr(realStdErr);
}
return 0;
}
Expand All @@ -706,7 +714,7 @@ public static void main(String[] args) throws Exception {
if (args.length > 0 && args[0].equals("--persistent_worker")) {
System.exit(runPersistentWorker());
} else {
System.exit(processRequest(Arrays.asList(args)));
System.exit(processRequest(Arrays.asList(args), System.err));
}
}
}

0 comments on commit 788d00e

Please sign in to comment.