You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2017/12/19 21:35:44 UTC
mesos git commit: Windows: Deleted unused and unnecessary OS version
functions.
Repository: mesos
Updated Branches:
refs/heads/master 45669854d -> b7c218fda
Windows: Deleted unused and unnecessary OS version functions.
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.
Review: https://reviews.apache.org/r/64697
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b7c218fd
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b7c218fd
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b7c218fd
Branch: refs/heads/master
Commit: b7c218fda59ce2c66ac1b5775a4d48b03c830305
Parents: 4566985
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Tue Dec 12 14:24:57 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue Dec 19 13:35:06 2017 -0800
----------------------------------------------------------------------
3rdparty/stout/include/stout/os.hpp | 4 +
3rdparty/stout/include/stout/windows/os.hpp | 101 +----------------------
3rdparty/stout/tests/CMakeLists.txt | 6 +-
3rdparty/stout/tests/os/systems_tests.cpp | 8 --
4 files changed, 9 insertions(+), 110 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b7c218fd/3rdparty/stout/include/stout/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os.hpp b/3rdparty/stout/include/stout/os.hpp
index 1a81db6..c366ad0 100644
--- a/3rdparty/stout/include/stout/os.hpp
+++ b/3rdparty/stout/include/stout/os.hpp
@@ -163,6 +163,9 @@ inline void appendPaths(const std::string& newPaths)
} // namespace libraries {
+#ifdef __WINDOWS__
+inline Try<std::string> sysname() = delete;
+#else
// Return the operating system name (e.g. Linux).
inline Try<std::string> sysname()
{
@@ -173,6 +176,7 @@ inline Try<std::string> sysname()
return info.get().sysname;
}
+#endif // __WINDOWS__
inline Try<std::list<Process>> processes()
http://git-wip-us.apache.org/repos/asf/mesos/blob/b7c218fd/3rdparty/stout/include/stout/windows/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows/os.hpp b/3rdparty/stout/include/stout/windows/os.hpp
index 26a1a04..8c6f7a3 100644
--- a/3rdparty/stout/include/stout/windows/os.hpp
+++ b/3rdparty/stout/include/stout/windows/os.hpp
@@ -55,19 +55,6 @@
namespace os {
namespace internal {
-inline Try<OSVERSIONINFOEXW> os_version()
-{
- OSVERSIONINFOEXW os_version;
- os_version.dwOSVersionInfoSize = sizeof(os_version);
- if (!::GetVersionExW(reinterpret_cast<LPOSVERSIONINFO>(&os_version))) {
- return WindowsError(
- "os::internal::os_version: Call to `GetVersionEx` failed");
- }
-
- return os_version;
-}
-
-
inline Try<std::string> nodename()
{
// MSDN documentation states "The names are established at system startup,
@@ -95,58 +82,6 @@ inline Try<std::string> nodename()
return stringify(std::wstring(buffer.data()));
}
-
-inline std::string machine()
-{
- SYSTEM_INFO system_info;
- ::GetNativeSystemInfo(&system_info);
-
- switch (system_info.wProcessorArchitecture) {
- case PROCESSOR_ARCHITECTURE_AMD64:
- return "AMD64";
- case PROCESSOR_ARCHITECTURE_ARM:
- return "ARM";
- case PROCESSOR_ARCHITECTURE_IA64:
- return "IA64";
- case PROCESSOR_ARCHITECTURE_INTEL:
- return "x86";
- default:
- return "Unknown";
- }
-}
-
-
-inline std::string sysname(OSVERSIONINFOEXW os_version)
-{
- switch (os_version.wProductType) {
- case VER_NT_DOMAIN_CONTROLLER:
- case VER_NT_SERVER:
- return "Windows Server";
- default:
- return "Windows";
- }
-}
-
-
-inline std::string release(OSVERSIONINFOEXW os_version)
-{
- return stringify(
- Version(os_version.dwMajorVersion, os_version.dwMinorVersion, 0));
-}
-
-
-inline std::string version(OSVERSIONINFOEXW os_version)
-{
- std::string version = std::to_string(os_version.dwBuildNumber);
-
- if (os_version.szCSDVersion[0] != L'\0') {
- version.append(" ");
- version.append(stringify(os_version.szCSDVersion));
- }
-
- return version;
-}
-
} // namespace internal {
@@ -444,43 +379,11 @@ inline Try<Memory> memory()
}
-inline Try<Version> release()
-{
- OSVERSIONINFOEXW os_version;
- os_version.dwOSVersionInfoSize = sizeof(os_version);
- if (!::GetVersionExW(reinterpret_cast<LPOSVERSIONINFO>(&os_version))) {
- return WindowsError("os::release: Call to `GetVersionEx` failed");
- }
-
- return Version(os_version.dwMajorVersion, os_version.dwMinorVersion, 0);
-}
+inline Try<Version> release() = delete;
// Return the system information.
-inline Try<UTSInfo> uname()
-{
- Try<OSVERSIONINFOEXW> os_version = internal::os_version();
- if (os_version.isError()) {
- return Error(os_version.error());
- }
-
- // Add nodename to `UTSInfo` object.
- Try<std::string> nodename = internal::nodename();
- if (nodename.isError()) {
- return Error(nodename.error());
- }
-
- // Populate `UTSInfo`.
- UTSInfo info;
-
- info.sysname = internal::sysname(os_version.get());
- info.release = internal::release(os_version.get());
- info.version = internal::version(os_version.get());
- info.nodename = nodename.get();
- info.machine = internal::machine();
-
- return info;
-}
+inline Try<UTSInfo> uname() = delete;
inline tm* gmtime_r(const time_t* timep, tm* result)
http://git-wip-us.apache.org/repos/asf/mesos/blob/b7c218fd/3rdparty/stout/tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/CMakeLists.txt b/3rdparty/stout/tests/CMakeLists.txt
index 940fc9f..28674c9 100644
--- a/3rdparty/stout/tests/CMakeLists.txt
+++ b/3rdparty/stout/tests/CMakeLists.txt
@@ -70,13 +70,13 @@ set(STOUT_OS_TESTS_SRC
os/process_tests.cpp
os/rmdir_tests.cpp
os/socket_tests.cpp
- os/strerror_tests.cpp
- os/systems_tests.cpp)
+ os/strerror_tests.cpp)
if (NOT WIN32)
list(APPEND STOUT_OS_TESTS_SRC
os/sendfile_tests.cpp
- os/signals_tests.cpp)
+ os/signals_tests.cpp
+ os/systems_tests.cpp)
endif ()
if (${CMAKE_SYSTEM_NAME} MATCHES "Linux")
http://git-wip-us.apache.org/repos/asf/mesos/blob/b7c218fd/3rdparty/stout/tests/os/systems_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/os/systems_tests.cpp b/3rdparty/stout/tests/os/systems_tests.cpp
index 286d34e..44a9221 100644
--- a/3rdparty/stout/tests/os/systems_tests.cpp
+++ b/3rdparty/stout/tests/os/systems_tests.cpp
@@ -35,14 +35,6 @@ TEST_F(SystemsTests, Uname)
// Machine arch must be non-empty.
EXPECT_FALSE(info.get().machine.empty());
-#elif defined(__WINDOWS__)
- // On Windows, `sysname` is one of 2 options.
- hashset<string> server_types{"Windows", "Windows Server"};
- EXPECT_TRUE(server_types.contains(info.get().sysname));
-
- // On Windows, we `machine` takes one of 5 values.
- hashset<string> arch_types{"AMD64", "ARM", "IA64", "x86", "Unknown"};
- EXPECT_TRUE(arch_types.contains(info.get().machine));
#endif // __linux__
// The `release`, `version`, and `nodename` properties should all be