Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace sys-info crate dependency with systemstat #4496

Closed
ry opened this issue Mar 26, 2020 · 14 comments
Closed

Replace sys-info crate dependency with systemstat #4496

ry opened this issue Mar 26, 2020 · 14 comments
Labels
good first issue Good for newcomers

Comments

@ry
Copy link
Member

ry commented Mar 26, 2020

sys-info is broken on Windows for 3 releases now, and doesn't have proper CI. We cannot depend on that.

This other crate https://github.com/myfreeweb/systemstat looks like a good replacement

This should be a relatively easy Rust issue for someone who wants to look around the code base.

@ry ry added the good first issue Good for newcomers label Mar 26, 2020
@Nimpruda
Copy link

I'm bored because of all this corona confinement stuff (I'm French) I'll take it.

@Nimpruda
Copy link

@ry There is no way to get the hostname or the os release version with systemstat shoul I just read from /proc/... for unix and execute hostname for windows (as there is no os_release).
I'm sorry I'm a bit noob with this stuff

@ry
Copy link
Member Author

ry commented Mar 27, 2020

@Nimpruda you can return OpError::not_implemented() for those. We can get to it later.

For the tests that will break, use ignored: true.

Thanks for looking into this!

@bsene
Copy link

bsene commented Apr 6, 2020

Hello @Nimpruda ,
Are you still working in this issue ?

@Axighi
Copy link

Axighi commented May 12, 2020

@ry I'll work on it.

@Axighi
Copy link

Axighi commented May 12, 2020

I didn't find any usage of sys-info in the code base. What am I missing here?

@oberzan
Copy link

oberzan commented May 13, 2020

The sys_info seems to have fixed the issue: FillZpp/sys-info-rs#50
@ry Can you confirm that?

@kmlroot
Copy link

kmlroot commented May 14, 2020

@oberzan #4504 (review)

@ColinHarrington
Copy link
Contributor

Looks like @ry is reconsidering systemstat lacking hostname and release parsing functionality, dependency issues among others. It's not a 1:1 replacement. Should the good first issue label be removed?

@joshdover
Copy link

sys-info was upgraded successfully to 0.6.1 a few days ago (#5104) but the issue remains that they don't have a windows build in their CI.

@ktfth
Copy link
Contributor

ktfth commented May 17, 2020

Have another way to test the changes?

@Gum-Joe
Copy link

Gum-Joe commented Jun 14, 2020

I know it's been 28 days since the last comment, but would it be worth submitting a PR to sys-info to add windows CI? It looks like it's as simple as adding an entry to Travis build files. Or am I missing something here, as it seems it's still to be decided if systemstat or sys-info will be used, particularly given Dahl comments on #4504?

@ry
Copy link
Member Author

ry commented Jun 15, 2020

@Gum-Joe The latest releases of sys-info are working on Windows. It would be quite nice to help them get Windows CI working. For now I'm going to close this issue since it's not a problem anymore - and IIRC systemstat had its own issues.

@Gum-Joe
Copy link

Gum-Joe commented Jun 16, 2020

@ry Ok, thank you. I've opened FillZpp/sys-info-rs#65 to add Windows CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

10 participants