mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mesos Reviewbot Windows <revi...@mesos.apache.org>
Subject Re: Review Request 64697: Windows: Deleted unused and unnecessary OS version functions.
Date Tue, 19 Dec 2017 03:10:54 GMT

-----------------------------------------------------------
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
> 
>


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