mesos git commit: Windows: Fixed `os::stat::size()`.

[an...@apache.org]

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 <an...@schwartzmeyer.com>
Authored: Thu Dec 7 16:57:02 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
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__
 }