Propagate detected value of BAZEL_SH to --client_env.

Also adds some extra debug logging.
Also removes some msys-only code paths.

Fixes #3498.

This is a reland of commit 2f38404.

Change-Id: I1690a64f427070cc3f127b857650e6a32d1aab35
PiperOrigin-RevId: 166049222

3 files changed, 17 insertions, 25 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index ec5b8f3676..87a10571a7 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -1229,7 +1229,10 @@ static void ComputeBaseDirectories(const WorkspaceLayout *workspace_layout,
       blaze_util::JoinPath(globals->options->output_base, "server/jvm.out");
 }
 
-static void CheckEnvironmentOrDie() {
+// 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() {
   if (!blaze::GetEnv("http_proxy").empty()) {
     PrintWarning("ignoring http_proxy in environment.");
     blaze::UnsetEnv("http_proxy");
@@ -1271,8 +1274,6 @@ static void CheckEnvironmentOrDie() {
   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");
-
-  blaze::DetectBashOrDie();
 }
 
 static string CheckAndGetBinaryPath(const string &argv0) {
@@ -1341,13 +1342,19 @@ int Main(int argc, const char *argv[], WorkspaceLayout *workspace_layout,
 
   // Must be done before command line parsing.
   ComputeWorkspace(workspace_layout);
+
+  // Must be done before command line parsing.
+  // ParseOptions already populate --client_env, so detect bash before it
+  // happens.
+  DetectBashOrDie();
+
   globals->binary_path = CheckAndGetBinaryPath(argv[0]);
   ParseOptions(argc, argv);
 
   blaze::SetDebugLog(globals->options->client_debug);
   debug_log("Debug logging active");
 
-  CheckEnvironmentOrDie();
+  PrepareEnvironmentForJvm();
   blaze::CreateSecureOutputRoot(globals->options->output_user_root);
 
   const string self_path = GetSelfPath();
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index cffe06f350..42095e19c2 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -739,10 +739,6 @@ void ExecuteDaemon(const string& exe, const std::vector<string>& args_vector,
   startupInfo.dwFlags |= STARTF_USESTDHANDLES;
   CmdLine cmdline;
   CreateCommandLine(&cmdline, exe, args_vector);
-  // Propagate BAZEL_SH environment variable to a sub-process.
-  // TODO(dslomov): More principled approach to propagating
-  // environment variables.
-  SetEnvironmentVariableA("BAZEL_SH", getenv("BAZEL_SH"));
 
   BOOL ok = CreateProcessA(
       /* lpApplicationName */ NULL,
@@ -835,11 +831,6 @@ void ExecuteProgram(const string& exe, const std::vector<string>& args_vector) {
 
   PROCESS_INFORMATION processInfo = {0};
 
-  // Propagate BAZEL_SH environment variable to a sub-process.
-  // todo(dslomov): More principled approach to propagating
-  // environment variables.
-  SetEnvironmentVariableA("BAZEL_SH", getenv("BAZEL_SH"));
-
   HANDLE job = CreateJobObject(NULL, NULL);
   if (job == NULL) {
     pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
@@ -1151,17 +1142,8 @@ string GetEnv(const string& name) {
 }
 
 void SetEnv(const string& name, const string& value) {
-  if (value.empty()) {
-    ::SetEnvironmentVariableA(name.c_str(), NULL);
-#ifndef COMPILER_MSVC
-    unsetenv(name.c_str());
-#endif  // not COMPILER_MSVC
-  } else {
-    ::SetEnvironmentVariableA(name.c_str(), value.c_str());
-#ifndef COMPILER_MSVC
-    setenv(name.c_str(), value.c_str(), 1);
-#endif  // not COMPILER_MSVC
-  }
+  // _putenv_s both calls ::SetEnvionmentVariableA and updates environ(5).
+  _putenv_s(name.c_str(), value.c_str());
 }
 
 void UnsetEnv(const string& name) { SetEnv(name, ""); }
@@ -1698,9 +1680,11 @@ void DetectBashOrDie() {
 
   string bash = LocateBash();
   uint64_t end = blaze::GetMillisecondsMonotonic();
-  debug_log("BAZEL_SH detection took %lu msec", end - start);
+  debug_log("BAZEL_SH detection took %lu msec, found %s", end - start,
+            bash.c_str());
 
   if (!bash.empty()) {
+    // Set process environment variable.
     blaze::SetEnv("BAZEL_SH", bash);
   } else {
     printf(
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 325e8ce881..170103c3eb 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -531,6 +531,7 @@ std::vector<std::string> OptionProcessor::GetBlazercAndEnvCommandArgs(
     string env_str(*env);
     if (IsValidEnvName(*env)) {
       PreprocessEnvString(&env_str);
+      debug_log("--client_env=%s", env_str.c_str());
       result.push_back("--client_env=" + env_str);
     }
   }