Skip to content

/ deno

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

Jump to bottom

Replace sys-info crate dependency with systemstat #4496

Open
ry opened this issue Mar 26, 2020 · 12 comments
Open

Replace sys-info crate dependency with systemstat #4496

ry opened this issue Mar 26, 2020 · 12 comments
Labels
good first issue

Comments

@ry
Copy link
Collaborator

@ry ry commented Mar 26, 2020 •
edited

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 label Mar 26, 2020
@Nimpruda
Copy link

@Nimpruda Nimpruda commented Mar 26, 2020

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

@Nimpruda Nimpruda commented Mar 26, 2020

@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
Collaborator Author

@ry ry commented Mar 27, 2020 •
edited

@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 bsene commented Apr 6, 2020

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

@Axighi Axighi commented May 12, 2020

@ry I'll work on it.
@Axighi
Copy link

@Axighi 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 oberzan commented May 13, 2020

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

@fmauricios fmauricios commented May 14, 2020 •
edited

#4504 (review)
@ColinHarrington
Copy link
Contributor

@ColinHarrington ColinHarrington commented May 14, 2020

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

@joshdover joshdover commented May 15, 2020

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

@ktfth ktfth commented May 17, 2020

Have another way to test the changes?
@Gum-Joe
Copy link

@Gum-Joe 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?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
@ry @ColinHarrington @Axighi @joshdover @bsene @Nimpruda @fmauricios @Gum-Joe @oberzan @ktfth
You can’t perform that action at this time.

Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant