----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64697/#review194116 -----------------------------------------------------------
PASS: Mesos patch 64697 was successfully built and tested. Reviews applied: `['64697']` All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64697 - Mesos Reviewbot Windows On Dec. 18, 2017, 9: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, 9: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 > >
