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

Suggestion: using new system info crate #8357

Closed
jopemachine opened this issue Nov 12, 2020 · 4 comments
Closed

Suggestion: using new system info crate #8357

jopemachine opened this issue Nov 12, 2020 · 4 comments
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@jopemachine
Copy link
Contributor

jopemachine commented Nov 12, 2020

Issue

The sys-info crate seems that it doesn't provide enough APIs for deno and deno's node compatibility.

I think in this issue (#7774), Deno.systemCpuInfo discarded node compatibility because of it.

Suggestion

How about using a new crate like heim?

Example

For example, I think the uptime function could be implemented by the below code using heim

fn op_uptime(
  state: &mut OpState,
  _args: Value,
  _zero_copy: &mut [ZeroCopyBuf],
) -> Result<Value, AnyError> {
  super::check_unstable(state, "Deno.uptime");
  state.borrow::<Permissions>().check_env()?;

  let uptime = (host::uptime().await?.get::<time::second>() as u32);

  Ok(json!({
    "uptime": uptime
  }))
}

This function would be needed to implement node compatibility.

Related issue: #3802.

Notes

If we need another package other than heim, I would be appreciated if you let me know.

@jopemachine jopemachine changed the title Suggestion: Introducing new system info crate Suggestion: using new system info crate Nov 12, 2020
@kitsonk kitsonk added cli related to cli/ dir suggestion suggestions for new features (yet to be agreed) labels Nov 12, 2020
@kitsonk
Copy link
Contributor

kitsonk commented Nov 12, 2020

Achieving 100% compatibility with Node.js is not achievable with Deno, nor is it a goal of Deno.

What APIs that are critical that are missing where there are use cases where mocking the API isn't feasible?

heim is a lot of crates, and haven't gone through all the dependencies yet, and it seems like it is fairly slow development that has slowed, wit the biggest version being downloaded being a beta release. That doesn't feel comfortable having such an "immature" set of a crates in Deno.

@littledivy
Copy link
Member

Looking at host::uptime https://github.com/heim-rs/heim/blob/master/heim-host/src/sys/linux/uptime.rs - heim basically reads /proc/uptime on linux. Adding a heavy dependency for such small task is unnecessary.

@jopemachine
Copy link
Contributor Author

jopemachine commented Nov 13, 2020

Looking at host::uptime https://github.com/heim-rs/heim/blob/master/heim-host/src/sys/linux/uptime.rs - heim basically reads /proc/uptime on linux. Adding a heavy dependency for such small task is unnecessary.

Thanks for letting me know this :)

I think I could implement uptime in this way.

I think I need to look into if other methods could be implemented without additional dependencies.

@bartlomieju
Copy link
Member

I'm -1 on bringing heim as dependency. As pointed out it's pretty heavy dependency for the task at hand. Respectfully closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

4 participants