Env.vars: server won't ignore the client env

Make the --ignore_client_env flag a no-op.
The client will pass --client_env flags to the
server even in --batch mode. This simplifies the
code as well as ensuring that the server always
uses the up-do-date client environment.

We'll gradually get rid of all System.getenv calls
in the server, because the server should always
respect the client env.

Roll forward of 149403129 with fixes.

--
PiperOrigin-RevId: 149435060
MOS_MIGRATED_REVID=149435060

4 files changed, 24 insertions, 26 deletions

diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index cfd390d003..d61587d3ae 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -372,7 +372,7 @@ blaze_exit_code::ExitCode OptionProcessor::ParseOptions(
 
   command_ = args[startup_args_ + 1];
 
-  AddRcfileArgsAndOptions(parsed_startup_options_->batch, cwd);
+  AddRcfileArgsAndOptions(cwd);
   for (unsigned int cmd_arg = startup_args_ + 2;
        cmd_arg < args.size(); cmd_arg++) {
     command_arguments_.push_back(args[cmd_arg]);
@@ -463,7 +463,7 @@ blaze_exit_code::ExitCode OptionProcessor::ParseStartupOptions(string *error) {
 // and also splices in some additional terminal and environment options between
 // the command and the arguments. NB: Keep the options added here in sync with
 // BlazeCommandDispatcher.INTERNAL_COMMAND_OPTIONS!
-void OptionProcessor::AddRcfileArgsAndOptions(bool batch, const string& cwd) {
+void OptionProcessor::AddRcfileArgsAndOptions(const string& cwd) {
   // Provide terminal options as coming from the least important rc file.
   command_arguments_.push_back("--rc_source=client");
   command_arguments_.push_back("--default_override=0:common=--isatty=" +
@@ -495,25 +495,21 @@ void OptionProcessor::AddRcfileArgsAndOptions(bool batch, const string& cwd) {
     }
   }
 
-  // Pass the client environment to the server in server mode.
-  if (batch) {
-    command_arguments_.push_back("--ignore_client_env");
-  } else {
-    for (char** env = environ; *env != NULL; env++) {
-      string env_str(*env);
-      int pos = env_str.find("=");
-      if (pos != string::npos) {
-        string name = env_str.substr(0, pos);
-        if (name == "PATH") {
-          env_str = "PATH=" + ConvertPathList(env_str.substr(pos + 1));
-        } else if (name == "TMP") {
-          // A valid Windows path "c:/foo" is also a valid Unix path list of
-          // ["c", "/foo"] so must use ConvertPath here. See GitHub issue #1684.
-          env_str = "TMP=" + ConvertPath(env_str.substr(pos + 1));
-        }
+  // Pass the client environment to the server.
+  for (char** env = environ; *env != NULL; env++) {
+    string env_str(*env);
+    int pos = env_str.find("=");
+    if (pos != string::npos) {
+      string name = env_str.substr(0, pos);
+      if (name == "PATH") {
+        env_str = "PATH=" + ConvertPathList(env_str.substr(pos + 1));
+      } else if (name == "TMP") {
+        // A valid Windows path "c:/foo" is also a valid Unix path list of
+        // ["c", "/foo"] so must use ConvertPath here. See GitHub issue #1684.
+        env_str = "TMP=" + ConvertPath(env_str.substr(pos + 1));
       }
-      command_arguments_.push_back("--client_env=" + env_str);
     }
+    command_arguments_.push_back("--client_env=" + env_str);
   }
   command_arguments_.push_back("--client_cwd=" + blaze::ConvertPath(cwd));
 
diff --git a/src/main/cpp/option_processor.h b/src/main/cpp/option_processor.h
index 8ea705816a..84866fdb81 100644
--- a/src/main/cpp/option_processor.h
+++ b/src/main/cpp/option_processor.h
@@ -142,7 +142,7 @@ class OptionProcessor {
     int index_;
   };
 
-  void AddRcfileArgsAndOptions(bool batch, const std::string& cwd);
+  void AddRcfileArgsAndOptions(const std::string& cwd);
   blaze_exit_code::ExitCode ParseStartupOptions(std::string* error);
 
   std::vector<RcFile*> blazercs_;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index f981715f8a..84d5a20825 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -206,11 +206,10 @@ public final class CommandEnvironment {
   }
 
   @VisibleForTesting
-  void updateClientEnv(List<Map.Entry<String, String>> clientEnvList, boolean ignoreClientEnv) {
+  void updateClientEnv(List<Map.Entry<String, String>> clientEnvList) {
     Preconditions.checkState(clientEnv.isEmpty());
 
-    Collection<Map.Entry<String, String>> env =
-        ignoreClientEnv ? System.getenv().entrySet() : clientEnvList;
+    Collection<Map.Entry<String, String>> env = clientEnvList;
     for (Map.Entry<String, String> entry : env) {
       clientEnv.put(entry.getKey(), entry.getValue());
     }
@@ -557,7 +556,7 @@ public final class CommandEnvironment {
     this.relativeWorkingDirectory = workingDirectory.relativeTo(workspace);
     this.workingDirectory = workingDirectory;
 
-    updateClientEnv(options.clientEnv, options.ignoreClientEnv);
+    updateClientEnv(options.clientEnv);
 
     // Fail fast in the case where a Blaze command forgets to install the package path correctly.
     skyframeExecutor.setActive(false);
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 fb521aee96..2afb774217 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
@@ -20,7 +20,6 @@ import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParsingException;
-
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
@@ -128,7 +127,11 @@ public class CommonCommandOptions extends OptionsBase {
   @Option(name = "ignore_client_env",
       defaultValue = "false",
       category = "hidden",
-      help = "If true, ignore the '--client_env' flag, and use the JVM environment instead")
+      deprecationWarning = "Deprecated, no-op.",
+      help = "Deprecated, no-op."
+  )
+  // TODO(laszlocsomor, dslomov) 2017-03-07: remove this flag after 2017-06-01 (~3 months from now)
+  // and all of its occurrences.
   public boolean ignoreClientEnv;
 
   @Option(name = "client_cwd",