Skip to content

Commit

Permalink
Add windows-specific call for OS version
Browse files Browse the repository at this point in the history
Summary:
Right now, we rely on the sys_info library to get the OS version.

However, that functionality is broken for Windows: FillZpp/sys-info-rs#86
Specifically, it produces `6.2.9200` for all versions across Windows: https://fburl.com/scuba/buck2_builds/iq6hsudp

This diff swaps the check to use the winver library, which has much better
support for fetching the Windows version.

Reviewed By: JakobDegen

Differential Revision: D56546196
  • Loading branch information
Eric J Nguyen authored and facebook-github-bot committed Apr 29, 2024
1 parent ad611a7 commit 38472e6
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 2 deletions.
15 changes: 14 additions & 1 deletion app/buck2_events/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ rust_library(
srcs = glob(
["src/**/*.rs"],
),
os_deps = [
(
"windows",
["fbsource//third-party/rust:winver"],
),
(
"linux",
["fbsource//third-party/rust:sys-info"],
),
(
"macos",
["fbsource//third-party/rust:sys-info"],
),
],
test_deps = ["fbsource//third-party/rust:tokio"],
deps = [
"fbsource//third-party/rust:anyhow",
Expand All @@ -22,7 +36,6 @@ rust_library(
"fbsource//third-party/rust:serde",
# @oss-disable: "fbsource//third-party/rust:serde_json",
"fbsource//third-party/rust:smallvec",
"fbsource//third-party/rust:sys-info",
"fbsource//third-party/rust:tokio",
"fbsource//third-party/rust:uuid",
"//buck2/allocative/allocative:allocative",
Expand Down
3 changes: 3 additions & 0 deletions app/buck2_events/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ buck2_data = { workspace = true }
buck2_error = { workspace = true }
buck2_util = { workspace = true }
buck2_wrapper_common = { workspace = true }

[target."cfg(windows)".dependencies]
winver = "1"
29 changes: 28 additions & 1 deletion app/buck2_events/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub fn system_info() -> SystemInfo {
hostname,
username,
os: os_type(),
os_version: sys_info::os_release().ok(),
os_version: os_version(),
}
}

Expand All @@ -119,6 +119,16 @@ fn os_type() -> String {
}
}

#[cfg(target_os = "windows")]
fn os_version() -> Option<String> {
winver::WindowsVersion::detect().map(|v| v.to_string())
}

#[cfg(any(target_os = "linux", target_os = "macos"))]
fn os_version() -> Option<String> {
sys_info::os_release().ok()
}

pub fn hostname() -> Option<String> {
static CELL: OnceLock<Option<String>> = OnceLock::new();

Expand All @@ -129,3 +139,20 @@ pub fn hostname() -> Option<String> {
})
.clone()
}

#[cfg(all(test, target_os = "windows"))]
mod tests {
use super::*;

#[test]
fn os_version_produces_reasonable_windows_version() {
let data = collect();
// This logic used to use the `GetVersionExW` win32 API, which
// always returns the value below on recent versions of windows. See
// https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getversionexw
// for more details.
assert_ne!(data["os_version"], "6.2.9200");
// This is true for both Windows 10 and Windows 11: https://learn.microsoft.com/en-us/windows/win32/sysinfo/operating-system-version
assert!(data["os_version"].starts_with("10.0."));
}
}

0 comments on commit 38472e6

Please sign in to comment.