aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--WORKSPACE9
-rw-r--r--src/main/cpp/blaze.cc13
-rw-r--r--src/main/cpp/option_processor.cc56
-rw-r--r--src/main/cpp/option_processor.h15
-rw-r--r--src/main/cpp/startup_options.cc35
-rw-r--r--src/main/cpp/startup_options.h7
-rw-r--r--src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java80
-rw-r--r--src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java18
-rw-r--r--src/main/java/com/google/devtools/build/lib/runtime/CommandExecutor.java19
-rw-r--r--src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java5
-rw-r--r--src/main/java/com/google/devtools/build/lib/server/GrpcServerImpl.java16
-rw-r--r--src/main/java/com/google/devtools/build/lib/server/ServerCommand.java15
-rw-r--r--src/main/protobuf/command_server.proto15
-rw-r--r--src/test/cpp/BUILD2
-rw-r--r--src/test/cpp/option_processor_test.cc168
-rw-r--r--src/test/cpp/startup_options_test.cc73
-rwxr-xr-xsrc/test/shell/integration/loading_phase_tests.sh5
17 files changed, 491 insertions, 60 deletions
diff --git a/WORKSPACE b/WORKSPACE
index a86ba4e6b4..e9665ae9a2 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -119,3 +119,12 @@ http_archive(
strip_prefix = "bazel-toolchains-bccee4855c049d34bac481083b4c68e2fab8cc50",
sha256 = "3903fd93b96b42067e00b7973a2c16c34e761ad7a0b55e1557d408f352849e41",
)
+
+http_archive(
+ name = "com_googlesource_code_re2",
+ urls = [
+ "https://github.com/google/re2/archive/2017-08-01.tar.gz",
+ ],
+ strip_prefix = "re2-2017-08-01",
+ sha256 = "938723dc197125392698c5fcf41acb74877866ff140b81fd50b7314bf26f1636",
+)
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index cca002de10..ec5b8f3676 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -761,6 +761,7 @@ static void StartServerAndConnect(const WorkspaceLayout *workspace_layout,
std::this_thread::sleep_until(next_attempt_time);
if (!server_startup->IsStillAlive()) {
+ globals->option_processor->PrintStartupOptionsProvenanceMessage();
fprintf(stderr,
"\nunexpected pipe read status: %s\n"
"Server presumed dead. Now printing '%s':\n",
@@ -1148,6 +1149,7 @@ static void ParseOptions(int argc, const char *argv[]) {
args, globals->workspace, globals->cwd, &error);
if (parse_exit_code != blaze_exit_code::SUCCESS) {
+ globals->option_processor->PrintStartupOptionsProvenanceMessage();
die(parse_exit_code, "%s", error.c_str());
}
globals->options = globals->option_processor->GetParsedStartupOptions();
@@ -1624,6 +1626,17 @@ unsigned int GrpcBlazeServer::Communicate() {
request.set_invocation_policy(globals->options->invocation_policy);
}
+ const StartupOptions *startup_options(
+ globals->option_processor->GetParsedStartupOptions());
+ for (const auto &startup_option :
+ startup_options->original_startup_options_) {
+ command_server::StartupOption *proto_option_field =
+ request.add_startup_options();
+ request.add_startup_options();
+ proto_option_field->set_source(startup_option.source);
+ proto_option_field->set_option(startup_option.value);
+ }
+
grpc::ClientContext context;
command_server::RunResponse response;
std::unique_ptr<grpc::ClientReader<command_server::RunResponse>> reader(
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index b3b4afa009..325e8ce881 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -86,7 +86,6 @@ blaze_exit_code::ExitCode OptionProcessor::RcFile::Parse(
// A '\' at the end of a line continues the line.
blaze_util::Replace("\\\r\n", "", &contents);
blaze_util::Replace("\\\n", "", &contents);
- vector<string> startup_options;
vector<string> lines = blaze_util::Split(contents, '\n');
for (string& line : lines) {
@@ -153,19 +152,10 @@ blaze_exit_code::ExitCode OptionProcessor::RcFile::Parse(
words_it++; // Advance past command.
for (; words_it != words.end(); words_it++) {
(*rcoptions)[command].push_back(RcOption(index, *words_it));
- if (command == "startup") {
- startup_options.push_back(*words_it);
- }
}
}
}
- if (!startup_options.empty()) {
- string startup_args;
- blaze_util::JoinStrings(startup_options, ' ', &startup_args);
- fprintf(stderr, "INFO: Reading 'startup' options from %s: %s\n",
- filename.c_str(), startup_args.c_str());
- }
return blaze_exit_code::SUCCESS;
}
@@ -389,8 +379,50 @@ blaze_exit_code::ExitCode OptionProcessor::ParseOptions(
return blaze_exit_code::SUCCESS;
}
+static void PrintStartupOptions(const std::string& source,
+ const std::vector<std::string>& options) {
+ if (!source.empty()) {
+ std::string startup_args;
+ blaze_util::JoinStrings(options, ' ', &startup_args);
+ fprintf(stderr, "INFO: Reading 'startup' options from %s: %s\n",
+ source.c_str(), startup_args.c_str());
+ }
+}
+
+void OptionProcessor::PrintStartupOptionsProvenanceMessage() const {
+ StartupOptions* parsed_startup_options = GetParsedStartupOptions();
+
+ // Print the startup flags in the order they are parsed, to keep the
+ // precendence clear. In order to minimize the number of lines of output in
+ // the terminal, group sequential flags by origin. Note that an rc file may
+ // turn up multiple times in this list, if, for example, it imports another
+ // rc file and contains startup options on either side of the import
+ // statement. This is done intentionally to make option priority clear.
+ std::string command_line_source;
+ std::string& most_recent_blazerc = command_line_source;
+ std::vector<std::string> accumulated_options;
+ for (auto flag : parsed_startup_options->original_startup_options_) {
+ if (flag.source == most_recent_blazerc) {
+ accumulated_options.push_back(flag.value);
+ } else {
+ PrintStartupOptions(most_recent_blazerc, accumulated_options);
+ // Start accumulating again.
+ accumulated_options.clear();
+ accumulated_options.push_back(flag.value);
+ most_recent_blazerc = flag.source;
+ }
+ }
+ // Don't forget to print out the last ones.
+ PrintStartupOptions(most_recent_blazerc, accumulated_options);
+}
+
blaze_exit_code::ExitCode OptionProcessor::ParseStartupOptions(
std::string *error) {
+ // Rc files can import other files at any point, and these imported rcs are
+ // expanded in place. The effective ordering of rc flags is stored in
+ // rcoptions_ and should be processed in that order. Here, we isolate just the
+ // startup options, but keep the file they came from attached for the
+ // option_sources tracking and for sending to the server.
std::vector<RcStartupFlag> rcstartup_flags;
auto iter = rcoptions_.find("startup");
@@ -482,8 +514,8 @@ std::vector<std::string> OptionProcessor::GetBlazercAndEnvCommandArgs(
result.push_back("--rc_source=" + blaze::ConvertPath(blazerc->Filename()));
}
- // Process RcOptions exept for the startup flags that are already parsed
- // by the client and shouldn't be handled by defult_overrides.
+ // Process RcOptions except for the startup flags that are already parsed
+ // by the client and shouldn't be included in the command args.
for (auto it = rcoptions.begin(); it != rcoptions.end(); ++it) {
if (it->first != "startup") {
for (const RcOption& rcoption : it->second) {
diff --git a/src/main/cpp/option_processor.h b/src/main/cpp/option_processor.h
index 7054ac1dc1..0fe45ba015 100644
--- a/src/main/cpp/option_processor.h
+++ b/src/main/cpp/option_processor.h
@@ -28,6 +28,10 @@ namespace blaze {
class WorkspaceLayout;
+// Broken down structure of the command line into logical components. The raw
+// arguments should not be referenced after this structure exists. This
+// breakdown should suffice to access the parts of the command line that the
+// client cares about, notably the binary and startup startup options.
struct CommandLine {
const std::string path_to_binary;
const std::vector<std::string> startup_args;
@@ -63,7 +67,7 @@ class OptionProcessor {
// result.path_to_binary = "output/bazel"
// result.startup_args = {"--foo", "--bar=42", "--bar=blah"}
// result.command = "build"
- // result.command_args = {"--some_flag", "value", ":mytarget"}
+ // result.command_args = {"--myflag", "value", ":mytarget"}
//
// Note that result.startup_args is guaranteed to contain only valid
// startup options (w.r.t. StartupOptions::IsUnary and
@@ -104,6 +108,11 @@ class OptionProcessor {
const std::string& workspace, std::string* user_blazerc_file,
std::string* error);
+ // Prints a message about the origin of startup options. This should be called
+ // if the server is not started or called, in case the options are related
+ // to the failure. Otherwise, the server will handle any required logging.
+ void PrintStartupOptionsProvenanceMessage() const;
+
private:
class RcOption {
public:
@@ -123,7 +132,7 @@ class OptionProcessor {
blaze_exit_code::ExitCode Parse(
const std::string& workspace, const WorkspaceLayout* workspace_layout,
std::vector<RcFile*>* rcfiles,
- std::map<std::string, std::vector<RcOption> >* rcoptions,
+ std::map<std::string, std::vector<RcOption>>* rcoptions,
std::string* error);
const std::string& Filename() const { return filename_; }
const int Index() const { return index_; }
@@ -133,7 +142,7 @@ class OptionProcessor {
const std::string& workspace, const std::string& filename,
const int index, const WorkspaceLayout* workspace_layout,
std::vector<RcFile*>* rcfiles,
- std::map<std::string, std::vector<RcOption> >* rcoptions,
+ std::map<std::string, std::vector<RcOption>>* rcoptions,
std::list<std::string>* import_stack, std::string* error);
std::string filename_;
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index d54dbb2969..27c3d71848 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -90,7 +90,8 @@ StartupOptions::StartupOptions(const string &product_name,
invocation_policy(NULL),
client_debug(false),
java_logging_formatter(
- "com.google.devtools.build.lib.util.SingleLineFormatter") {
+ "com.google.devtools.build.lib.util.SingleLineFormatter"),
+ original_startup_options_(std::vector<RcStartupFlag>()) {
bool testing = !blaze::GetEnv("TEST_TMPDIR").empty();
if (testing) {
output_root = MakeAbsolute(blaze::GetEnv("TEST_TMPDIR"));
@@ -381,21 +382,31 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArgs(
std::string *error) {
std::vector<RcStartupFlag>::size_type i = 0;
while (i < rcstartup_flags.size()) {
- bool is_space_separated;
- const bool is_last_elem = i == rcstartup_flags.size() - 1;
+ bool is_space_separated = false;
+ const std::string next_value =
+ (i == rcstartup_flags.size() - 1) ? "" : rcstartup_flags[i + 1].value;
const blaze_exit_code::ExitCode process_arg_exit_code =
- ProcessArg(rcstartup_flags[i].value,
- is_last_elem ? "" : rcstartup_flags[i + 1].value,
- rcstartup_flags[i].source,
- &is_space_separated,
- error);
- if (process_arg_exit_code != blaze_exit_code::SUCCESS) {
- return process_arg_exit_code;
- }
+ ProcessArg(rcstartup_flags[i].value, next_value,
+ rcstartup_flags[i].source, &is_space_separated, error);
+ // Store the provided option in --flag(=value)? form. Store these before
+ // propagating any error code, since we want to have the correct
+ // information for the output. The fact that the options aren't parseable
+ // doesn't matter for this step.
if (is_space_separated) {
+ const std::string combined_value =
+ rcstartup_flags[i].value + "=" + next_value;
+ original_startup_options_.push_back(
+ RcStartupFlag(rcstartup_flags[i].source, combined_value));
+ i += 2;
+ } else {
+ original_startup_options_.push_back(
+ RcStartupFlag(rcstartup_flags[i].source, rcstartup_flags[i].value));
i++;
}
- i++;
+
+ if (process_arg_exit_code != blaze_exit_code::SUCCESS) {
+ return process_arg_exit_code;
+ }
}
return blaze_exit_code::SUCCESS;
}
diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h
index 81cfe787ac..9ddc98d48c 100644
--- a/src/main/cpp/startup_options.h
+++ b/src/main/cpp/startup_options.h
@@ -61,7 +61,8 @@ class UnaryStartupFlag : public StartupFlag {
const std::string name_;
};
-// A startup flag parsed from a bazelrc file.
+// A startup flag tagged with its origin, either an rc file or the empty
+// string for the ones specified in the command line.
// For instance, RcStartupFlag("somepath/.bazelrc", "--foo") is used to
// represent that the line "startup --foo" was found when parsing
// "somepath/.bazelrc".
@@ -267,6 +268,10 @@ class StartupOptions {
// Value of the java.util.logging.FileHandler.formatter Java property.
std::string java_logging_formatter;
+ // The startup options as received from the user and rc files, tagged with
+ // their origin. This is populated by ProcessArgs.
+ std::vector<RcStartupFlag> original_startup_options_;
+
protected:
// Constructor for subclasses only so that site-specific extensions of this
// class can override the product name. The product_name must be the
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 453cd7c24b..54803ea944 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
@@ -65,6 +65,7 @@ import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Level;
@@ -261,9 +262,9 @@ public class BlazeCommandDispatcher {
}
/**
- * Executes a single command. Returns the Unix exit status for the Blaze
- * client process, or throws {@link ShutdownBlazeServerException} to
- * indicate that a command wants to shutdown the Blaze server.
+ * Executes a single command. Returns the Unix exit status for the Blaze client process, or throws
+ * {@link ShutdownBlazeServerException} to indicate that a command wants to shutdown the Blaze
+ * server.
*/
int exec(
InvocationPolicy invocationPolicy,
@@ -271,7 +272,9 @@ public class BlazeCommandDispatcher {
OutErr outErr,
LockingMode lockingMode,
String clientDescription,
- long firstContactTime) throws ShutdownBlazeServerException, InterruptedException {
+ long firstContactTime,
+ Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
+ throws ShutdownBlazeServerException, InterruptedException {
OriginalCommandLineEvent originalCommandLine = new OriginalCommandLineEvent(args);
Preconditions.checkNotNull(clientDescription);
if (args.isEmpty()) { // Default to help command if no arguments specified.
@@ -327,8 +330,16 @@ public class BlazeCommandDispatcher {
outErr.printErrLn("Server shut down " + shutdownReason);
return ExitCode.LOCAL_ENVIRONMENTAL_ERROR.getNumericExitCode();
}
- return execExclusively(originalCommandLine, invocationPolicy, args, outErr, firstContactTime,
- commandName, command, waitTimeInMs);
+ return execExclusively(
+ originalCommandLine,
+ invocationPolicy,
+ args,
+ outErr,
+ firstContactTime,
+ commandName,
+ command,
+ waitTimeInMs,
+ startupOptionsTaggedWithBazelRc);
} catch (ShutdownBlazeServerException e) {
shutdownReason = "explicitly by client " + currentClientDescription;
throw e;
@@ -348,7 +359,8 @@ public class BlazeCommandDispatcher {
long firstContactTime,
String commandName,
BlazeCommand command,
- long waitTimeInMs)
+ long waitTimeInMs,
+ Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
throws ShutdownBlazeServerException {
// Record the start time for the profiler. Do not put anything before this!
long execStartTimeNanos = runtime.getClock().nanoTime();
@@ -489,6 +501,39 @@ public class BlazeCommandDispatcher {
}
if (commonOptions.announceRcOptions) {
+ if (startupOptionsTaggedWithBazelRc.isPresent()) {
+ String lastBlazerc = "";
+ List<String> accumulatedStartupOptions = new ArrayList<>();
+ for (Pair<String, String> option : startupOptionsTaggedWithBazelRc.get()) {
+ // Do not include the command line options, marked by the empty string.
+ if (option.getFirst().isEmpty()) {
+ continue;
+ }
+
+ // If we've moved to a new blazerc in the list, print out the info from the last one,
+ // and clear the accumulated list.
+ if (!lastBlazerc.isEmpty() && !option.getFirst().equals(lastBlazerc)) {
+ String logMessage =
+ String.format(
+ "Reading 'startup' options from %s: %s",
+ lastBlazerc, String.join(", ", accumulatedStartupOptions));
+ reporter.handle(Event.info(logMessage));
+ accumulatedStartupOptions = new ArrayList<>();
+ }
+
+ lastBlazerc = option.getFirst();
+ accumulatedStartupOptions.add(option.getSecond());
+ }
+ // Print out the final blazerc-grouped list, if any startup options were provided by
+ // blazerc.
+ if (!lastBlazerc.isEmpty()) {
+ String logMessage =
+ String.format(
+ "Reading 'startup' options from %s: %s",
+ lastBlazerc, String.join(", ", accumulatedStartupOptions));
+ reporter.handle(Event.info(logMessage));
+ }
+ }
for (String note : rcfileNotes) {
reporter.handle(Event.info(note));
}
@@ -546,14 +591,21 @@ public class BlazeCommandDispatcher {
}
/**
- * For testing ONLY. Same as {@link #exec}, but automatically
- * uses the current time.
+ * For testing ONLY. Same as {@link #exec(InvocationPolicy, List, OutErr, LockingMode, String,
+ * long, Optional<List<Pair<String, String>>>)}, but automatically uses the current time.
*/
@VisibleForTesting
- public int exec(List<String> args, LockingMode lockingMode, String clientDescription,
- OutErr originalOutErr) throws ShutdownBlazeServerException, InterruptedException {
- return exec(InvocationPolicy.getDefaultInstance(), args, originalOutErr, LockingMode.ERROR_OUT,
- clientDescription, runtime.getClock().currentTimeMillis());
+ public int exec(
+ List<String> args, LockingMode lockingMode, String clientDescription, OutErr originalOutErr)
+ throws ShutdownBlazeServerException, InterruptedException {
+ return exec(
+ InvocationPolicy.getDefaultInstance(),
+ args,
+ originalOutErr,
+ LockingMode.ERROR_OUT,
+ clientDescription,
+ runtime.getClock().currentTimeMillis(),
+ Optional.empty() /* startupOptionBundles */);
}
/**
@@ -590,7 +642,7 @@ public class BlazeCommandDispatcher {
// handler, and later replayed.
ExitCode earlyExitCode =
checkCwdInWorkspace(workspace, commandAnnotation, commandName, eventHandler);
- if (earlyExitCode != ExitCode.SUCCESS) {
+ if (!earlyExitCode.equals(ExitCode.SUCCESS)) {
return earlyExitCode;
}
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 d7bd5941d8..2d5e8f0510 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
@@ -63,6 +63,7 @@ import com.google.devtools.build.lib.util.CustomExitCodePublisher;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.ProcessUtils;
import com.google.devtools.build.lib.util.ThreadUtils;
@@ -95,6 +96,7 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
@@ -741,12 +743,24 @@ public final class BlazeRuntime {
return e.getExitCode().getNumericExitCode();
}
+ ImmutableList.Builder<Pair<String, String>> startupOptionsFromCommandLine =
+ ImmutableList.builder();
+ for (String option : commandLineOptions.getStartupArgs()) {
+ startupOptionsFromCommandLine.add(new Pair<>("", option));
+ }
+
BlazeCommandDispatcher dispatcher = new BlazeCommandDispatcher(runtime);
try {
LOG.info(getRequestLogString(commandLineOptions.getOtherArgs()));
- return dispatcher.exec(policy, commandLineOptions.getOtherArgs(), OutErr.SYSTEM_OUT_ERR,
- LockingMode.ERROR_OUT, "batch client", runtime.getClock().currentTimeMillis());
+ return dispatcher.exec(
+ policy,
+ commandLineOptions.getOtherArgs(),
+ OutErr.SYSTEM_OUT_ERR,
+ LockingMode.ERROR_OUT,
+ "batch client",
+ runtime.getClock().currentTimeMillis(),
+ Optional.of(startupOptionsFromCommandLine.build()));
} catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) {
return e.getExitStatus();
} catch (InterruptedException e) {
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 3b70c8656a..3d2b46d310 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,12 +13,15 @@
// limitations under the License.
package com.google.devtools.build.lib.runtime;
+import com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.LockingMode;
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.Pair;
import com.google.devtools.build.lib.util.io.OutErr;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.List;
+import java.util.Optional;
import java.util.logging.Logger;
/**
@@ -44,14 +47,22 @@ public class CommandExecutor implements ServerCommand {
InvocationPolicy invocationPolicy,
List<String> args,
OutErr outErr,
- BlazeCommandDispatcher.LockingMode lockingMode,
+ LockingMode lockingMode,
String clientDescription,
- long firstContactTime) throws InterruptedException {
+ long firstContactTime,
+ Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
+ throws InterruptedException {
LOG.info(BlazeRuntime.getRequestLogString(args));
try {
- return dispatcher.exec(invocationPolicy, args, outErr, lockingMode, clientDescription,
- firstContactTime);
+ return dispatcher.exec(
+ invocationPolicy,
+ args,
+ outErr,
+ lockingMode,
+ clientDescription,
+ firstContactTime,
+ startupOptionsTaggedWithBazelRc);
} catch (BlazeCommandDispatcher.ShutdownBlazeServerException e) {
if (e.getCause() != null) {
StringWriter message = new StringWriter();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
index 9284db7688..d5131c29d7 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
@@ -190,9 +190,10 @@ public class CommonCommandOptions extends OptionsBase {
public boolean announceRcOptions;
/**
- * These are the actual default overrides. Each value is a pair of (command name, value).
+ * These are the actual default overrides. Each value is a tuple of (bazelrc index, command name,
+ * value). The blazerc index is a number used to find the blazerc in --rc_source's values.
*
- * <p>For example: "--default_override=build=--cpu=piii"
+ * <p>For example: "--default_override=rc:build=--cpu=piii"
*/
@Option(
name = "default_override",
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 6abef0e4ac..3f3b46b413 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
@@ -31,9 +31,11 @@ import com.google.devtools.build.lib.server.CommandProtos.PingRequest;
import com.google.devtools.build.lib.server.CommandProtos.PingResponse;
import com.google.devtools.build.lib.server.CommandProtos.RunRequest;
import com.google.devtools.build.lib.server.CommandProtos.RunResponse;
+import com.google.devtools.build.lib.server.CommandProtos.StartupOption;
import com.google.devtools.build.lib.util.BlazeClock;
import com.google.devtools.build.lib.util.Clock;
import com.google.devtools.build.lib.util.ExitCode;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.ThreadUtils;
import com.google.devtools.build.lib.util.io.OutErr;
@@ -59,6 +61,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.Exchanger;
import java.util.concurrent.ExecutionException;
@@ -809,6 +812,16 @@ public class GrpcServerImpl implements RPCServer {
String commandId;
int exitCode;
+ // TODO(b/63925394): This information needs to be passed to the GotOptionsEvent, which does not
+ // currently have the explicit startup options. See Improved Command Line Reporting design doc
+ // for details.
+ // Convert the startup options record to Java strings, source first.
+ ImmutableList.Builder<Pair<String, String>> startupOptions = ImmutableList.builder();
+ for (StartupOption option : request.getStartupOptionsList()) {
+ startupOptions.add(
+ new Pair<>(option.getSource().toString(CHARSET), option.getOption().toString(CHARSET)));
+ }
+
try (RunningCommand command = new RunningCommand()) {
commandId = command.id;
@@ -837,7 +850,8 @@ public class GrpcServerImpl implements RPCServer {
rpcOutErr,
request.getBlockForLock() ? LockingMode.WAIT : LockingMode.ERROR_OUT,
request.getClientDescription(),
- clock.currentTimeMillis());
+ clock.currentTimeMillis(),
+ Optional.of(startupOptions.build()));
} catch (OptionsParsingException e) {
rpcOutErr.printErrLn(e.getMessage());
exitCode = ExitCode.COMMAND_LINE_ERROR.getNumericExitCode();
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 ef5f727bdb..f0574cdbaa 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
@@ -15,8 +15,10 @@ 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.Pair;
import com.google.devtools.build.lib.util.io.OutErr;
import java.util.List;
+import java.util.Optional;
/**
* The {@link RPCServer} calls an arbitrary command implementing this
@@ -25,8 +27,13 @@ import java.util.List;
public interface ServerCommand {
/**
- * Executes the request, writing any output or error messages into err.
- * Returns 0 on success; any other value or exception indicates an error.
+ * Executes the request, writing any output or error messages into err. Returns 0 on success; any
+ * other value or exception indicates an error.
+ *
+ * @param startupOptionsTaggedWithBazelRc List of startup options in Pair(bazelRc, option) form.
+ * The empty string bazelRc is interpreted as the command line, and option should be in
+ * --[no]flag or --flag=value form. If we don't have access to this information (--batch),
+ * leave this parameter as Optional.empty().
*/
int exec(
InvocationPolicy policy,
@@ -34,7 +41,9 @@ public interface ServerCommand {
OutErr outErr,
BlazeCommandDispatcher.LockingMode lockingMode,
String clientDescription,
- long firstContactTime) throws InterruptedException;
+ long firstContactTime,
+ Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc)
+ 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 79f44b9930..b55a68817d 100644
--- a/src/main/protobuf/command_server.proto
+++ b/src/main/protobuf/command_server.proto
@@ -51,6 +51,21 @@ message RunRequest {
// between batch and server mode, so the parsing logic is only in the Java
// code.
string invocation_policy = 5;
+
+ // Startup arguments, in the order they were applied, tagged with where they
+ // came from. These options have already been parsed and already have had
+ // their effect. This information should only be used for logging.
+ repeated StartupOption startup_options = 6;
+}
+
+// Contains the a startup option with its source file. Uses bytes to preserve
+// the way the user inputted the arguments, like the args in RunRequest.
+message StartupOption {
+ // Startup option in --nullaryflag or --unaryflag=value form.
+ bytes option = 1;
+ // Where the option came from, such as an rc file or an empty string for the
+ // command line.
+ bytes source = 2;
}
// Contains metadata and result data for a command execution.
diff --git a/src/test/cpp/BUILD b/src/test/cpp/BUILD
index 20c36d552e..500974501d 100644
--- a/src/test/cpp/BUILD
+++ b/src/test/cpp/BUILD
@@ -37,12 +37,14 @@ cc_test(
name = "option_processor_test",
size = "small",
srcs = ["option_processor_test.cc"],
+ tags = ["requires_internet"],
deps = [
"//src/main/cpp:blaze_util",
"//src/main/cpp:option_processor",
"//src/main/cpp:workspace_layout",
"//src/main/cpp/util",
"//third_party:gtest",
+ "@com_googlesource_code_re2//:re2",
],
)
diff --git a/src/test/cpp/option_processor_test.cc b/src/test/cpp/option_processor_test.cc
index ab8ea59847..88fb52f42c 100644
--- a/src/test/cpp/option_processor_test.cc
+++ b/src/test/cpp/option_processor_test.cc
@@ -17,10 +17,11 @@
#include "src/main/cpp/blaze_util.h"
#include "src/main/cpp/blaze_util_platform.h"
#include "src/main/cpp/option_processor-internal.h"
-#include "src/main/cpp/workspace_layout.h"
#include "src/main/cpp/util/file.h"
#include "src/main/cpp/util/file_platform.h"
+#include "src/main/cpp/workspace_layout.h"
#include "gtest/gtest.h"
+#include "re2/re2.h"
namespace blaze {
@@ -167,8 +168,7 @@ TEST_F(OptionProcessorTest, CanParseDifferentStartupArgs) {
option_processor_->GetExplicitCommandArguments());
}
-
-TEST_F(OptionProcessorTest, CommandLineBlazercTest) {
+TEST_F(OptionProcessorTest, CommandLineBazelrcTest) {
const std::string cmdline_rc_path =
blaze_util::JoinPath(workspace_, "mybazelrc");
ASSERT_TRUE(blaze_util::MakeDirectories(
@@ -185,9 +185,25 @@ TEST_F(OptionProcessorTest, CommandLineBlazercTest) {
option_processor_->ParseOptions(args, workspace_, cwd_, &error))
<< error;
ASSERT_EQ(expected_error, error);
+
+ // Check that the startup option option provenance message prints the correct
+ // information for the incorrect flag, and does not print the command-line
+ // provided startup flags.
+ testing::internal::CaptureStderr();
+ option_processor_->PrintStartupOptionsProvenanceMessage();
+ const std::string& output = testing::internal::GetCapturedStderr();
+
+ EXPECT_PRED1(
+ [](std::string actualOutput) {
+ return RE2::FullMatch(
+ actualOutput,
+ "INFO: Reading 'startup' options from .*mybazelrc: "
+ "--foo\n");
+ },
+ output);
}
-TEST_F(OptionProcessorTest, NoMasterBlazercAndBlazercWorkTogetherCorrectly) {
+TEST_F(OptionProcessorTest, NoMasterBazelrcAndBazelrcWorkTogetherCorrectly) {
const std::string cmdline_rc_path =
blaze_util::JoinPath(workspace_, "mybazelrc");
ASSERT_TRUE(blaze_util::MakeDirectories(
@@ -211,6 +227,150 @@ TEST_F(OptionProcessorTest, NoMasterBlazercAndBlazercWorkTogetherCorrectly) {
<< error;
EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
+
+ // Check that the startup option option provenance message prints the correct
+ // information for the provided rc, and prints nothing for the master bazelrc.
+ testing::internal::CaptureStderr();
+ option_processor_->PrintStartupOptionsProvenanceMessage();
+ const std::string& output = testing::internal::GetCapturedStderr();
+
+ EXPECT_PRED1(
+ [](std::string actualOutput) {
+ return RE2::FullMatch(
+ actualOutput,
+ "INFO: Reading 'startup' options from .*mybazelrc: "
+ "--max_idle_secs=123\n");
+ },
+ output);
+}
+
+TEST_F(OptionProcessorTest, MultipleStartupArgsInMasterBazelrcWorksCorrectly) {
+ // Add startup flags to the master bazelrc.
+ const std::string master_rc_path =
+ blaze_util::JoinPath(workspace_, "tools/bazel.rc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(master_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile(
+ "startup --max_idle_secs=42\nstartup --io_nice_level=6", master_rc_path,
+ 0755));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ std::string error;
+ ASSERT_EQ(blaze_exit_code::SUCCESS,
+ option_processor_->ParseOptions(args, workspace_, cwd_, &error))
+ << error;
+
+ EXPECT_EQ(42, option_processor_->GetParsedStartupOptions()->max_idle_secs);
+ EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
+
+ // Check that the startup options get grouped together properly in the output
+ // message.
+ testing::internal::CaptureStderr();
+ option_processor_->PrintStartupOptionsProvenanceMessage();
+ const std::string& output = testing::internal::GetCapturedStderr();
+
+ EXPECT_PRED1(
+ [](std::string actualOutput) {
+ return RE2::FullMatch(
+ actualOutput,
+ "INFO: Reading 'startup' options from .*tools.*bazel.rc: "
+ "--max_idle_secs=42 --io_nice_level=6\n");
+ },
+ output);
+}
+
+TEST_F(OptionProcessorTest, CustomBazelrcOverridesMasterBazelrc) {
+ // Add startup flags to the master bazelrc.
+ const std::string master_rc_path =
+ blaze_util::JoinPath(workspace_, "tools/bazel.rc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(master_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile(
+ "startup --max_idle_secs=42\nstartup --io_nice_level=6", master_rc_path,
+ 0755));
+
+ // Override one of the master bazelrc's flags in the custom bazelrc.
+ const std::string cmdline_rc_path =
+ blaze_util::JoinPath(workspace_, "mybazelrc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(cmdline_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("startup --max_idle_secs=123",
+ cmdline_rc_path, 0755));
+ const std::vector<std::string> args = {
+ "bazel", "--bazelrc=" + cmdline_rc_path, "build"};
+ std::string error;
+ ASSERT_EQ(blaze_exit_code::SUCCESS,
+ option_processor_->ParseOptions(args, workspace_, cwd_, &error))
+ << error;
+
+ EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
+ EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
+
+ // Check that the options are reported in the correct order in the provenance
+ // message.
+ testing::internal::CaptureStderr();
+ option_processor_->PrintStartupOptionsProvenanceMessage();
+ const std::string& output = testing::internal::GetCapturedStderr();
+
+ EXPECT_PRED1(
+ [](std::string actualOutput) {
+ return RE2::FullMatch(
+ actualOutput,
+ "INFO: Reading 'startup' options from .*tools.*bazel.rc: "
+ "--max_idle_secs=42 --io_nice_level=6\n"
+ "INFO: Reading 'startup' options from .*mybazelrc: "
+ "--max_idle_secs=123\n");
+ },
+ output);
+}
+
+TEST_F(OptionProcessorTest, BazelRcImportsMaintainsFlagOrdering) {
+ // Override one of the master bazelrc's flags in the custom bazelrc.
+ const std::string imported_rc_path =
+ blaze_util::JoinPath(workspace_, "myimportedbazelrc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(imported_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile(
+ "startup --max_idle_secs=123\nstartup --io_nice_level=4",
+ imported_rc_path, 0755));
+
+ // Add startup flags the imported bazelrc.
+ const std::string master_rc_path =
+ blaze_util::JoinPath(workspace_, "tools/bazel.rc");
+ ASSERT_TRUE(
+ blaze_util::MakeDirectories(blaze_util::Dirname(master_rc_path), 0755));
+ ASSERT_TRUE(blaze_util::WriteFile("startup --max_idle_secs=42\nimport " +
+ imported_rc_path +
+ "\nstartup --io_nice_level=6",
+ master_rc_path, 0755));
+
+ const std::vector<std::string> args = {"bazel", "build"};
+ std::string error;
+ ASSERT_EQ(blaze_exit_code::SUCCESS,
+ option_processor_->ParseOptions(args, workspace_, cwd_, &error))
+ << error;
+
+ EXPECT_EQ(123, option_processor_->GetParsedStartupOptions()->max_idle_secs);
+ EXPECT_EQ(6, option_processor_->GetParsedStartupOptions()->io_nice_level);
+
+ // Check that the options are reported in the correct order in the provenance
+ // message, the imported file between the two master flags
+ testing::internal::CaptureStderr();
+ option_processor_->PrintStartupOptionsProvenanceMessage();
+ const std::string& output = testing::internal::GetCapturedStderr();
+
+ EXPECT_PRED1(
+ [](std::string actualOutput) {
+ return RE2::FullMatch(
+ actualOutput,
+ "INFO: Reading 'startup' options from .*tools.*bazel.rc: "
+ "--max_idle_secs=42\n"
+ "INFO: Reading 'startup' options from .*myimportedbazelrc: "
+ "--max_idle_secs=123 --io_nice_level=4\n"
+ "INFO: Reading 'startup' options from .*tools.*bazel.rc: "
+ "--io_nice_level=6\n");
+ },
+ output);
}
TEST_F(OptionProcessorTest, SplitCommandLineWithEmptyArgs) {
diff --git a/src/test/cpp/startup_options_test.cc b/src/test/cpp/startup_options_test.cc
index 164bdc3c90..a4c52a2621 100644
--- a/src/test/cpp/startup_options_test.cc
+++ b/src/test/cpp/startup_options_test.cc
@@ -155,4 +155,77 @@ TEST_F(StartupOptionsTest, ValidStartupFlagsTest) {
SuccessfulIsUnaryTest("output_user_root");
}
+TEST_F(StartupOptionsTest, ProcessSpaceSeparatedArgsTest) {
+ std::string error;
+ const std::vector<RcStartupFlag> flags{
+ RcStartupFlag("somewhere", "--max_idle_secs"),
+ RcStartupFlag("somewhere", "42")};
+
+ const blaze_exit_code::ExitCode ec =
+ startup_options_->ProcessArgs(flags, &error);
+ ASSERT_EQ(blaze_exit_code::SUCCESS, ec)
+ << "ProcessArgs failed with error " << error;
+ EXPECT_EQ(42, startup_options_->max_idle_secs);
+
+ EXPECT_EQ("somewhere", startup_options_->original_startup_options_[0].source);
+ EXPECT_EQ("--max_idle_secs=42",
+ startup_options_->original_startup_options_[0].value);
+}
+
+TEST_F(StartupOptionsTest, ProcessEqualsSeparatedArgsTest) {
+ std::string error;
+ const std::vector<RcStartupFlag> flags{
+ RcStartupFlag("somewhere", "--max_idle_secs=36")};
+
+ const blaze_exit_code::ExitCode ec =
+ startup_options_->ProcessArgs(flags, &error);
+ ASSERT_EQ(ec, blaze_exit_code::SUCCESS)
+ << "ProcessArgs failed with error " << error;
+ EXPECT_EQ(36, startup_options_->max_idle_secs);
+
+ EXPECT_EQ("somewhere", startup_options_->original_startup_options_[0].source);
+ EXPECT_EQ("--max_idle_secs=36",
+ startup_options_->original_startup_options_[0].value);
+}
+
+TEST_F(StartupOptionsTest, ProcessIncorrectArgValueTest) {
+ std::string error;
+ const std::vector<RcStartupFlag> flags{
+ RcStartupFlag("somewhere", "--max_idle_secs=notANumber")};
+
+ const blaze_exit_code::ExitCode ec =
+ startup_options_->ProcessArgs(flags, &error);
+ ASSERT_EQ(blaze_exit_code::BAD_ARGV, ec)
+ << "ProcessArgs failed with the wrong error " << error;
+
+ // Even for a failing args processing step, expect the original value
+ // to be stored.
+ EXPECT_EQ("somewhere", startup_options_->original_startup_options_[0].source);
+ EXPECT_EQ("--max_idle_secs=notANumber",
+ startup_options_->original_startup_options_[0].value);
+}
+
+TEST_F(StartupOptionsTest, ProcessArgsWithMultipleArgstest) {
+ const std::vector<RcStartupFlag> flags{
+ RcStartupFlag("somewhere", "--max_idle_secs=36"),
+ RcStartupFlag("somewhereElse", "--nowrite_command_log")};
+
+ std::string error;
+ const blaze_exit_code::ExitCode ec =
+ startup_options_->ProcessArgs(flags, &error);
+ ASSERT_EQ(ec, blaze_exit_code::SUCCESS)
+ << "ProcessArgs failed with error " << error;
+ EXPECT_EQ(36, startup_options_->max_idle_secs);
+ EXPECT_FALSE(startup_options_->write_command_log);
+
+ EXPECT_EQ("somewhere", startup_options_->original_startup_options_[0].source);
+ EXPECT_EQ("--max_idle_secs=36",
+ startup_options_->original_startup_options_[0].value);
+
+ EXPECT_EQ("somewhereElse",
+ startup_options_->original_startup_options_[1].source);
+ EXPECT_EQ("--nowrite_command_log",
+ startup_options_->original_startup_options_[1].value);
+}
+
} // namespace blaze
diff --git a/src/test/shell/integration/loading_phase_tests.sh b/src/test/shell/integration/loading_phase_tests.sh
index 5c13b8fb18..33d3539b56 100755
--- a/src/test/shell/integration/loading_phase_tests.sh
+++ b/src/test/shell/integration/loading_phase_tests.sh
@@ -110,13 +110,14 @@ function test_bazelrc_option() {
cp ${bazelrc} ${new_workspace_dir}/.${PRODUCT_NAME}rc
echo "build --cpu=armeabi-v7a" >>.${PRODUCT_NAME}rc # default bazelrc
- $PATH_TO_BAZEL_BIN info >/dev/null 2>$TEST_log
+ $PATH_TO_BAZEL_BIN info --announce_rc >/dev/null 2>$TEST_log
expect_log "Reading.*$(pwd)/.${PRODUCT_NAME}rc:
.*--cpu=armeabi-v7a"
cp .${PRODUCT_NAME}rc foo
echo "build --cpu=armeabi-v7a" >>foo # non-default bazelrc
- $PATH_TO_BAZEL_BIN --${PRODUCT_NAME}rc=foo info >/dev/null 2>$TEST_log
+ $PATH_TO_BAZEL_BIN --${PRODUCT_NAME}rc=foo info --announce_rc >/dev/null \
+ 2>$TEST_log
expect_log "Reading.*$(pwd)/foo:
.*--cpu=armeabi-v7a"
}
generated by cgit v1.2.3 (git 2.39.1) at 2024-05-14 07:19:48 +0000