Skip to content
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
20 changes: 18 additions & 2 deletions extensions/scarb-execute/src/hint_processor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use cairo_lang_casm::hints::{Hint, StarknetHint};
use cairo_lang_casm::hints::{CoreHint, CoreHintBase, Hint, StarknetHint};
use cairo_lang_runner::casm_run::{
MemBuffer, cell_ref_to_relocatable, extract_relocatable, vm_get_range,
MemBuffer, cell_ref_to_relocatable, extract_relocatable, read_felts, vm_get_range,
};
use cairo_lang_runner::{CairoHintProcessor, insert_value_to_cellref};
use cairo_vm::Felt252;
Expand All @@ -19,6 +19,10 @@ use std::sync::Arc;
pub struct ExecuteHintProcessor<'a> {
pub cairo_hint_processor: CairoHintProcessor<'a>,
pub oracle_hint_service: OracleHintService,
/// Captured felts from `printl!` and `println!` statements.
/// Only populated if `capture_enabled` is `true`.
pub captured_print_felts: Vec<Vec<Felt252>>,
pub capture_enabled: bool,
}

impl<'a> HintProcessorLogic for ExecuteHintProcessor<'a> {
Expand All @@ -28,6 +32,18 @@ impl<'a> HintProcessorLogic for ExecuteHintProcessor<'a> {
exec_scopes: &mut ExecutionScopes,
hint_data: &Box<dyn Any>,
) -> Result<(), HintError> {
// Handle print hints
if self.capture_enabled
&& let Some(Hint::Core(CoreHintBase::Core(CoreHint::DebugPrint { start, end }))) =
hint_data.downcast_ref::<Hint>()
{
let felts = read_felts(vm, start, end)?;
self.captured_print_felts.push(felts);
return self
.cairo_hint_processor
.execute_hint(vm, exec_scopes, hint_data);
}
// Handle Oracle cheatcodes
if let Some(Hint::Starknet(StarknetHint::Cheatcode {
selector,
input_start,
Expand Down
35 changes: 31 additions & 4 deletions extensions/scarb-execute/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::profiler::{build_profiler_call_trace, get_profiler_tracked_resource};
use anyhow::{Context, Result, anyhow, bail, ensure};
use bincode::enc::write::Writer;
use cairo_lang_executable::executable::{EntryPointKind, Executable};
use cairo_lang_runner::casm_run::format_for_panic;
use cairo_lang_runner::casm_run::{format_for_debug, format_for_panic};
use cairo_lang_runner::{Arg, CairoHintProcessor, build_hints_dict};
use cairo_lang_utils::bigint::BigUintAsHex;
use cairo_vm::cairo_run::CairoRunConfig;
Expand All @@ -17,7 +17,9 @@ use cairo_vm::types::program::Program;
use cairo_vm::types::relocatable::MaybeRelocatable;
use cairo_vm::{Felt252, cairo_run};
use camino::{Utf8Path, Utf8PathBuf};
use create_output_dir::create_output_dir;
use create_output_dir::{
EXECUTE_PRINT_OUTPUT_FILENAME, EXECUTE_PROGRAM_OUTPUT_FILENAME, create_output_dir,
};
use indoc::formatdoc;
use scarb_extensions_cli::execute::{
Args, BuildTargetSpecifier, ExecutionArgs, OutputFormat, ProgramArguments,
Expand Down Expand Up @@ -161,6 +163,8 @@ pub fn execute(
let mut hint_processor = ExecuteHintProcessor {
cairo_hint_processor,
oracle_hint_service: OracleHintService::new(Some(executable_path.as_std_path())),
capture_enabled: args.run.save_print_output,
captured_print_felts: Default::default(),
};

let proof_mode = args.run.target.is_standalone();
Expand Down Expand Up @@ -193,12 +197,23 @@ pub fn execute(
.then(|| ExecutionResources::try_new(&runner, hint_processor.cairo_hint_processor).ok())
.flatten();

let captured_print_output = hint_processor
.captured_print_felts
.into_iter()
.map(|felts| format_for_debug(felts.into_iter()).to_string())
.collect::<Vec<_>>()
.join("");

let execution_output = (args.run.print_program_output || args.run.save_program_output)
.then(|| ExecutionOutput::try_new(&mut runner))
.transpose()?;

ui.print(ExecutionSummary {
output: args
.run
.print_program_output
.then(|| ExecutionOutput::try_new(&mut runner))
.transpose()?,
.then_some(execution_output.clone())
.flatten(),
resources: args
.run
.print_resource_usage
Expand Down Expand Up @@ -318,6 +333,18 @@ pub fn execute(
fs::write(profiler_trace_path, serialized_trace)?;
}

if args.run.save_program_output
&& let Some(output) = &execution_output
{
let program_output_path = execution_output_dir.join(EXECUTE_PROGRAM_OUTPUT_FILENAME);
fs::write(program_output_path, output.as_str())?;
}

if args.run.save_print_output {
let print_output_path = execution_output_dir.join(EXECUTE_PRINT_OUTPUT_FILENAME);
fs::write(print_output_path, &captured_print_output)?;
}

Comment on lines +336 to +347
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how useful is it, to save this to files, instead of printing to stdout? Can't we just parse the stdout wherever we need to use this? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I based this off of the decision we made in the past regarding scarb prove: Add scarb prove #1900 (comment)

    Although I'm not sure that's super problematic since we have control over the logic of both extensions, I suppose if extracting N from Saving output to [..]N is unstable, then extracting multiline outputs could be considered even less stable 🤔
    That's why we use SCARB_EXECUTION_ID, and it seemed reasonable to me not to rethink this here

  2. Another thing that concerned me - verbosity.

    • If we pass verbosity from doc to execute, and that verbosity is --quiet, the output is lost
    • If we force a normal verbosity for execute, we'd have to capture execute stdout, and then somehow figure out based on doc verbosity what to print and not to print. Since we'd have only raw text at our disposal, there would be no way to filter out messages based on their intended level and doc verbosity
      Maybe there's some elegant solution to this, but I just didn't think it's worth going there since the files should just work 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

I see your point, but I don't really think these two are comparable.

For starters, I don't think that using HintProcessor to capture print is reliably what we want. It seems it can be bypassed (see running with bootloader), but also it runs on the assumption that println! it's the only possible way to access stdout (which I am not sure is true).

It shouldn't require any unpredictable parsing, as we can expect the program output to be the last message when run with --program-output flag. We can use json output format in ui, to output it as easily parsable json. We can also do what we do in scarb metadata, that is, run the command with --quite, but set the output to always be printed regardless of verbosity (imho makes sense, if you are explicitly for it to be printed).

Ok(())
}

Expand Down
9 changes: 6 additions & 3 deletions extensions/scarb-execute/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,27 @@ impl Message for ExecutionSummary {
}
}

#[derive(Serialize)]
#[derive(Clone, Serialize)]
pub struct ExecutionOutput(String);

impl ExecutionOutput {
pub fn try_new(runner: &mut CairoRunner) -> Result<Self> {
let mut output_buffer = "Program output:\n".to_string();
let mut output_buffer = String::new();
runner.vm.write_output(&mut output_buffer)?;
let output = output_buffer.trim_end().to_string();
Ok(Self(output))
}
pub fn as_str(&self) -> &str {
&self.0
}
}

impl Message for ExecutionOutput {
fn print_text(self)
where
Self: Sized,
{
println!("{}", self.0);
println!("Program output:\n{}", self.0);
}

fn structured<S: Serializer>(self, ser: S) -> std::result::Result<S::Ok, S::Error>
Expand Down
67 changes: 67 additions & 0 deletions extensions/scarb-execute/tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,3 +584,70 @@ fn invalid_tracked_resource_for_profiler_trace_file() {
help: valid options are: `cairo-steps` or `sierra-gas`
"#});
}

#[test]
fn can_save_program_output_to_file() {
let t = TempDir::new().unwrap();
executable_project_builder()
.lib_cairo(indoc! {r#"
#[executable]
fn main() -> felt252 {
println!("Hello, world!");
42
}
"#})
.build(&t);

Scarb::quick_command()
.arg("execute")
.arg("--save-program-output")
.current_dir(&t)
.assert()
.success()
.stdout_eq(indoc! {r#"
[..]Compiling hello v0.1.0 ([..]Scarb.toml)
[..]Finished `dev` profile target(s) in [..]
[..]Executing hello
Hello, world!
Saving output to: target/execute/hello/execution1
"#});

let program_output_path = t.child("target/execute/hello/execution1/program_output.txt");
program_output_path.assert(predicates::path::exists().and(is_file_empty().not()));
program_output_path.assert(predicates::str::contains("42"));
}

#[test]
fn can_save_stdout_output_to_file() {
let t = TempDir::new().unwrap();
executable_project_builder()
.lib_cairo(indoc! {r#"
#[executable]
fn main() -> felt252 {
println!("Hello");
println!("world!");
42
}
"#})
.build(&t);

Scarb::quick_command()
.arg("execute")
.arg("--save-print-output")
.current_dir(&t)
.assert()
.success()
.stdout_eq(indoc! {r#"
[..]Compiling hello v0.1.0 ([..]Scarb.toml)
[..]Finished `dev` profile target(s) in [..]
[..]Executing hello
Hello
world!
Saving output to: target/execute/hello/execution1
"#});

let stdout_output_path = t.child("target/execute/hello/execution1/stdout_output.txt");
stdout_output_path.assert(predicates::path::exists().and(is_file_empty().not()));
let output = std::fs::read_to_string(stdout_output_path.path()).unwrap();
assert_eq!(output, "Hello\nworld!\n");
}
4 changes: 3 additions & 1 deletion utils/create-output-dir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
//!
//! [cargo-util-fn]: https://docs.rs/cargo-util/latest/cargo_util/paths/fn.create_dir_all_excluded_from_backups_atomic.html

use anyhow::{Context, Result};
use std::ffi::OsStr;
use std::path::Path;
use std::{env, fs};

use anyhow::{Context, Result};
pub const EXECUTE_PROGRAM_OUTPUT_FILENAME: &str = "program_output.txt";
pub const EXECUTE_PRINT_OUTPUT_FILENAME: &str = "stdout_output.txt";

/// Creates an excluded from cache directory atomically with its parents as needed.
///
Expand Down
16 changes: 16 additions & 0 deletions utils/scarb-extensions-cli/src/execute/checked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ pub struct RunArgs {
/// Whether to save cairo-profiler trace data.
#[arg(long, default_value_t = false)]
pub save_profiler_trace_data: bool,

/// Whether to save program output to a file.
#[arg(long, default_value_t = false)]
pub save_program_output: bool,

/// Whether to save stdout output to a file.
#[arg(long, default_value_t = false)]
pub save_print_output: bool,
}

impl ToArgs for RunArgs {
Expand All @@ -136,6 +144,8 @@ impl ToArgs for RunArgs {
print_program_output,
print_resource_usage,
save_profiler_trace_data,
save_program_output,
save_print_output,
} = self;
let mut args = arguments.to_args();
if let Some(output) = output {
Expand All @@ -153,6 +163,12 @@ impl ToArgs for RunArgs {
if *save_profiler_trace_data {
args.push("--save-profiler-trace-data".to_string());
}
if *save_program_output {
args.push("--save-program-output".to_string());
}
if *save_print_output {
args.push("--save-print-output".to_string());
}
args
}
}
Expand Down
16 changes: 16 additions & 0 deletions utils/scarb-extensions-cli/src/execute/unchecked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ pub struct RunArgs {
/// Whether to save cairo-profiler trace data.
#[arg(long, default_value_t = false)]
pub save_profiler_trace_data: bool,

/// Whether to save program output to a file.
#[arg(long, default_value_t = false)]
pub save_program_output: bool,

/// Whether to save stdout output to a file.
#[arg(long, default_value_t = false)]
pub save_print_output: bool,
}

impl ToArgs for RunArgs {
Expand All @@ -135,6 +143,8 @@ impl ToArgs for RunArgs {
print_program_output,
print_resource_usage,
save_profiler_trace_data,
save_program_output,
save_print_output,
} = self;
let mut args = arguments.to_args();
if let Some(output) = output {
Expand All @@ -152,6 +162,12 @@ impl ToArgs for RunArgs {
if *save_profiler_trace_data {
args.push("--save-profiler-trace-data".to_string());
}
if *save_program_output {
args.push("--save-program-output".to_string());
}
if *save_print_output {
args.push("--save-print-output".to_string());
}
args
}
}
Expand Down