Keep LANG/LANGUAGE and related environment variables for the process spawned by "blaze run --direct_run".

RELNOTES: None.
PiperOrigin-RevId: 193391379

6 files changed, 152 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());