From 6fd08016ce395046be4ce18360a7fad1158efb87 Mon Sep 17 00:00:00 2001 From: Ratan Kaliani Date: Tue, 16 Jul 2024 17:25:27 -0700 Subject: [PATCH 1/7] fix: build program same workspace --- build/src/lib.rs | 68 ++++++++++++++++++++++++++++++++++------------- helper/src/lib.rs | 5 +++- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/build/src/lib.rs b/build/src/lib.rs index 8b7374442f..7632ded85a 100644 --- a/build/src/lib.rs +++ b/build/src/lib.rs @@ -5,7 +5,7 @@ use std::{ fs, io::{BufRead, BufReader}, path::PathBuf, - process::{exit, Command, Stdio}, + process::{Command, Stdio}, thread, }; @@ -128,8 +128,11 @@ fn get_rust_compiler_flags() -> String { /// Get the command to build the program locally. fn create_local_command(args: &BuildArgs, program_dir: &Utf8PathBuf) -> Command { let mut command = Command::new("cargo"); + let canonicalized_program_dir = program_dir + .canonicalize() + .expect("Failed to canonicalize program directory"); command - .current_dir(program_dir.clone()) + .current_dir(canonicalized_program_dir) .env("RUSTUP_TOOLCHAIN", "succinct") .env("CARGO_ENCODED_RUSTFLAGS", get_rust_compiler_flags()) .args(&get_program_build_args(args)); @@ -138,12 +141,27 @@ fn create_local_command(args: &BuildArgs, program_dir: &Utf8PathBuf) -> Command /// Execute the command and handle the output. Note: Strip the rustc configuration if this is called /// by sp1-helper so it uses the Succinct toolchain. -fn execute_command(mut command: Command, build_with_helper: bool, docker: bool) -> Result<()> { - // Strip the rustc configuration if this is called by sp1-helper, otherwise it will attempt to - // compile the SP1 program with the toolchain of the normal build process, rather than the - // Succinct toolchain. +fn execute_command( + mut command: Command, + build_with_helper: bool, + docker: bool, + program_metadata: &cargo_metadata::Metadata, +) -> Result<()> { if build_with_helper { + // Strip the rustc configuration if this is called by sp1-helper, otherwise it will attempt to + // compile the SP1 program with the toolchain of the normal build process, rather than the + // Succinct toolchain. command.env_remove("RUSTC"); + + // Set the target directory to a subdirectory of the program's target directory to avoid + // build conflicts with the parent process. If removed, programs that share the same target + // directory (i.e. same workspace) as the script will hang indefinitely due to a file lock + // when building. + // Source: https://github.com/rust-lang/cargo/issues/6412 + command.env( + "CARGO_TARGET_DIR", + program_metadata.target_directory.join("elf-compilation"), + ); } // Add necessary tags for stdout and stderr from the command. @@ -178,25 +196,30 @@ fn execute_command(mut command: Command, build_with_helper: bool, docker: bool) let result = child.wait()?; if !result.success() { // Error message is already printed by cargo. - exit(result.code().unwrap_or(1)) + panic!( + "cargo build failed with status code: {}", + result.code().unwrap_or(1) + ) } Ok(()) } /// Copy the ELF to the specified output directory. -fn copy_elf_to_output_dir(args: &BuildArgs, program_dir: &Utf8PathBuf) -> Result { - let program_metadata_file = program_dir.join("Cargo.toml"); - let mut program_metadata_cmd = cargo_metadata::MetadataCommand::new(); - let program_metadata = program_metadata_cmd - .manifest_path(program_metadata_file) - .exec() - .unwrap(); +fn copy_elf_to_output_dir( + args: &BuildArgs, + is_helper: bool, + program_metadata: &cargo_metadata::Metadata, +) -> Result { let root_package = program_metadata.root_package(); let root_package_name = root_package.as_ref().map(|p| &p.name); // The ELF is written to a target folder specified by the program's package. - let original_elf_path = program_metadata - .target_directory + // Conditionally add "elf-compilation" to the path based on is_helper. + let mut original_elf_path = program_metadata.target_directory.clone(); + if is_helper { + original_elf_path = original_elf_path.join("elf-compilation"); + } + original_elf_path = original_elf_path .join(BUILD_TARGET) .join("release") .join(root_package_name.unwrap()); @@ -256,12 +279,19 @@ pub fn build_program(args: &BuildArgs, program_dir: Option) -> Result Date: Tue, 16 Jul 2024 17:33:21 -0700 Subject: [PATCH 2/7] clean --- build/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/build/src/lib.rs b/build/src/lib.rs index 7632ded85a..d81ef1210d 100644 --- a/build/src/lib.rs +++ b/build/src/lib.rs @@ -15,6 +15,7 @@ use cargo_metadata::camino::Utf8PathBuf; const BUILD_TARGET: &str = "riscv32im-succinct-zkvm-elf"; const DEFAULT_TAG: &str = "latest"; const DEFAULT_OUTPUT_DIR: &str = "elf"; +const HELPER_TARGET_SUBDIR: &str = "elf-compilation"; /// [`BuildArgs`] is a struct that holds various arguments used for building a program. /// @@ -160,7 +161,7 @@ fn execute_command( // Source: https://github.com/rust-lang/cargo/issues/6412 command.env( "CARGO_TARGET_DIR", - program_metadata.target_directory.join("elf-compilation"), + program_metadata.target_directory.join(HELPER_TARGET_SUBDIR), ); } @@ -217,7 +218,7 @@ fn copy_elf_to_output_dir( // Conditionally add "elf-compilation" to the path based on is_helper. let mut original_elf_path = program_metadata.target_directory.clone(); if is_helper { - original_elf_path = original_elf_path.join("elf-compilation"); + original_elf_path = original_elf_path.join(HELPER_TARGET_SUBDIR); } original_elf_path = original_elf_path .join(BUILD_TARGET) From 2f01c44ac620507323dabb8c087723dca7034b82 Mon Sep 17 00:00:00 2001 From: Ratan Kaliani Date: Tue, 16 Jul 2024 17:33:44 -0700 Subject: [PATCH 3/7] fix --- build/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/src/lib.rs b/build/src/lib.rs index d81ef1210d..27e14a2393 100644 --- a/build/src/lib.rs +++ b/build/src/lib.rs @@ -215,7 +215,7 @@ fn copy_elf_to_output_dir( let root_package_name = root_package.as_ref().map(|p| &p.name); // The ELF is written to a target folder specified by the program's package. - // Conditionally add "elf-compilation" to the path based on is_helper. + // Conditionally add HELPER_TARGET_SUBDIR to the path based on is_helper. let mut original_elf_path = program_metadata.target_directory.clone(); if is_helper { original_elf_path = original_elf_path.join(HELPER_TARGET_SUBDIR); From 7ce065f4d63fa2a1c0849e132631a9b833380618 Mon Sep 17 00:00:00 2001 From: Ratan Kaliani Date: Tue, 16 Jul 2024 17:42:41 -0700 Subject: [PATCH 4/7] fix --- build/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/build/src/lib.rs b/build/src/lib.rs index 27e14a2393..727a0866c4 100644 --- a/build/src/lib.rs +++ b/build/src/lib.rs @@ -140,8 +140,7 @@ fn create_local_command(args: &BuildArgs, program_dir: &Utf8PathBuf) -> Command command } -/// Execute the command and handle the output. Note: Strip the rustc configuration if this is called -/// by sp1-helper so it uses the Succinct toolchain. +/// Execute the command and handle the output depending on the context. fn execute_command( mut command: Command, build_with_helper: bool, @@ -214,8 +213,9 @@ fn copy_elf_to_output_dir( let root_package = program_metadata.root_package(); let root_package_name = root_package.as_ref().map(|p| &p.name); - // The ELF is written to a target folder specified by the program's package. - // Conditionally add HELPER_TARGET_SUBDIR to the path based on is_helper. + // The ELF is written to a target folder specified by the program's package. Note: If the ELF + // is built by sp1-helper, the target directory is a subdirectory of the program's target + // directory. let mut original_elf_path = program_metadata.target_directory.clone(); if is_helper { original_elf_path = original_elf_path.join(HELPER_TARGET_SUBDIR); From aa1857bbb1f3c4d249199a1307181ed888d3d424 Mon Sep 17 00:00:00 2001 From: Ratan Kaliani Date: Wed, 17 Jul 2024 16:40:23 -0700 Subject: [PATCH 5/7] simplify configuration, always write to target subdir and remove rustc --- build/src/lib.rs | 48 ++++++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/build/src/lib.rs b/build/src/lib.rs index 727a0866c4..575f900d6e 100644 --- a/build/src/lib.rs +++ b/build/src/lib.rs @@ -143,26 +143,23 @@ fn create_local_command(args: &BuildArgs, program_dir: &Utf8PathBuf) -> Command /// Execute the command and handle the output depending on the context. fn execute_command( mut command: Command, - build_with_helper: bool, + is_helper: bool, docker: bool, program_metadata: &cargo_metadata::Metadata, ) -> Result<()> { - if build_with_helper { - // Strip the rustc configuration if this is called by sp1-helper, otherwise it will attempt to - // compile the SP1 program with the toolchain of the normal build process, rather than the - // Succinct toolchain. - command.env_remove("RUSTC"); - - // Set the target directory to a subdirectory of the program's target directory to avoid - // build conflicts with the parent process. If removed, programs that share the same target - // directory (i.e. same workspace) as the script will hang indefinitely due to a file lock - // when building. - // Source: https://github.com/rust-lang/cargo/issues/6412 - command.env( - "CARGO_TARGET_DIR", - program_metadata.target_directory.join(HELPER_TARGET_SUBDIR), - ); - } + // Strip the rustc configuration, otherwise in the helper it will attempt to compile the SP1 + // program with the toolchain of the normal build process, rather than the Succinct toolchain. + command.env_remove("RUSTC"); + + // Set the target directory to a subdirectory of the program's target directory to avoid + // build conflicts with the parent process. If removed, programs that share the same target + // directory (i.e. same workspace) as the script will hang indefinitely due to a file lock + // when building in the helper. + // Source: https://github.com/rust-lang/cargo/issues/6412 + command.env( + "CARGO_TARGET_DIR", + program_metadata.target_directory.join(HELPER_TARGET_SUBDIR), + ); // Add necessary tags for stdout and stderr from the command. let mut child = command @@ -174,7 +171,7 @@ fn execute_command( let stderr = BufReader::new(child.stderr.take().unwrap()); // Add the [sp1] or [docker] prefix to the output of the child process depending on the context. - let msg = match (build_with_helper, docker) { + let msg = match (is_helper, docker) { (true, true) => "[sp1] [docker] ", (true, false) => "[sp1] ", (false, true) => "[docker] ", @@ -207,20 +204,15 @@ fn execute_command( /// Copy the ELF to the specified output directory. fn copy_elf_to_output_dir( args: &BuildArgs, - is_helper: bool, program_metadata: &cargo_metadata::Metadata, ) -> Result { let root_package = program_metadata.root_package(); let root_package_name = root_package.as_ref().map(|p| &p.name); - // The ELF is written to a target folder specified by the program's package. Note: If the ELF - // is built by sp1-helper, the target directory is a subdirectory of the program's target - // directory. - let mut original_elf_path = program_metadata.target_directory.clone(); - if is_helper { - original_elf_path = original_elf_path.join(HELPER_TARGET_SUBDIR); - } - original_elf_path = original_elf_path + // The ELF is written to a target folder specified by the program's package. + let original_elf_path = program_metadata + .target_directory + .join(HELPER_TARGET_SUBDIR) .join(BUILD_TARGET) .join("release") .join(root_package_name.unwrap()); @@ -294,5 +286,5 @@ pub fn build_program(args: &BuildArgs, program_dir: Option) -> Result Date: Wed, 17 Jul 2024 16:46:37 -0700 Subject: [PATCH 6/7] simplify --- build/src/lib.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/build/src/lib.rs b/build/src/lib.rs index 575f900d6e..70f29e547f 100644 --- a/build/src/lib.rs +++ b/build/src/lib.rs @@ -143,7 +143,6 @@ fn create_local_command(args: &BuildArgs, program_dir: &Utf8PathBuf) -> Command /// Execute the command and handle the output depending on the context. fn execute_command( mut command: Command, - is_helper: bool, docker: bool, program_metadata: &cargo_metadata::Metadata, ) -> Result<()> { @@ -170,12 +169,10 @@ fn execute_command( let stdout = BufReader::new(child.stdout.take().unwrap()); let stderr = BufReader::new(child.stderr.take().unwrap()); - // Add the [sp1] or [docker] prefix to the output of the child process depending on the context. - let msg = match (is_helper, docker) { - (true, true) => "[sp1] [docker] ", - (true, false) => "[sp1] ", - (false, true) => "[docker] ", - (false, false) => "", + // Add prefix to the output of the process depending on the context. + let msg = match docker { + true => "[sp1] [docker] ", + false => "[sp1] ", }; // Pipe stdout and stderr to the parent process with [docker] prefix @@ -256,9 +253,6 @@ fn copy_elf_to_output_dir( /// /// * `Result` - The path to the built program as a `Utf8PathBuf` on success, or an error on failure. pub fn build_program(args: &BuildArgs, program_dir: Option) -> Result { - // If the program directory is specified, this function was called by sp1-helper. - let is_helper = program_dir.is_some(); - // If the program directory is not specified, use the current directory. let program_dir = program_dir .unwrap_or_else(|| std::env::current_dir().expect("Failed to get current directory.")); @@ -284,7 +278,7 @@ pub fn build_program(args: &BuildArgs, program_dir: Option) -> Result Date: Wed, 17 Jul 2024 17:37:40 -0700 Subject: [PATCH 7/7] small fixes --- build/src/lib.rs | 7 ++----- helper/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/build/src/lib.rs b/build/src/lib.rs index 70f29e547f..735f7edbf1 100644 --- a/build/src/lib.rs +++ b/build/src/lib.rs @@ -5,7 +5,7 @@ use std::{ fs, io::{BufRead, BufReader}, path::PathBuf, - process::{Command, Stdio}, + process::{exit, Command, Stdio}, thread, }; @@ -190,10 +190,7 @@ fn execute_command( let result = child.wait()?; if !result.success() { // Error message is already printed by cargo. - panic!( - "cargo build failed with status code: {}", - result.code().unwrap_or(1) - ) + exit(result.code().unwrap_or(1)) } Ok(()) } diff --git a/helper/src/lib.rs b/helper/src/lib.rs index 75435319b7..523c81509d 100644 --- a/helper/src/lib.rs +++ b/helper/src/lib.rs @@ -70,7 +70,7 @@ fn execute_build_cmd( ) }; if let Err(err) = path_output { - panic!("Failed to build program: {}.", err); + panic!("Failed to build SP1 program: {}.", err); } Ok(ExitStatus::default())