-
Notifications
You must be signed in to change notification settings - Fork 355
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
base: dev
Are you sure you want to change the base?
Changes from 9 commits
385a9e6
1cdb3b2
418678b
5018a27
872c431
c062eef
b636df5
eaa6ffa
0f29948
8f5a636
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,7 +1,6 @@ | ||||||
use std::path::PathBuf; | ||||||
|
||||||
use anyhow::Result; | ||||||
use cargo_metadata::camino::Utf8PathBuf; | ||||||
use std::path::{Path, PathBuf}; | ||||||
|
||||||
use crate::{ | ||||||
command::{docker::create_docker_command, local::create_local_command, utils::execute_command}, | ||||||
|
@@ -32,6 +31,8 @@ pub fn execute_build_program( | |||||
let program_dir: Utf8PathBuf = | ||||||
program_dir.try_into().expect("Failed to convert PathBuf to Utf8PathBuf"); | ||||||
|
||||||
verify_locked_version(&program_dir, args.verbose).expect("Version check mismatch"); | ||||||
|
||||||
// Get the program metadata. | ||||||
let program_metadata_file = program_dir.join("Cargo.toml"); | ||||||
let mut program_metadata_cmd = cargo_metadata::MetadataCommand::new(); | ||||||
|
@@ -54,9 +55,15 @@ pub fn execute_build_program( | |||||
} | ||||||
|
||||||
/// Internal helper function to build the program with or without arguments. | ||||||
/// | ||||||
/// Note: This function is not intended to be used by the CLI, as it looks for the sp1-sdk, | ||||||
/// which is probably in the same crate lockfile as this function is only called by build script. | ||||||
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, args.as_ref().map(|args| args.verbose).unwrap_or(false)) | ||||||
.expect("Locked version mismatch"); | ||||||
|
||||||
let metadata_file = program_dir.join("Cargo.toml"); | ||||||
let mut metadata_cmd = cargo_metadata::MetadataCommand::new(); | ||||||
let metadata = metadata_cmd.manifest_path(metadata_file).exec().unwrap(); | ||||||
|
@@ -180,3 +187,120 @@ fn print_elf_paths_cargo_directives(target_elf_paths: &[(String, Utf8PathBuf)]) | |||||
println!("cargo:rustc-env=SP1_ELF_{}={}", target_name, elf_path); | ||||||
} | ||||||
} | ||||||
|
||||||
/// Verify that the locked version of `sp1-zkvm` in the Cargo.lock file is compatible with the | ||||||
/// current version of this crate. | ||||||
/// | ||||||
/// This also checks to ensure that `sp1-sdk` is also the correct version. | ||||||
/// | ||||||
/// ## Note: This function assumes that version compatibility is given by matching major and minor | ||||||
/// semver. | ||||||
/// | ||||||
/// This is also correct if future releases sharing the workspace version, which should be the case. | ||||||
/// | ||||||
/// # Errors: | ||||||
/// - If the locked version of `sp1-zkvm` is incompatible with the current toolchain version. | ||||||
/// - If the locked version of `sp1-sdk` is incompatible with the current toolchain version. | ||||||
/// - | ||||||
/// | ||||||
/// # Panics | ||||||
/// - If we cant IO | ||||||
/// - If we cant find your lockfile | ||||||
/// - If we cant parse the version from the `cargo-prove` | ||||||
fn verify_locked_version(program_dir: impl AsRef<Path>, verbose: bool) -> Result<()> { | ||||||
if std::env::var("DEV_SP1_SKIP_VERSION_CHECK").is_ok() { | ||||||
println!("cargo:warning=Skipping version check"); | ||||||
return Ok(()); | ||||||
} | ||||||
|
||||||
// We need an exception for our test fixtures. | ||||||
if std::env::var("CARGO_PKG_NAME").unwrap_or_default() == "test-artifacts" { | ||||||
return Ok(()); | ||||||
} | ||||||
|
||||||
// This might be a workspace, need to optionally search parent dirs for lock files | ||||||
let canon = | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit punc |
||||||
program_dir.as_ref().canonicalize().expect("Failed to canonicalize path to program dir"); | ||||||
|
||||||
let mut lock_path = canon.join("Cargo.lock"); | ||||||
if !lock_path.is_file() { | ||||||
let mut curr_path: &Path = canon.as_ref(); | ||||||
|
||||||
while let Some(parent) = curr_path.parent() { | ||||||
let maybe_lock_path = parent.join("Cargo.lock"); | ||||||
|
||||||
if maybe_lock_path.is_file() { | ||||||
lock_path = maybe_lock_path; | ||||||
break; | ||||||
} else { | ||||||
curr_path = parent; | ||||||
} | ||||||
} | ||||||
|
||||||
if !lock_path.is_file() { | ||||||
panic!("Failed to find Cargo.lock in the program directory or its parent directories"); | ||||||
} | ||||||
} | ||||||
|
||||||
println!("cargo:warning=Found Cargo.lock at {}", lock_path.display()); | ||||||
|
||||||
let lock_file = cargo_lock::Lockfile::load(lock_path).expect("Failed to load Cargo.lock"); | ||||||
|
||||||
// You need an entrypoint, therefore you need sp1-zkvm. | ||||||
let vm_version = &lock_file | ||||||
.packages | ||||||
.iter() | ||||||
.find(|p| p.name.as_str() == "sp1-zkvm") | ||||||
.expect("The guest program should have a sp1-zkvm dependency") | ||||||
.version; | ||||||
|
||||||
let maybe_sdk = | ||||||
lock_file.packages.iter().find(|p| p.name.as_str() == "sp1-sdk").map(|p| &p.version); | ||||||
|
||||||
// Print these just to be useful. | ||||||
// Most people will install the actual rust toolchain along with thier cargo-prove, | ||||||
// so we can just use the cargo-prove version. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typos:
Suggested change
|
||||||
let toolchain_version = { | ||||||
let output = std::process::Command::new("cargo") | ||||||
.args(["prove", "--version"]) | ||||||
.output() | ||||||
.expect("Failed to find check toolchain version, see https://docs.succinct.xyz/getting-started/install.html"); | ||||||
|
||||||
let version = String::from_utf8(output.stdout).expect("Failed to parse version string"); | ||||||
|
||||||
semver::Version::parse( | ||||||
version | ||||||
.split_once('\n') | ||||||
.expect("The version should have whitespace, this is a bug") | ||||||
.1 | ||||||
.trim(), | ||||||
) | ||||||
.expect("Failed to parse version string, this is a bug") | ||||||
}; | ||||||
|
||||||
if verbose { | ||||||
maybe_sdk.inspect(|v| { | ||||||
println!("cargo:warning=Locked version of sp1-sdk is {}", v); | ||||||
}); | ||||||
println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_version); | ||||||
println!("cargo:warning=Current toolchain version = {}", toolchain_version); | ||||||
} | ||||||
|
||||||
if vm_version.major != toolchain_version.major || vm_version.minor != toolchain_version.minor { | ||||||
return Err(anyhow::anyhow!( | ||||||
"Locked version of sp1-zkvm is incompatible with the current toolchain version" | ||||||
)); | ||||||
} | ||||||
|
||||||
if let Some(sdk_version) = maybe_sdk { | ||||||
if sdk_version.major != toolchain_version.major | ||||||
|| sdk_version.minor != toolchain_version.minor | ||||||
{ | ||||||
return Err(anyhow::anyhow!( | ||||||
"Locked version of sp1-sdk is incompatible with the current toolchain version" | ||||||
)); | ||||||
} | ||||||
} | ||||||
|
||||||
Ok(()) | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: cant -> can't