Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`. The functions `mode()`, `dev()`, and `inode()` are unused and do not make sense on Windows, so they were explicitly deleted. The function `mtime()` is used and has a logical mapping, `GetFileTime()`. However, Windows reports time differently from POSIX, so a conversion must also be performed such that the API `os::stat::mtime()` remains consistent with its POSIX version.
Review: https://reviews.apache.org/r/66773 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3b89d187 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3b89d187 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3b89d187 Branch: refs/heads/master Commit: 3b89d187a296bf09570184bcff7991318d4aff1a Parents: 942d495 Author: Andrew Schwartzmeyer <[email protected]> Authored: Fri Apr 20 11:32:15 2018 -0700 Committer: Andrew Schwartzmeyer <[email protected]> Committed: Tue May 1 18:36:04 2018 -0700 ---------------------------------------------------------------------- 3rdparty/stout/include/stout/os/permissions.hpp | 5 + .../stout/include/stout/os/windows/stat.hpp | 123 ++++++++----------- 3rdparty/stout/tests/os_tests.cpp | 26 ++++ 3 files changed, 81 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/3b89d187/3rdparty/stout/include/stout/os/permissions.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/permissions.hpp b/3rdparty/stout/include/stout/os/permissions.hpp index 453e60c..096ad7f 100644 --- a/3rdparty/stout/include/stout/os/permissions.hpp +++ b/3rdparty/stout/include/stout/os/permissions.hpp @@ -60,12 +60,17 @@ struct Permissions inline Try<Permissions> permissions(const std::string& path) { +#ifdef __WINDOWS__ + VLOG(2) << "`os::permissions` has been called, but is a stub on Windows"; + return Permissions(0); +#else struct stat status; if (::stat(path.c_str(), &status) < 0) { return ErrnoError(); } return Permissions(status.st_mode); +#endif // __WINDOWS__ } http://git-wip-us.apache.org/repos/asf/mesos/blob/3b89d187/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 93bc949..7838bac 100644 --- a/3rdparty/stout/include/stout/os/windows/stat.hpp +++ b/3rdparty/stout/include/stout/os/windows/stat.hpp @@ -14,6 +14,7 @@ #define __STOUT_OS_WINDOWS_STAT_HPP__ #include <string> +#include <type_traits> #include <stout/bytes.hpp> #include <stout/try.hpp> @@ -27,11 +28,6 @@ #include <stout/internal/windows/reparsepoint.hpp> #include <stout/internal/windows/symlink.hpp> -#ifdef _USE_32BIT_TIME_T -#error "Implementation of `os::stat::mtime` assumes 64-bit `time_t`." -#endif // _USE_32BIT_TIME_T - - namespace os { namespace stat { @@ -144,94 +140,75 @@ inline Try<Bytes> size(const int_fd& fd) } -// TODO(andschwa): Replace `::_stat`. See MESOS-8275. inline Try<long> mtime( const std::string& path, const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK) { - if (follow == FollowSymlink::DO_NOT_FOLLOW_SYMLINK) { - Try<::internal::windows::SymbolicLink> symlink = - ::internal::windows::query_symbolic_link_data(path); - - if (symlink.isSome()) { - return Error( - "Requested mtime for '" + path + - "', but symbolic links don't have an mtime on Windows"); - } + if (follow == FollowSymlink::DO_NOT_FOLLOW_SYMLINK && islink(path)) { + return Error( + "Requested mtime for '" + path + + "', but symbolic links don't have an mtime on Windows"); } - struct _stat s; + 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(handle.error()); + } - if (::_stat(path.c_str(), &s) < 0) { - return ErrnoError("Error invoking stat for '" + path + "'"); + FILETIME filetime; + // The last argument is file write time, AKA modification time. + const BOOL result = + ::GetFileTime(handle->get_handle(), nullptr, nullptr, &filetime); + if (result == FALSE) { + return WindowsError(); } - // To be safe, we assert that `st_mtime` is represented as `__int64`. To - // conform to the POSIX, we also cast `st_mtime` to `long`; we choose to make - // this conversion explicit because we expect the truncation to not cause - // information loss. + // Convert to 64-bit integer using Windows magic. + ULARGE_INTEGER largetime; + largetime.LowPart = filetime.dwLowDateTime; + largetime.HighPart = filetime.dwHighDateTime; + // Now the `QuadPart` field is the 64-bit representation due to the + // layout of the `ULARGE_INTEGER` struct. static_assert( - std::is_same<__int64, __time64_t>::value, - "Mesos assumes `__time64_t` is represented as `__int64`"); - return static_cast<long>(s.st_mtime); + sizeof(largetime.QuadPart) == sizeof(__int64), + "Expected `QuadPart` to be of type `__int64`"); + const __int64 windowstime = largetime.QuadPart; + // A file time is a 64-bit value that represents the number of + // 100-nanosecond intervals that have elapsed since 1601-01-01 + // 00:00:00 +0000. However, users of this function expect UNIX time, + // which is seconds elapsed since the Epoch, 1970-01-01 00:00:00 + // +0000. + // + // So first we convert 100-nanosecond intervals into seconds by + // doing `(x * 100) / (1,000^3)`, or `x / 10,000,000`, and then + // substracting the number of seconds between 1601-01-01 and + // 1970-01-01, or `11,644,473,600`. + const __int64 unixtime = (windowstime / 10000000) - 11644473600; + // We choose to make this conversion explicit because we expect the + // truncation to not cause information loss. + return static_cast<long>(unixtime); } -// TODO(andschwa): Replace `::_stat`. See MESOS-8275. +// NOTE: The following are deleted because implementing them would use +// the CRT API `_stat`, which we want to avoid, and they're not +// currently used on Windows. inline Try<mode_t> mode( - const std::string& path, - const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK) -{ - struct _stat s; + const std::string& path, + const FollowSymlink follow) = delete; - if (follow == FollowSymlink::DO_NOT_FOLLOW_SYMLINK && islink(path)) { - return Error("lstat not supported for symlink '" + path + "'"); - } - - if (::_stat(path.c_str(), &s) < 0) { - return ErrnoError("Error invoking stat for '" + path + "'"); - } - return s.st_mode; -} - - -// TODO(andschwa): Replace `::_stat`. See MESOS-8275. inline Try<dev_t> dev( - const std::string& path, - const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK) -{ - struct _stat s; - - if (follow == FollowSymlink::DO_NOT_FOLLOW_SYMLINK && islink(path)) { - return Error("lstat not supported for symlink '" + path + "'"); - } - - if (::_stat(path.c_str(), &s) < 0) { - return ErrnoError("Error invoking stat for '" + path + "'"); - } - - return s.st_dev; -} + const std::string& path, + const FollowSymlink follow) = delete; -// TODO(andschwa): Replace `::_stat`. See MESOS-8275. inline Try<ino_t> inode( - const std::string& path, - const FollowSymlink follow = FollowSymlink::FOLLOW_SYMLINK) -{ - struct _stat s; - - if (follow == FollowSymlink::DO_NOT_FOLLOW_SYMLINK) { - return Error("Non-following stat not supported for '" + path + "'"); - } - - if (::_stat(path.c_str(), &s) < 0) { - return ErrnoError("Error invoking stat for '" + path + "'"); - } - - return s.st_ino; -} + const std::string& path, + const FollowSymlink follow) = delete; } // namespace stat { } // namespace os { http://git-wip-us.apache.org/repos/asf/mesos/blob/3b89d187/3rdparty/stout/tests/os_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp index 752d9e5..419879a 100644 --- a/3rdparty/stout/tests/os_tests.cpp +++ b/3rdparty/stout/tests/os_tests.cpp @@ -20,6 +20,7 @@ #include <sys/types.h> #endif +#include <algorithm> #include <chrono> #include <cstdlib> // For rand. #include <list> @@ -1049,11 +1050,36 @@ TEST_F(OsTest, SYMLINK_Realpath) ASSERT_SOME(fs::symlink(testFile, testLink)); // Validate the symlink. +#ifdef __WINDOWS__ + Try<int_fd> handle = os::open(testFile, O_RDONLY); + ASSERT_SOME(handle); + FILE_ID_INFO fileInfo; + BOOL result = ::GetFileInformationByHandleEx( + handle.get(), FileIdInfo, &fileInfo, sizeof(fileInfo)); + ASSERT_SOME(os::close(handle.get())); + ASSERT_EQ(TRUE, result); + + handle = os::open(testLink, O_RDONLY); + ASSERT_SOME(handle); + FILE_ID_INFO linkInfo; + result = ::GetFileInformationByHandleEx( + handle.get(), FileIdInfo, &linkInfo, sizeof(linkInfo)); + ASSERT_SOME(os::close(handle.get())); + ASSERT_EQ(TRUE, result); + + ASSERT_EQ(fileInfo.VolumeSerialNumber, linkInfo.VolumeSerialNumber); + ASSERT_TRUE(std::equal( + std::begin(fileInfo.FileId.Identifier), + std::end(fileInfo.FileId.Identifier), + std::begin(linkInfo.FileId.Identifier), + std::end(linkInfo.FileId.Identifier))); +#else const Try<ino_t> fileInode = os::stat::inode(testFile); ASSERT_SOME(fileInode); const Try<ino_t> linkInode = os::stat::inode(testLink); ASSERT_SOME(linkInode); ASSERT_EQ(fileInode.get(), linkInode.get()); +#endif // __WINDOWS__ // Verify that the symlink resolves correctly. Result<string> resolved = os::realpath(testLink);
