diff --git a/e2e/tests-dfx/deploy.bash b/e2e/tests-dfx/deploy.bash index 0e2b3a4173..0605dca9e6 100644 --- a/e2e/tests-dfx/deploy.bash +++ b/e2e/tests-dfx/deploy.bash @@ -66,14 +66,14 @@ teardown() { ( cd src assert_command dfx deploy - assert_match "Installing code for" + assert_match "Installed code for" ) assert_command dfx canister call hello_backend greet '("Banzai")' assert_eq '("Hello, Banzai!")' assert_command dfx deploy - assert_not_match "Installing code for" + assert_not_match "Installed code for" assert_match "is already installed" } diff --git a/e2e/tests-dfx/install.bash b/e2e/tests-dfx/install.bash index b08950c35c..4450c4827a 100644 --- a/e2e/tests-dfx/install.bash +++ b/e2e/tests-dfx/install.bash @@ -42,7 +42,7 @@ teardown() { assert_command dfx canister install --all - assert_match "Installing code for canister e2e_project_backend" + assert_match "Installed code for canister e2e_project_backend" } @test "install succeeds with network name" { @@ -52,7 +52,7 @@ teardown() { assert_command dfx canister install --all --network local - assert_match "Installing code for canister e2e_project_backend" + assert_match "Installed code for canister e2e_project_backend" } @test "install fails with network name that is not in dfx.json" { diff --git a/e2e/tests-dfx/mode_reinstall.bash b/e2e/tests-dfx/mode_reinstall.bash index 6af316840a..65949b93d5 100644 --- a/e2e/tests-dfx/mode_reinstall.bash +++ b/e2e/tests-dfx/mode_reinstall.bash @@ -41,7 +41,7 @@ teardown() { assert_command dfx canister install --mode=reinstall hello_backend assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_match "Reinstalling code for canister hello_backend" + assert_match "Reinstalled code for canister hello_backend" ) } @@ -54,7 +54,7 @@ teardown() { assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_not_match "Installing code for canister" + assert_not_match "Installed code for canister" assert_contains "Refusing to install canister without approval" assert_contains "User declined consent" ) @@ -77,7 +77,7 @@ teardown() { assert_command dfx deploy --mode=reinstall hello_backend assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_match "Reinstalling code for canister hello_backend" + assert_match "Reinstalled code for canister hello_backend" ) } @@ -90,7 +90,7 @@ teardown() { assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_not_match "Installing code for canister" + assert_not_match "Installed code for canister" assert_contains "Refusing to install canister without approval" assert_contains "User declined consent" ) @@ -123,7 +123,7 @@ teardown() { assert_match "You are about to reinstall the hello_frontend canister." assert_not_match "You are about to reinstall the hello_backend canister." assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_match "Reinstalling code for canister hello_frontend," + assert_match "Reinstalled code for canister hello_frontend," ) # the hello_backend canister should not have been upgraded (which would reset the non-stable var) @@ -141,24 +141,24 @@ teardown() { assert_command dfx deploy --mode=reinstall hello_backend assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_match "Reinstalling code for canister hello_backend" + assert_match "Reinstalled code for canister hello_backend" ) echo y | ( assert_command dfx deploy --mode=reinstall hello_backend assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_match "Reinstalling code for canister hello_backend" + assert_match "Reinstalled code for canister hello_backend" ) echo YES | ( assert_command dfx deploy --mode=reinstall hello_backend assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_match "Reinstalling code for canister hello_backend" + assert_match "Reinstalled code for canister hello_backend" ) echo YeS | ( assert_command dfx deploy --mode=reinstall hello_backend assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER" - assert_match "Reinstalling code for canister hello_backend" + assert_match "Reinstalled code for canister hello_backend" ) } diff --git a/e2e/tests-replica/deploy.bash b/e2e/tests-replica/deploy.bash index 3416318bde..5ad367176c 100644 --- a/e2e/tests-replica/deploy.bash +++ b/e2e/tests-replica/deploy.bash @@ -60,7 +60,7 @@ teardown() { assert_command dfx canister create --all assert_command dfx deploy - assert_match 'Installing code for canister' + assert_match 'Installed code for canister' } @test "dfx deploy supports arguments" { @@ -85,13 +85,13 @@ teardown() { # Therefore, there is no "attempting (install|upgrade)" message. assert_command dfx deploy hello_backend - assert_match 'Installing code for canister' + assert_match 'Installed code for canister' assert_command dfx canister call hello_backend greet '("First")' assert_eq '("Hello, First!")' assert_command dfx deploy hello_backend --upgrade-unchanged - assert_match 'Upgrading code for canister' + assert_match 'Upgraded code for canister' assert_command dfx canister call hello_backend greet '("Second")' assert_eq '("Hello, Second!")' diff --git a/src/dfx-core/src/canister/mod.rs b/src/dfx-core/src/canister/mod.rs index e57d7c16a0..4244666fd8 100644 --- a/src/dfx-core/src/canister/mod.rs +++ b/src/dfx-core/src/canister/mod.rs @@ -1,6 +1,8 @@ use crate::{ - cli::ask_for_consent, - error::canister::{CanisterBuilderError, CanisterInstallError}, + error::{ + canister::{CanisterBuilderError, CanisterInstallError}, + cli::UserConsent, + }, identity::CallSender, }; use candid::Principal; @@ -28,7 +30,7 @@ pub async fn build_wallet_canister( .map_err(CanisterBuilderError::WalletCanisterCaller) } -pub fn install_mode_to_prompt(mode: &InstallMode) -> &'static str { +pub fn install_mode_to_present_tense(mode: &InstallMode) -> &'static str { match mode { InstallMode::Install => "Installing", InstallMode::Reinstall => "Reinstalling", @@ -36,6 +38,14 @@ pub fn install_mode_to_prompt(mode: &InstallMode) -> &'static str { } } +pub fn install_mode_to_past_tense(mode: &InstallMode) -> &'static str { + match mode { + InstallMode::Install => "Installed", + InstallMode::Reinstall => "Reinstalled", + InstallMode::Upgrade { .. } => "Upgraded", + } +} + pub async fn install_canister_wasm( agent: &Agent, canister_id: Principal, @@ -44,10 +54,10 @@ pub async fn install_canister_wasm( mode: InstallMode, call_sender: &CallSender, wasm_module: Vec, - skip_consent: bool, + ask_for_consent: impl FnOnce(&str) -> Result<(), UserConsent>, ) -> Result<(), CanisterInstallError> { let mgr = ManagementCanister::create(agent); - if !skip_consent && mode == InstallMode::Reinstall { + if mode == InstallMode::Reinstall { let msg = if let Some(name) = canister_name { format!("You are about to reinstall the {name} canister") } else { diff --git a/src/dfx-core/src/cli/mod.rs b/src/dfx-core/src/cli/mod.rs deleted file mode 100644 index 764db7358b..0000000000 --- a/src/dfx-core/src/cli/mod.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::error::cli::UserConsent; -use std::io::stdin; - -pub fn ask_for_consent(message: &str) -> Result<(), UserConsent> { - eprintln!("WARNING!"); - eprintln!("{}", message); - eprintln!("Do you want to proceed? yes/No"); - let mut input_string = String::new(); - stdin() - .read_line(&mut input_string) - .map_err(UserConsent::ReadError)?; - let input_string = input_string.trim_end().to_lowercase(); - if input_string != "yes" && input_string != "y" { - return Err(UserConsent::Declined); - } - Ok(()) -} diff --git a/src/dfx-core/src/lib.rs b/src/dfx-core/src/lib.rs index 8fa53d6f81..05261e6f36 100644 --- a/src/dfx-core/src/lib.rs +++ b/src/dfx-core/src/lib.rs @@ -1,5 +1,4 @@ pub mod canister; -pub mod cli; pub mod config; pub mod error; pub mod extension; diff --git a/src/dfx/src/actors/pocketic.rs b/src/dfx/src/actors/pocketic.rs index 10cd0c48c1..68be82d27c 100644 --- a/src/dfx/src/actors/pocketic.rs +++ b/src/dfx/src/actors/pocketic.rs @@ -159,7 +159,7 @@ impl Actor for PocketIc { } fn stopping(&mut self, _ctx: &mut Self::Context) -> Running { - warn!(self.logger, "Stopping PocketIC..."); + debug!(self.logger, "Stopping PocketIC..."); if let Some(sender) = self.stop_sender.take() { let _ = sender.send(()); } @@ -168,7 +168,7 @@ impl Actor for PocketIc { let _ = join.join(); } - warn!(self.logger, "Stopped."); + debug!(self.logger, "Stopped."); Running::Stop } } diff --git a/src/dfx/src/commands/canister/delete.rs b/src/dfx/src/commands/canister/delete.rs index 6869a3d9ad..b50804dee9 100644 --- a/src/dfx/src/commands/canister/delete.rs +++ b/src/dfx/src/commands/canister/delete.rs @@ -8,13 +8,12 @@ use crate::lib::operations::canister::{ use crate::lib::operations::cycles_ledger::wallet_deposit_to_cycles_ledger; use crate::lib::root_key::fetch_root_key_if_needed; use crate::util::assets::wallet_wasm; -use crate::util::blob_from_arguments; use crate::util::clap::parsers::{cycle_amount_parser, icrc_subaccount_parser}; +use crate::util::{ask_for_consent, blob_from_arguments}; use anyhow::{bail, Context}; use candid::Principal; use clap::Parser; use dfx_core::canister::build_wallet_canister; -use dfx_core::cli::ask_for_consent; use dfx_core::identity::wallet::wallet_canister_id; use dfx_core::identity::CallSender; use fn_error_context::context; @@ -177,9 +176,10 @@ async fn delete_canister( // Determine how many cycles we can withdraw. let status = canister::get_canister_status(env, canister_id, call_sender).await?; if status.status != CanisterStatus::Stopped && !skip_confirmation { - ask_for_consent(&format!( - "Canister {canister} has not been stopped. Delete anyway?" - ))?; + ask_for_consent( + env, + &format!("Canister {canister} has not been stopped. Delete anyway?"), + )?; } let agent = env.get_agent(); let mgr = ManagementCanister::create(agent); diff --git a/src/dfx/src/commands/canister/install.rs b/src/dfx/src/commands/canister/install.rs index bb8e1de50b..7e9f253c49 100644 --- a/src/dfx/src/commands/canister/install.rs +++ b/src/dfx/src/commands/canister/install.rs @@ -4,10 +4,12 @@ use crate::lib::environment::Environment; use crate::lib::error::DfxResult; use crate::lib::operations::canister::install_canister::install_canister; use crate::lib::root_key::fetch_root_key_if_needed; -use crate::util::blob_from_arguments; use crate::util::clap::argument_from_cli::ArgumentFromCliLongOpt; use crate::util::clap::install_mode::{InstallModeHint, InstallModeOpt}; -use dfx_core::canister::{install_canister_wasm, install_mode_to_prompt}; +use crate::util::{ask_for_consent, blob_from_arguments}; +use dfx_core::canister::{ + install_canister_wasm, install_mode_to_past_tense, install_mode_to_present_tense, +}; use dfx_core::identity::CallSender; use crate::lib::operations::canister::skip_remote_canister; @@ -103,12 +105,15 @@ pub async fn exec( )?; let wasm_module = dfx_core::fs::read(wasm_path)?; let mode = mode_hint.to_install_mode_with_wasm_path()?; - info!( - env.get_logger(), - "{} code for canister {}", - install_mode_to_prompt(&mode), - canister_id, + let spinner = env.new_spinner( + format!( + "{} code for canister {}", + install_mode_to_present_tense(&mode), + canister_id, + ) + .into(), ); + install_canister_wasm( env.get_agent(), canister_id, @@ -117,9 +122,22 @@ pub async fn exec( mode, call_sender, wasm_module, - opts.yes, + |message| { + if opts.yes { + Ok(()) + } else { + ask_for_consent(env, message) + } + }, ) .await?; + spinner.finish_and_clear(); + info!( + env.get_logger(), + "{} code for canister {}", + install_mode_to_past_tense(&mode), + canister_id + ); Ok(()) } else { bail!("When installing a canister by its ID, you must specify `--wasm` option.") diff --git a/src/dfx/src/commands/canister/update_settings.rs b/src/dfx/src/commands/canister/update_settings.rs index d55c6fd6a1..90ecf45884 100644 --- a/src/dfx/src/commands/canister/update_settings.rs +++ b/src/dfx/src/commands/canister/update_settings.rs @@ -10,6 +10,7 @@ use crate::lib::operations::canister::{ get_canister_status, skip_remote_canister, update_settings, }; use crate::lib::root_key::fetch_root_key_if_needed; +use crate::util::ask_for_consent; use crate::util::clap::parsers::{ compute_allocation_parser, freezing_threshold_parser, memory_allocation_parser, reserved_cycles_limit_parser, wasm_memory_limit_parser, @@ -19,7 +20,6 @@ use byte_unit::Byte; use candid::Principal as CanisterId; use candid::Principal; use clap::{ArgAction, Parser}; -use dfx_core::cli::ask_for_consent; use dfx_core::error::identity::InstantiateIdentityFromNameError::GetIdentityPrincipalFailed; use dfx_core::identity::CallSender; use fn_error_context::context; @@ -146,7 +146,7 @@ pub async fn exec( fetch_root_key_if_needed(env).await?; if !opts.yes && user_is_removing_themselves_as_controller(env, call_sender, &opts)? { - ask_for_consent("You are trying to remove yourself as a controller of this canister. This may leave this canister un-upgradeable.")? + ask_for_consent(env, "You are trying to remove yourself as a controller of this canister. This may leave this canister un-upgradeable.")? } let controllers: Option>> = opts.set_controller.as_ref().map(|controllers| { diff --git a/src/dfx/src/commands/deploy.rs b/src/dfx/src/commands/deploy.rs index 78751a62de..ffc0ef32c9 100644 --- a/src/dfx/src/commands/deploy.rs +++ b/src/dfx/src/commands/deploy.rs @@ -237,7 +237,7 @@ fn display_urls(env: &dyn Environment) -> DfxResult { info!(log, "URLs:"); let green = Style::new().green(); if !frontend_urls.is_empty() { - info!(log, " Frontend canister via browser"); + info!(log, " Frontend canister via browser:"); for (name, (url1, url2)) in frontend_urls { if let Some(url2) = url2 { info!(log, " {}:", name); diff --git a/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs b/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs index 29946a48e7..9d342f7553 100644 --- a/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs +++ b/src/dfx/src/commands/wallet/redeem_faucet_coupon.rs @@ -4,11 +4,10 @@ use crate::lib::environment::Environment; use crate::lib::error::DfxResult; use crate::lib::identity::wallet::set_wallet_id; use crate::lib::root_key::fetch_root_key_if_needed; -use crate::util::{format_as_trillions, pretty_thousand_separators}; +use crate::util::{ask_for_consent, format_as_trillions, pretty_thousand_separators}; use anyhow::{anyhow, bail, Context}; use candid::{encode_args, Decode, Principal}; use clap::Parser; -use dfx_core::cli::ask_for_consent; use slog::{info, warn}; pub const DEFAULT_FAUCET_PRINCIPAL: Principal = @@ -74,7 +73,7 @@ pub async fn exec(env: &dyn Environment, opts: RedeemFaucetCouponOpts) -> DfxRes // identity has no wallet yet - faucet will provide one _ => { if !opts.yes { - ask_for_consent("`dfx cycles` is now recommended instead of `dfx wallet`. Are you sure you want to create a new cycles wallet anyway?")?; + ask_for_consent(env, "`dfx cycles` is now recommended instead of `dfx wallet`. Are you sure you want to create a new cycles wallet anyway?")?; } let identity = env .get_selected_identity() diff --git a/src/dfx/src/lib/builders/assets.rs b/src/dfx/src/lib/builders/assets.rs index 6540ea7a19..03225bfa9c 100644 --- a/src/dfx/src/lib/builders/assets.rs +++ b/src/dfx/src/lib/builders/assets.rs @@ -14,9 +14,10 @@ use candid::Principal as CanisterId; use console::style; use dfx_core::config::model::network_descriptor::NetworkDescriptor; use fn_error_context::context; -use slog::{o, Logger}; +use slog::{debug, info, o, Logger}; use std::fs; use std::path::Path; +use std::process::{Command, Stdio}; /// Set of extras that can be specified in the dfx.json. struct AssetsBuilderExtra { @@ -106,7 +107,7 @@ impl CanisterBuilder for AssetsBuilder { fn postbuild( &self, - _: &dyn Environment, + env: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, config: &BuildConfig, @@ -126,6 +127,7 @@ impl CanisterBuilder for AssetsBuilder { )?; build_frontend( + env, pool.get_logger(), info.get_workspace_root(), &config.network_name, @@ -179,6 +181,7 @@ fn unpack_did(generate_output_dir: &Path) -> DfxResult<()> { #[context("Failed to build frontend for network '{}'.", network_name)] fn build_frontend( + env: &dyn Environment, logger: &slog::Logger, project_root: &Path, network_name: &str, @@ -192,20 +195,20 @@ fn build_frontend( if custom_build_frontend { for command in build { - slog::info!( + info!( logger, r#"{} '{}'"#, style("Executing").green().bold(), command ); - super::run_command(command, &vars, project_root) - .with_context(|| format!("Failed to run {}.", command))?; + super::run_command(env, command, &vars, project_root) + .with_context(|| format!("Failed to run {command}.",))?; } } else if build_frontend { // Frontend build. - slog::info!(logger, "Building frontend..."); - let mut cmd = std::process::Command::new(program::NPM); + let spinner = env.new_spinner("Building frontend...".into()); + let mut cmd = Command::new(program::NPM); // Provide DFX_NETWORK at build time cmd.env("DFX_NETWORK", network_name); @@ -225,27 +228,28 @@ fn build_frontend( } cmd.current_dir(project_root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()); - slog::debug!(logger, "Running {:?}...", cmd); + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + debug!(logger, "Running {cmd:?}..."); let output = cmd .output() - .with_context(|| format!("Error executing {:#?}", cmd))?; + .with_context(|| format!("Error executing {cmd:#?}"))?; if !output.status.success() { return Err(DfxError::new(BuildError::CommandError( - format!("{:?}", cmd), + format!("{cmd:?}",), output.status, String::from_utf8_lossy(&output.stdout).to_string(), String::from_utf8_lossy(&output.stderr).to_string(), ))); } else if !output.stderr.is_empty() { // Cannot use eprintln, because it would interfere with the progress bar. - slog::debug!( + debug!( logger, "Frontend build succeed:\n{}", String::from_utf8_lossy(&output.stderr) ); + spinner.finish_and_clear(); } } Ok(()) diff --git a/src/dfx/src/lib/builders/custom.rs b/src/dfx/src/lib/builders/custom.rs index 31d9e54b62..ab8d29fe0d 100644 --- a/src/dfx/src/lib/builders/custom.rs +++ b/src/dfx/src/lib/builders/custom.rs @@ -94,7 +94,7 @@ impl CanisterBuilder for CustomBuilder { #[context("Failed to build custom canister {}.", info.get_name())] fn build( &self, - _: &dyn Environment, + env: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, config: &BuildConfig, @@ -123,7 +123,7 @@ impl CanisterBuilder for CustomBuilder { command ); - super::run_command(&command, &vars, info.get_workspace_root()) + super::run_command(env, &command, &vars, info.get_workspace_root()) .with_context(|| format!("Failed to run {}.", command))?; } diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index 8a77ac6ea2..23e5bde2e8 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -4,6 +4,7 @@ use crate::lib::environment::Environment; use crate::lib::error::{BuildError, DfxError, DfxResult}; use crate::lib::models::canister::CanisterPool; use crate::util::command::direct_or_shell_command; +use crate::util::with_suspend_all_spinners; use anyhow::{bail, Context}; use candid::Principal as CanisterId; use candid_parser::utils::CandidSource; @@ -331,6 +332,7 @@ fn ensure_trailing_newline(s: String) -> String { /// Execute a command and return its output bytes. /// If the catch_output is false, the return bytes will always be empty. pub fn execute_command( + env: &dyn Environment, command: &str, vars: &[Env<'_>], cwd: &Path, @@ -350,9 +352,12 @@ pub fn execute_command( for (key, value) in vars { cmd.env(key.as_ref(), value); } - let output = cmd - .output() - .with_context(|| format!("Error executing custom build step {cmd:#?}"))?; + let output = if catch_output { + cmd.output() + } else { + with_suspend_all_spinners(env, || cmd.output()) + } + .with_context(|| format!("Error executing custom build step {cmd:#?}"))?; if output.status.success() { Ok(output.stdout) } else { @@ -362,13 +367,23 @@ pub fn execute_command( } } -pub fn run_command(command: &str, vars: &[Env<'_>], cwd: &Path) -> DfxResult<()> { - execute_command(command, vars, cwd, false)?; +pub fn run_command( + env: &dyn Environment, + command: &str, + vars: &[Env<'_>], + cwd: &Path, +) -> DfxResult<()> { + execute_command(env, command, vars, cwd, false)?; Ok(()) } -pub fn command_output(command: &str, vars: &[Env<'_>], cwd: &Path) -> DfxResult> { - execute_command(command, vars, cwd, true) +pub fn command_output( + env: &dyn Environment, + command: &str, + vars: &[Env<'_>], + cwd: &Path, +) -> DfxResult> { + execute_command(env, command, vars, cwd, true) } type Env<'a> = (Cow<'static, str>, Cow<'a, OsStr>); diff --git a/src/dfx/src/lib/builders/rust.rs b/src/dfx/src/lib/builders/rust.rs index 8f539b0b5a..32c279444e 100644 --- a/src/dfx/src/lib/builders/rust.rs +++ b/src/dfx/src/lib/builders/rust.rs @@ -6,6 +6,7 @@ use crate::lib::canister_info::CanisterInfo; use crate::lib::environment::Environment; use crate::lib::error::DfxResult; use crate::lib::models::canister::CanisterPool; +use crate::util::with_suspend_all_spinners; use anyhow::{anyhow, bail, Context}; use candid::Principal as CanisterId; use fn_error_context::context; @@ -94,7 +95,10 @@ impl CanisterBuilder for RustBuilder { "Executing: cargo build --target wasm32-unknown-unknown --release -p {} --locked", package ); - let output = cargo.output().context("Failed to run 'cargo build'. You might need to run `cargo update` (or a similar command like `cargo vendor`) if you have updated `Cargo.toml`, because `dfx build` uses the --locked flag with Cargo.")?; + + let output = with_suspend_all_spinners(env, || { + cargo.output().context("Failed to run 'cargo build'. You might need to run `cargo update` (or a similar command like `cargo vendor`) if you have updated `Cargo.toml`, because `dfx build` uses the --locked flag with Cargo.") + })?; if !output.status.success() { bail!("Failed to compile the rust package: {}", package); diff --git a/src/dfx/src/lib/canister_logs/log_visibility.rs b/src/dfx/src/lib/canister_logs/log_visibility.rs index 9d3fbf622b..3327547731 100644 --- a/src/dfx/src/lib/canister_logs/log_visibility.rs +++ b/src/dfx/src/lib/canister_logs/log_visibility.rs @@ -1,10 +1,10 @@ use crate::lib::environment::Environment; use crate::lib::error::DfxResult; +use crate::util::ask_for_consent; use crate::util::clap::parsers::{log_visibility_parser, principal_parser}; use anyhow::anyhow; use candid::Principal; use clap::{ArgAction, Args}; -use dfx_core::cli::ask_for_consent; use ic_utils::interfaces::management_canister::{LogVisibility, StatusCallResult}; #[derive(Args, Clone, Debug, Default)] @@ -104,7 +104,7 @@ impl LogVisibilityOpt { if let Some(added) = self.add_log_viewer.as_ref() { if let Some(LogVisibility::Public) = current_visibility { let msg = "Current log is public to everyone. Adding log reviewers will make the log only visible to the reviewers."; - ask_for_consent(msg)?; + ask_for_consent(env, msg)?; } for principal in added { diff --git a/src/dfx/src/lib/environment.rs b/src/dfx/src/lib/environment.rs index 8626e358ed..1efe47ae36 100644 --- a/src/dfx/src/lib/environment.rs +++ b/src/dfx/src/lib/environment.rs @@ -55,6 +55,7 @@ pub trait Environment { fn get_logger(&self) -> &slog::Logger; fn get_verbose_level(&self) -> i64; fn new_spinner(&self, message: Cow<'static, str>) -> ProgressBar; + fn with_suspend_all_spinners(&self, f: Box); // box needed for dyn Environment fn new_progress(&self, message: &str) -> ProgressBar; fn new_identity_manager(&self) -> Result { @@ -255,6 +256,10 @@ impl Environment for EnvironmentImpl { } } + fn with_suspend_all_spinners(&self, f: Box) { + self.spinners.suspend(f); + } + fn new_progress(&self, _message: &str) -> ProgressBar { ProgressBar::discard() } @@ -415,6 +420,10 @@ impl<'a> Environment for AgentEnvironment<'a> { self.backend.new_spinner(message) } + fn with_suspend_all_spinners(&self, f: Box) { + self.backend.with_suspend_all_spinners(f); + } + fn new_progress(&self, message: &str) -> ProgressBar { self.backend.new_progress(message) } @@ -535,6 +544,9 @@ pub mod test_env { fn new_progress(&self, _message: &str) -> ProgressBar { ProgressBar::discard() } + fn with_suspend_all_spinners(&self, f: Box) { + f() + } fn new_spinner(&self, _message: Cow<'static, str>) -> ProgressBar { ProgressBar::discard() } diff --git a/src/dfx/src/lib/metadata/dfx.rs b/src/dfx/src/lib/metadata/dfx.rs index b111e401f9..6c1101fa60 100644 --- a/src/dfx/src/lib/metadata/dfx.rs +++ b/src/dfx/src/lib/metadata/dfx.rs @@ -2,7 +2,7 @@ //! //! The cli tool dfx should consolidate its usage of canister metadata into this single section //! It's originally for pulling dependencies. But open to extend for other usage. -use crate::lib::{builders::command_output, error::DfxResult}; +use crate::lib::{builders::command_output, environment::Environment, error::DfxResult}; use anyhow::{bail, Context}; use dfx_core::config::model::dfinity::{Pullable, TechStack, TechStackCategoryMap}; use schemars::JsonSchema; @@ -39,15 +39,16 @@ impl DfxMetadata { pub fn set_tech_stack( &mut self, + env: &dyn Environment, tech_stack_config: &TechStack, project_root: &Path, ) -> DfxResult<()> { let mut tech_stack = tech_stack_config.clone(); - overwrite_field_from_command("cdk", tech_stack.cdk.as_mut(), project_root)?; - overwrite_field_from_command("language", tech_stack.language.as_mut(), project_root)?; - overwrite_field_from_command("lib", tech_stack.lib.as_mut(), project_root)?; - overwrite_field_from_command("tool", tech_stack.tool.as_mut(), project_root)?; - overwrite_field_from_command("other", tech_stack.other.as_mut(), project_root)?; + overwrite_field_from_command(env, "cdk", tech_stack.cdk.as_mut(), project_root)?; + overwrite_field_from_command(env, "language", tech_stack.language.as_mut(), project_root)?; + overwrite_field_from_command(env, "lib", tech_stack.lib.as_mut(), project_root)?; + overwrite_field_from_command(env, "tool", tech_stack.tool.as_mut(), project_root)?; + overwrite_field_from_command(env, "other", tech_stack.other.as_mut(), project_root)?; self.tech_stack = Some(tech_stack); Ok(()) } @@ -56,6 +57,7 @@ impl DfxMetadata { // If the value of a field starts with "$(", and ends with ")", then it's a command to calculate the value. // The field value will be overwritten by the output of the command. fn overwrite_field_from_command( + env: &dyn Environment, category: &str, category_map: Option<&mut TechStackCategoryMap>, project_root: &Path, @@ -66,7 +68,7 @@ fn overwrite_field_from_command( if value.starts_with("$(") && value.ends_with(')') { let triple = format!("{}->{}->{}", category, name, field); let command = &value[2..value.len() - 1]; - let bytes = command_output(command, &[], project_root) + let bytes = command_output(env, command, &[], project_root) .with_context(|| format!("Failed to run the value_command: {}.", triple))?; let calculated_value = String::from_utf8(bytes).with_context(|| { format!( diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 0084143514..3c9e1c2936 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -8,7 +8,7 @@ use crate::lib::error::{BuildError, DfxError, DfxResult}; use crate::lib::metadata::dfx::DfxMetadata; use crate::lib::metadata::names::{CANDID_ARGS, CANDID_SERVICE, DFX}; use crate::lib::wasm::file::{compress_bytes, read_wasm_module}; -use crate::util::assets; +use crate::util::{assets, with_suspend_all_spinners}; use anyhow::{anyhow, bail, Context}; use candid::Principal as CanisterId; use candid_parser::utils::CandidSource; @@ -129,6 +129,7 @@ impl Canister { #[context("Failed to post-process wasm of canister '{}'.", self.info.get_name())] pub(crate) fn wasm_post_process( &self, + env: &dyn Environment, logger: &Logger, build_output: &BuildOutput, ) -> DfxResult { @@ -187,7 +188,7 @@ impl Canister { if let Some(tech_stack_config) = info.get_tech_stack() { set_dfx_metadata = true; - dfx_metadata.set_tech_stack(tech_stack_config, info.get_workspace_root())?; + dfx_metadata.set_tech_stack(env, tech_stack_config, info.get_workspace_root())?; } else if info.is_rust() { // TODO: remove this when we have rust extension set_dfx_metadata = true; @@ -204,7 +205,7 @@ impl Canister { } }"#; let tech_stack_config: TechStack = serde_json::from_str(s)?; - dfx_metadata.set_tech_stack(&tech_stack_config, info.get_workspace_root())?; + dfx_metadata.set_tech_stack(env, &tech_stack_config, info.get_workspace_root())?; } else if info.is_motoko() { // TODO: remove this when we have motoko extension set_dfx_metadata = true; @@ -214,7 +215,7 @@ impl Canister { } }"#; let tech_stack_config: TechStack = serde_json::from_str(s)?; - dfx_metadata.set_tech_stack(&tech_stack_config, info.get_workspace_root())?; + dfx_metadata.set_tech_stack(env, &tech_stack_config, info.get_workspace_root())?; } if set_dfx_metadata { @@ -639,7 +640,12 @@ impl CanisterPool { } #[context("Failed step_prebuild_all.")] - fn step_prebuild_all(&self, log: &Logger, build_config: &BuildConfig) -> DfxResult<()> { + fn step_prebuild_all( + &self, + env: &dyn Environment, + log: &Logger, + build_config: &BuildConfig, + ) -> DfxResult<()> { // moc expects all .did files of dependencies to be in with name .did. // Because some canisters don't get built these .did files have to be copied over manually. for canister in self.canisters.iter().filter(|c| { @@ -684,7 +690,7 @@ impl CanisterPool { .iter() .any(|can| can.info.should_cargo_audit()) { - self.run_cargo_audit()?; + self.run_cargo_audit(env)?; } else { trace!( self.logger, @@ -722,7 +728,7 @@ impl CanisterPool { ) -> DfxResult<()> { canister.candid_post_process(self.get_logger(), build_config, build_output)?; - canister.wasm_post_process(self.get_logger(), build_output)?; + canister.wasm_post_process(env, self.get_logger(), build_output)?; build_canister_js(&canister.canister_id(), &canister.info)?; @@ -756,7 +762,7 @@ impl CanisterPool { log: &Logger, build_config: &BuildConfig, ) -> DfxResult>> { - self.step_prebuild_all(log, build_config) + self.step_prebuild_all(env, log, build_config) .map_err(|e| DfxError::new(BuildError::PreBuildAllStepFailed(Box::new(e))))?; let canisters_to_build = self.canisters_to_build(build_config); @@ -861,7 +867,7 @@ impl CanisterPool { } /// If `cargo-audit` is installed this runs `cargo audit` and displays any vulnerable dependencies. - fn run_cargo_audit(&self) -> DfxResult { + fn run_cargo_audit(&self, env: &dyn Environment) -> DfxResult { let location = Command::new("cargo") .args(["locate-project", "--message-format=plain", "--workspace"]) .output() @@ -896,12 +902,14 @@ impl CanisterPool { self.logger, "Checking for vulnerabilities in rust canisters." ); - let out = Command::new("cargo") - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .arg("audit") - .output() - .context("Failed to run 'cargo audit'.")?; + let out = with_suspend_all_spinners(env, || { + Command::new("cargo") + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .arg("audit") + .output() + .context("Failed to run 'cargo audit'.") + })?; if out.status.success() { info!(self.logger, "Audit found no vulnerabilities.") } else { diff --git a/src/dfx/src/lib/operations/canister/deploy_canisters.rs b/src/dfx/src/lib/operations/canister/deploy_canisters.rs index 079645361f..27a5a3d7c5 100644 --- a/src/dfx/src/lib/operations/canister/deploy_canisters.rs +++ b/src/dfx/src/lib/operations/canister/deploy_canisters.rs @@ -210,12 +210,13 @@ async fn register_canisters( if canisters_to_create.is_empty() { info!(env.get_logger(), "All canisters have already been created."); } else if env.get_network_descriptor().is_playground() { - info!(env.get_logger(), "Reserving canisters in playground..."); + let spinner = env.new_spinner("Reserving canisters in playground...".into()); for canister_name in &canisters_to_create { reserve_canister_with_playground(env, canister_name).await?; } + spinner.finish_and_clear(); } else { - info!(env.get_logger(), "Creating canisters..."); + let spinner = env.new_spinner("Creating canisters...".into()); for canister_name in &canisters_to_create { let config_interface = config.get_config(); let compute_allocation = config_interface @@ -285,6 +286,7 @@ async fn register_canisters( ) .await?; } + spinner.finish_and_clear(); } Ok(()) } @@ -297,15 +299,17 @@ async fn build_canisters( config: &Config, env_file: Option, ) -> DfxResult { - let log = env.get_logger(); - info!(log, "Building canisters..."); + let spinner = env.new_spinner("Building canisters...".into()); let build_mode_check = false; let canister_pool = CanisterPool::load(env, build_mode_check, canisters_to_load)?; let build_config = BuildConfig::from_config(config)? .with_canisters_to_build(canisters_to_build.into()) .with_env_file(env_file); - canister_pool.build_or_fail(env, log, &build_config).await?; + canister_pool + .build_or_fail(env, env.get_logger(), &build_config) + .await?; + spinner.finish_and_clear(); Ok(canister_pool) } @@ -325,7 +329,7 @@ async fn install_canisters( no_asset_upgrade: bool, always_assist: bool, ) -> DfxResult { - info!(env.get_logger(), "Installing canisters..."); + let spinner = env.new_spinner("Installing canisters...".into()); let mut canister_id_store = env.get_canister_id_store()?; @@ -352,7 +356,7 @@ async fn install_canisters( ) .await?; } - + spinner.finish_and_clear(); Ok(()) } diff --git a/src/dfx/src/lib/operations/canister/install_canister.rs b/src/dfx/src/lib/operations/canister/install_canister.rs index c75cf8cacc..c8e349cec4 100644 --- a/src/dfx/src/lib/operations/canister/install_canister.rs +++ b/src/dfx/src/lib/operations/canister/install_canister.rs @@ -10,13 +10,18 @@ use crate::lib::operations::canister::motoko_playground::authorize_asset_uploade use crate::lib::state_tree::canister_info::read_state_tree_canister_module_hash; use crate::util::assets::wallet_wasm; use crate::util::clap::install_mode::InstallModeHint; -use crate::util::{blob_from_arguments, get_candid_init_type, read_module_metadata}; +use crate::util::{ + ask_for_consent, blob_from_arguments, get_candid_init_type, read_module_metadata, + with_suspend_all_spinners, +}; use anyhow::{anyhow, bail, Context}; use backoff::backoff::Backoff; use backoff::ExponentialBackoff; use candid::Principal; -use dfx_core::canister::{build_wallet_canister, install_canister_wasm, install_mode_to_prompt}; -use dfx_core::cli::ask_for_consent; +use dfx_core::canister::{ + build_wallet_canister, install_canister_wasm, install_mode_to_past_tense, + install_mode_to_present_tense, +}; use dfx_core::config::model::canister_id_store::CanisterIdStore; use dfx_core::config::model::network_descriptor::NetworkDescriptor; use dfx_core::identity::CallSender; @@ -83,11 +88,13 @@ pub async fn install_canister( installed_module_hash.is_some(), wasm_memory_persistence_embedded, ); - let mode_str = install_mode_to_prompt(&mode); let canister_name = canister_info.get_name(); - info!( - log, - "{mode_str} code for canister {canister_name}, with canister ID {canister_id}", + let spinner = env.new_spinner( + format!( + "{mode_str} code for canister {canister_name}, with canister ID {canister_id}", + mode_str = install_mode_to_present_tense(&mode) + ) + .into(), ); if !skip_consent && matches!(mode, InstallMode::Reinstall | InstallMode::Upgrade { .. }) { let candid = read_module_metadata(agent, canister_id, "candid:service").await; @@ -96,11 +103,11 @@ pub async fn install_canister( Ok(None) => (), Ok(Some(err)) => { let msg = format!("Candid interface compatibility check failed for canister '{}'.\nYou are making a BREAKING change. Other canisters or frontend clients relying on your canister may stop working.\n\n", canister_info.get_name()) + &err; - ask_for_consent(&msg)?; + ask_for_consent(env, &msg)?; } Err(e) => { let msg = format!("An error occurred during Candid interface compatibility check for canister '{}'.\n\n", canister_info.get_name()) + &e.to_string(); - ask_for_consent(&msg)?; + ask_for_consent(env, &msg)?; } } } @@ -112,15 +119,15 @@ pub async fn install_canister( Ok(StableCompatibility::Okay) => (), Ok(StableCompatibility::Warning(details)) => { let msg = format!("Stable interface compatibility check issued a WARNING for canister '{}'.\n\n", canister_info.get_name()) + &details; - ask_for_consent(&msg)?; + ask_for_consent(env, &msg)?; } Ok(StableCompatibility::Error(details)) => { let msg = format!("Stable interface compatibility check issued an ERROR for canister '{}'.\nUpgrade will either FAIL or LOSE some stable variable data.\n\n", canister_info.get_name()) + &details; - ask_for_consent(&msg)?; + ask_for_consent(env, &msg)?; } Err(e) => { let msg = format!("An error occurred during stable interface compatibility check for canister '{}'.\n\n", canister_info.get_name()) + &e.to_string(); - ask_for_consent(&msg)?; + ask_for_consent(env, &msg)?; } } } @@ -143,7 +150,8 @@ pub async fn install_canister( && matches!(&installed_module_hash, Some(old_hash) if old_hash[..] == new_hash[..]) && !upgrade_unchanged { - println!( + info!( + log, "Module hash {} is already installed.", hex::encode(installed_module_hash.as_ref().unwrap()) ); @@ -215,7 +223,13 @@ The command line value will be used.", mode, call_sender, wasm_module, - skip_consent, + |message| { + if skip_consent { + Ok(()) + } else { + ask_for_consent(env, message) + } + }, ) .await?; } @@ -279,6 +293,12 @@ The command line value will be used.", env_file.or_else(|| config.as_ref()?.get_config().output_env_file.as_deref()), )?; } + spinner.finish_and_clear(); + info!( + log, + "{mode_str} code for canister {canister_name}, with canister ID {canister_id}", + mode_str = install_mode_to_past_tense(&mode) + ); Ok(()) } @@ -458,6 +478,7 @@ fn run_customized_install_tasks( }; for task in tasks { run_customized_install_task( + env, canister, task, is_pre_install, @@ -472,6 +493,7 @@ fn run_customized_install_tasks( #[context("Failed to run {}-install task {}", if is_pre_install { "pre" } else { "post" }, task)] fn run_customized_install_task( + env: &dyn Environment, canister: &CanisterInfo, task: &str, is_pre_install: bool, @@ -500,7 +522,7 @@ fn run_customized_install_task( .stdout(Stdio::inherit()) .stderr(Stdio::inherit()); - let status = command.status()?; + let status = with_suspend_all_spinners(env, || command.status())?; if !status.success() { match status.code() { Some(code) => { diff --git a/src/dfx/src/util/mod.rs b/src/dfx/src/util/mod.rs index c338b72d08..972e3f78f2 100644 --- a/src/dfx/src/util/mod.rs +++ b/src/dfx/src/util/mod.rs @@ -9,6 +9,7 @@ use candid::types::{value::IDLValue, Function, Type, TypeEnv, TypeInner}; use candid::{Decode, Encode, IDLArgs, Principal}; use candid_parser::error::pretty_wrap; use candid_parser::utils::CandidSource; +use dfx_core::error::cli::UserConsent; use dfx_core::fs::create_dir_all; use fn_error_context::context; use idl2json::{idl2json, Idl2JsonOptions}; @@ -230,34 +231,36 @@ pub fn blob_from_arguments( } else if is_terminal { use candid_parser::assist::{input_args, Context}; let mut ctx = Context::new(env.clone()); - if let Some(env) = dfx_env { - let principals = gather_principals_from_env(env); - if !principals.is_empty() { - let mut map = BTreeMap::new(); - map.insert("principal".to_string(), principals); - ctx.set_completion(map); - } - } - if is_init_arg { - eprintln!("This canister requires an initialization argument."); - } else { - eprintln!("This method requires arguments."); - } - let args = input_args(&ctx, &func.args)?; - eprintln!("Sending the following argument:\n{}\n", args); - if is_init_arg { - eprintln!( - "Do you want to initialize the canister with this argument? [y/N]" - ); - } else { - eprintln!("Do you want to send this message? [y/N]"); - } - let mut input = String::new(); - stdin().read_line(&mut input)?; - if !["y", "Y", "yes", "Yes", "YES"].contains(&input.trim()) { - return Err(error_invalid_data!("User cancelled.")); + let dfx_env = dfx_env.expect( + "internal error: requiring interactive args without Environment", + ); + let principals = gather_principals_from_env(dfx_env); + if !principals.is_empty() { + let mut map = BTreeMap::new(); + map.insert("principal".to_string(), principals); + ctx.set_completion(map); } - args.to_bytes_with_types(env, &func.args) + return with_suspend_all_spinners(dfx_env, || { + if is_init_arg { + eprintln!("This canister requires an initialization argument."); + } else { + eprintln!("This method requires arguments."); + } + let args = input_args(&ctx, &func.args)?; + eprintln!("Sending the following argument:\n{}\n", args); + if is_init_arg { + eprintln!("Do you want to initialize the canister with this argument? [y/N]"); + } else { + eprintln!("Do you want to send this message? [y/N]"); + } + let mut input = String::new(); + stdin().read_line(&mut input)?; + if !["y", "Y", "yes", "Yes", "YES"].contains(&input.trim()) { + return Err(error_invalid_data!("User cancelled.")); + } + let bytes = args.to_bytes_with_types(env, &func.args)?; + Ok(bytes) + }) } else { return Err(error_invalid_data!("Expected arguments but found none.")); } @@ -425,6 +428,29 @@ async fn attempt_download(client: &Client, url: &Url) -> DfxResult } } +pub fn ask_for_consent(env: &dyn Environment, message: &str) -> Result<(), UserConsent> { + with_suspend_all_spinners(env, || { + eprintln!("WARNING!"); + eprintln!("{}", message); + eprintln!("Do you want to proceed? yes/No"); + let mut input_string = String::new(); + stdin() + .read_line(&mut input_string) + .map_err(UserConsent::ReadError)?; + let input_string = input_string.trim_end().to_lowercase(); + if input_string != "yes" && input_string != "y" { + return Err(UserConsent::Declined); + } + Ok(()) + }) +} + +pub fn with_suspend_all_spinners(env: &dyn Environment, f: impl FnOnce() -> R) -> R { + let mut r = None; + env.with_suspend_all_spinners(Box::new(|| r = Some(f()))); + r.unwrap() +} + #[cfg(test)] mod tests { use super::{download_file, format_as_trillions, pretty_thousand_separators};