Skip to content

Commit

Permalink
fix: cli build with profile (#1143)
Browse files Browse the repository at this point in the history
* fix: cli build with profile

* fix: tempdir early drop
  • Loading branch information
jonathanpwang authored Dec 30, 2024
1 parent 1e683b0 commit fff23b3
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 46 deletions.
4 changes: 3 additions & 1 deletion benchmarks/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ pub fn build_bench_program(program_name: &str) -> Result<Elf> {
std::process::exit(code);
}
// Assumes the package has a single target binary
let elf_path = guest_methods(&pkg, &target_dir, &[]).pop().unwrap();
let elf_path = guest_methods(&pkg, &target_dir, &guest_opts.features, &guest_opts.profile)
.pop()
.unwrap();
let data = read(elf_path)?;
Elf::decode(&data, MEM_SIZE as u32)
}
Expand Down
12 changes: 12 additions & 0 deletions book/src/writing-apps/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ The following flags are available for the `cargo openvm build` command:

This ensures the build command is executed in that directory.

- `--target-dir <TARGET_DIR>`

**Description**: Specifies the directory where the guest binary will be built. If not specified, the default target directory is used.

**Default**: The `target` directory in the package root directory.

**Usage Example**: To build the guest binary in the `my_target` directory:

```bash
cargo openvm build --target-dir my_target
```

- `--features <FEATURES>`

**Description**: Passes a list of feature flags to the Cargo build process. These flags enable or disable conditional compilation features defined in your `Cargo.toml`.
Expand Down
1 change: 1 addition & 0 deletions crates/cli/example/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[workspace]
[package]
name = "openvm-cli-example-test"
version = "0.0.0"
Expand Down
50 changes: 43 additions & 7 deletions crates/cli/src/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ pub struct BuildArgs {
)]
pub manifest_dir: PathBuf,

#[arg(long, help = "Path to the target directory")]
pub target_dir: Option<PathBuf>,

#[arg(long, value_delimiter = ',', help = "Feature flags passed to cargo")]
pub features: Vec<String>,

Expand Down Expand Up @@ -67,13 +70,13 @@ pub struct BuildArgs {
pub profile: String,
}

#[derive(Clone, clap::Args)]
#[derive(Clone, Default, clap::Args)]
#[group(required = false, multiple = false)]
pub struct BinTypeFilter {
#[arg(long, help = "Specifies that the bin target to build")]
#[arg(long, help = "Specifies the bin target to build")]
pub bin: Option<String>,

#[arg(long, help = "Specifies that the example target to build")]
#[arg(long, help = "Specifies the example target to build")]
pub example: Option<String>,
}

Expand All @@ -95,10 +98,10 @@ pub(crate) fn build(build_args: &BuildArgs) -> Result<Option<PathBuf>> {
kind: "example".to_string(),
})
};
let guest_options = GuestOptions {
features: build_args.features.clone(),
..Default::default()
};
let mut guest_options = GuestOptions::default()
.with_features(build_args.features.clone())
.with_profile(build_args.profile.clone());
guest_options.target_dir = build_args.target_dir.clone();

let pkg = get_package(&build_args.manifest_dir);
// We support builds of libraries with 0 or >1 executables.
Expand Down Expand Up @@ -142,3 +145,36 @@ pub(crate) fn build(build_args: &BuildArgs) -> Result<Option<PathBuf>> {
Ok(None)
}
}

#[cfg(test)]
mod tests {
use std::path::PathBuf;

use eyre::Result;
use openvm_build::RUSTC_TARGET;

use super::*;

#[test]
fn test_build_with_profile() -> Result<()> {
let temp_dir = tempfile::tempdir()?;
let target_dir = temp_dir.path();
let build_args = BuildArgs {
manifest_dir: PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("example"),
features: vec![],
bin_type_filter: Default::default(),
no_transpile: true,
config: PathBuf::from(DEFAULT_APP_CONFIG_PATH),
exe_output: PathBuf::from(DEFAULT_APP_EXE_PATH),
profile: "dev".to_string(),
target_dir: Some(target_dir.to_path_buf()),
};
build(&build_args)?;
assert!(
target_dir.join(RUSTC_TARGET).join("debug").exists(),
"did not build with dev profile"
);
temp_dir.close()?;
Ok(())
}
}
6 changes: 3 additions & 3 deletions crates/toolchain/build/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ use serde::{Deserialize, Serialize};
pub struct GuestOptions {
/// Features for cargo to build the guest with.
pub features: Vec<String>,
/// Custom options to pass as args to `cargo build`.
pub options: Vec<String>,
/// Configuration flags to build the guest with.
pub rustc_flags: Vec<String>,
/// Cargo profile
pub profile: Option<String>,
/// Target directory
pub target_dir: Option<PathBuf>,
/// Custom options to pass as args to `cargo build`.
pub(crate) options: Vec<String>,
}

