Skip to content

Commit

Permalink
Env.vars: server won't ignore the client env
Browse files Browse the repository at this point in the history
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.

--
PiperOrigin-RevId: 149403129
MOS_MIGRATED_REVID=149403129
  • Loading branch information
laszlocsomor authored and dslomov committed Mar 7, 2017
1 parent 70cbebe commit 94d8f4e
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 30 deletions.
1 change: 0 additions & 1 deletion scripts/bootstrap/compile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ function run_bazel_jar() {
--ignore_unsupported_sandboxing \
--startup_time=329 --extract_data_time=523 \
--rc_source=/dev/null --isatty=1 \
--ignore_client_env \
--client_cwd=${PWD} \
"${@}"
}
33 changes: 14 additions & 19 deletions src/main/cpp/option_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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=" +
Expand Down Expand Up @@ -495,25 +495,20 @@ 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));
}
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));

Expand Down
2 changes: 1 addition & 1 deletion src/main/cpp/option_processor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,10 @@ public Map<String, String> getWhitelistedClientEnv() {
}

@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());
}
Expand Down Expand Up @@ -557,7 +556,7 @@ void beforeCommand(Command command, OptionsParser optionsParser,
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
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;
Expand Down Expand Up @@ -125,10 +124,14 @@ public String getTypeDescription() {
help = "A system-generated parameter which specifies the client's environment")
public List<Map.Entry<String, String>> clientEnv;

@Option(name = "ignore_client_env",
defaultValue = "false",
category = "hidden",
help = "If true, ignore the '--client_env' flag, and use the JVM environment instead")
@Option(
name = "ignore_client_env",
defaultValue = "false",
category = "hidden",
help = "Deprecated, no-op."
)
// TODO(laszlocsomor) 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",
Expand Down

0 comments on commit 94d8f4e

Please sign in to comment.