remote: quit early if no remote options specified

If neither remote caching nor remote execution is enabled then quit the remote initialization logic early. The current null check on the RemoteOptions never worked. Also, don't create a remote logs directory and don't print an invocation id if remoting is disabled.
bazelbuild · Jan 21, 2019 · 0690ef1 · 0690ef1
1 parent f4c4e07
commit 0690ef1
Showing 1 changed file with 36 additions and 28 deletions.
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
@@ -112,35 +112,13 @@ public void serverInit(OptionsParsingResult startupOptions, ServerBuilder builde
 
   @Override
   public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
-    env.getEventBus().register(this);
-
-    String invocationId = env.getCommandId().toString();
-    String buildRequestId = env.getBuildRequestId();
-    env.getReporter().handle(Event.info(String.format("Invocation ID: %s", invocationId)));
-
-    Path logDir =
-        env.getOutputBase().getRelative(env.getRuntime().getProductName() + "-remote-logs");
-    try {
-      // Clean out old logs files.
-      if (logDir.exists()) {
-        FileSystemUtils.deleteTree(logDir);
-      }
-      logDir.createDirectory();
-    } catch (IOException e) {
-      env.getReporter()
-          .handle(Event.error("Could not create base directory for remote logs: " + logDir));
-      throw new AbruptExitException(ExitCode.LOCAL_ENVIRONMENTAL_ERROR, e);
-    }
-    RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
-    AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
+    RemoteOptions remoteOptions = Preconditions.checkNotNull(
+        env.getOptions().getOptions(RemoteOptions.class), "Could not get RemoteOptions");
+    AuthAndTLSOptions authAndTlsOptions = Preconditions.checkNotNull(
+        env.getOptions().getOptions(AuthAndTLSOptions.class), "Could not get AuthAndTLSOptions");
     DigestHashFunction hashFn = env.getRuntime().getFileSystem().getDigestFunction();
     DigestUtil digestUtil = new DigestUtil(hashFn);
 
-    // Quit if no remote options specified.
-    if (remoteOptions == null) {
-      return;
-    }
-
     boolean enableRestCache = SimpleBlobStoreFactory.isRestUrlOptions(remoteOptions);
     boolean enableDiskCache = SimpleBlobStoreFactory.isDiskCache(remoteOptions);
     if (enableRestCache && enableDiskCache) {
@@ -150,12 +128,27 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     }
     boolean enableBlobStoreCache = enableRestCache || enableDiskCache;
     boolean enableGrpcCache = GrpcRemoteCache.isRemoteCacheOptions(remoteOptions);
-    if (enableBlobStoreCache && !Strings.isNullOrEmpty(remoteOptions.remoteExecutor)) {
+    boolean enableRemoteExecution = !Strings.isNullOrEmpty(remoteOptions.remoteExecutor);
+    if (enableBlobStoreCache && enableRemoteExecution) {
       throw new AbruptExitException(
           "Cannot combine gRPC based remote execution with local disk or HTTP-based caching",
           ExitCode.COMMAND_LINE_ERROR);
     }
 
+    if (!enableBlobStoreCache && !enableGrpcCache && !enableRemoteExecution) {
+      // Quit if no remote caching or execution was enabled.
+      return;
+    }
+
+    env.getEventBus().register(this);
+    String invocationId = env.getCommandId().toString();
+    String buildRequestId = env.getBuildRequestId();
+    env.getReporter().handle(Event.info(String.format("Invocation ID: %s", invocationId)));
+
+    Path logDir =
+        env.getOutputBase().getRelative(env.getRuntime().getProductName() + "-remote-logs");
+    cleanAndCreateRemoteLogsDir(logDir);
+
     try {
       List<ClientInterceptor> interceptors = new ArrayList<>();
       if (!remoteOptions.experimentalRemoteGrpcLog.isEmpty()) {
@@ -267,7 +260,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       }
 
       GrpcRemoteExecutor executor = null;
-      if (!Strings.isNullOrEmpty(remoteOptions.remoteExecutor)) {
+      if (enableRemoteExecution) {
         RemoteRetrier retrier =
             new RemoteRetrier(
                 remoteOptions,
@@ -292,6 +285,21 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     }
   }
 
+  private static void cleanAndCreateRemoteLogsDir(Path logDir) throws AbruptExitException{
+    try {
+      // Clean out old logs files.
+      if (logDir.exists()) {
+        FileSystemUtils.deleteTree(logDir);
+      }
+      logDir.createDirectory();
+    } catch (IOException e) {
+      env.getReporter()
+          .handle(Event.error("Could not create base directory for remote logs: " + logDir));
+      throw new AbruptExitException(ExitCode.LOCAL_ENVIRONMENTAL_ERROR, e);
+    }
+  }
+
+
   private void checkClientServerCompatibility(
       ServerCapabilities capabilities,
       RemoteOptions remoteOptions,