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

feat: version compat checking in build #1791

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open

feat: version compat checking in build #1791

wants to merge 7 commits into from

Conversation

nhtyy
Copy link
Contributor

@nhtyy nhtyy commented Nov 14, 2024

Version checking the sp1-zkvm crate with the current toolchain to ensure compatibly.

Assumes that major/minor of sp1-zkvm and sp1-build matching implies compat, which would be inline if sp1-zkvm inherits workspace version in the future

edit: closes #1625

Copy link

github-actions bot commented Nov 14, 2024

SP1 Performance Test Results

Branch: n/version-check
Commit: 6c0d8be
Author: nhtyy

program cycles execute (mHz) core (kHZ) compress (KHz) time success
fibonacci 11291 0.17 2.82 0.46 24s
ssz-withdrawals 2757356 17.79 124.78 34.88 1m19s
tendermint 12593597 6.63 267.21 99.26 2m9s

Copy link
Contributor

@yuwen01 yuwen01 left a comment

Choose a reason for hiding this comment

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

https://crates.io/crates/cargo-lock

Is there a reason we're parsing the cargo.lock by hand instead of using this crate?

crates/build/Cargo.toml Outdated Show resolved Hide resolved
pub(crate) fn build_program_internal(path: &str, args: Option<BuildArgs>) {
// Get the root package name and metadata.
let program_dir = std::path::Path::new(path);
verify_locked_version(program_dir).expect("locked version mismatch");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this error message more descriptive? As someone reading this error message, I don't understand what is mismatched between lock files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So with the expect also displaying the inner error msg I think its sufficent wdyt
ex: locked version mismatch: Locked version of sp1-sdk is incompatible with the current toolchain version

or itll say zkvm depending on the mismatch

crates/build/src/build.rs Outdated Show resolved Hide resolved
crates/build/src/build.rs Outdated Show resolved Hide resolved
crates/build/src/build.rs Outdated Show resolved Hide resolved
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.

Nicer error message when toolchain version and sp1-zkvm versions are incompatible
2 participants