blaze_util::ConvertPath should not make paths absolute.
It does not claim to, and this was already true for posix platforms. Windows platforms, however, always made the path absolute, which was a hard-to-diagnose difference between the two.
Similarly, MakeAbsolute was relying on this to be correct for windows, so this change splits the implementation and keeps the behavior consistent. While we're here, also remove the empty-string behavior from MakeAbsolute, and instead make it clear at all sites that this behavior is present and affects accepted flag syntax. We may want to remove this later.
RELNOTES: None.
PiperOrigin-RevId: 199663395
diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc
index 10c4e1a..d8ec359 100644
--- a/src/main/cpp/blaze_util.cc
+++ b/src/main/cpp/blaze_util.cc
@@ -28,6 +28,7 @@
#include "src/main/cpp/util/file.h"
#include "src/main/cpp/util/logging.h"
#include "src/main/cpp/util/numbers.h"
+#include "src/main/cpp/util/path_platform.h"
#include "src/main/cpp/util/port.h"
#include "src/main/cpp/util/strings.h"
@@ -118,6 +119,14 @@
&& (arg != "-help") && (arg != "-h");
}
+std::string AbsolutePathFromFlag(const std::string& value) {
+ if (value.empty()) {
+ return blaze_util::GetCwd();
+ } else {
+ return blaze_util::MakeAbsolute(value);
+ }
+}
+
void LogWait(unsigned int elapsed_seconds, unsigned int wait_seconds) {
SigPrintf("WARNING: Waiting for server process to terminate "
"(waited %d seconds, waiting at most %d)\n",
diff --git a/src/main/cpp/blaze_util.h b/src/main/cpp/blaze_util.h
index 084e8d6..b9f5a51 100644
--- a/src/main/cpp/blaze_util.h
+++ b/src/main/cpp/blaze_util.h
@@ -61,6 +61,12 @@
// Returns true iff arg is a valid command line argument for bazel.
bool IsArg(const std::string& arg);
+// Returns the flag value as an absolute path. For legacy reasons, it accepts
+// the empty string as cwd.
+// TODO(b/109874628): Assess if removing the empty string case would break
+// legitimate uses, and if not, remove it.
+std::string AbsolutePathFromFlag(const std::string& value);
+
// Wait to see if the server process terminates. Checks the server's status
// immediately, and repeats the check every 100ms until approximately
// wait_seconds elapses or the server process terminates. Returns true if a
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index a1d7f9d..c2f480f 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -138,7 +138,7 @@
"." + parsed_startup_options_->GetLowercaseProductName() + "rc";
if (cmd_line_rc_file != nullptr) {
- string rcFile = blaze_util::MakeAbsolute(cmd_line_rc_file);
+ string rcFile = blaze::AbsolutePathFromFlag(cmd_line_rc_file);
if (!blaze_util::CanReadFile(rcFile)) {
blaze_util::StringPrintf(error,
"Error: Unable to read %s file '%s'.", rc_basename.c_str(),
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index 3699d40..67e0d55a 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -189,19 +189,19 @@
const char* value = NULL;
if ((value = GetUnaryOption(arg, next_arg, "--output_base")) != NULL) {
- output_base = blaze_util::MakeAbsolute(value);
+ output_base = blaze::AbsolutePathFromFlag(value);
option_sources["output_base"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg,
"--install_base")) != NULL) {
- install_base = blaze_util::MakeAbsolute(value);
+ install_base = blaze::AbsolutePathFromFlag(value);
option_sources["install_base"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg,
"--output_user_root")) != NULL) {
- output_user_root = blaze_util::MakeAbsolute(value);
+ output_user_root = blaze::AbsolutePathFromFlag(value);
option_sources["output_user_root"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg,
"--server_jvm_out")) != NULL) {
- server_jvm_out = blaze_util::MakeAbsolute(value);
+ server_jvm_out = blaze::AbsolutePathFromFlag(value);
option_sources["server_jvm_out"] = rcfile;
} else if (GetNullaryOption(arg, "--deep_execroot")) {
deep_execroot = true;
@@ -223,7 +223,7 @@
"--host_javabase")) != NULL) {
// TODO(bazel-team): Consider examining the javabase and re-execing in case
// of architecture mismatch.
- host_javabase = blaze_util::MakeAbsolute(value);
+ host_javabase = blaze::AbsolutePathFromFlag(value);
option_sources["host_javabase"] = rcfile;
} else if ((value = GetUnaryOption(arg, next_arg, "--host_jvm_args")) !=
NULL) {
diff --git a/src/main/cpp/util/path.cc b/src/main/cpp/util/path.cc
index efa10b8..b69a594 100644
--- a/src/main/cpp/util/path.cc
+++ b/src/main/cpp/util/path.cc
@@ -48,17 +48,4 @@
}
}
-std::string MakeAbsolute(const std::string &path) {
- std::string converted_path = ConvertPath(path);
- if (converted_path.empty()) {
- return GetCwd();
- }
- if (IsDevNull(converted_path.c_str()) ||
- blaze_util::IsAbsolute(converted_path)) {
- return converted_path;
- }
-
- return JoinPath(blaze_util::GetCwd(), converted_path);
-}
-
} // namespace blaze_util
diff --git a/src/main/cpp/util/path.h b/src/main/cpp/util/path.h
index 38e735b..23edd83 100644
--- a/src/main/cpp/util/path.h
+++ b/src/main/cpp/util/path.h
@@ -29,15 +29,6 @@
std::string JoinPath(const std::string &path1, const std::string &path2);
-// Returns the given path in absolute form. Does not change paths that are
-// already absolute.
-//
-// If called from working directory "/bar":
-// MakeAbsolute("foo") --> "/bar/foo"
-// MakeAbsolute("/foo") ---> "/foo"
-// MakeAbsolute("C:/foo") ---> "C:/foo"
-std::string MakeAbsolute(const std::string &path);
-
} // namespace blaze_util
#endif // BAZEL_SRC_MAIN_CPP_UTIL_PATH_H_
diff --git a/src/main/cpp/util/path_platform.h b/src/main/cpp/util/path_platform.h
index abb25dc..1f14bd6 100644
--- a/src/main/cpp/util/path_platform.h
+++ b/src/main/cpp/util/path_platform.h
@@ -45,6 +45,15 @@
// Returns true if `path` is absolute.
bool IsAbsolute(const std::string &path);
+// Returns the given path in absolute form. Does not change paths that are
+// already absolute.
+//
+// If called from working directory "/bar":
+// MakeAbsolute("foo") --> "/bar/foo"
+// MakeAbsolute("/foo") ---> "/foo"
+// MakeAbsolute("C:/foo") ---> "C:/foo"
+std::string MakeAbsolute(const std::string &path);
+
// TODO(bazel-team) consider changing the path(_platform) header split to be a
// path.h and path_windows.h split, which would make it clearer what functions
// are included by an import statement. The downside to this gain in clarity
diff --git a/src/main/cpp/util/path_posix.cc b/src/main/cpp/util/path_posix.cc
index bbeffca..267dc22 100644
--- a/src/main/cpp/util/path_posix.cc
+++ b/src/main/cpp/util/path_posix.cc
@@ -20,7 +20,9 @@
#include <unistd.h> // access, open, close, fsync
#include "src/main/cpp/util/errors.h"
#include "src/main/cpp/util/exit_code.h"
+#include "src/main/cpp/util/file_platform.h"
#include "src/main/cpp/util/logging.h"
+#include "src/main/cpp/util/path.h"
namespace blaze_util {
@@ -57,4 +59,11 @@
return !path.empty() && path[0] == '/';
}
+std::string MakeAbsolute(const std::string &path) {
+ if (blaze_util::IsAbsolute(path) || path.empty()) {
+ return path;
+ }
+
+ return JoinPath(blaze_util::GetCwd(), path);
+}
} // namespace blaze_util
diff --git a/src/main/cpp/util/path_windows.cc b/src/main/cpp/util/path_windows.cc
index d3756c7..041a0e8 100644
--- a/src/main/cpp/util/path_windows.cc
+++ b/src/main/cpp/util/path_windows.cc
@@ -62,11 +62,24 @@
std::string ConvertPath(const std::string& path) {
// The path may not be Windows-style and may not be normalized, so convert it.
+ std::string converted_path;
+ std::string error;
+ if (!blaze_util::AsWindowsPath(path, &converted_path, &error)) {
+ BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
+ << "ConvertPath(" << path << "): AsWindowsPath failed: " << error;
+ }
+ std::transform(converted_path.begin(), converted_path.end(),
+ converted_path.begin(), ::towlower);
+ return converted_path;
+}
+
+std::string MakeAbsolute(const std::string& path) {
+ // The path may not be Windows-style and may not be normalized, so convert it.
std::wstring wpath;
std::string error;
if (!AsAbsoluteWindowsPath(path, &wpath, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
- << "ConvertPath(" << path
+ << "MakeAbsolute(" << path
<< "): AsAbsoluteWindowsPath failed: " << error;
}
std::transform(wpath.begin(), wpath.end(), wpath.begin(), ::towlower);
diff --git a/src/test/cpp/util/path_posix_test.cc b/src/test/cpp/util/path_posix_test.cc
index 7d0e497..dd22f9b 100644
--- a/src/test/cpp/util/path_posix_test.cc
+++ b/src/test/cpp/util/path_posix_test.cc
@@ -155,8 +155,10 @@
EXPECT_EQ(MakeAbsolute("/foo/bar"), "/foo/bar");
EXPECT_EQ(MakeAbsolute("/foo/bar/"), "/foo/bar/");
EXPECT_EQ(MakeAbsolute("foo"), blaze_util::GetCwd() + "/foo");
- EXPECT_EQ(MakeAbsolute(std::string()), blaze_util::GetCwd());
+
EXPECT_EQ(MakeAbsolute("/dev/null"), "/dev/null");
+
+ EXPECT_EQ(MakeAbsolute(""), "");
}
} // namespace blaze_util
diff --git a/src/test/cpp/util/path_windows_test.cc b/src/test/cpp/util/path_windows_test.cc
index 789f67c..003fc9a 100644
--- a/src/test/cpp/util/path_windows_test.cc
+++ b/src/test/cpp/util/path_windows_test.cc
@@ -296,9 +296,6 @@
EXPECT_EQ("c:\\foo", ConvertPath("C:\\FOO"));
EXPECT_EQ("c:\\", ConvertPath("c:/"));
EXPECT_EQ("c:\\foo\\bar", ConvertPath("c:/../foo\\BAR\\.\\"));
- EXPECT_EQ("nul", MakeAbsolute("NUL"));
- EXPECT_EQ("nul", MakeAbsolute("nul"));
- EXPECT_EQ("nul", MakeAbsolute("/dev/null"));
}
TEST(PathWindowsTest, TestMakeAbsolute) {
@@ -308,11 +305,13 @@
EXPECT_EQ("c:\\foo\\bar", MakeAbsolute("C:/foo/bar/"));
EXPECT_EQ(blaze_util::AsLower(blaze_util::GetCwd()) + "\\foo",
MakeAbsolute("foo"));
+
EXPECT_EQ("nul", MakeAbsolute("NUL"));
EXPECT_EQ("nul", MakeAbsolute("Nul"));
EXPECT_EQ("nul", MakeAbsolute("nul"));
- EXPECT_EQ(blaze_util::AsLower(blaze_util::GetCwd()), MakeAbsolute(""));
EXPECT_EQ("nul", MakeAbsolute("/dev/null"));
+
+ EXPECT_EQ("", MakeAbsolute(""));
}
} // namespace blaze_util
