Bazel client: add and use blaze::GetPathEnv()
Use GetPathEnv() instead of GetEnv() for envvars
with paths.
On Linux/macOS/POSIX, GetPathEnv and GetEnv do the
same.
On Windows, GetPathEnv removes the UNC prefix from
the result, calls AsWindowsPath, then converts
backslashes to forward slashes. (As callers expect
the result.)
Fixes https://github.com/bazelbuild/bazel/issues/7705
Closes #7707.
PiperOrigin-RevId: 238236143
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 4acd627d..0d10e0c 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -1440,7 +1440,7 @@
result[var] = EnvVarValue(EnvVarAction::SET, value);
}
- if (!blaze::GetEnv("LD_ASSUME_KERNEL").empty()) {
+ if (blaze::ExistsEnv("LD_ASSUME_KERNEL")) {
// Fix for bug: if ulimit -s and LD_ASSUME_KERNEL are both
// specified, the JVM fails to create threads. See thread_stack_regtest.
// This is also provoked by LD_LIBRARY_PATH=/usr/lib/debug,
@@ -1449,12 +1449,12 @@
result["LD_ASSUME_KERNEL"] = EnvVarValue(EnvVarAction::UNSET, "");
}
- if (!blaze::GetEnv("LD_PRELOAD").empty()) {
+ if (blaze::ExistsEnv("LD_PRELOAD")) {
BAZEL_LOG(WARNING) << "ignoring LD_PRELOAD in environment.";
result["LD_PRELOAD"] = EnvVarValue(EnvVarAction::UNSET, "");
}
- if (!blaze::GetEnv("_JAVA_OPTIONS").empty()) {
+ if (blaze::ExistsEnv("_JAVA_OPTIONS")) {
// This would override --host_jvm_args
BAZEL_LOG(WARNING) << "ignoring _JAVA_OPTIONS in environment.";
result["_JAVA_OPTIONS"] = EnvVarValue(EnvVarAction::UNSET, "");
diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc
index b35af9b..ce48149 100644
--- a/src/main/cpp/blaze_util.cc
+++ b/src/main/cpp/blaze_util.cc
@@ -181,7 +181,7 @@
}
}
-bool IsRunningWithinTest() { return !GetEnv("TEST_TMPDIR").empty(); }
+bool IsRunningWithinTest() { return ExistsEnv("TEST_TMPDIR"); }
void WithEnvVars::SetEnvVars(const map<string, EnvVarValue>& vars) {
for (const auto& var : vars) {
diff --git a/src/main/cpp/blaze_util_darwin.cc b/src/main/cpp/blaze_util_darwin.cc
index d20169c..aadee1b 100644
--- a/src/main/cpp/blaze_util_darwin.cc
+++ b/src/main/cpp/blaze_util_darwin.cc
@@ -161,7 +161,7 @@
}
string GetSystemJavabase() {
- string java_home = GetEnv("JAVA_HOME");
+ string java_home = GetPathEnv("JAVA_HOME");
if (!java_home.empty()) {
string javac = blaze_util::JoinPath(java_home, "bin/javac");
if (access(javac.c_str(), X_OK) == 0) {
diff --git a/src/main/cpp/blaze_util_freebsd.cc b/src/main/cpp/blaze_util_freebsd.cc
index b972168..9cb89bd 100644
--- a/src/main/cpp/blaze_util_freebsd.cc
+++ b/src/main/cpp/blaze_util_freebsd.cc
@@ -140,7 +140,7 @@
string GetSystemJavabase() {
// if JAVA_HOME is defined, then use it as default.
- string javahome = GetEnv("JAVA_HOME");
+ string javahome = GetPathEnv("JAVA_HOME");
if (!javahome.empty()) {
string javac = blaze_util::JoinPath(javahome, "bin/javac");
diff --git a/src/main/cpp/blaze_util_linux.cc b/src/main/cpp/blaze_util_linux.cc
index a365ac9..e6e6d64 100644
--- a/src/main/cpp/blaze_util_linux.cc
+++ b/src/main/cpp/blaze_util_linux.cc
@@ -130,7 +130,7 @@
}
static string Which(const string &executable) {
- string path(GetEnv("PATH"));
+ string path(GetPathEnv("PATH"));
if (path.empty()) {
return "";
}
@@ -154,7 +154,7 @@
string GetSystemJavabase() {
// if JAVA_HOME is defined, then use it as default.
- string javahome = GetEnv("JAVA_HOME");
+ string javahome = GetPathEnv("JAVA_HOME");
if (!javahome.empty()) {
string javac = blaze_util::JoinPath(javahome, "bin/javac");
if (access(javac.c_str(), X_OK) == 0) {
diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h
index 3686aa7..56d815b 100644
--- a/src/main/cpp/blaze_util_platform.h
+++ b/src/main/cpp/blaze_util_platform.h
@@ -213,6 +213,8 @@
std::string GetEnv(const std::string& name);
+std::string GetPathEnv(const std::string& name);
+
bool ExistsEnv(const std::string& name);
void SetEnv(const std::string& name, const std::string& value);
diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc
index c420658..04e8696 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -206,7 +206,7 @@
return ToString(getpid());
}
-string GetHomeDir() { return GetEnv("HOME"); }
+string GetHomeDir() { return GetPathEnv("HOME"); }
string GetJavaBinaryUnderJavabase() { return "bin/java"; }
@@ -459,6 +459,8 @@
return result != NULL ? string(result) : "";
}
+string GetPathEnv(const string& name) { return GetEnv(name); }
+
bool ExistsEnv(const string& name) {
return getenv(name.c_str()) != NULL;
}
@@ -676,12 +678,11 @@
bool IsEmacsTerminal() {
string emacs = GetEnv("EMACS");
- string inside_emacs = GetEnv("INSIDE_EMACS");
// GNU Emacs <25.1 (and ~all non-GNU emacsen) set EMACS=t, but >=25.1 doesn't
// do that and instead sets INSIDE_EMACS=<stuff> (where <stuff> can look like
// e.g. "25.1.1,comint"). So we check both variables for maximum
// compatibility.
- return emacs == "t" || !inside_emacs.empty();
+ return emacs == "t" || ExistsEnv("INSIDE_EMACS");
}
// Returns true if stderr is connected to a terminal, and it can support color
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index a89a9af..cf91e12 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -409,7 +409,7 @@
if (IsRunningWithinTest()) {
// Bazel is running inside of a test. Respect $HOME that the test setup has
// set instead of using the actual home directory of the current user.
- return GetEnv("HOME");
+ return GetPathEnv("HOME");
}
PWSTR wpath;
@@ -424,12 +424,12 @@
// On Windows 2016 Server, Nano server: FOLDERID_Profile is unknown but
// %USERPROFILE% is set. See https://github.com/bazelbuild/bazel/issues/6701
- string userprofile = GetEnv("USERPROFILE");
+ string userprofile = GetPathEnv("USERPROFILE");
if (!userprofile.empty()) {
return userprofile;
}
- return GetEnv("HOME"); // only defined in MSYS/Cygwin
+ return GetPathEnv("HOME"); // only defined in MSYS/Cygwin
}
string FindSystemWideBlazerc() {
@@ -458,7 +458,7 @@
}
string GetSystemJavabase() {
- string javahome(GetEnv("JAVA_HOME"));
+ string javahome(GetPathEnv("JAVA_HOME"));
if (!javahome.empty()) {
string javac = blaze_util::JoinPath(javahome, "bin/javac.exe");
if (blaze_util::PathExists(javac.c_str())) {
@@ -1002,6 +1002,24 @@
return string(value.get());
}
+string GetPathEnv(const string& name) {
+ string value = GetEnv(name);
+ if (value.empty()) {
+ return value;
+ }
+ if (bazel::windows::HasUncPrefix(value.c_str())) {
+ value = value.substr(4);
+ }
+ string wpath, error;
+ if (!blaze_util::AsWindowsPath(value, &wpath, &error)) {
+ BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
+ << "Invalid path in envvar \"" << name << "\": " << error;
+ }
+ // Callers of GetPathEnv expect a path with forward slashes.
+ std::replace(wpath.begin(), wpath.end(), '\\', '/');
+ return wpath;
+}
+
bool ExistsEnv(const string& name) {
return ::GetEnvironmentVariableA(name.c_str(), NULL, 0) != 0;
}
@@ -1218,12 +1236,11 @@
bool IsEmacsTerminal() {
string emacs = GetEnv("EMACS");
- string inside_emacs = GetEnv("INSIDE_EMACS");
// GNU Emacs <25.1 (and ~all non-GNU emacsen) set EMACS=t, but >=25.1 doesn't
// do that and instead sets INSIDE_EMACS=<stuff> (where <stuff> can look like
// e.g. "25.1.1,comint"). So we check both variables for maximum
// compatibility.
- return emacs == "t" || !inside_emacs.empty();
+ return emacs == "t" || ExistsEnv("INSIDE_EMACS");
}
// Returns true if stderr is connected to a terminal, and it can support color
@@ -1418,7 +1435,7 @@
static string GetBinaryFromPath(const string& binary_name) {
char found[MAX_PATH];
- string path_list = blaze::GetEnv("PATH");
+ string path_list = blaze::GetPathEnv("PATH");
// We do not fully replicate all the quirks of search in PATH.
// There is no system function to do so, and that way lies madness.
@@ -1458,7 +1475,7 @@
}
string DetectBashAndExportBazelSh() {
- string bash = blaze::GetEnv("BAZEL_SH");
+ string bash = blaze::GetPathEnv("BAZEL_SH");
if (!bash.empty()) {
return bash;
}
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index 9fc35fb..3d2cb48 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -98,7 +98,7 @@
original_startup_options_(std::vector<RcStartupFlag>()),
unlimit_coredumps(false) {
if (blaze::IsRunningWithinTest()) {
- output_root = blaze_util::MakeAbsolute(blaze::GetEnv("TEST_TMPDIR"));
+ output_root = blaze_util::MakeAbsolute(blaze::GetPathEnv("TEST_TMPDIR"));
max_idle_secs = 15;
BAZEL_LOG(USER) << "$TEST_TMPDIR defined: output root default is '"
<< output_root << "' and max_idle_secs default is '"
diff --git a/src/test/cpp/bazel_startup_options_test.cc b/src/test/cpp/bazel_startup_options_test.cc
index 18467a3..27d692f 100644
--- a/src/test/cpp/bazel_startup_options_test.cc
+++ b/src/test/cpp/bazel_startup_options_test.cc
@@ -32,7 +32,7 @@
// This knowingly ignores the possibility of these environment variables
// being unset because we expect our test runner to set them in all cases.
// Otherwise, we'll crash here, but this keeps our code simpler.
- old_test_tmpdir_ = GetEnv("TEST_TMPDIR");
+ old_test_tmpdir_ = GetPathEnv("TEST_TMPDIR");
ReinitStartupOptions();
}
diff --git a/src/test/cpp/blaze_util_windows_test.cc b/src/test/cpp/blaze_util_windows_test.cc
index f577b48..47d0aa9 100644
--- a/src/test/cpp/blaze_util_windows_test.cc
+++ b/src/test/cpp/blaze_util_windows_test.cc
@@ -96,27 +96,28 @@
}
TEST(BlazeUtilWindowsTest, TestGetEnv) {
- ASSERT_ENVVAR_UNSET("DOES_not_EXIST");
+#define _STR(x) #x
+#define STR(x) _STR(x)
+ const char* envvar = "BAZEL_TEST_" STR(__LINE__);
+#undef STR
+#undef _STR
- string actual(GetEnv("TEST_SRCDIR"));
- ASSERT_NE(actual, "");
+ ASSERT_TRUE(SetEnvironmentVariableA(envvar, "A\\B c"));
+ ASSERT_EQ(GetEnv(envvar), "A\\B c");
+}
- std::replace(actual.begin(), actual.end(), '/', '\\');
- ASSERT_NE(actual.find(":\\"), string::npos);
+TEST(BlazeUtilWindowsTest, TestGetPathEnv) {
+#define _STR(x) #x
+#define STR(x) _STR(x)
+ const char* envvar = "BAZEL_TEST_" STR(__LINE__);
+#undef STR
+#undef _STR
- ASSERT_ENVVAR_UNSET("Bazel_TEST_Key1");
- ASSERT_TRUE(::SetEnvironmentVariableA("Bazel_TEST_Key1", "some_VALUE"));
- ASSERT_ENVVAR("Bazel_TEST_Key1", "some_VALUE");
- ASSERT_TRUE(::SetEnvironmentVariableA("Bazel_TEST_Key1", NULL));
+ ASSERT_TRUE(SetEnvironmentVariableA(envvar, "A\\B c"));
+ ASSERT_EQ(GetPathEnv(envvar), "A/B c");
- string long_string(MAX_PATH, 'a');
- string long_key = string("Bazel_TEST_Key2_") + long_string;
- string long_value = string("Bazel_TEST_Value2_") + long_string;
-
- ASSERT_ENVVAR_UNSET(long_key.c_str());
- ASSERT_TRUE(::SetEnvironmentVariableA(long_key.c_str(), long_value.c_str()));
- ASSERT_ENVVAR(long_key, long_value);
- ASSERT_TRUE(::SetEnvironmentVariableA(long_key.c_str(), NULL));
+ ASSERT_TRUE(SetEnvironmentVariableA(envvar, "\\\\?\\A:\\B c"));
+ ASSERT_EQ(GetPathEnv(envvar), "A:/B c");
}
TEST(BlazeUtilWindowsTest, TestSetEnv) {
diff --git a/src/test/cpp/option_processor_test.cc b/src/test/cpp/option_processor_test.cc
index 461b425..e9f09cb 100644
--- a/src/test/cpp/option_processor_test.cc
+++ b/src/test/cpp/option_processor_test.cc
@@ -30,7 +30,7 @@
protected:
OptionProcessorTest()
: workspace_(
- blaze_util::JoinPath(blaze::GetEnv("TEST_TMPDIR"), "testdir")),
+ blaze_util::JoinPath(blaze::GetPathEnv("TEST_TMPDIR"), "testdir")),
cwd_("cwd"),
workspace_layout_(new WorkspaceLayout()) {}
diff --git a/src/test/cpp/rc_file_test.cc b/src/test/cpp/rc_file_test.cc
index 25baaaa..bfbdbea 100644
--- a/src/test/cpp/rc_file_test.cc
+++ b/src/test/cpp/rc_file_test.cc
@@ -41,11 +41,11 @@
class RcFileTest : public ::testing::Test {
protected:
RcFileTest()
- : workspace_(
- blaze_util::JoinPath(blaze::GetEnv("TEST_TMPDIR"), "workspace")),
- cwd_(blaze_util::JoinPath(blaze::GetEnv("TEST_TMPDIR"), "cwd")),
+ : workspace_(blaze_util::JoinPath(blaze::GetPathEnv("TEST_TMPDIR"),
+ "workspace")),
+ cwd_(blaze_util::JoinPath(blaze::GetPathEnv("TEST_TMPDIR"), "cwd")),
binary_dir_(
- blaze_util::JoinPath(blaze::GetEnv("TEST_TMPDIR"), "bazeldir")),
+ blaze_util::JoinPath(blaze::GetPathEnv("TEST_TMPDIR"), "bazeldir")),
binary_path_(blaze_util::JoinPath(binary_dir_, "bazel")),
workspace_layout_(new WorkspaceLayout()) {}
diff --git a/src/test/cpp/rc_options_test.cc b/src/test/cpp/rc_options_test.cc
index daaba7f..418639e 100644
--- a/src/test/cpp/rc_options_test.cc
+++ b/src/test/cpp/rc_options_test.cc
@@ -34,8 +34,7 @@
class RcOptionsTest : public ::testing::Test {
protected:
RcOptionsTest()
- : test_file_dir_(blaze::GetEnv("TEST_TMPDIR")),
- workspace_layout_() {}
+ : test_file_dir_(blaze::GetPathEnv("TEST_TMPDIR")), workspace_layout_() {}
const string test_file_dir_;
const WorkspaceLayout workspace_layout_;
diff --git a/src/test/cpp/startup_options_test.cc b/src/test/cpp/startup_options_test.cc
index 9a0b01c..0c2cf2b 100644
--- a/src/test/cpp/startup_options_test.cc
+++ b/src/test/cpp/startup_options_test.cc
@@ -47,7 +47,7 @@
// being unset because we expect our test runner to set them in all cases.
// Otherwise, we'll crash here, but this keeps our code simpler.
old_home_ = GetHomeDir();
- old_test_tmpdir_ = GetEnv("TEST_TMPDIR");
+ old_test_tmpdir_ = GetPathEnv("TEST_TMPDIR");
ReinitStartupOptions();
}
diff --git a/src/test/cpp/util/logging_test.cc b/src/test/cpp/util/logging_test.cc
index 7891a07..dce3623 100644
--- a/src/test/cpp/util/logging_test.cc
+++ b/src/test/cpp/util/logging_test.cc
@@ -40,7 +40,7 @@
// Set the value of $TMP first, because CaptureStderr retrieves a temp
// directory path and on Windows, the corresponding function (GetTempPathA)
// reads $TMP.
- blaze::SetEnv("TMP", blaze::GetEnv("TEST_TMPDIR"));
+ blaze::SetEnv("TMP", blaze::GetPathEnv("TEST_TMPDIR"));
}
void TearDown() { blaze_util::SetLogHandler(nullptr); }
};
@@ -530,7 +530,7 @@
TEST(LoggingDeathTest,
BazelLogHandler_CustomStream_BazelDiePrintsToStderrAndCustomStream) {
std::string logfile =
- blaze_util::JoinPath(blaze::GetEnv("TEST_TMPDIR"), "logfile");
+ blaze_util::JoinPath(blaze::GetPathEnv("TEST_TMPDIR"), "logfile");
ASSERT_EXIT(
{
diff --git a/src/test/cpp/workspace_layout_test.cc b/src/test/cpp/workspace_layout_test.cc
index 4e17d64..865005a 100644
--- a/src/test/cpp/workspace_layout_test.cc
+++ b/src/test/cpp/workspace_layout_test.cc
@@ -27,10 +27,10 @@
class WorkspaceLayoutTest : public ::testing::Test {
protected:
- WorkspaceLayoutTest() :
- build_root_(blaze_util::JoinPath(
- blaze::GetEnv("TEST_TMPDIR"), "build_root")),
- workspace_layout_(new WorkspaceLayout()) {}
+ WorkspaceLayoutTest()
+ : build_root_(blaze_util::JoinPath(blaze::GetPathEnv("TEST_TMPDIR"),
+ "build_root")),
+ workspace_layout_(new WorkspaceLayout()) {}
void SetUp() override {
ASSERT_TRUE(blaze_util::MakeDirectories(build_root_, 0755));
