----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64697/#review194181 -----------------------------------------------------------
Ship it! This is reasonable. The reason why we need these methods on Posix is to differentiate between Darwin and Linux, or check the kernel version. On Windows, we have the `#ifdef __WINDOWS__`, so we shouldn't need runtime OS version checks at all. (And we only support the latest Windows version anyway). - Joseph Wu On Dec. 18, 2017, 6:07 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64697/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2017, 6:07 p.m.) > > > Review request for mesos, Akash Gupta, Jeff Coffler, and Joseph Wu. > > > Bugs: MESOS-7781 > https://issues.apache.org/jira/browse/MESOS-7781 > > > Repository: mesos > > > Description > ------- > > The Windows API `GetVersionEx` was deprecated in Windows 8. Using it in > the `os.hpp` header caused hundreds of warnings that it was deprecated > to be emitted when building. While the function still exists, it stopped > returning the correct value when it was deprecated (so the version it > returns is 6.3, that is, Windows 8). > > However, replacing it was less than straightforward. The recommended > replacement is to use the "version helper functions", but these are not > capable of providing the version information of the system. The intent > of the deprecation and the new APIs is to prevent developers from using > the version of Windows as a feature check. The new APIs are all of the > form `bool IsWindows10OrGreater()`. While it is possible to use > `IsWindowsServer()` to determine if we're on a client or server version > of Windows, no current user-mode API is provided to query the build > information of the OS. This is unfortunate, as retrieving this > information is a valid use case of the now deprecated API. > > An explored alternative was to query the registry, but this was > discarded as it was not consistently available. > > Another alternative was to parse the output of `systeminfo`, which can > return CSV formatted version information. However, we chose not to do > this currently as it is slow (on the order of seconds to invoke the > command). > > There still exists a kernel-mode API to retrieve the version > information. However, to use it would entail either including WDK (which > is massive and not something we're going to do), or to invoke it > dynamically by getting the address from `nt.dll`. This is possible, but > it would be hacky, and was not necessary. > > The chosen route was to simply delete the use of `GetVersionEx`. After > reviewing the use of these functions, it turned out to be entirely > possible to `delete` them. > > Note that this means the entirety of `systems_tests.cpp` was rendered > pointless for Windows. > > > Diffs > ----- > > 3rdparty/stout/include/stout/os.hpp > 1a81db61faa55d7043d75a012fe1e66b49bf601c > 3rdparty/stout/include/stout/windows/os.hpp > 26a1a049c7a4e1b6a3b6767a9c2c86e7538f6012 > 3rdparty/stout/tests/CMakeLists.txt > 940fc9f6997695cf6c181ecbc72d6fd6e72dc7e7 > 3rdparty/stout/tests/os/systems_tests.cpp > 286d34edacab932aaecacfa6259a0bc549f01b1b > > > Diff: https://reviews.apache.org/r/64697/diff/1/ > > > Testing > ------- > > `ctest -V -C Debug` on Windows 10, `make check` on CentOS 7. > > > Thanks, > > Andrew Schwartzmeyer > >
