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

Support rust standard library in guest #161

Merged
merged 6 commits into from
Jun 19, 2022
Merged

Support rust standard library in guest #161

merged 6 commits into from
Jun 19, 2022

Conversation

shkoo
Copy link
Contributor

@shkoo shkoo commented Jun 15, 2022

Automatically download standard library from risc0's rust repository

Bump current rust nightly channel up to 2022-06-13

Fixes #148

@shkoo shkoo requested a review from flaub June 15, 2022 04:53
@shkoo
Copy link
Contributor Author

shkoo commented Jun 15, 2022

Unfortunately this also breaks #116 again

Comment on lines +62 to +63
// This is an alias for either std::Error, or serde's no_std error replacement.
impl serde::ser::StdError for Error {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@shkoo shkoo marked this pull request as ready for review June 15, 2022 05:01
@flaub
Copy link
Member

flaub commented Jun 15, 2022

My inclination would be to wait until #116 is fixed upstream since build reproducibility is a larger need than std support at the moment.

I'm also curious what it would take to get a standard main entrypoint. I suspect we need to adjust the zkVM circuit since we rely on a specific ABI for the entry (both on input and the 'result' lands in registers currently).

@flaub
Copy link
Member

flaub commented Jun 15, 2022

@jbruestle WDYT?

@jbruestle
Copy link
Contributor

I am so psyched about std support (and might grab this branch and try out a few things for fun!), but I do also agree that annoyingly for our actual use cases, non-reproducibility is a critical bug, whereas support for std is a hugely valuable but not required feature. I'm a bit concerned that there hasn't been any review on our cargo PR, but it's only been a few day, so I think it's still impolite to poke. It seems likely that it could be OK'd at which point this issue goes away, but I guess overall I'm also of the mindset that we shouldn't land this yet.

Copy link
Member

@flaub flaub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to block this for now, but very interested to see if we can workaround #116 or wait for rust-lang/cargo#10746 to land

@shkoo
Copy link
Contributor Author

shkoo commented Jun 15, 2022

From https://github.com/rust-lang/cargo/blob/master/CONTRIBUTING.md:

NOTICE: Due to limited review capacity, the Cargo team is not accepting new features or major changes at this time. Please consult with the team before opening a new PR. Only issues that have been explicitly marked as accepted will be reviewed.

So, it could take some additional work to get this into cargo. :(

I'll see if I can figure out another workaround.

@shkoo shkoo marked this pull request as draft June 15, 2022 17:04
@jbruestle
Copy link
Contributor

Can we also hit up the discussion forum they mention on the page and talk to the devs about what we are doing? Even if we can hack around it, I think this is a case where it's a useful feature for them to put in upstream, and developing relations with the Rust devs is probably a good call.

@shkoo shkoo marked this pull request as ready for review June 19, 2022 17:54
@shkoo shkoo merged commit 2e8a495 into main Jun 19, 2022
@shkoo shkoo deleted the nils/riscv_std branch June 19, 2022 17:55
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.

Add std support for guest
3 participants