Skip to content

Replace sys-info with sysinfo crate - #17

Merged
sharkdp merged 2 commits into
sharkdp:masterfrom
sola-contrib:migrate-sysinfo
Jan 4, 2025
Merged

Replace sys-info with sysinfo crate#17
sharkdp merged 2 commits into
sharkdp:masterfrom
sola-contrib:migrate-sysinfo

Conversation

@unlimitedsola

@unlimitedsola unlimitedsola commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

Closes #16 :

Rumors say cargo install bat is failing for some people due to a compilation failure from the sys-info crate. However the fix has not been merged for over half a year and the crate itself hasn't been active for over 2 years now.

There is an alternative crate sysinfo that's more regularly maintained and I believe it should provide similar functionality needed by this project.


Examples copied from sysinfo::System::long_os_version's documentation:

example platform value of System::long_os_version()
linux laptop "Linux (Ubuntu 24.04)"
android phone "Android 15 on Pixel 9 Pro"
apple laptop "macOS 15.1.1 Sequoia"
windows server "Windows Server 2022 Datacenter"

I can add the kernel version information too if someone can give suggestions on how to format them together.


EDIT:

This PR depends on the MSRV bump to 1.74 proposed in #18. merged.

A more comprehensive list of outputs is also available in a successful action run based on #18:
https://github.com/sola-contrib/bugreport/actions/runs/12552288110

@unlimitedsola

Copy link
Copy Markdown
Contributor Author

The current master branch that this is based on doesn't seem to be able to pass the CI configured: https://github.com/sola-contrib/bugreport/actions/runs/12551564715/job/34996187540

@unlimitedsola unlimitedsola mentioned this pull request Dec 30, 2024
@unlimitedsola

Copy link
Copy Markdown
Contributor Author

I made another PR #18 to fix the CI issues, which also bumps the MSRV for the sysinfo crate for this PR.

@sharkdp

sharkdp commented Dec 31, 2024

Copy link
Copy Markdown
Owner

Thank you!

Would you mind rebasing this, so we can compare the outputs in CI with master?

@unlimitedsola

Copy link
Copy Markdown
Contributor Author

Thank you!

Would you mind rebasing this, so we can compare the outputs in CI with master?

Done!

@unlimitedsola

unlimitedsola commented Dec 31, 2024

Copy link
Copy Markdown
Contributor Author

so we can compare the outputs in CI with master?

The main difference should be that the kernel version is replaced with the OS/distribution version.

I can add the kernel version information too if someone can give suggestions on how to format them together.

I could change this to include the kernel version as well like below:

Ok(ReportEntry::List(vec![
    ReportEntry::Text(format!(
        "OS: {}",
        sysinfo::System::long_os_version().unwrap_or_else(|| "Unknown".to_owned()),
    )),
    ReportEntry::Text(format!(
        "Kernel: {}",
        sysinfo::System::kernel_version().unwrap_or_else(|| "Unknown".to_owned()),
    )),
]))

@sharkdp Please let me know your thoughts on the formatting, or any additional information that's available in the sysinfo crate that you would like to include. Thanks!

@unlimitedsola

Copy link
Copy Markdown
Contributor Author

I could change this to include the kernel version as well like below:

To save us time from back-and-forth, I pushed an additional commit to add the kernel version information as an option. You can choose whether to include it when applying the patches from this PR.

@sharkdp
sharkdp merged commit 73e84cd into sharkdp:master Jan 4, 2025
@sharkdp

sharkdp commented Jan 4, 2025

Copy link
Copy Markdown
Owner

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider replacing sys-info with sysinfo crate?

2 participants