Move InvocationPolicy from a startup argument to part of the RunRequest proto.
The user interface is not changing. The policy will still be accepted as a flag passed to the client, as a startup flag (before the command), it will just no longer trigger server restarts and will not be passed on to a bazel server as part of the startup arguments. In batch mode, however, it will still be a startup argument, because the RunRequest proto is not sent, and all invocations restart bazel in batch mode anyway.
Since invocation policy only affects command arguments, and changes in command arguments do not cause server restarts, this is consistent with other server restart behavior.
RELNOTES: Changing --invocation_policy will no longer force a server restart.
PiperOrigin-RevId: 152426207
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 30692da..d1b2298 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -492,7 +492,8 @@
}
}
- if (globals->options->invocation_policy != NULL &&
+ // Pass in invocation policy as a startup argument for batch mode only.
+ if (globals->options->batch && globals->options->invocation_policy != NULL &&
strlen(globals->options->invocation_policy) > 0) {
result.push_back(string("--invocation_policy=") +
globals->options->invocation_policy);
@@ -1613,6 +1614,10 @@
for (const string &arg : arg_vector) {
request.add_arg(arg);
}
+ if (globals->options->invocation_policy != NULL &&
+ strlen(globals->options->invocation_policy) > 0) {
+ request.set_invocation_policy(globals->options->invocation_policy);
+ }
grpc::ClientContext context;
command_server::RunResponse response;
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 7d275a4..f7fadb5 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
@@ -33,7 +33,6 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.flags.InvocationPolicyEnforcer;
-import com.google.devtools.build.lib.flags.InvocationPolicyParser;
import com.google.devtools.build.lib.runtime.commands.ProjectFileSupport;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.util.AbruptExitException;
@@ -246,7 +245,12 @@
* client process, or throws {@link ShutdownBlazeServerException} to
* indicate that a command wants to shutdown the Blaze server.
*/
- int exec(List<String> args, OutErr outErr, LockingMode lockingMode, String clientDescription,
+ int exec(
+ InvocationPolicy invocationPolicy,
+ List<String> args,
+ OutErr outErr,
+ LockingMode lockingMode,
+ String clientDescription,
long firstContactTime) throws ShutdownBlazeServerException, InterruptedException {
OriginalCommandLineEvent originalCommandLine = new OriginalCommandLineEvent(args);
Preconditions.checkNotNull(clientDescription);
@@ -303,8 +307,8 @@
outErr.printErrLn("Server shut down " + shutdownReason);
return ExitCode.LOCAL_ENVIRONMENTAL_ERROR.getNumericExitCode();
}
- return execExclusively(
- originalCommandLine, args, outErr, firstContactTime, commandName, command, waitTimeInMs);
+ return execExclusively(originalCommandLine, invocationPolicy, args, outErr, firstContactTime,
+ commandName, command, waitTimeInMs);
} catch (ShutdownBlazeServerException e) {
shutdownReason = "explicitly by client " + currentClientDescription;
throw e;
@@ -318,6 +322,7 @@
private int execExclusively(
OriginalCommandLineEvent originalCommandLine,
+ InvocationPolicy invocationPolicy,
List<String> args,
OutErr outErr,
long firstContactTime,
@@ -395,12 +400,7 @@
InvocationPolicy combinedPolicy =
InvocationPolicy.newBuilder()
.mergeFrom(runtime.getModuleInvocationPolicy())
- .mergeFrom(
- InvocationPolicyParser.parsePolicy(
- getRuntime()
- .getStartupOptionsProvider()
- .getOptions(BlazeServerStartupOptions.class)
- .invocationPolicy))
+ .mergeFrom(invocationPolicy)
.build();
InvocationPolicyEnforcer optionsPolicyEnforcer = new InvocationPolicyEnforcer(combinedPolicy);
optionsPolicyEnforcer.enforce(optionsParser, commandName);
@@ -546,8 +546,8 @@
@VisibleForTesting
public int exec(List<String> args, LockingMode lockingMode, String clientDescription,
OutErr originalOutErr) throws ShutdownBlazeServerException, InterruptedException {
- return exec(args, originalOutErr, LockingMode.ERROR_OUT, clientDescription,
- runtime.getClock().currentTimeMillis());
+ return exec(InvocationPolicy.getDefaultInstance(), args, originalOutErr, LockingMode.ERROR_OUT,
+ clientDescription, runtime.getClock().currentTimeMillis());
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index cf8aff0..10812a4 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.OutputFilter;
import com.google.devtools.build.lib.flags.CommandNameCache;
+import com.google.devtools.build.lib.flags.InvocationPolicyParser;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -747,13 +748,17 @@
+ commandLineOptions.getStartupArgs());
BlazeRuntime runtime;
+ InvocationPolicy policy;
try {
runtime = newRuntime(modules, commandLineOptions.getStartupArgs(), null);
+ policy = InvocationPolicyParser.parsePolicy(
+ runtime.getStartupOptionsProvider().getOptions(BlazeServerStartupOptions.class)
+ .invocationPolicy);
} catch (OptionsParsingException e) {
- OutErr.SYSTEM_OUT_ERR.printErr(e.getMessage());
+ OutErr.SYSTEM_OUT_ERR.printErrLn(e.getMessage());
return ExitCode.COMMAND_LINE_ERROR.getNumericExitCode();
} catch (AbruptExitException e) {
- OutErr.SYSTEM_OUT_ERR.printErr(e.getMessage());
+ OutErr.SYSTEM_OUT_ERR.printErrLn(e.getMessage());
return e.getExitCode().getNumericExitCode();
}
@@ -761,7 +766,7 @@
try {
LOG.info(getRequestLogString(commandLineOptions.getOtherArgs()));
- return dispatcher.exec(commandLineOptions.getOtherArgs(), OutErr.SYSTEM_OUT_ERR,
+ return dispatcher.exec(policy, commandLineOptions.getOtherArgs(), OutErr.SYSTEM_OUT_ERR,
LockingMode.ERROR_OUT, "batch client", runtime.getClock().currentTimeMillis());
} catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) {
return e.getExitStatus();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java
index 6f5b7b6..3b70c86 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.runtime;
+import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.server.ServerCommand;
import com.google.devtools.build.lib.util.io.OutErr;
import java.io.PrintWriter;
@@ -39,12 +40,18 @@
}
@Override
- public int exec(List<String> args, OutErr outErr, BlazeCommandDispatcher.LockingMode lockingMode,
- String clientDescription, long firstContactTime) throws InterruptedException {
+ public int exec(
+ InvocationPolicy invocationPolicy,
+ List<String> args,
+ OutErr outErr,
+ BlazeCommandDispatcher.LockingMode lockingMode,
+ String clientDescription,
+ long firstContactTime) throws InterruptedException {
LOG.info(BlazeRuntime.getRequestLogString(args));
try {
- return dispatcher.exec(args, outErr, lockingMode, clientDescription, firstContactTime);
+ return dispatcher.exec(invocationPolicy, args, outErr, lockingMode, clientDescription,
+ firstContactTime);
} catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) {
if (e.getCause() != null) {
StringWriter message = new StringWriter();
diff --git a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
index 19e038f..676337e 100644
--- a/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java
@@ -22,8 +22,10 @@
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.common.util.concurrent.Uninterruptibles;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.flags.InvocationPolicyParser;
import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.LockingMode;
import com.google.devtools.build.lib.runtime.CommandExecutor;
+import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.server.CommandProtos.CancelRequest;
import com.google.devtools.build.lib.server.CommandProtos.CancelResponse;
import com.google.devtools.build.lib.server.CommandProtos.PingRequest;
@@ -38,6 +40,7 @@
import com.google.devtools.build.lib.util.io.OutErr;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.common.options.OptionsParsingException;
import com.google.protobuf.ByteString;
import io.grpc.Server;
import io.grpc.StatusRuntimeException;
@@ -840,14 +843,20 @@
new RpcOutputStream(command.id, responseCookie, StreamType.STDOUT, sink),
new RpcOutputStream(command.id, responseCookie, StreamType.STDERR, sink));
- exitCode =
- commandExecutor.exec(
- args.build(),
- rpcOutErr,
- request.getBlockForLock() ? LockingMode.WAIT : LockingMode.ERROR_OUT,
- request.getClientDescription(),
- clock.currentTimeMillis());
-
+ try {
+ InvocationPolicy policy = InvocationPolicyParser.parsePolicy(request.getInvocationPolicy());
+ exitCode =
+ commandExecutor.exec(
+ policy,
+ args.build(),
+ rpcOutErr,
+ request.getBlockForLock() ? LockingMode.WAIT : LockingMode.ERROR_OUT,
+ request.getClientDescription(),
+ clock.currentTimeMillis());
+ } catch (OptionsParsingException e) {
+ rpcOutErr.printErrLn(e.getMessage());
+ exitCode = ExitCode.COMMAND_LINE_ERROR.getNumericExitCode();
+ }
} catch (InterruptedException e) {
exitCode = ExitCode.INTERRUPTED.getNumericExitCode();
commandId = ""; // The default value, the client will ignore it
diff --git a/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java b/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java
index 7b0a075..ef5f727 100644
--- a/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/server/ServerCommand.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.server;
import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher;
+import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.util.io.OutErr;
import java.util.List;
@@ -27,8 +28,13 @@
* Executes the request, writing any output or error messages into err.
* Returns 0 on success; any other value or exception indicates an error.
*/
- int exec(List<String> args, OutErr outErr, BlazeCommandDispatcher.LockingMode lockingMode,
- String clientDescription, long firstContactTime) throws InterruptedException;
+ int exec(
+ InvocationPolicy policy,
+ List<String> args,
+ OutErr outErr,
+ BlazeCommandDispatcher.LockingMode lockingMode,
+ String clientDescription,
+ long firstContactTime) throws InterruptedException;
/**
* Whether the server needs to be shut down.
diff --git a/src/main/protobuf/command_server.proto b/src/main/protobuf/command_server.proto
index cf7eb74..3dfedd4 100644
--- a/src/main/protobuf/command_server.proto
+++ b/src/main/protobuf/command_server.proto
@@ -43,6 +43,14 @@
// A simple description of the client for reporting purposes. This value is
// required.
string client_description = 4;
+
+ // Invocation policy affects how command arguments are interpreted and should
+ // be passed separately. This is a proto message, either a human readable
+ // String or base64-encoded binary-serialized version of the message. It is
+ // not typed directly as an InvocationPolicy message due to distinctions
+ // between batch and server mode, so the parsing logic is only in the Java
+ // code.
+ string invocation_policy = 5;
}
// Contains metadata and result data for a command execution.
