Support for Windows Long Paths#398
Support for Windows Long Paths#398ashishk98 wants to merge 4 commits intotriton-inference-server:mainfrom
Conversation
|
|
||
| private: | ||
| inline std::string getOSValidPath(const std::string& _path); | ||
| const char* kWindowsLongPathPrefix = "\\\\?\\"; |
There was a problem hiding this comment.
| const char* kWindowsLongPathPrefix = "\\\\?\\"; | |
| const char* windows_long_path_prefix_ = "\\\\?\\"; |
To match Triton member variable convention.
There was a problem hiding this comment.
But that's a constant, not a variable? AFAIK, constants should be named like the former, not the latter.
There was a problem hiding this comment.
It might be more idiomatic to use static constexpr char* kWindowsLongPathPrefix = "\\\\?\\"; to drive the fact this is, indeed, a constant.
There was a problem hiding this comment.
| Status DeletePath(const std::string& path) override; | ||
|
|
||
| private: | ||
| inline std::string getOSValidPath(const std::string& _path); |
There was a problem hiding this comment.
| inline std::string getOSValidPath(const std::string& _path); | |
| inline std::string GetOSValidPath(const std::string& path); |
To match triton syntax conventions
| //! https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry | ||
| //! | ||
| inline std::string | ||
| LocalFileSystem::getOSValidPath(const std::string& _path) |
There was a problem hiding this comment.
| LocalFileSystem::getOSValidPath(const std::string& _path) | |
| LocalFileSystem::getOSValidPath(const std::string& path) |
Cascading effect where the local path variable will need to change to something like local_path or l_path
|
Great work @ashishk98, just a few minor stylistic comments to be consistent with Triton. Also, to get this merged, I need two things:
|
Add support for windows long paths