Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for Windows Long Paths #398

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Support for Windows Long Paths #398

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c30a554
Support long paths on windows
Akarale98 Oct 14, 2024
1d57d3e
Pre commit fixes
Akarale98 Oct 14, 2024
3d196b2
Merge remote-tracking branch 'upstream/main' into windows-long-paths
Akarale98 Oct 30, 2024
8ff3bca
Addressing review comments
Akarale98 Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions src/filesystem/implementations/local.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,45 @@ class LocalFileSystem : public FileSystem {
Status MakeTemporaryDirectory(
std::string dir_path, std::string* temp_dir) override;
Status DeletePath(const std::string& path) override;

private:
inline std::string getOSValidPath(const std::string& _path);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
inline std::string getOSValidPath(const std::string& _path);
inline std::string GetOSValidPath(const std::string& path);

To match triton syntax conventions

const char* WindowsLongPathPrefix = "\\\\?\\";
};


//! 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
//!
inline std::string
LocalFileSystem::getOSValidPath(const std::string& path)
{
std::string l_path(path);
#ifdef _WIN32
// On Windows long paths must be marked correctly otherwise, due to backwards
// compatibility, all paths are limited to MAX_PATH length
if (l_path.size() >= MAX_PATH) {
// Must be prefixed with "\\?\" to be considered long path
if (l_path.substr(0, 4) != (WindowsLongPathPrefix)) {
// Long path but not "tagged" correctly
l_path = (WindowsLongPathPrefix) + l_path;
}
}
std::replace(l_path.begin(), l_path.end(), '/', '\\');
#endif
return l_path;
}

Status
LocalFileSystem::FileExists(const std::string& path, bool* exists)
{
*exists = (access(path.c_str(), F_OK) == 0);
*exists = (access(getOSValidPath(path).c_str(), F_OK) == 0);
return Status::Success;
}

Expand All @@ -75,7 +108,7 @@ LocalFileSystem::IsDirectory(const std::string& path, bool* is_dir)
*is_dir = false;

struct stat st;
if (stat(path.c_str(), &st) != 0) {
if (stat(getOSValidPath(path).c_str(), &st) != 0) {
return Status(Status::Code::INTERNAL, "failed to stat file " + path);
}

Expand All @@ -88,7 +121,7 @@ LocalFileSystem::FileModificationTime(
const std::string& path, int64_t* mtime_ns)
{
struct stat st;
if (stat(path.c_str(), &st) != 0) {
if (stat(getOSValidPath(path).c_str(), &st) != 0) {
return Status(Status::Code::INTERNAL, "failed to stat file " + path);
}

Expand Down Expand Up @@ -187,7 +220,7 @@ LocalFileSystem::GetDirectoryFiles(
Status
LocalFileSystem::ReadTextFile(const std::string& path, std::string* contents)
{
std::ifstream in(path, std::ios::in | std::ios::binary);
std::ifstream in(getOSValidPath(path), std::ios::in | std::ios::binary);
if (!in) {
return Status(
Status::Code::INTERNAL,
Expand Down