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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ anyhow = { version = "1.0.83" }
clap = { version = "4.5.9", features = ["derive", "env"] }
dirs = "5.0.1"
chrono = { version = "0.4.38", default-features = false, features = ["clock"] }
serde = { workspace = true, features = ["derive"] }
semver = "1.0.23"
cargo-lock = "10.0.1"
88 changes: 86 additions & 2 deletions crates/build/src/build.rs
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},
Expand Down Expand Up @@ -54,9 +53,14 @@ 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).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

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();
Expand Down Expand Up @@ -180,3 +184,83 @@ 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.
fn verify_locked_version(program_dir: impl AsRef<Path>) -> Result<()> {
// We need an exception for our test fixtures.
if env!("CARGO_PKG_NAME") != "test-artifacts" {
return Ok(());
}

// This might be a workspace, so we need optionally search parent dirs for lock files
let canon = program_dir.as_ref().canonicalize()?;
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() {
return Err(anyhow::anyhow!("Cargo.lock not found"));
}
}

println!("cargo:warning=Found Cargo.lock at {}", lock_path.display());

let lock_file = cargo_lock::Lockfile::load(lock_path)?;

let vm_package = lock_file
.packages
.iter()
.find(|p| p.name.as_str() == "sp1-zkvm")
.ok_or_else(|| anyhow::anyhow!("sp1-zkvm not found in lock file!"))?;

let sp1_sdk = lock_file
.packages
.iter()
.find(|p| p.name.as_str() == "sp1-sdk")
.ok_or_else(|| anyhow::anyhow!("sp1-sdk not found in lock file!"))?;

// Print these just to be useful.
let toolchain_version = env!("CARGO_PKG_VERSION");
println!("cargo:warning=Locked version of sp1-zkvm is {}", vm_package.version);
println!("cargo:warning=Locked version of sp1-sdk is {}", sp1_sdk.version);
println!("cargo:warning=Current toolchain version = {}", toolchain_version);

let toolchain_version = semver::Version::parse(toolchain_version)?;
let vm_version = &vm_package.version;
let sp1_sdk_version = &sp1_sdk.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 sp1_sdk_version.major != toolchain_version.major
|| sp1_sdk_version.minor != toolchain_version.minor
{
return Err(anyhow::anyhow!(
"Locked version of sp1-sdk is incompatible with the current toolchain version"
));
}

Ok(())
}
2 changes: 1 addition & 1 deletion crates/test-artifacts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ categories.workspace = true
sp1-build = { workspace = true }

[build-dependencies]
sp1-build = { workspace = true }
sp1-build = { workspace = true }
Loading