mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Review Request 64697: Windows: Deleted unused and unnecessary OS version functions.
Date Tue, 19 Dec 2017 02:07:01 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64697/
-----------------------------------------------------------

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message