impl GuestOptions {
Expand Down Expand Up @@ -71,7 +71,7 @@ pub(crate) struct GuestMetadata {

impl From<&Package> for GuestMetadata {
fn from(value: &Package) -> Self {
let Some(obj) = value.metadata.get("risc0") else {
let Some(obj) = value.metadata.get("openvm") else {
return Default::default();
};
serde_json::from_value(obj.clone()).unwrap()
Expand Down
54 changes: 28 additions & 26 deletions crates/toolchain/build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ pub use self::config::GuestOptions;

mod config;

/// The rustc compiler [target](https://doc.rust-lang.org/rustc/targets/index.html).
pub const RUSTC_TARGET: &str = "riscv32im-risc0-zkvm-elf";
const RUSTUP_TOOLCHAIN_NAME: &str = "nightly-2024-10-30";
const BUILD_LOCKED_ENV: &str = "OPENVM_BUILD_LOCKED";
const BUILD_DEBUG_ENV: &str = "OPENVM_BUILD_DEBUG";
const SKIP_BUILD_ENV: &str = "OPENVM_SKIP_BUILD";
const GUEST_LOGFILE_ENV: &str = "OPENVM_GUEST_LOGFILE";

Expand Down Expand Up @@ -77,10 +78,14 @@ pub fn get_dir_with_profile(
profile: &str,
examples: bool,
) -> PathBuf {
let res = target_dir
.as_ref()
.join("riscv32im-risc0-zkvm-elf")
.join(profile);
let mut res = target_dir.as_ref().join(RUSTC_TARGET).to_path_buf();
if profile == "dev" || profile == "test" {
res.push("debug");
} else if profile == "bench" {
res.push("release");
} else {
res.push(profile);
}
if examples {
res.join("examples")
} else {
Expand All @@ -93,11 +98,6 @@ pub fn current_package() -> Package {
get_package(env::var("CARGO_MANIFEST_DIR").unwrap())
}

/// Reads the value of the environment variable `OPENVM_BUILD_DEBUG` and returns true if it is set to 1.
pub fn is_debug() -> bool {
get_env_var(BUILD_DEBUG_ENV) == "1"
}

/// Reads the value of the environment variable `OPENVM_SKIP_BUILD` and returns true if it is set to 1.
pub fn is_skip_build() -> bool {
!get_env_var(SKIP_BUILD_ENV).is_empty()
Expand All @@ -109,12 +109,13 @@ fn get_env_var(name: &str) -> String {
}

/// Returns all target ELF paths associated with the given guest crate.
pub fn guest_methods(
pub fn guest_methods<S: AsRef<str>>(
pkg: &Package,
target_dir: impl AsRef<Path>,
guest_features: &[String],
profile: &Option<S>,
) -> Vec<PathBuf> {
let profile = if is_debug() { "debug" } else { "release" };
let profile = profile.as_ref().map(|s| s.as_ref()).unwrap_or("release");
pkg.targets
.iter()
.filter(|target| {
Expand All @@ -129,13 +130,19 @@ pub fn guest_methods(
.iter()
.all(|required_feature| guest_features.contains(required_feature))
})
.map(|target| {
target_dir
.as_ref()
.join("riscv32im-risc0-zkvm-elf")
.join(profile)
.join(&target.name)
.to_path_buf()
.flat_map(|target| {
let path_prefix = target_dir.as_ref().join(RUSTC_TARGET).join(profile);
target
.kind
.iter()
.map(|target_kind| {
let mut path = path_prefix.clone();
if target_kind == "example" {
path.push(target_kind);
}
path.join(&target.name).to_path_buf()
})
.collect::<Vec<_>>()
})
.collect()
}
Expand Down Expand Up @@ -167,7 +174,7 @@ pub fn cargo_command(subcmd: &str, rust_flags: &[&str]) -> Command {
println!("Using rustc: {rustc}");

let mut cmd = sanitized_cmd("cargo");
let mut args = vec![&toolchain, subcmd, "--target", "riscv32im-risc0-zkvm-elf"];
let mut args = vec![&toolchain, subcmd, "--target", RUSTC_TARGET];

if std::env::var(BUILD_LOCKED_ENV).is_ok() {
args.push("--locked");
Expand Down Expand Up @@ -299,8 +306,6 @@ pub fn build_guest_package(

let profile = if let Some(profile) = &guest_opts.profile {
profile
} else if is_debug() {
"dev"
} else {
"release"
};
Expand All @@ -325,10 +330,7 @@ pub fn build_guest_package(
.expect("cargo build failed");
let stderr = child.stderr.take().unwrap();

tty_println(&format!(
"{}: Starting build for riscv32im-risc0-zkvm-elf",
pkg.name
));
tty_println(&format!("{}: Starting build for {RUSTC_TARGET}", pkg.name));

for line in BufReader::new(stderr).lines() {
tty_println(&format!("{}: {}", pkg.name, line.unwrap()));
Expand Down
17 changes: 8 additions & 9 deletions crates/toolchain/tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ use std::{
path::{Path, PathBuf},
};

use eyre::Result;
use openvm_build::{build_guest_package, get_package, is_debug, GuestOptions, TargetFilter};
use eyre::{Context, Result};
use openvm_build::{
build_guest_package, get_dir_with_profile, get_package, GuestOptions, TargetFilter,
};
use openvm_transpiler::{elf::Elf, openvm_platform::memory::MEM_SIZE};
use tempfile::tempdir;

Expand Down Expand Up @@ -67,21 +69,18 @@ pub fn build_example_program_at_path_with_features<S: AsRef<str>>(
std::process::exit(code);
}
// Assumes the package has a single target binary
let profile = if is_debug() { "debug" } else { "release" };
let profile = "release";
let elf_path = pkg
.targets
.iter()
.find(|target| target.name == example_name)
.map(|target| {
target_dir
.as_ref()
.join("riscv32im-risc0-zkvm-elf")
.join(profile)
.join("examples")
get_dir_with_profile(&target_dir, profile, true)
.join(&target.name)
.to_path_buf()
})
.expect("Could not find target binary");
let data = read(elf_path)?;
let data = read(&elf_path).with_context(|| format!("Path not found: {:?}", elf_path))?;
target_dir.close()?;
Elf::decode(&data, MEM_SIZE as u32)
}

0 comments on commit fff23b3

Please sign in to comment.