Conversation
3208750 to
1b673c0
Compare
|
Validated on Android in #18333 |
bf6d16a to
092bf5c
Compare
| fs_err::read_to_string("/etc/os-release") | ||
| .ok()? | ||
| .parse() | ||
| .unwrap(), |
There was a problem hiding this comment.
Technically we should fall back to /usr/lib/os-release if we don't find anything in /etc/os-release
There was a problem hiding this comment.
Is there a reason (e.g. more distro coverage)? Curious. I can also see falling back to /etc/lsb-release, but not sure if it's needed.
Note, https://github.com/FillZpp/sys-info-rs/blob/master/lib.rs#L483 only checked /etc/os-release
edit (found answer): Per https://www.freedesktop.org/software/systemd/man/latest/os-release.html
The file /etc/os-release takes precedence over /usr/lib/os-release. Applications should check for the former, and exclusively use its data if it exists, and only fall back to /usr/lib/os-release if that is missing
| let key = windows_registry::LOCAL_MACHINE | ||
| .open(r"SOFTWARE\Microsoft\Windows NT\CurrentVersion") | ||
| .ok()?; | ||
| Some(Self::Windows(key.get_string("CurrentBuildNumber").ok()?)) |
There was a problem hiding this comment.
This may not match previous sys-info/linehaul behavior, do we care?
| match self { | ||
| Self::Linux(os_type) => f.write_str(os_type), | ||
| Self::Darwin => f.write_str("Darwin"), | ||
| Self::WindowsNt => f.write_str("Windows_NT"), |
There was a problem hiding this comment.
I'd expect this to be Windows, to match previous linehaul impl and the output of platform.system() on Windows.
There was a problem hiding this comment.
Hm I should have checked this more carefully. I intended to mirror sys-info.
…dows version (#18383) ## Summary In #18324, sys-info-rs was dropped in favor of a more native reimplementation to provide OsType and OsRelease. This PR adjusts two areas for windows to match closely previous behavior: 1. OsType should be `Windows` rather than `Windows_NT` #18324 (comment) as seen in https://github.com/FillZpp/sys-info-rs/blob/60ecf1470a5b7c90242f429934a3bacb6023ec4d/c/windows.c#L12. This also matches the output of `platform.system()` in CPython. 2. OsRelease previously used `GetVersionEx` in sys-info-rs. Looking closely, this was used primarily in `crates/uv-python/src/interpreter.rs` and not in linehaul as linehaul uses `platform.release()`. The problem with `GetVersionEx` is that it returns often the wrong version due to legacy reasons (e.g. may be stuck returning `6.2.9200`). The current implementation only returns the build number from the registry which is prone to problems across windows older variants. The implementation should use `RtlGetVersion` system call which returns the current major, minor, build in the same way as reported by `sys.getwindowsversion()` in CPython. In order to strike balance, this switches the implementation to use four octects `{major}.{minor}.{build}.{revision}` as recent windows versioning relies on major, build and revision where as older versions rely on major, minor and service pack. A windows-version crate by the same author as windows-rs was added as it includes the correct system calls for the windows versions. ## Test Plan Tested manually on both Windows desktop (10, 11) and Windows Server (2016).
Part of #14574 and termux/termux-packages#27547
Avoids the downstream patch at https://github.com/termux/termux-packages/blob/master/packages/uv/0001-sys-info-replace-index-with-strchr.diff
sys-info-rshas not been updated in years https://github.com/FillZpp/sys-info-rs — it includes C code and can't be built on Termux FillZpp/sys-info-rs#116I considered switching to https://github.com/GuillaumeGomez/sysinfo but our usage is trivial, so we implement our requirements in
uv-platforminstead