diff --git a/src/ray/util/filesystem.cc b/src/ray/util/filesystem.cc index ae961083c..4e1c119e5 100644 --- a/src/ray/util/filesystem.cc +++ b/src/ray/util/filesystem.cc @@ -24,49 +24,10 @@ namespace ray { -std::string GetExeSuffix() { - std::string result; -#ifdef _WIN32 - result = ".exe"; -#endif - return result; -} - std::string GetFileName(const std::string &path) { - size_t i = GetRootPathLength(path), j = path.size(); - while (j > i && !IsDirSep(path[j - 1])) { - --j; - } - return path.substr(j); + return std::filesystem::path(path).filename().string(); } -size_t GetRootPathLength(const std::string &path) { - size_t i = 0; -#ifdef _WIN32 - if (i + 2 < path.size() && IsDirSep(path[i]) && IsDirSep(path[i + 1]) && - !IsDirSep(path[i + 2])) { - // UNC paths begin with two separators (but not 1 or 3) - i += 2; - for (int k = 0; k < 2; ++k) { - while (i < path.size() && !IsDirSep(path[i])) { - ++i; - } - while (i < path.size() && IsDirSep(path[i])) { - ++i; - } - } - } else if (i + 1 < path.size() && path[i + 1] == ':') { - i += 2; - } -#endif - while (i < path.size() && IsDirSep(path[i])) { - ++i; - } - return i; -} - -std::string GetRayTempDir() { return JoinPaths(GetUserTempDir(), "ray"); } - std::string GetUserTempDir() { std::string result; #if defined(__APPLE__) || defined(__linux__) diff --git a/src/ray/util/filesystem.h b/src/ray/util/filesystem.h index 32db3243a..995bf5586 100644 --- a/src/ray/util/filesystem.h +++ b/src/ray/util/filesystem.h @@ -14,6 +14,7 @@ #pragma once +#include #include #include @@ -25,65 +26,36 @@ namespace ray { /// \return The portable directory separator (slash on all OSes). static inline char GetAltDirSep() { return '/'; } -/// \return The platform directory separator (backslash on Windows, slash on other OSes). -static inline char GetDirSep() { - char result; -#ifdef _WIN32 - result = '\\'; -#else - result = '/'; -#endif - return result; -} - -/// \return The platform PATH separator (semicolon on Windows, colon on other OSes). -static inline char GetPathSep() { - char result; -#ifdef _WIN32 - result = ';'; -#else - result = ':'; -#endif - return result; -} - -/// Returns the executable binary suffix for the platform, if any. -std::string GetExeSuffix(); - /// Equivalent to Python's os.path.basename() for file system paths. std::string GetFileName(const std::string &path); -size_t GetRootPathLength(const std::string &path); - -/// \return A non-volatile temporary directory in which Ray can stores its files. -std::string GetRayTempDir(); - /// \return The non-volatile temporary directory for the current user (often /tmp). std::string GetUserTempDir(); /// \return Whether or not the given character is a directory separator on this platform. static inline bool IsDirSep(char ch) { - bool result = ch == GetDirSep(); + bool result = ch == std::filesystem::path::preferred_separator; #ifdef _WIN32 result |= ch == GetAltDirSep(); #endif return result; } -/// \return Whether or not the given character is a PATH separator on this platform. -static inline bool IsPathSep(char ch) { return ch == GetPathSep(); } - /// \return The result of joining multiple path components. template -std::string JoinPaths(std::string base, Paths... components) { - std::string to_append[] = {components...}; - for (size_t i = 0; i < sizeof(to_append) / sizeof(*to_append); ++i) { - const std::string &s = to_append[i]; - if (!base.empty() && !IsDirSep(base.back()) && !s.empty() && !IsDirSep(s[0])) { - base += GetDirSep(); +std::string JoinPaths(std::string base, const Paths &...components) { + auto join = [](auto &joined_path, const auto &component) { + // if the components begin with "/" or "////", just get the path name. + if (!component.empty() && + component.front() == std::filesystem::path::preferred_separator) { + joined_path = std::filesystem::path(joined_path) + .append(std::filesystem::path(component).filename().string()) + .string(); + } else { + joined_path = std::filesystem::path(joined_path).append(component).string(); } - base += s; - } + }; + (join(base, std::string_view(components)), ...); return base; } } // namespace ray diff --git a/src/ray/util/filesystem_test.cc b/src/ray/util/filesystem_test.cc index 3434f756a..179839af9 100644 --- a/src/ray/util/filesystem_test.cc +++ b/src/ray/util/filesystem_test.cc @@ -18,6 +18,21 @@ namespace ray { +namespace testing { +template +std::string JoinPaths(std::string base, Paths... components) { + std::string to_append[] = {components...}; + for (size_t i = 0; i < sizeof(to_append) / sizeof(*to_append); ++i) { + const std::string &s = to_append[i]; + if (!base.empty() && !IsDirSep(base.back()) && !s.empty() && !IsDirSep(s[0])) { + base += std::filesystem::path::preferred_separator; + } + base += s; + } + return base; +} +} // namespace testing + TEST(FileSystemTest, PathParseTest) { ASSERT_EQ(GetFileName("."), "."); ASSERT_EQ(GetFileName(".."), ".."); @@ -32,6 +47,22 @@ TEST(FileSystemTest, PathParseTest) { #endif } +TEST(FileSystemTest, JoinPathTest) { +#ifdef _WIN32 + auto old_path = + testing::JoinPaths(GetUserTempDir(), "hello", "\\subdir", "more", "", "last/"); + auto new_path = + ray::JoinPaths(GetUserTempDir(), "hello", "\\subdir", "more", "", "last/"); + ASSERT_EQ(old_path, new_path); +#else + auto old_path = + testing::JoinPaths(GetUserTempDir(), "hello", "/subdir", "more", "", "last/"); + auto new_path = + ray::JoinPaths(GetUserTempDir(), "hello", "/subdir", "more", "", "last/"); + ASSERT_EQ(old_path, new_path); +#endif +} + } // namespace ray int main(int argc, char **argv) { diff --git a/src/ray/util/logging.cc b/src/ray/util/logging.cc index b2bb97156..ed9beffab 100644 --- a/src/ray/util/logging.cc +++ b/src/ray/util/logging.cc @@ -201,10 +201,6 @@ void RayLog::StartRayLog(const std::string &app_name, RayLogLevel severity_thres if (!log_dir_.empty()) { // Enable log file if log_dir_ is not empty. - std::string dir_ends_with_slash = log_dir_; - if (!ray::IsDirSep(log_dir_[log_dir_.length() - 1])) { - dir_ends_with_slash += ray::GetDirSep(); - } std::string app_name_without_path = app_name; if (app_name.empty()) { app_name_without_path = "DefaultApp"; @@ -247,7 +243,7 @@ void RayLog::StartRayLog(const std::string &app_name, RayLogLevel severity_thres spdlog::drop(RayLog::GetLoggerName()); } auto file_sink = std::make_shared( - dir_ends_with_slash + app_name_without_path + "_" + std::to_string(pid) + ".log", + JoinPaths(log_dir_, app_name_without_path + "_" + std::to_string(pid) + ".log"), log_rotation_max_size_, log_rotation_file_num_); sinks.push_back(file_sink); } else { diff --git a/src/ray/util/logging_test.cc b/src/ray/util/logging_test.cc index fc7c555e4..217166866 100644 --- a/src/ray/util/logging_test.cc +++ b/src/ray/util/logging_test.cc @@ -190,7 +190,7 @@ TEST(PrintLogTest, TestRayLogEveryMs) { TEST(PrintLogTest, LogTestWithInit) { // Test empty app name. - RayLog::StartRayLog("", RayLogLevel::DEBUG, ray::GetUserTempDir() + ray::GetDirSep()); + RayLog::StartRayLog("", RayLogLevel::DEBUG, ray::GetUserTempDir()); PrintLog(); RayLog::ShutDownRayLog(); } @@ -198,8 +198,8 @@ TEST(PrintLogTest, LogTestWithInit) { // This test will output large amount of logs to stderr, should be disabled in travis. TEST(LogPerfTest, PerfTest) { RayLog::StartRayLog("/fake/path/to/appdire/LogPerfTest", RayLogLevel::ERROR, - ray::GetUserTempDir() + ray::GetDirSep()); - int rounds = 100000; + ray::GetUserTempDir()); + int rounds = 10; int64_t start_time = current_time_ms(); for (int i = 0; i < rounds; ++i) {