diff options
-rw-r--r-- | src/main/cpp/blaze.cc | 44 | ||||
-rw-r--r-- | src/main/cpp/blaze_util.cc | 35 | ||||
-rw-r--r-- | src/main/cpp/blaze_util.h | 33 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_platform.h | 6 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_posix.cc | 43 | ||||
-rw-r--r-- | src/main/cpp/blaze_util_windows.cc | 40 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java | 15 |
7 files changed, 167 insertions, 49 deletions
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 1e80b492ca..6c9aea3895 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -44,6 +44,7 @@ #include <algorithm> #include <chrono> // NOLINT (gRPC requires this) #include <cinttypes> +#include <map> #include <mutex> // NOLINT #include <set> #include <sstream> @@ -74,6 +75,7 @@ using blaze_util::GetLastErrorString; namespace blaze { +using std::map; using std::set; using std::string; using std::vector; @@ -318,6 +320,8 @@ class CompoundZipProcessor : public devtools_ijar::ZipExtractorProcessor { const vector<PureZipExtractorProcessor*> processors_; }; +static map<string, EnvVarValue> PrepareEnvironmentForJvm(); + // A PureZipExtractorProcessor to extract the InstallKeyFile class GetInstallKeyFileProcessor : public PureZipExtractorProcessor { public: @@ -671,9 +675,9 @@ static int StartServer(const WorkspaceLayout *workspace_layout, // we can still print errors to the terminal. GoToWorkspace(workspace_layout); - return ExecuteDaemon(exe, jvm_args_vector, globals->jvm_log_file, - globals->jvm_log_file_append, server_dir, - server_startup); + return ExecuteDaemon(exe, jvm_args_vector, PrepareEnvironmentForJvm(), + globals->jvm_log_file, globals->jvm_log_file_append, + server_dir, server_startup); } // Replace this process with blaze in standalone/batch mode. @@ -718,12 +722,15 @@ static void StartStandalone(const WorkspaceLayout *workspace_layout, command_arguments.end()); GoToWorkspace(workspace_layout); - string exe = globals->options->GetExe(globals->jvm_path, globals->ServerJarPath()); - ExecuteProgram(exe, jvm_args_vector); - BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) - << "execv of '" << exe << "' failed: " << GetLastErrorString(); + + { + WithEnvVars env_obj(PrepareEnvironmentForJvm()); + ExecuteProgram(exe, jvm_args_vector); + BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) + << "execv of '" << exe << "' failed: " << GetLastErrorString(); + } } static void WriteFileToStderrOrDie(const char *file_name) { @@ -1343,10 +1350,12 @@ static void ComputeBaseDirectories(const WorkspaceLayout *workspace_layout, // Prepares the environment to be suitable to start a JVM. // Changes made to the environment in this function *will not* be part // of '--client_env'. -static void PrepareEnvironmentForJvm() { +static map<string, EnvVarValue> PrepareEnvironmentForJvm() { + map<string, EnvVarValue> result; + if (!blaze::GetEnv("http_proxy").empty()) { BAZEL_LOG(WARNING) << "ignoring http_proxy in environment."; - blaze::UnsetEnv("http_proxy"); + result["http_proxy"] = EnvVarValue(EnvVarAction::UNSET, ""); } if (!blaze::GetEnv("LD_ASSUME_KERNEL").empty()) { @@ -1355,18 +1364,18 @@ static void PrepareEnvironmentForJvm() { // This is also provoked by LD_LIBRARY_PATH=/usr/lib/debug, // or anything else that causes the JVM to use LinuxThreads. BAZEL_LOG(WARNING) << "ignoring LD_ASSUME_KERNEL in environment."; - blaze::UnsetEnv("LD_ASSUME_KERNEL"); + result["LD_ASSUME_KERNEL"] = EnvVarValue(EnvVarAction::UNSET, ""); } if (!blaze::GetEnv("LD_PRELOAD").empty()) { BAZEL_LOG(WARNING) << "ignoring LD_PRELOAD in environment."; - blaze::UnsetEnv("LD_PRELOAD"); + result["LD_PRELOAD"] = EnvVarValue(EnvVarAction::UNSET, ""); } if (!blaze::GetEnv("_JAVA_OPTIONS").empty()) { // This would override --host_jvm_args BAZEL_LOG(WARNING) << "ignoring _JAVA_OPTIONS in environment."; - blaze::UnsetEnv("_JAVA_OPTIONS"); + result["_JAVA_OPTIONS"] = EnvVarValue(EnvVarAction::UNSET, ""); } // TODO(bazel-team): We've also seen a failure during loading (creating @@ -1375,10 +1384,12 @@ static void PrepareEnvironmentForJvm() { // Make the JVM use ISO-8859-1 for parsing its command line because "blaze // run" doesn't handle non-ASCII command line arguments. This is apparently // the most reliable way to select the platform default encoding. - blaze::SetEnv("LANG", "en_US.ISO-8859-1"); - blaze::SetEnv("LANGUAGE", "en_US.ISO-8859-1"); - blaze::SetEnv("LC_ALL", "en_US.ISO-8859-1"); - blaze::SetEnv("LC_CTYPE", "en_US.ISO-8859-1"); + result["LANG"] = EnvVarValue(EnvVarAction::SET, "en_US.ISO-8859-1"); + result["LANGUAGE"] = EnvVarValue(EnvVarAction::SET, "en_US.ISO-8859-1"); + result["LC_ALL"] = EnvVarValue(EnvVarAction::SET, "en_US.ISO-8859-1"); + result["LC_CTYPE"] = EnvVarValue(EnvVarAction::SET, "en_US.ISO-8859-1"); + + return result; } static string CheckAndGetBinaryPath(const string &argv0) { @@ -1462,7 +1473,6 @@ int Main(int argc, const char *argv[], WorkspaceLayout *workspace_layout, BAZEL_LOG(INFO) << "Debug logging requested, sending all client log " "statements to stderr"; - PrepareEnvironmentForJvm(); blaze::CreateSecureOutputRoot(globals->options->output_user_root); const string self_path = GetSelfPath(); diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc index fd742e2392..d7ab44ca01 100644 --- a/src/main/cpp/blaze_util.cc +++ b/src/main/cpp/blaze_util.cc @@ -19,6 +19,7 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <cassert> #include <iostream> #include "src/main/cpp/blaze_util_platform.h" @@ -32,6 +33,7 @@ namespace blaze { +using std::map; using std::string; using std::vector; @@ -182,4 +184,37 @@ void SetDebugLog(bool enabled) { } } +void WithEnvVars::SetEnvVars(const map<string, EnvVarValue>& vars) { + for (const auto& var : vars) { + switch (var.second.action) { + case EnvVarAction::UNSET: + UnsetEnv(var.first); + break; + + case EnvVarAction::SET: + SetEnv(var.first, var.second.value); + break; + + default: + assert(false); + } + } +} + +WithEnvVars::WithEnvVars(const map<string, EnvVarValue>& vars) { + for (const auto& v : vars) { + if (ExistsEnv(v.first)) { + _old_values[v.first] = EnvVarValue(EnvVarAction::SET, GetEnv(v.first)); + } else { + _old_values[v.first] = EnvVarValue(EnvVarAction::UNSET, ""); + } + } + + SetEnvVars(vars); +} + +WithEnvVars::~WithEnvVars() { + SetEnvVars(_old_values); +} + } // namespace blaze diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h index b8055c3b0a..e7f8ba1c46 100644 --- a/src/main/cpp/blaze_util.h +++ b/src/main/cpp/blaze_util.h @@ -21,6 +21,7 @@ #include <sys/types.h> +#include <map> #include <sstream> #include <string> #include <vector> @@ -106,6 +107,38 @@ std::string ToString(const T &value) { // Revisit once client logging is fixed (b/32939567). void SetDebugLog(bool enabled); +// What WithEnvVar should do with an environment variable +enum EnvVarAction { UNSET, SET }; + +// What WithEnvVar should do with an environment variable +struct EnvVarValue { + // What should be done with the given variable + EnvVarAction action; + + // The value of the variable; ignored if action == UNSET + std::string value; + + EnvVarValue() {} + + EnvVarValue(EnvVarAction action, const std::string& value) + : action(action), + value(value) {} +}; + +// While this class is in scope, the specified environment variables will be set +// to a specified value (or unset). When it leaves scope, changed variables will +// be set to their original values. +class WithEnvVars { + private: + std::map<std::string, EnvVarValue> _old_values; + + void SetEnvVars(const std::map<std::string, EnvVarValue>& vars); + + public: + WithEnvVars(const std::map<std::string, EnvVarValue>& vars); + ~WithEnvVars(); +}; + } // namespace blaze #endif // BAZEL_SRC_MAIN_CPP_BLAZE_UTIL_H_ diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h index 6818c95ef6..50ac21933f 100644 --- a/src/main/cpp/blaze_util_platform.h +++ b/src/main/cpp/blaze_util_platform.h @@ -16,10 +16,12 @@ #define BAZEL_SRC_MAIN_CPP_BLAZE_UTIL_PLATFORM_H_ #include <cinttypes> +#include <map> #include <string> #include <vector> #include "src/main/cpp/util/port.h" +#include "src/main/cpp/blaze_util.h" namespace blaze { @@ -101,6 +103,7 @@ class BlazeServerStartup { virtual bool IsStillAlive() = 0; }; + // Starts a daemon process with its standard output and standard error // redirected (and conditionally appended) to the file "daemon_output". Sets // server_startup to an object that can be used to query if the server is @@ -109,6 +112,7 @@ class BlazeServerStartup { // caller. int ExecuteDaemon(const std::string& exe, const std::vector<std::string>& args_vector, + const std::map<std::string, EnvVarValue>& env, const std::string& daemon_output, const bool daemon_output_append, const std::string& server_dir, @@ -184,6 +188,8 @@ void CreateSecureOutputRoot(const std::string& path); std::string GetEnv(const std::string& name); +bool ExistsEnv(const std::string& name); + void SetEnv(const std::string& name, const std::string& value); void UnsetEnv(const std::string& name); diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc index a0ce475479..4f8d9c766c 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -332,6 +332,7 @@ void WriteSystemSpecificProcessIdentifier( // localized here. int ExecuteDaemon(const string& exe, const std::vector<string>& args_vector, + const std::map<string, EnvVarValue>& env, const string& daemon_output, const bool daemon_output_append, const string& server_dir, @@ -379,24 +380,26 @@ int ExecuteDaemon(const string& exe, // before ExecuteDaemon() to understand why. close(fds[0]); // ...child keeps the other. - Daemonize(daemon_output_chars, daemon_output_append); - - pid_t server_pid = getpid(); - WriteToFdWithRetryEintr(fds[1], &server_pid, sizeof server_pid, - "cannot communicate server PID to client"); - // We wait until the client writes the PID file so that there is no race - // condition; the server expects the PID file to already be there so that - // it can read it and know its own PID (see the ctor GrpcServerImpl) and so - // that it can kill itself if the PID file is deleted (see - // GrpcServerImpl.PidFileWatcherThread) - char dummy; - ReadFromFdWithRetryEintr( - fds[1], &dummy, 1, - "cannot get PID file write acknowledgement from client"); - - execv(exe_chars, const_cast<char**>(argv)); - DieAfterFork("Cannot execute daemon"); - return -1; + { + WithEnvVars env_obj(env); + Daemonize(daemon_output_chars, daemon_output_append); + pid_t server_pid = getpid(); + WriteToFdWithRetryEintr(fds[1], &server_pid, sizeof server_pid, + "cannot communicate server PID to client"); + // We wait until the client writes the PID file so that there is no race + // condition; the server expects the PID file to already be there so that + // it can read it and know its own PID (see the ctor GrpcServerImpl) and + // so that it can kill itself if the PID file is deleted (see + // GrpcServerImpl.PidFileWatcherThread) + char dummy; + ReadFromFdWithRetryEintr( + fds[1], &dummy, 1, + "cannot get PID file write acknowledgement from client"); + + execv(exe_chars, const_cast<char**>(argv)); + DieAfterFork("Cannot execute daemon"); + return -1; + } } } @@ -461,6 +464,10 @@ string GetEnv(const string& name) { return result != NULL ? string(result) : ""; } +bool ExistsEnv(const string& name) { + return getenv(name.c_str()) != NULL; +} + void SetEnv(const string& name, const string& value) { setenv(name.c_str(), value.c_str(), 1); } diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc index 924ceb0fe9..9916d04b6a 100644 --- a/src/main/cpp/blaze_util_windows.cc +++ b/src/main/cpp/blaze_util_windows.cc @@ -432,8 +432,11 @@ class ProcessHandleBlazeServerStartup : public BlazeServerStartup { }; -int ExecuteDaemon(const string& exe, const std::vector<string>& args_vector, - const string& daemon_output, const bool daemon_out_append, +int ExecuteDaemon(const string& exe, + const std::vector<string>& args_vector, + const std::map<string, EnvVarValue>& env, + const string& daemon_output, + const bool daemon_out_append, const string& server_dir, BlazeServerStartup** server_startup) { wstring wdaemon_output; @@ -512,18 +515,23 @@ int ExecuteDaemon(const string& exe, const std::vector<string>& args_vector, CmdLine cmdline; CreateCommandLine(&cmdline, exe, args_vector); - BOOL ok = CreateProcessA( - /* lpApplicationName */ NULL, - /* lpCommandLine */ cmdline.cmdline, - /* lpProcessAttributes */ NULL, - /* lpThreadAttributes */ NULL, - /* bInheritHandles */ TRUE, - /* dwCreationFlags */ DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | - EXTENDED_STARTUPINFO_PRESENT, - /* lpEnvironment */ NULL, - /* lpCurrentDirectory */ NULL, - /* lpStartupInfo */ &startupInfoEx.StartupInfo, - /* lpProcessInformation */ &processInfo); + BOOL ok; + { + WithEnvVars env_obj(env); + + ok = CreateProcessA( + /* lpApplicationName */ NULL, + /* lpCommandLine */ cmdline.cmdline, + /* lpProcessAttributes */ NULL, + /* lpThreadAttributes */ NULL, + /* bInheritHandles */ TRUE, + /* dwCreationFlags */ DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | + EXTENDED_STARTUPINFO_PRESENT, + /* lpEnvironment */ NULL, + /* lpCurrentDirectory */ NULL, + /* lpStartupInfo */ &startupInfoEx.StartupInfo, + /* lpProcessInformation */ &processInfo); + } if (!ok) { BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) @@ -816,6 +824,10 @@ string GetEnv(const string& name) { return string(value.get()); } +bool ExistsEnv(const string& name) { + return ::GetEnvironmentVariableA(name.c_str(), NULL, 0) != 0; +} + void SetEnv(const string& name, const string& value) { // _putenv_s both calls ::SetEnvionmentVariableA and updates environ(5). _putenv_s(name.c_str(), value.c_str()); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index 8170ca2b14..14b05a721a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -55,6 +55,7 @@ import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.pkgcache.LoadingFailedException; import com.google.devtools.build.lib.runtime.BlazeCommand; import com.google.devtools.build.lib.runtime.BlazeCommandResult; +import com.google.devtools.build.lib.runtime.BlazeServerStartupOptions; import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.runtime.ProcessWrapperUtil; @@ -532,6 +533,20 @@ public class RunCommand implements BlazeCommand { } } + // We need to do update runEnvironment so that the environment of --batch is not contaminated + // with that required for the server. Note that some differences between the environment of + // the process being run and the environment of the client are still possible if the environment + // variables added for the server were not in the original client environment. + // + // This is done after writing the script for --script_path so that that is not contaminated + // with the original client environment (CommandFailureUtils.describeCommand() puts + // runEnvironment into the written script) + boolean batchMode = env.getRuntime().getStartupOptionsProvider() + .getOptions(BlazeServerStartupOptions.class).batch; + if (batchMode) { + runEnvironment.putAll(env.getClientEnv()); + } + env.getReporter().handle(Event.info( null, "Running command line: " + ShellEscaper.escapeJoinAll(cmdLine))); |