Repository: mesos Updated Branches: refs/heads/master b780d873b -> 3f862f332
Windows: Fixed `os::stat::size()`. Removed the use of CRT runtime API `::_stat`, and instead implemented with `get_handle_[no_]follow()` plus `GetFileSizeEx` to correctly obtain the size of a file with long-path and symlink-aware semantics. This resolves MESOS-5939 and so enables `OsTest.SYMLINK_Size`. Review: https://reviews.apache.org/r/64434 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3f862f33 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3f862f33 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3f862f33 Branch: refs/heads/master Commit: 3f862f332b55b9b6124e098144fac44734553438 Parents: b780d87 Author: Andrew Schwartzmeyer <[email protected]> Authored: Thu Dec 7 16:57:02 2017 -0800 Committer: Andrew Schwartzmeyer <[email protected]> Committed: Mon Dec 11 13:32:48 2017 -0800 ---------------------------------------------------------------------- .../stout/include/stout/os/windows/stat.hpp | 44 +++++++------------- 3rdparty/stout/tests/os_tests.cpp | 15 ++++--- 2 files changed, 25 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/3f862f33/3rdparty/stout/include/stout/os/windows/stat.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/windows/stat.hpp b/3rdparty/stout/include/stout/os/windows/stat.hpp index 2ba6969..c04953e 100644 --- a/3rdparty/stout/include/stout/os/windows/stat.hpp +++ b/3rdparty/stout/include/stout/os/windows/stat.hpp @@ -93,41 +93,27 @@ inline bool islink(const std::string& path) } -// Returns the size in Bytes of a given file system entry. When -// applied to a symbolic link with `follow` set to -// `DO_NOT_FOLLOW_SYMLINK`, this will return the length of the entry -// name (strlen). -// -// TODO(andschwa): Replace `::_stat`. See MESOS-8275. +// Returns the size in Bytes of a given file system entry. When applied to a +// symbolic link with `follow` set to `DO_NOT_FOLLOW_SYMLINK`, this will return +// zero because that's what Windows says. inline Try<Bytes> size( const std::string& path, const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK) { - switch (follow) { - case FollowSymlink::DO_NOT_FOLLOW_SYMLINK: { - Try<::internal::windows::SymbolicLink> symlink = - ::internal::windows::query_symbolic_link_data(path); - - if (symlink.isError()) { - return Error(symlink.error()); - } else { - return Bytes(symlink.get().substitute_name.length()); - } - break; - } - case FollowSymlink::FOLLOW_SYMLINK: { - struct _stat s; - - if (::_stat(path.c_str(), &s) < 0) { - return ErrnoError("Error invoking stat for '" + path + "'"); - } else { - return Bytes(s.st_size); - } - break; - } + const Try<SharedHandle> handle = (follow == FollowSymlink::FOLLOW_SYMLINK) + ? ::internal::windows::get_handle_follow(path) + : ::internal::windows::get_handle_no_follow(path); + if (handle.isError()) { + return Error("Error obtaining handle to file: " + handle.error()); + } + + LARGE_INTEGER file_size; + + if (::GetFileSizeEx(handle->get_handle(), &file_size) == 0) { + return WindowsError(); } - UNREACHABLE(); + return Bytes(file_size.QuadPart); } http://git-wip-us.apache.org/repos/asf/mesos/blob/3f862f33/3rdparty/stout/tests/os_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp index 93a23a8..02f2a18 100644 --- a/3rdparty/stout/tests/os_tests.cpp +++ b/3rdparty/stout/tests/os_tests.cpp @@ -210,14 +210,11 @@ TEST_F(OsTest, Nonblock) #endif // __WINDOWS__ -// TODO(hausdorff): Fix this issue and enable the test on Windows. It fails -// because `os::size` is not following symlinks correctly on Windows. See -// MESOS-5939. // Tests whether a file's size is reported by os::stat::size as expected. // Tests all four combinations of following a link or not and of a file // or a link as argument. Also tests that an error is returned for a // non-existing file. -TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, SYMLINK_Size) +TEST_F(OsTest, SYMLINK_Size) { const string file = path::join(os::getcwd(), UUID::random().toString()); @@ -241,9 +238,17 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(OsTest, SYMLINK_Size) // Following links we expect the file's size, not the link's. EXPECT_SOME_EQ(size, os::stat::size(link, FollowSymlink::FOLLOW_SYMLINK)); +#ifdef __WINDOWS__ + // On Windows, the reported size of a symlink is zero. + EXPECT_SOME_EQ( + Bytes(0), + os::stat::size(link, FollowSymlink::DO_NOT_FOLLOW_SYMLINK)); +#else // Not following links, we expect the string length of the linked path. - EXPECT_SOME_EQ(Bytes(file.size()), + EXPECT_SOME_EQ( + Bytes(file.size()), os::stat::size(link, FollowSymlink::DO_NOT_FOLLOW_SYMLINK)); +#endif // __WINDOWS__ }
