Merge handleOptions into beforeCommand

Now that we have the options before calling beforeCommand, there's no need for
a separate handleOptions method in the BlazeModule API. Remove it.

PiperOrigin-RevId: 162002300

9 files changed, 75 insertions, 177 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index 23eec44981..e8d744b91d 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -180,11 +180,12 @@ public class BazelRepositoryModule extends BlazeModule {
   }
 
   @Override
-  public void handleOptions(OptionsProvider optionsProvider) {
-    PackageCacheOptions pkgOptions = optionsProvider.getOptions(PackageCacheOptions.class);
+  public void beforeCommand(CommandEnvironment env) {
+    delegator.setClientEnvironment(env.getActionClientEnv());
+    PackageCacheOptions pkgOptions = env.getOptions().getOptions(PackageCacheOptions.class);
     isFetch.set(pkgOptions != null && pkgOptions.fetch);
 
-    RepositoryOptions repoOptions = optionsProvider.getOptions(RepositoryOptions.class);
+    RepositoryOptions repoOptions = env.getOptions().getOptions(RepositoryOptions.class);
     if (repoOptions != null) {
       if (repoOptions.experimentalRepositoryCache != null) {
         Path repositoryCachePath = filesystem.getPath(repoOptions.experimentalRepositoryCache);
@@ -216,11 +217,6 @@ public class BazelRepositoryModule extends BlazeModule {
   }
 
   @Override
-  public void beforeCommand(CommandEnvironment env) {
-    delegator.setClientEnvironment(env.getActionClientEnv());
-  }
-
-  @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
     return ImmutableSet.of("fetch", "build", "query").contains(command.name())
         ? ImmutableList.<Class<? extends OptionsBase>>of(RepositoryOptions.class)
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
index 60df1913a4..ead187d005 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.buildeventservice;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static com.google.devtools.build.lib.buildeventservice.BuildEventServiceTransport.UPLOAD_FAILED_MESSAGE;
 import static java.lang.String.format;
@@ -23,10 +22,8 @@ import static java.lang.String.format;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.eventbus.Subscribe;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
 import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
-import com.google.devtools.build.lib.buildeventstream.BuildEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.build.lib.buildeventstream.transports.BuildEventStreamOptions;
@@ -45,8 +42,6 @@ import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsProvider;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Set;
 import java.util.UUID;
 import java.util.logging.Logger;
@@ -64,25 +59,7 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
 
   private static final Logger logger = Logger.getLogger(BuildEventServiceModule.class.getName());
 
-  private CommandEnvironment commandEnvironment;
-  private SynchronizedOutputStream out;
-  private SynchronizedOutputStream err;
-  private boolean disabled;
-
-  private static class BuildEventRecorder {
-    private final List<BuildEvent> events = new ArrayList<>();
-
-    @Subscribe
-    public void buildEvent(BuildEvent event) {
-      events.add(event);
-    }
-
-    List<BuildEvent> getEvents() {
-      return events;
-    }
-  }
-
-  private BuildEventRecorder buildEventRecorder;
+  private OutErr outErr;
 
   @Override
   public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
@@ -92,25 +69,15 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
   @Override
   public void beforeCommand(CommandEnvironment commandEnvironment)
       throws AbruptExitException {
-    disabled = false;
+    // Reset to null in case afterCommand was not called.
+    this.outErr = null;
     if (!whitelistedCommands().contains(commandEnvironment.getCommandName())) {
-      disabled = true;
       return;
     }
-    this.commandEnvironment = commandEnvironment;
-    this.buildEventRecorder = new BuildEventRecorder();
-    commandEnvironment.getEventBus().register(buildEventRecorder);
-  }
 
-  @Override
-  public void handleOptions(OptionsProvider optionsProvider) {
-    if (disabled) {
-      return;
-    }
-    checkState(commandEnvironment != null, "Methods called out of order");
     BuildEventStreamer streamer =
         tryCreateStreamer(
-            optionsProvider,
+            commandEnvironment.getOptions(),
             commandEnvironment.getReporter(),
             commandEnvironment.getBlazeModuleEnvironment(),
             commandEnvironment.getRuntime().getClock(),
@@ -122,58 +89,35 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
       commandEnvironment.getReporter().addHandler(streamer);
       commandEnvironment.getEventBus().register(streamer);
 
-      final SynchronizedOutputStream theOut = this.out;
-      final SynchronizedOutputStream theErr = this.err;
-      // out and err should be non-null at this point, as getOutputListener is supposed to
-      // be always called before handleOptions. But let's still prefer a stream with no
-      // stdout/stderr over an aborted build.
+      final SynchronizedOutputStream out = new SynchronizedOutputStream();
+      final SynchronizedOutputStream err = new SynchronizedOutputStream();
+      this.outErr = OutErr.create(out, err);
       streamer.registerOutErrProvider(
           new BuildEventStreamer.OutErrProvider() {
             @Override
             public String getOut() {
-              if (theOut == null) {
-                return null;
-              }
-              return theOut.readAndReset();
+              return out.readAndReset();
             }
 
             @Override
             public String getErr() {
-              if (theErr == null) {
-                return null;
-              }
-              return theErr.readAndReset();
+              return err.readAndReset();
             }
           });
-      if (theErr != null) {
-        theErr.registerStreamer(streamer);
-      }
-      if (theOut != null) {
-        theOut.registerStreamer(streamer);
-      }
-      for (BuildEvent event : buildEventRecorder.getEvents()) {
-        streamer.buildEvent(event);
-      }
+      err.registerStreamer(streamer);
+      out.registerStreamer(streamer);
       logger.fine("BuildEventStreamer created and registered successfully.");
-    } else {
-      // If there is no streamer to consume the output, we should not try to accumulate it.
-      this.out.setDiscardAll();
-      this.err.setDiscardAll();
     }
-    commandEnvironment.getEventBus().unregister(buildEventRecorder);
-    this.buildEventRecorder = null;
-    this.out = null;
-    this.err = null;
   }
 
   @Override
   public OutErr getOutputListener() {
-    if (disabled) {
-      return null;
-    }
-    this.out = new SynchronizedOutputStream();
-    this.err = new SynchronizedOutputStream();
-    return OutErr.create(this.out, this.err);
+    return outErr;
+  }
+
+  @Override
+  public void afterCommand() {
+    this.outErr = null;
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index d5c0575115..1a4b874db9 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -14,8 +14,6 @@
 
 package com.google.devtools.build.lib.remote;
 
-import static com.google.common.base.Preconditions.checkState;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
@@ -77,7 +75,6 @@ public final class RemoteModule extends BlazeModule {
 
   private final CasPathConverter converter = new CasPathConverter();
 
-  private CommandEnvironment env;
   private RemoteActionContextProvider actionContextProvider;
 
   @Override
@@ -89,15 +86,9 @@ public final class RemoteModule extends BlazeModule {
   @Override
   public void beforeCommand(CommandEnvironment env) {
     env.getEventBus().register(this);
-    this.env = env;
-  }
-
-  @Override
-  public void handleOptions(OptionsProvider optionsProvider) {
-    checkState(env != null);
 
-    RemoteOptions remoteOptions = optionsProvider.getOptions(RemoteOptions.class);
-    AuthAndTLSOptions authAndTlsOptions = optionsProvider.getOptions(AuthAndTLSOptions.class);
+    RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
+    AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
     converter.options = remoteOptions;
 
     // Quit if no remote options specified.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index cbd8e564ec..453cd7c24b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -367,10 +367,13 @@ public class BlazeCommandDispatcher {
     OptionsProvider options = optionsResult.get();
 
     // The initCommand call also records the start time for the timestamp granularity monitor.
-    CommandEnvironment env = workspace.initCommand(commandAnnotation);
+    CommandEnvironment env = workspace.initCommand(commandAnnotation, options);
     // Record the command's starting time for use by the commands themselves.
     env.recordCommandStartTime(firstContactTime);
 
+    // Temporary: there is one module that outputs events during beforeCommand, but the reporter
+    // isn't setup yet. Add the stored event handler to catch those events.
+    env.getReporter().addHandler(eventHandler);
     for (BlazeModule module : runtime.getBlazeModules()) {
       try {
         module.beforeCommand(env);
@@ -385,6 +388,7 @@ public class BlazeCommandDispatcher {
         earlyExitCode = e.getExitCode();
       }
     }
+    env.getReporter().removeHandler(eventHandler);
 
     // We may only start writing to outErr once we've given the modules the chance to hook into it.
     for (BlazeModule module : runtime.getBlazeModules()) {
@@ -502,9 +506,6 @@ public class BlazeCommandDispatcher {
         reporter.handle(Event.error(e.getMessage()));
         return e.getExitCode().getNumericExitCode();
       }
-      for (BlazeModule module : runtime.getBlazeModules()) {
-        module.handleOptions(options);
-      }
 
       env.getEventBus().post(originalCommandLine);
 
@@ -573,6 +574,16 @@ public class BlazeCommandDispatcher {
       // applied the invocation policy.
       AtomicReference<OptionsProvider> parsedOptions,
       List<String> rcfileNotes) {
+    OptionsParser optionsParser;
+    try {
+      optionsParser = createOptionsParser(command);
+      // We need to set this early so it's not null when we return.
+      parsedOptions.set(optionsParser);
+    } catch (OptionsParser.ConstructionException e) {
+      // This should never happen.
+      throw new IllegalStateException(e);
+    }
+
     // The initialization code here was carefully written to parse the options early before we call
     // into the BlazeModule APIs, which means we must not generate any output to outErr, return, or
     // throw an exception. All the events happening here are instead stored in a temporary event
@@ -584,8 +595,6 @@ public class BlazeCommandDispatcher {
     }
 
     try {
-      OptionsParser optionsParser = createOptionsParser(command);
-
       // TODO(ulfjack): The second parameter is supposed to be the working directory, except that
       // the client passes that as part of CommonCommandOptions, and we can't know those until
       // after we've parsed them.
@@ -624,10 +633,6 @@ public class BlazeCommandDispatcher {
       for (String warning : optionsParser.getWarnings()) {
         eventHandler.handle(Event.warn(warning));
       }
-      parsedOptions.set(optionsParser);
-    } catch (OptionsParser.ConstructionException e) {
-      // This should never happen.
-      throw new IllegalStateException(e);
     } catch (OptionsParsingException e) {
       eventHandler.handle(Event.error(e.getMessage()));
       return ExitCode.COMMAND_LINE_ERROR;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 40b420e9ac..49265e0e91 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -173,14 +173,6 @@ public abstract class BlazeModule {
   }
 
   /**
-   * Does any handling of options needed by the command.
-   *
-   * <p>This method will be called at the beginning of each command (after #beforeCommand).
-   */
-  @SuppressWarnings("unused")
-  public void handleOptions(OptionsProvider optionsProvider) {}
-
-  /**
    * Returns extra options this module contributes to a specific command. Note that option
    * inheritance applies: if this method returns a non-empty list, then the returned options are
    * added to every command that depends on this command.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
index 47b0cddc9e..5877ec2331 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
@@ -185,22 +185,10 @@ public final class BlazeWorkspace {
    * <p>This method should be called from the "main" thread on which the command will execute;
    * that thread will receive interruptions if a module requests an early exit.
    */
-  public CommandEnvironment initCommand(Command command) {
+  public CommandEnvironment initCommand(Command command, OptionsProvider options) {
     CommandEnvironment env = new CommandEnvironment(
-        runtime, this, new EventBus(eventBusExceptionHandler), Thread.currentThread(), command);
-    skyframeExecutor.setClientEnv(env.getClientEnv());
-    return env;
-  }
-
-  /**
-   * Same as {@code #initCommand()} but setting the command name and the options manually since
-   * those values are set by {@code CommandEnvironment#beforeCommand()} which is not called for
-   * testing. Use ONLY for testing purposes.
-   */
-  public CommandEnvironment initCommandForTesting(Command command, OptionsProvider options) {
-    CommandEnvironment env = new CommandEnvironment(
-        runtime, this, new EventBus(eventBusExceptionHandler), Thread.currentThread(),
-        command, options);
+        runtime, this, new EventBus(eventBusExceptionHandler), Thread.currentThread(), command,
+        options);
     skyframeExecutor.setClientEnv(env.getClientEnv());
     return env;
   }
@@ -214,7 +202,7 @@ public final class BlazeWorkspace {
   /**
    * Reinitializes the Skyframe evaluator.
    */
-  public void resetEvaluator() throws IOException {
+  public void resetEvaluator() {
     skyframeExecutor.resetEvaluator();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index 43c5faa4fd..df19077a78 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.runtime;
 
 import static com.google.devtools.build.lib.profiler.AutoProfiler.profiled;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.PackageRootResolver;
 import com.google.devtools.build.lib.actions.cache.ActionCache;
@@ -80,6 +79,7 @@ public final class CommandEnvironment {
   private final TimestampGranularityMonitor timestampGranularityMonitor;
   private final Thread commandThread;
   private final Command command;
+  private final OptionsProvider options;
 
   private String[] crashData;
 
@@ -89,8 +89,6 @@ public final class CommandEnvironment {
   private Path workingDirectory;
   private String workspaceName;
 
-  private OptionsProvider options;
-
   private AtomicReference<AbruptExitException> pendingException = new AtomicReference<>();
 
   private class BlazeModuleEnvironment implements BlazeModule.ModuleEnvironment {
@@ -121,7 +119,7 @@ public final class CommandEnvironment {
    */
   CommandEnvironment(
       BlazeRuntime runtime, BlazeWorkspace workspace, EventBus eventBus, Thread commandThread,
-      Command command) {
+      Command command, OptionsProvider options) {
     this.runtime = runtime;
     this.workspace = workspace;
     this.directories = workspace.getDirectories();
@@ -130,6 +128,7 @@ public final class CommandEnvironment {
     this.eventBus = eventBus;
     this.commandThread = commandThread;
     this.command = command;
+    this.options = options;
     this.blazeModuleEnvironment = new BlazeModuleEnvironment();
     this.timestampGranularityMonitor = new TimestampGranularityMonitor(runtime.getClock());
     // Record the command's starting time again, for use by
@@ -144,22 +143,31 @@ public final class CommandEnvironment {
     this.workspaceName = null;
 
     workspace.getSkyframeExecutor().setEventBus(eventBus);
-  }
 
-  /**
-   * Same as CommandEnvironment(BlazeRuntime, BlazeWorkspace, EventBus, Thread) but with an
-   * explicit commandName and options.
-   *
-   * ONLY for testing.
-   */
-  @VisibleForTesting
-  CommandEnvironment(
-      BlazeRuntime runtime, BlazeWorkspace workspace, EventBus eventBus, Thread commandThread,
-      Command command, OptionsProvider optionsForTesting) {
-    this(runtime, workspace, eventBus, commandThread, command);
-    // Options are normally set by beforeCommand(); however this method is not called in tests (i.e.
-    // tests use BlazeRuntimeWrapper). These fields should only be set for testing.
-    this.options = optionsForTesting;
+    updateClientEnv(options.getOptions(CommonCommandOptions.class).clientEnv);
+
+    // actionClientEnv contains the environment where values from actionEnvironment are overridden.
+    actionClientEnv.putAll(clientEnv);
+
+    if (command.builds()) {
+      // Compute the set of environment variables that are whitelisted on the commandline
+      // for inheritance.
+      for (Map.Entry<String, String> entry :
+          options.getOptions(BuildConfiguration.Options.class).actionEnvironment) {
+        if (entry.getValue() == null) {
+          visibleActionEnv.add(entry.getKey());
+        } else {
+          visibleActionEnv.remove(entry.getKey());
+          actionClientEnv.put(entry.getKey(), entry.getValue());
+        }
+      }
+      for (Map.Entry<String, String> entry :
+          options.getOptions(BuildConfiguration.Options.class).testEnvironment) {
+        if (entry.getValue() == null) {
+          visibleTestEnv.add(entry.getKey());
+        }
+      }
+    }
   }
 
   public BlazeRuntime getRuntime() {
@@ -236,8 +244,7 @@ public final class CommandEnvironment {
     return Collections.unmodifiableMap(result);
   }
 
-  @VisibleForTesting
-  void updateClientEnv(List<Map.Entry<String, String>> clientEnvList) {
+  private void updateClientEnv(List<Map.Entry<String, String>> clientEnvList) {
     Preconditions.checkState(clientEnv.isEmpty());
 
     Collection<Map.Entry<String, String>> env = clientEnvList;
@@ -545,7 +552,6 @@ public final class CommandEnvironment {
       InvocationPolicy invocationPolicy)
       throws AbruptExitException {
     commandStartTime -= commonOptions.startupTime;
-    this.options = options;
 
     eventBus.post(
         new GotOptionsEvent(runtime.getStartupOptionsProvider(), options, invocationPolicy));
@@ -584,8 +590,6 @@ public final class CommandEnvironment {
     this.relativeWorkingDirectory = workingDirectory.relativeTo(workspace);
     this.workingDirectory = workingDirectory;
 
-    updateClientEnv(commonOptions.clientEnv);
-
     // Fail fast in the case where a Blaze command forgets to install the package path correctly.
     skyframeExecutor.setActive(false);
     // Let skyframe figure out if it needs to store graph edges for this build.
@@ -596,29 +600,6 @@ public final class CommandEnvironment {
     // Start the performance and memory profilers.
     runtime.beforeCommand(this, commonOptions, execStartTimeNanos);
 
-    // actionClientEnv contains the environment where values from actionEnvironment are overridden.
-    actionClientEnv.putAll(clientEnv);
-
-    if (command.builds()) {
-      // Compute the set of environment variables that are whitelisted on the commandline
-      // for inheritance.
-      for (Map.Entry<String, String> entry :
-          options.getOptions(BuildConfiguration.Options.class).actionEnvironment) {
-        if (entry.getValue() == null) {
-          visibleActionEnv.add(entry.getKey());
-        } else {
-          visibleActionEnv.remove(entry.getKey());
-          actionClientEnv.put(entry.getKey(), entry.getValue());
-        }
-      }
-      for (Map.Entry<String, String> entry :
-          options.getOptions(BuildConfiguration.Options.class).testEnvironment) {
-        if (entry.getValue() == null) {
-          visibleTestEnv.add(entry.getKey());
-        }
-      }
-    }
-
     eventBus.post(new CommandStartEvent(
         command.name(), getCommandId(), getClientEnv(), workingDirectory, getDirectories(),
         waitTimeInMs + commonOptions.waitTime));
diff --git a/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java b/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java
index 00b5ad370c..2e73896516 100644
--- a/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java
+++ b/src/main/java/com/google/devtools/build/lib/ssd/SsdModule.java
@@ -16,8 +16,8 @@ package com.google.devtools.build.lib.ssd;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.cache.DigestUtils;
 import com.google.devtools.build.lib.runtime.BlazeModule;
+import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.common.options.OptionsBase;
-import com.google.devtools.common.options.OptionsProvider;
 
 /**
  * BlazeModule that applies optimizations to Bazel's internals in order to improve performance when
@@ -30,8 +30,8 @@ public final class SsdModule extends BlazeModule {
   }
 
   @Override
-  public void handleOptions(OptionsProvider optionsProvider) {
-    SsdOptions options = optionsProvider.getOptions(SsdOptions.class);
+  public void beforeCommand(CommandEnvironment env) {
+    SsdOptions options = env.getOptions().getOptions(SsdOptions.class);
     if (options.experimentalMultiThreadedDigest) {
       DigestUtils.setMultiThreadedDigest(options.experimentalMultiThreadedDigest);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java b/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java
index d5cafab2d4..233e613ef6 100644
--- a/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java
+++ b/src/main/java/com/google/devtools/build/lib/util/io/DelegatingOutErr.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.util.io;
 
+import com.google.common.base.Preconditions;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.util.ArrayList;
@@ -68,7 +69,7 @@ public final class DelegatingOutErr extends OutErr {
     private final List<OutputStream> sinks = new ArrayList<>();
 
     public void addSink(OutputStream sink) {
-      sinks.add(sink);
+      sinks.add(Preconditions.checkNotNull(sink));
     }
 
     public void removeSink(OutputStream sink) {