From 1d396c23d7e623e524e19f340c3edfff53f6873d Mon Sep 17 00:00:00 2001 From: Ashish Karale Date: Mon, 14 Oct 2024 09:37:10 -0700 Subject: [PATCH 1/4] Long Paths support for Windows --- include/triton/backend/backend_common.h | 3 ++ src/backend_common.cc | 48 +++++++++++++++++++++---- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/include/triton/backend/backend_common.h b/include/triton/backend/backend_common.h index bbc80dd..5ee2d31 100644 --- a/include/triton/backend/backend_common.h +++ b/include/triton/backend/backend_common.h @@ -504,6 +504,9 @@ TRITONSERVER_Error* CopyBuffer( void* dst, cudaStream_t cuda_stream, bool* cuda_used, const bool copy_on_stream = false); +TRITONSERVER_Error* getOSValidPath( + const std::string& _path, std::string& retPath); + /// Does a file or directory exist? /// \param path The path to check for existence. /// \param exists Returns true if file/dir exists diff --git a/src/backend_common.cc b/src/backend_common.cc index fb705aa..2576117 100644 --- a/src/backend_common.cc +++ b/src/backend_common.cc @@ -781,21 +781,56 @@ GetDirectoryContents(const std::string& path, std::set* contents) return nullptr; // success } +//! Converts incoming utf-8 path to an OS valid path +//! +//! On Linux there is not much to do but make sure correct slashes are used +//! On Windows we need to take care of the long paths and handle them correctly +//! to avoid legacy issues with MAX_PATH +//! +//! More details: +//! https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry +//! +TRITONSERVER_Error* +getOSValidPath(const std::string& _path, std::string& retPath) +{ + std::string path(_path); +#ifdef _WIN32 + constexpr const char* kWindowsLongPathPrefix = "\\\\?\\"; + // On Windows long paths must be marked correctly otherwise, due to backwards + // compatibility, all paths are limited to MAX_PATH length + if (path.size() >= MAX_PATH) { + // Must be prefixed with "\\?\" to be considered long path + if (path.substr(0, 4) != (kWindowsLongPathPrefix)) { + // Long path but not "tagged" correctly + path = (kWindowsLongPathPrefix) + path; + } + } + std::replace(path.begin(), path.end(), '/', '\\'); +#endif + retPath = path; + return nullptr; +} + TRITONSERVER_Error* FileExists(const std::string& path, bool* exists) { - *exists = (access(path.c_str(), F_OK) == 0); + std::string valid_path; + getOSValidPath(path, valid_path); + *exists = (access(valid_path.c_str(), F_OK) == 0); return nullptr; // success } TRITONSERVER_Error* ReadTextFile(const std::string& path, std::string* contents) { - std::ifstream in(path, std::ios::in | std::ios::binary); + std::string valid_path; + getOSValidPath(path, valid_path); + + std::ifstream in(valid_path, std::ios::in | std::ios::binary); if (!in) { return TRITONSERVER_ErrorNew( TRITONSERVER_ERROR_INTERNAL, - ("failed to open/read file '" + path + "': " + strerror(errno)) + ("failed to open/read file '" + valid_path + "': " + strerror(errno)) .c_str()); } @@ -812,12 +847,13 @@ TRITONSERVER_Error* IsDirectory(const std::string& path, bool* is_dir) { *is_dir = false; - + std::string valid_path; + getOSValidPath(path, valid_path); struct stat st; - if (stat(path.c_str(), &st) != 0) { + if (stat(valid_path.c_str(), &st) != 0) { return TRITONSERVER_ErrorNew( TRITONSERVER_ERROR_INTERNAL, - (std::string("failed to stat file ") + path).c_str()); + (std::string("failed to stat file ") + valid_path).c_str()); } *is_dir = S_ISDIR(st.st_mode); From f9ebbc8e91da03899eb334862deee173d8f5b016 Mon Sep 17 00:00:00 2001 From: Ashish Karale Date: Wed, 30 Oct 2024 03:13:06 -0700 Subject: [PATCH 2/4] Adressing reveiw comments --- src/backend_common.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/backend_common.cc b/src/backend_common.cc index 2576117..3f5a3c6 100644 --- a/src/backend_common.cc +++ b/src/backend_common.cc @@ -791,23 +791,23 @@ GetDirectoryContents(const std::string& path, std::set* contents) //! https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry //! TRITONSERVER_Error* -getOSValidPath(const std::string& _path, std::string& retPath) +getOSValidPath(const std::string& path, std::string& retPath) { - std::string path(_path); + std::string l_path(path); #ifdef _WIN32 - constexpr const char* kWindowsLongPathPrefix = "\\\\?\\"; + constexpr const char* WindowsLongPathPrefix = "\\\\?\\"; // On Windows long paths must be marked correctly otherwise, due to backwards // compatibility, all paths are limited to MAX_PATH length - if (path.size() >= MAX_PATH) { + if (l_path.size() >= MAX_PATH) { // Must be prefixed with "\\?\" to be considered long path - if (path.substr(0, 4) != (kWindowsLongPathPrefix)) { + if (l_path.substr(0, 4) != (WindowsLongPathPrefix)) { // Long path but not "tagged" correctly - path = (kWindowsLongPathPrefix) + path; + l_path = (WindowsLongPathPrefix) + l_path; } } - std::replace(path.begin(), path.end(), '/', '\\'); + std::replace(l_path.begin(), l_path.end(), '/', '\\'); #endif - retPath = path; + retPath = l_path; return nullptr; } From cc35d2612724cd14799ea238ed26a7afee6d425f Mon Sep 17 00:00:00 2001 From: Ashish Karale Date: Wed, 30 Oct 2024 03:16:06 -0700 Subject: [PATCH 3/4] Review comments --- include/triton/backend/backend_common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/triton/backend/backend_common.h b/include/triton/backend/backend_common.h index 5ee2d31..bb6a2ee 100644 --- a/include/triton/backend/backend_common.h +++ b/include/triton/backend/backend_common.h @@ -505,7 +505,7 @@ TRITONSERVER_Error* CopyBuffer( const bool copy_on_stream = false); TRITONSERVER_Error* getOSValidPath( - const std::string& _path, std::string& retPath); + const std::string& path, std::string& retPath); /// Does a file or directory exist? /// \param path The path to check for existence. From 6c5960fe2ba25e64db7d3d00481a6aad05f28a49 Mon Sep 17 00:00:00 2001 From: Ashish Karale Date: Wed, 30 Oct 2024 04:30:09 -0700 Subject: [PATCH 4/4] Adding documentation for API --- include/triton/backend/backend_common.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/triton/backend/backend_common.h b/include/triton/backend/backend_common.h index bb6a2ee..38d2451 100644 --- a/include/triton/backend/backend_common.h +++ b/include/triton/backend/backend_common.h @@ -504,6 +504,18 @@ TRITONSERVER_Error* CopyBuffer( void* dst, cudaStream_t cuda_stream, bool* cuda_used, const bool copy_on_stream = false); +/// Converts incoming utf-8 path to an OS valid path +/// +/// On Linux there is not much to do. +/// On Windows we need to take care of the long paths and handle them correctly +/// to avoid legacy issues with MAX_PATH +/// +/// More details: +/// https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry +/// +/// \param path The path to validate +/// \param retPath The updated valid path as per the OS requirements +/// \return a TRITONSERVER_Error indicating success or failure. TRITONSERVER_Error* getOSValidPath( const std::string& path, std::string& retPath);