Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid writing D8 info and error messages directly to the Bazel console #17648

Closed
wants to merge 3 commits into from
Closed

Avoid writing D8 info and error messages directly to the Bazel console #17648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8e505b
Avoid writing dexing info and error messages directly to the Bazel co…
Bencodes Mar 2, 2023
ddbbf13
And apply the same to Desugar.java
Bencodes Mar 2, 2023
38616d9
Use the correct System.err stream
Bencodes Mar 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 58 additions & 28 deletions src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
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.errors.DexFileOverflowDiagnostic;
import com.android.tools.r8.SyntheticInfoConsumer;
import com.android.tools.r8.SyntheticInfoConsumerData;
import com.android.tools.r8.origin.ArchiveEntryOrigin;
import com.android.tools.r8.origin.PathOrigin;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.references.ClassReference;
import com.google.auto.value.AutoValue;
import com.google.common.cache.Cache;
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,47 @@ 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 +300,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 +333,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 +391,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);
}
}
}

78 changes: 42 additions & 36 deletions src/tools/android/java/com/google/devtools/build/android/r8/Desugar.java
View file
Open in desktop
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,42 @@ 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 +712,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));
}
}
}
Loading