From f79b3fc1ad06645b01ea537b38e134a411a09804 Mon Sep 17 00:00:00 2001 From: Adam Spofford <93943719+adamspofford-dfinity@users.noreply.github.com> Date: Mon, 27 Jan 2025 16:22:44 -0800 Subject: [PATCH] chore: clean up dfx canister create output (#4080) * change canister creation to spinners * keep spinners and logs from interfering with each other * Spinner for wallet creation --- Cargo.lock | 31 ++++-- clippy.toml | 6 +- e2e/tests-dfx/deploy.bash | 4 +- src/dfx-core/src/config/cache.rs | 18 +--- src/dfx/Cargo.toml | 2 +- src/dfx/src/actors/mod.rs | 14 +-- src/dfx/src/commands/build.rs | 6 +- src/dfx/src/commands/cache/install.rs | 4 +- src/dfx/src/commands/extension/install.rs | 4 +- src/dfx/src/commands/generate.rs | 8 +- src/dfx/src/commands/language_service.rs | 9 +- src/dfx/src/commands/new.rs | 6 +- src/dfx/src/commands/quickstart.rs | 33 +++--- src/dfx/src/config/cache.rs | 62 ++++++----- src/dfx/src/lib/builders/assets.rs | 9 +- src/dfx/src/lib/builders/custom.rs | 3 + src/dfx/src/lib/builders/mod.rs | 8 +- src/dfx/src/lib/builders/motoko.rs | 42 +++++--- src/dfx/src/lib/builders/pull.rs | 3 + src/dfx/src/lib/builders/rust.rs | 5 +- src/dfx/src/lib/environment.rs | 100 ++++++++++++++++-- src/dfx/src/lib/identity/wallet.rs | 13 ++- src/dfx/src/lib/logger.rs | 41 ++++++- src/dfx/src/lib/manifest.rs | 17 +-- src/dfx/src/lib/models/canister.rs | 63 ++++++++--- .../operations/canister/create_canister.rs | 3 +- .../operations/canister/deploy_canisters.rs | 2 +- .../operations/canister/install_canister.rs | 2 +- src/dfx/src/lib/package_arguments.rs | 8 +- src/dfx/src/lib/progress_bar.rs | 12 +-- src/dfx/src/main.rs | 10 +- 31 files changed, 365 insertions(+), 183 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6555b0a75d..42a9744dda 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -219,7 +219,7 @@ version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccaf7e9dfbb6ab22c82e473cd1a8a7bd313c19a5b7e40970f3d89ef5a5c9e81e" dependencies = [ - "unicode-width", + "unicode-width 0.1.14", ] [[package]] @@ -987,7 +987,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3538270d33cc669650c4b093848450d380def10c331d38c768e34cac80576e6e" dependencies = [ "termcolor", - "unicode-width", + "unicode-width 0.1.14", ] [[package]] @@ -1060,7 +1060,7 @@ dependencies = [ "encode_unicode", "lazy_static", "libc", - "unicode-width", + "unicode-width 0.1.14", "windows-sys 0.52.0", ] @@ -3633,14 +3633,15 @@ dependencies = [ [[package]] name = "indicatif" -version = "0.16.2" +version = "0.17.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d207dc617c7a380ab07ff572a6e52fa202a2a8f355860ac9c38e23f8196be1b" +checksum = "cbf675b85ed934d3c67b5c5469701eec7db22689d0a2139d856e0925fa28b281" dependencies = [ "console", - "lazy_static", "number_prefix", - "regex", + "portable-atomic", + "unicode-width 0.2.0", + "web-time", ] [[package]] @@ -4772,6 +4773,12 @@ dependencies = [ "universal-hash", ] +[[package]] +name = "portable-atomic" +version = "1.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "280dc24453071f1b63954171985a0b0d30058d287960968b9b2aca264c8d4ee6" + [[package]] name = "powerfmt" version = "0.2.0" @@ -4813,7 +4820,7 @@ checksum = "b55c4d17d994b637e2f4daf6e5dc5d660d209d5642377d675d7a1c3ab69fa579" dependencies = [ "arrayvec 0.5.2", "typed-arena", - "unicode-width", + "unicode-width 0.1.14", ] [[package]] @@ -6622,6 +6629,12 @@ version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" +[[package]] +name = "unicode-width" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fc81956842c57dac11422a97c3b8195a1ff727f06e85c84ed2e8aa277c9a0fd" + [[package]] name = "unicode-xid" version = "0.2.6" @@ -7000,7 +7013,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/clippy.toml b/clippy.toml index e5f7464702..b7646e6522 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,5 @@ -too-many-arguments-threshold = 16 \ No newline at end of file +too-many-arguments-threshold = 16 + +[[disallowed-types]] +path = "indicatif::ProgressBar" +reason = "use env.new_spinner() for log compatibility" diff --git a/e2e/tests-dfx/deploy.bash b/e2e/tests-dfx/deploy.bash index d893de8582..0e2b3a4173 100644 --- a/e2e/tests-dfx/deploy.bash +++ b/e2e/tests-dfx/deploy.bash @@ -182,9 +182,9 @@ teardown() { dfx_start dfx canister create --all --no-wallet assert_command dfx deploy - assert_not_contains "Creating a wallet canister" + assert_not_contains "Created a wallet canister" assert_command dfx identity get-wallet - assert_contains "Creating a wallet canister" + assert_contains "Created a wallet canister" } @test "can deploy gzip wasm" { diff --git a/src/dfx-core/src/config/cache.rs b/src/dfx-core/src/config/cache.rs index 13e39a2b61..6f7fd140b7 100644 --- a/src/dfx-core/src/config/cache.rs +++ b/src/dfx-core/src/config/cache.rs @@ -1,8 +1,8 @@ #[cfg(windows)] use crate::config::directories::project_dirs; use crate::error::cache::{ - DeleteCacheError, EnsureCacheVersionsDirError, GetBinaryCommandPathError, GetCacheRootError, - GetVersionFromCachePathError, IsCacheInstalledError, ListCacheVersionsError, + DeleteCacheError, EnsureCacheVersionsDirError, GetCacheRootError, GetVersionFromCachePathError, + IsCacheInstalledError, ListCacheVersionsError, }; #[cfg(not(windows))] use crate::foundation::get_user_home; @@ -10,20 +10,6 @@ use crate::fs::composite::ensure_dir_exists; use semver::Version; use std::path::{Path, PathBuf}; -pub trait Cache { - fn version_str(&self) -> String; - fn is_installed(&self) -> Result; - fn delete(&self) -> Result<(), DeleteCacheError>; - fn get_binary_command_path( - &self, - binary_name: &str, - ) -> Result; - fn get_binary_command( - &self, - binary_name: &str, - ) -> Result; -} - pub fn get_cache_root() -> Result { let cache_root = std::env::var_os("DFX_CACHE_ROOT"); // dirs-next is not used for *nix to preserve existing paths diff --git a/src/dfx/Cargo.toml b/src/dfx/Cargo.toml index 16ff073f34..d144603fc9 100644 --- a/src/dfx/Cargo.toml +++ b/src/dfx/Cargo.toml @@ -74,7 +74,7 @@ ic-utils.workspace = true ic-wasm = "0.8.0" icrc-ledger-types = "0.1.5" idl2json = "0.10.1" -indicatif = "0.16.0" +indicatif = "0.17.0" itertools.workspace = true json-patch = "1.0.0" keyring.workspace = true diff --git a/src/dfx/src/actors/mod.rs b/src/dfx/src/actors/mod.rs index a4a26c5776..77293a7f42 100644 --- a/src/dfx/src/actors/mod.rs +++ b/src/dfx/src/actors/mod.rs @@ -44,7 +44,9 @@ pub fn start_btc_adapter_actor( shutdown_controller: Addr, btc_adapter_pid_file_path: PathBuf, ) -> DfxResult> { - let btc_adapter_path = env.get_cache().get_binary_command_path("ic-btc-adapter")?; + let btc_adapter_path = env + .get_cache() + .get_binary_command_path(env, "ic-btc-adapter")?; let actor_config = btc_adapter::Config { btc_adapter_path, @@ -69,7 +71,7 @@ pub fn start_canister_http_adapter_actor( ) -> DfxResult> { let adapter_path = env .get_cache() - .get_binary_command_path("ic-https-outcalls-adapter")?; + .get_binary_command_path(env, "ic-https-outcalls-adapter")?; let actor_config = canister_http_adapter::Config { adapter_path, @@ -133,8 +135,8 @@ pub fn start_replica_actor( canister_http_adapter_ready_subscribe: Option>, ) -> DfxResult> { // get binary path - let replica_path = env.get_cache().get_binary_command_path("replica")?; - let ic_starter_path = env.get_cache().get_binary_command_path("ic-starter")?; + let replica_path = env.get_cache().get_binary_command_path(env, "replica")?; + let ic_starter_path = env.get_cache().get_binary_command_path(env, "ic-starter")?; setup_replica_env(local_server_descriptor, &replica_config)?; let replica_pid_path = local_server_descriptor.replica_pid_path(); @@ -169,7 +171,7 @@ pub fn start_pocketic_proxy_actor( pocketic_proxy_pid_path: PathBuf, pocketic_proxy_port_path: PathBuf, ) -> DfxResult> { - let pocketic_proxy_path = env.get_cache().get_binary_command_path("pocket-ic")?; + let pocketic_proxy_path = env.get_cache().get_binary_command_path(env, "pocket-ic")?; let actor_config = pocketic_proxy::Config { logger: Some(env.get_logger().clone()), port_ready_subscribe, @@ -190,7 +192,7 @@ pub fn start_pocketic_actor( shutdown_controller: Addr, pocketic_port_path: PathBuf, ) -> DfxResult> { - let pocketic_path = env.get_cache().get_binary_command_path("pocket-ic")?; + let pocketic_path = env.get_cache().get_binary_command_path(env, "pocket-ic")?; // Touch the port file. This ensures it is empty prior to // handing it over to PocketIC. If we read the file and it has diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index bccdcacba5..449912c9ce 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -1,4 +1,4 @@ -use crate::config::cache::DiskBasedCache; +use crate::config::cache::VersionCache; use crate::lib::agent::create_anonymous_agent_environment; use crate::lib::builders::BuildConfig; use crate::lib::environment::Environment; @@ -44,7 +44,7 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { // Check the cache. This will only install the cache if there isn't one installed // already. - DiskBasedCache::install(&env.get_cache().version_str())?; + VersionCache::install(&env, &env.get_cache().version_str())?; let build_mode_check = opts.check; @@ -90,7 +90,7 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { .with_build_mode_check(build_mode_check) .with_canisters_to_build(canisters_to_build) .with_env_file(env_file); - runtime.block_on(canister_pool.build_or_fail(logger, &build_config))?; + runtime.block_on(canister_pool.build_or_fail(&env, logger, &build_config))?; Ok(()) } diff --git a/src/dfx/src/commands/cache/install.rs b/src/dfx/src/commands/cache/install.rs index a09c8a52f4..ff53ec3c00 100644 --- a/src/dfx/src/commands/cache/install.rs +++ b/src/dfx/src/commands/cache/install.rs @@ -1,4 +1,4 @@ -use crate::config::cache::DiskBasedCache; +use crate::config::cache::VersionCache; use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; use clap::Parser; @@ -9,6 +9,6 @@ use clap::Parser; pub struct CacheInstall {} pub fn exec(env: &dyn Environment, _opts: CacheInstall) -> DfxResult { - DiskBasedCache::force_install(&env.get_cache().version_str()).map_err(DfxError::from)?; + VersionCache::force_install(env, &env.get_cache().version_str()).map_err(DfxError::from)?; Ok(()) } diff --git a/src/dfx/src/commands/extension/install.rs b/src/dfx/src/commands/extension/install.rs index 2e01006a5c..548430cdce 100644 --- a/src/dfx/src/commands/extension/install.rs +++ b/src/dfx/src/commands/extension/install.rs @@ -1,5 +1,5 @@ use crate::commands::DfxCommand; -use crate::config::cache::DiskBasedCache; +use crate::config::cache::VersionCache; use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; use anyhow::bail; @@ -30,7 +30,7 @@ pub struct InstallOpts { pub fn exec(env: &dyn Environment, opts: InstallOpts) -> DfxResult<()> { // creating an `extensions` directory in an otherwise empty cache directory would // cause the cache to be considered "installed" and later commands would fail - DiskBasedCache::install(&env.get_cache().version_str())?; + VersionCache::install(env, &env.get_cache().version_str())?; let spinner = env.new_spinner(format!("Installing extension: {}", opts.name).into()); let mgr = env.get_extension_manager(); let effective_extension_name = opts.install_as.clone().unwrap_or_else(|| opts.name.clone()); diff --git a/src/dfx/src/commands/generate.rs b/src/dfx/src/commands/generate.rs index 2844db9443..bc5debb914 100644 --- a/src/dfx/src/commands/generate.rs +++ b/src/dfx/src/commands/generate.rs @@ -1,4 +1,4 @@ -use crate::config::cache::DiskBasedCache; +use crate::config::cache::VersionCache; use crate::lib::agent::create_anonymous_agent_environment; use crate::lib::builders::BuildConfig; use crate::lib::environment::Environment; @@ -28,7 +28,7 @@ pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult { // Check the cache. This will only install the cache if there isn't one installed // already. - DiskBasedCache::install(&env.get_cache().version_str())?; + VersionCache::install(&env, &env.get_cache().version_str())?; // Option can be None which means generate types for all canisters let canisters_to_load = config @@ -73,11 +73,11 @@ pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult { let canister_pool_build = CanisterPool::load(&env, true, &build_dependees)?; slog::info!(log, "Building canisters before generate for Motoko"); let runtime = Runtime::new().expect("Unable to create a runtime"); - runtime.block_on(canister_pool_build.build_or_fail(log, &build_config))?; + runtime.block_on(canister_pool_build.build_or_fail(&env, log, &build_config))?; } for canister in canister_pool_load.canisters_to_build(&generate_config) { - canister.generate(log, &canister_pool_load, &generate_config)?; + canister.generate(&env, log, &canister_pool_load, &generate_config)?; } Ok(()) diff --git a/src/dfx/src/commands/language_service.rs b/src/dfx/src/commands/language_service.rs index f2a92f6335..69d0794b85 100644 --- a/src/dfx/src/commands/language_service.rs +++ b/src/dfx/src/commands/language_service.rs @@ -47,11 +47,8 @@ pub fn exec(env: &dyn Environment, opts: LanguageServiceOpts) -> DfxResult { .get_build() .get_packtool(); - let mut package_arguments = package_arguments::load( - env.get_cache().as_ref(), - packtool, - config.get_project_root(), - )?; + let mut package_arguments = + package_arguments::load(env, &env.get_cache(), packtool, config.get_project_root())?; // Include actor alias flags let canister_names = config @@ -143,7 +140,7 @@ fn run_ide( ) -> DfxResult { let output = env .get_cache() - .get_binary_command("mo-ide")? + .get_binary_command(env, "mo-ide")? .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) // Point at the right canister diff --git a/src/dfx/src/commands/new.rs b/src/dfx/src/commands/new.rs index 89f268c6b2..27362a46c5 100644 --- a/src/dfx/src/commands/new.rs +++ b/src/dfx/src/commands/new.rs @@ -1,4 +1,4 @@ -use crate::config::cache::DiskBasedCache; +use crate::config::cache::VersionCache; use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; use crate::lib::info::replica_rev; @@ -447,13 +447,13 @@ pub fn exec(env: &dyn Environment, mut opts: NewOpts) -> DfxResult { // It is fine for the following command to timeout or fail. We // drop the error. - let latest_version = get_latest_version(RELEASE_ROOT, Some(CHECK_VERSION_TIMEOUT)).ok(); + let latest_version = get_latest_version(env, RELEASE_ROOT, Some(CHECK_VERSION_TIMEOUT)).ok(); if is_upgrade_necessary(latest_version.as_ref(), current_version) { warn_upgrade(log, latest_version.as_ref(), current_version); } - DiskBasedCache::install(&env.get_cache().version_str())?; + VersionCache::install(env, &env.get_cache().version_str())?; if dry_run { warn!( diff --git a/src/dfx/src/commands/quickstart.rs b/src/dfx/src/commands/quickstart.rs index fd80a7e6b5..6933bedfda 100644 --- a/src/dfx/src/commands/quickstart.rs +++ b/src/dfx/src/commands/quickstart.rs @@ -30,7 +30,6 @@ use ic_agent::Agent; use ic_utils::interfaces::{ management_canister::builders::InstallMode, ManagementCanister, WalletCanister, }; -use indicatif::ProgressBar; use num_traits::Inv; use rust_decimal::Decimal; use slog::Logger; @@ -153,22 +152,21 @@ async fn step_deploy_wallet( eprintln!("Run this command again at any time to continue from here."); return Ok(()); } - let wallet = step_interact_ledger(agent, env.get_logger(), ident_principal, rounded).await?; + let wallet = + step_interact_ledger(env, agent, env.get_logger(), ident_principal, rounded).await?; step_finish_wallet(env, agent, wallet, ident).await?; Ok(()) } async fn step_interact_ledger( + env: &dyn Environment, agent: &Agent, logger: &Logger, ident_principal: Principal, to_spend: Decimal, ) -> DfxResult { - let send_spinner = ProgressBar::new_spinner(); - send_spinner.set_message(format!( - "Sending {to_spend:.8} ICP to the cycles minting canister..." - )); - send_spinner.enable_steady_tick(100); + let send_spinner = env + .new_spinner(format!("Sending {to_spend:.8} ICP to the cycles minting canister...").into()); let icpts = ICPTs::from_decimal(to_spend)?; let height = transfer_cmc( agent, @@ -182,12 +180,9 @@ async fn step_interact_ledger( ) .await .context("Failed to transfer to the cycles minting canister")?; - send_spinner.finish_with_message(format!( - "Sent {icpts} to the cycles minting canister at height {height}" - )); - let notify_spinner = ProgressBar::new_spinner(); - notify_spinner.set_message("Notifying the cycles minting canister..."); - notify_spinner.enable_steady_tick(100); + send_spinner.finish_and_clear(); + eprintln!("Sent {icpts} to the cycles minting canister at height {height}"); + let notify_spinner = env.new_spinner("Notifying the cycles minting canister...".into()); let res = notify_create( agent, ident_principal, @@ -211,9 +206,8 @@ async fn step_interact_ledger( Err(err) => Err(err), }.with_context(|| format!("Failed to notify the CMC of the transfer. Write down that height ({height}), and once the error is fixed, use `dfx ledger notify create-canister`."))?; - notify_spinner.finish_with_message(format!( - "Created wallet canister with principal ID {wallet}" - )); + notify_spinner.finish_and_clear(); + eprintln!("Created wallet canister with principal ID {wallet}"); Ok(wallet) } @@ -223,15 +217,14 @@ async fn step_finish_wallet( wallet: Principal, ident: &str, ) -> DfxResult { - let install_spinner = ProgressBar::new_spinner(); - install_spinner.set_message("Installing the wallet code to the canister..."); - install_spinner.enable_steady_tick(100); + let install_spinner = env.new_spinner("Installing the wallet code to the canister...".into()); install_wallet(env, agent, wallet, InstallMode::Install) .await .context("Failed to install the wallet code to the canister")?; set_wallet_id(env.get_network_descriptor(), ident, wallet) .context("Failed to record the wallet's principal as your associated wallet")?; - install_spinner.finish_with_message("Installed the wallet code to the canister"); + install_spinner.finish_and_clear(); + eprintln!("Installed the wallet code to the canister",); eprintln!("Success! Run this command again at any time to print all this information again."); Ok(()) } diff --git a/src/dfx/src/config/cache.rs b/src/dfx/src/config/cache.rs index d128ffef8c..e54c4a3962 100644 --- a/src/dfx/src/config/cache.rs +++ b/src/dfx/src/config/cache.rs @@ -1,17 +1,19 @@ use crate::config::dfx_version; +use crate::lib::environment::Environment; +use crate::lib::progress_bar::ProgressBar; use crate::util; use dfx_core; use dfx_core::config::cache::{ binary_command_from_version, delete_version, get_bin_cache, get_binary_path_from_version, - is_version_installed, Cache, + is_version_installed, }; use dfx_core::error::cache::{ DeleteCacheError, GetBinaryCommandPathError, InstallCacheError, IsCacheInstalledError, }; -use indicatif::{ProgressBar, ProgressDrawTarget}; use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use semver::Version; +use slog::info; use std::io::{stderr, IsTerminal}; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; @@ -23,58 +25,63 @@ const EXEC_READ_USER_ONLY_PERMISSION: u32 = 0o500; #[cfg(unix)] const READ_USER_ONLY_PERMISSION: u32 = 0o400; -pub struct DiskBasedCache { +#[derive(Debug, Clone)] +pub struct VersionCache { version: Version, } -impl DiskBasedCache { - pub fn with_version(version: &Version) -> DiskBasedCache { - DiskBasedCache { +impl VersionCache { + pub fn with_version(version: &Version) -> VersionCache { + VersionCache { version: version.clone(), } } - pub fn install(version: &str) -> Result<(), InstallCacheError> { - install_version(version, false).map(|_| {}) + pub fn install(env: &dyn Environment, version: &str) -> Result<(), InstallCacheError> { + install_version(env, version, false).map(|_| {}) } - pub fn force_install(version: &str) -> Result<(), InstallCacheError> { - install_version(version, true).map(|_| {}) + pub fn force_install(env: &dyn Environment, version: &str) -> Result<(), InstallCacheError> { + install_version(env, version, true).map(|_| {}) } -} -#[allow(dead_code)] -impl Cache for DiskBasedCache { - fn version_str(&self) -> String { + pub fn version_str(&self) -> String { format!("{}", self.version) } - fn is_installed(&self) -> Result { + #[allow(dead_code)] + pub fn is_installed(&self) -> Result { is_version_installed(&self.version_str()) } - fn delete(&self) -> Result<(), DeleteCacheError> { + pub fn delete(&self) -> Result<(), DeleteCacheError> { delete_version(&self.version_str()).map(|_| {}) } - fn get_binary_command_path( + pub fn get_binary_command_path( &self, + env: &dyn Environment, binary_name: &str, ) -> Result { - Self::install(&self.version_str())?; + Self::install(env, &self.version_str())?; let path = get_binary_path_from_version(&self.version_str(), binary_name)?; Ok(path) } - fn get_binary_command( + pub fn get_binary_command( &self, + env: &dyn Environment, binary_name: &str, ) -> Result { - Self::install(&self.version_str())?; + Self::install(env, &self.version_str())?; let path = binary_command_from_version(&self.version_str(), binary_name)?; Ok(path) } } -pub fn install_version(v: &str, force: bool) -> Result { +pub fn install_version( + env: &dyn Environment, + v: &str, + force: bool, +) -> Result { let p = get_bin_cache(v)?; if !force && is_version_installed(v).unwrap_or(false) { return Ok(p); @@ -88,10 +95,7 @@ pub fn install_version(v: &str, force: bool) -> Result = if stderr().is_terminal() { - let b = ProgressBar::new_spinner(); - b.set_draw_target(ProgressDrawTarget::stderr()); - b.set_message(format!("Installing version {} of dfx...", v)); - b.enable_steady_tick(80); + let b = env.new_spinner(format!("Installing version {v} of dfx...").into()); Some(b) } else { None @@ -153,16 +157,16 @@ pub fn install_version(v: &str, force: bool) -> Result, + _cache: VersionCache, logger: Logger, } unsafe impl Send for AssetsBuilder {} @@ -77,6 +76,7 @@ impl CanisterBuilder for AssetsBuilder { #[context("Failed to get dependencies for canister '{}'.", info.get_name())] fn get_dependencies( &self, + _: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, ) -> DfxResult> { @@ -86,6 +86,7 @@ impl CanisterBuilder for AssetsBuilder { #[context("Failed to build asset canister '{}'.", info.get_name())] fn build( &self, + _: &dyn Environment, _pool: &CanisterPool, info: &CanisterInfo, _config: &BuildConfig, @@ -106,6 +107,7 @@ impl CanisterBuilder for AssetsBuilder { fn postbuild( &self, + _: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, config: &BuildConfig, @@ -141,6 +143,7 @@ impl CanisterBuilder for AssetsBuilder { fn get_candid_path( &self, + _: &dyn Environment, _pool: &CanisterPool, info: &CanisterInfo, _config: &BuildConfig, diff --git a/src/dfx/src/lib/builders/custom.rs b/src/dfx/src/lib/builders/custom.rs index 6f360f7aab..5895c3ce01 100644 --- a/src/dfx/src/lib/builders/custom.rs +++ b/src/dfx/src/lib/builders/custom.rs @@ -84,6 +84,7 @@ impl CanisterBuilder for CustomBuilder { #[context("Failed to get dependencies for canister '{}'.", info.get_name())] fn get_dependencies( &self, + _: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, ) -> DfxResult> { @@ -93,6 +94,7 @@ impl CanisterBuilder for CustomBuilder { #[context("Failed to build custom canister {}.", info.get_name())] fn build( &self, + _: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, config: &BuildConfig, @@ -135,6 +137,7 @@ impl CanisterBuilder for CustomBuilder { fn get_candid_path( &self, + _: &dyn Environment, _pool: &CanisterPool, info: &CanisterInfo, _config: &BuildConfig, diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index 78b1c0974f..32eaf1ca96 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -59,6 +59,7 @@ pub trait CanisterBuilder { /// list. fn get_dependencies( &self, + _env: &dyn Environment, _pool: &CanisterPool, _info: &CanisterInfo, ) -> DfxResult> { @@ -67,6 +68,7 @@ pub trait CanisterBuilder { fn prebuild( &self, + _env: &dyn Environment, _pool: &CanisterPool, _info: &CanisterInfo, _config: &BuildConfig, @@ -78,6 +80,7 @@ pub trait CanisterBuilder { /// while the config contains information related to this particular build. fn build( &self, + env: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, config: &BuildConfig, @@ -85,6 +88,7 @@ pub trait CanisterBuilder { fn postbuild( &self, + _env: &dyn Environment, _pool: &CanisterPool, _info: &CanisterInfo, _config: &BuildConfig, @@ -95,6 +99,7 @@ pub trait CanisterBuilder { /// Generate type declarations for the canister fn generate( &self, + env: &dyn Environment, logger: &Logger, pool: &CanisterPool, info: &CanisterInfo, @@ -152,7 +157,7 @@ pub trait CanisterBuilder { ) })?; - let did_from_build = self.get_candid_path(pool, info, config)?; + let did_from_build = self.get_candid_path(env, pool, info, config)?; if !did_from_build.exists() { bail!( "Candid file: {} doesn't exist.", @@ -229,6 +234,7 @@ pub trait CanisterBuilder { /// No need to guarantee the file exists, as the caller will handle that. fn get_candid_path( &self, + env: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, config: &BuildConfig, diff --git a/src/dfx/src/lib/builders/motoko.rs b/src/dfx/src/lib/builders/motoko.rs index a7be1a0ef4..5f0ce7fbec 100644 --- a/src/dfx/src/lib/builders/motoko.rs +++ b/src/dfx/src/lib/builders/motoko.rs @@ -1,3 +1,4 @@ +use crate::config::cache::VersionCache; use crate::lib::builders::{ BuildConfig, BuildOutput, CanisterBuilder, IdlBuildOutput, WasmBuildOutput, }; @@ -11,7 +12,6 @@ use crate::lib::package_arguments::{self, PackageArguments}; use crate::util::assets::management_idl; use anyhow::Context; use candid::Principal as CanisterId; -use dfx_core::config::cache::Cache; use dfx_core::config::model::dfinity::{MetadataVisibility, Profile}; use fn_error_context::context; use slog::{info, o, trace, warn, Logger}; @@ -20,11 +20,10 @@ use std::convert::TryFrom; use std::fmt::Debug; use std::path::{Path, PathBuf}; use std::process::Output; -use std::sync::Arc; pub struct MotokoBuilder { logger: slog::Logger, - cache: Arc, + cache: VersionCache, } unsafe impl Send for MotokoBuilder {} unsafe impl Sync for MotokoBuilder {} @@ -42,10 +41,15 @@ impl MotokoBuilder { } #[context("Failed to find imports for canister at '{}'.", info.get_main_path().display())] -fn get_imports(cache: &dyn Cache, info: &MotokoCanisterInfo) -> DfxResult> { +fn get_imports( + env: &dyn Environment, + cache: &VersionCache, + info: &MotokoCanisterInfo, +) -> DfxResult> { #[context("Failed recursive dependency detection at {}.", file.display())] fn get_imports_recursive( - cache: &dyn Cache, + env: &dyn Environment, + cache: &VersionCache, workspace_root: &Path, file: &Path, result: &mut BTreeSet, @@ -56,7 +60,7 @@ fn get_imports(cache: &dyn Cache, info: &MotokoCanisterInfo) -> DfxResult DfxResult { - get_imports_recursive(cache, workspace_root, path.as_path(), result)?; + get_imports_recursive(env, cache, workspace_root, path.as_path(), result)?; } _ => { result.insert(import); @@ -81,6 +85,7 @@ fn get_imports(cache: &dyn Cache, info: &MotokoCanisterInfo) -> DfxResult DfxResult> { let motoko_info = info.as_info::()?; - let imports = get_imports(self.cache.as_ref(), &motoko_info)?; + let imports = get_imports(env, &self.cache, &motoko_info)?; Ok(imports .iter() @@ -116,6 +122,7 @@ impl CanisterBuilder for MotokoBuilder { #[context("Failed to build Motoko canister '{}'.", canister_info.get_name())] fn build( &self, + env: &dyn Environment, pool: &CanisterPool, canister_info: &CanisterInfo, config: &BuildConfig, @@ -143,7 +150,7 @@ impl CanisterBuilder for MotokoBuilder { .with_context(|| format!("Failed to create {}.", idl_dir_path.to_string_lossy()))?; // If the management canister is being imported, emit the candid file. - if get_imports(cache.as_ref(), &motoko_info)? + if get_imports(env, cache, &motoko_info)? .contains(&MotokoImport::Ic("aaaaa-aa".to_string())) { let management_idl_path = idl_dir_path.join("aaaaa-aa.did"); @@ -151,7 +158,7 @@ impl CanisterBuilder for MotokoBuilder { } let dependencies = self - .get_dependencies(pool, canister_info) + .get_dependencies(env, pool, canister_info) .unwrap_or_default(); super::get_and_write_environment_variables( canister_info, @@ -162,7 +169,8 @@ impl CanisterBuilder for MotokoBuilder { )?; let package_arguments = package_arguments::load( - cache.as_ref(), + env, + cache, motoko_info.get_packtool(), canister_info.get_workspace_root(), )?; @@ -202,7 +210,7 @@ impl CanisterBuilder for MotokoBuilder { idl_map: &id_map, workspace_root: canister_info.get_workspace_root(), }; - motoko_compile(&self.logger, cache.as_ref(), ¶ms)?; + motoko_compile(env, &self.logger, cache, ¶ms)?; Ok(BuildOutput { canister_id: canister_info @@ -215,6 +223,7 @@ impl CanisterBuilder for MotokoBuilder { fn get_candid_path( &self, + _: &dyn Environment, _pool: &CanisterPool, info: &CanisterInfo, _config: &BuildConfig, @@ -273,8 +282,13 @@ impl MotokoParams<'_> { /// Compile a motoko file. #[context("Failed to compile Motoko.")] -fn motoko_compile(logger: &Logger, cache: &dyn Cache, params: &MotokoParams<'_>) -> DfxResult { - let mut cmd = cache.get_binary_command("moc")?; +fn motoko_compile( + env: &dyn Environment, + logger: &Logger, + cache: &VersionCache, + params: &MotokoParams<'_>, +) -> DfxResult { + let mut cmd = cache.get_binary_command(env, "moc")?; cmd.current_dir(params.workspace_root); params.to_args(&mut cmd); run_command(logger, &mut cmd, params.suppress_warning).context("Failed to run 'moc'.")?; diff --git a/src/dfx/src/lib/builders/pull.rs b/src/dfx/src/lib/builders/pull.rs index 336611369e..e63e59e8f0 100644 --- a/src/dfx/src/lib/builders/pull.rs +++ b/src/dfx/src/lib/builders/pull.rs @@ -30,6 +30,7 @@ impl CanisterBuilder for PullBuilder { #[context("Failed to get dependencies for canister '{}'.", info.get_name())] fn get_dependencies( &self, + _: &dyn Environment, _pool: &CanisterPool, info: &CanisterInfo, ) -> DfxResult> { @@ -39,6 +40,7 @@ impl CanisterBuilder for PullBuilder { #[context("Failed to build Pull canister '{}'.", canister_info.get_name())] fn build( &self, + _: &dyn Environment, _pool: &CanisterPool, canister_info: &CanisterInfo, _config: &BuildConfig, @@ -54,6 +56,7 @@ impl CanisterBuilder for PullBuilder { fn get_candid_path( &self, + _: &dyn Environment, _pool: &CanisterPool, info: &CanisterInfo, _config: &BuildConfig, diff --git a/src/dfx/src/lib/builders/rust.rs b/src/dfx/src/lib/builders/rust.rs index cd47e4d54a..3a5d03051e 100644 --- a/src/dfx/src/lib/builders/rust.rs +++ b/src/dfx/src/lib/builders/rust.rs @@ -33,6 +33,7 @@ impl CanisterBuilder for RustBuilder { #[context("Failed to get dependencies for canister '{}'.", info.get_name())] fn get_dependencies( &self, + _: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, ) -> DfxResult> { @@ -53,6 +54,7 @@ impl CanisterBuilder for RustBuilder { #[context("Failed to build Rust canister '{}'.", canister_info.get_name())] fn build( &self, + env: &dyn Environment, pool: &CanisterPool, canister_info: &CanisterInfo, config: &BuildConfig, @@ -76,7 +78,7 @@ impl CanisterBuilder for RustBuilder { .arg("--locked"); let dependencies = self - .get_dependencies(pool, canister_info) + .get_dependencies(env, pool, canister_info) .unwrap_or_default(); let vars = super::get_and_write_environment_variables( canister_info, @@ -109,6 +111,7 @@ impl CanisterBuilder for RustBuilder { fn get_candid_path( &self, + _: &dyn Environment, _pool: &CanisterPool, info: &CanisterInfo, _config: &BuildConfig, diff --git a/src/dfx/src/lib/environment.rs b/src/dfx/src/lib/environment.rs index 2b701f233d..8626e358ed 100644 --- a/src/dfx/src/lib/environment.rs +++ b/src/dfx/src/lib/environment.rs @@ -1,11 +1,10 @@ -use crate::config::cache::DiskBasedCache; +use crate::config::cache::VersionCache; use crate::config::dfx_version; use crate::lib::error::DfxResult; use crate::lib::progress_bar::ProgressBar; use crate::lib::warning::{is_warning_disabled, DfxWarning::MainnetPlainTextIdentity}; use anyhow::{anyhow, bail}; use candid::Principal; -use dfx_core::config::cache::Cache; use dfx_core::config::model::canister_id_store::CanisterIdStore; use dfx_core::config::model::dfinity::{Config, NetworksConfig}; use dfx_core::config::model::network_descriptor::{NetworkDescriptor, NetworkTypeDescriptor}; @@ -17,6 +16,7 @@ use dfx_core::extension::manager::ExtensionManager; use dfx_core::identity::identity_manager::{IdentityManager, InitializeIdentity}; use fn_error_context::context; use ic_agent::{Agent, Identity}; +use indicatif::MultiProgress; use pocket_ic::nonblocking::PocketIc; use semver::Version; use slog::{Logger, Record}; @@ -28,7 +28,7 @@ use std::time::Duration; use url::Url; pub trait Environment { - fn get_cache(&self) -> Arc; + fn get_cache(&self) -> VersionCache; fn get_config(&self) -> Result>, LoadDfxConfigError>; fn get_networks_config(&self) -> Arc; fn get_config_or_anyhow(&self) -> anyhow::Result>; @@ -100,13 +100,15 @@ pub struct EnvironmentImpl { project_config: RefCell, shared_networks_config: Arc, - cache: Arc, + cache: VersionCache, version: Version, logger: Option, verbose_level: i64, + spinners: MultiProgress, + identity_override: Option, effective_canister_id: Option, @@ -120,7 +122,7 @@ impl EnvironmentImpl { let version = dfx_version().clone(); Ok(EnvironmentImpl { - cache: Arc::new(DiskBasedCache::with_version(&version)), + cache: VersionCache::with_version(&version), project_config: RefCell::new(ProjectConfig::NotLoaded), shared_networks_config: Arc::new(shared_networks_config), version: version.clone(), @@ -129,6 +131,7 @@ impl EnvironmentImpl { identity_override: None, effective_canister_id: None, extension_manager, + spinners: MultiProgress::new(), }) } @@ -163,6 +166,11 @@ impl EnvironmentImpl { } } + pub fn with_spinners(mut self, spinners: MultiProgress) -> Self { + self.spinners = spinners; + self + } + fn load_config(&self) -> Result<(), LoadDfxConfigError> { let config = Config::from_current_dir(Some(&self.extension_manager))?; @@ -175,8 +183,8 @@ impl EnvironmentImpl { } impl Environment for EnvironmentImpl { - fn get_cache(&self) -> Arc { - Arc::clone(&self.cache) + fn get_cache(&self) -> VersionCache { + self.cache.clone() } fn get_config(&self) -> Result>, LoadDfxConfigError> { @@ -241,7 +249,7 @@ impl Environment for EnvironmentImpl { fn new_spinner(&self, message: Cow<'static, str>) -> ProgressBar { // Only show the progress bar if the level is INFO or more. if self.verbose_level >= 0 { - ProgressBar::new_spinner(message) + ProgressBar::new_spinner(message, &self.spinners) } else { ProgressBar::discard() } @@ -353,7 +361,7 @@ impl<'a> AgentEnvironment<'a> { } impl<'a> Environment for AgentEnvironment<'a> { - fn get_cache(&self) -> Arc { + fn get_cache(&self) -> VersionCache { self.backend.get_cache() } @@ -458,3 +466,77 @@ pub fn create_agent( pub fn create_pocketic(url: &Url) -> PocketIc { PocketIc::new_from_existing_instance(url.clone(), 0, None) } + +#[cfg(test)] +pub mod test_env { + use super::*; + + /// Provides access to log-message-generating functions in test mode. + pub struct TestEnv; + impl Environment for TestEnv { + fn get_agent(&self) -> &Agent { + unimplemented!() + } + fn get_cache(&self) -> VersionCache { + unimplemented!() + } + fn get_canister_id_store(&self) -> Result { + unimplemented!() + } + fn get_config(&self) -> Result>, LoadDfxConfigError> { + unimplemented!() + } + fn get_config_or_anyhow(&self) -> anyhow::Result> { + bail!("dummy env") + } + fn get_effective_canister_id(&self) -> Principal { + unimplemented!() + } + fn get_extension_manager(&self) -> &ExtensionManager { + unimplemented!() + } + fn get_identity_override(&self) -> Option<&str> { + None + } + fn get_logger(&self) -> &slog::Logger { + unimplemented!() + } + fn get_network_descriptor(&self) -> &NetworkDescriptor { + unimplemented!() + } + fn get_networks_config(&self) -> Arc { + unimplemented!() + } + fn get_override_effective_canister_id(&self) -> Option { + None + } + fn get_pocketic(&self) -> Option<&PocketIc> { + None + } + fn get_project_temp_dir(&self) -> DfxResult> { + Ok(None) + } + fn get_selected_identity(&self) -> Option<&String> { + unimplemented!() + } + fn get_selected_identity_principal(&self) -> Option { + unimplemented!() + } + fn get_verbose_level(&self) -> i64 { + 0 + } + fn get_version(&self) -> &Version { + unimplemented!() + } + fn log(&self, _record: &Record) {} + fn new_identity_manager(&self) -> Result { + unimplemented!() + } + fn new_progress(&self, _message: &str) -> ProgressBar { + ProgressBar::discard() + } + fn new_spinner(&self, _message: Cow<'static, str>) -> ProgressBar { + ProgressBar::discard() + } + } +} diff --git a/src/dfx/src/lib/identity/wallet.rs b/src/dfx/src/lib/identity/wallet.rs index 473a10651f..0b4581f399 100644 --- a/src/dfx/src/lib/identity/wallet.rs +++ b/src/dfx/src/lib/identity/wallet.rs @@ -72,9 +72,12 @@ pub async fn create_wallet( fetch_root_key_if_needed(env).await?; let agent = env.get_agent(); let mgr = ManagementCanister::create(agent); - info!( - env.get_logger(), - "Creating a wallet canister on the {} network.", network.name + let spinner = env.new_spinner( + format!( + "Creating a wallet canister on the {} network.", + network.name + ) + .into(), ); let wasm = wallet_wasm(env.get_logger())?; @@ -117,10 +120,10 @@ pub async fn create_wallet( .context("Failed to store wallet wasm.")?; set_wallet_id(network, name, canister_id)?; - + spinner.finish_and_clear(); info!( env.get_logger(), - r#"The wallet canister on the "{}" network for user "{}" is "{}""#, + r#"Created a wallet canister on the "{}" network for user "{}" with ID "{}""#, network.name, name, canister_id, diff --git a/src/dfx/src/lib/logger.rs b/src/dfx/src/lib/logger.rs index 28c13b94da..142ceca8f9 100644 --- a/src/dfx/src/lib/logger.rs +++ b/src/dfx/src/lib/logger.rs @@ -1,5 +1,6 @@ use crate::config::dfx_version_str; -use slog::{Drain, Level, Logger}; +use indicatif::{MultiProgress, ProgressDrawTarget}; +use slog::{Drain, Level, Logger, OwnedKVList, Record}; use std::fs::File; use std::path::PathBuf; @@ -87,10 +88,12 @@ fn create_drain(mode: LoggingMode) -> Logger { } } -/// Create a root logger. +/// Create a root logger. The logger will freeze any spinners from the accompanying MultiProgress while logging; +/// accordingly, all spinners should be made from `env.new_spinner()`. +/// /// The verbose_level can be negative, in which case it's a quiet mode which removes warnings, /// then errors entirely. -pub fn create_root_logger(verbose_level: i64, mode: LoggingMode) -> Logger { +pub fn create_root_logger(verbose_level: i64, mode: LoggingMode) -> (Logger, MultiProgress) { let log_level = match verbose_level { -3 => Level::Critical, -2 => Level::Error, @@ -101,13 +104,41 @@ pub fn create_root_logger(verbose_level: i64, mode: LoggingMode) -> Logger { if x > 0 { Level::Trace } else { - return Logger::root(slog::Discard, slog::o!()); + return ( + Logger::root(slog::Discard, slog::o!()), + MultiProgress::new(), + ); } } }; + let spinners = MultiProgress::new(); + spinners.set_draw_target(ProgressDrawTarget::stderr()); let drain = slog::LevelFilter::new(create_drain(mode), log_level).fuse(); + let drain = IndicatifCompat { + inner: drain, + spinners: spinners.clone(), + }; let drain = slog_async::Async::new(drain).build().fuse(); + ( + Logger::root(drain, slog::o!("version" => dfx_version_str())), + spinners, + ) +} + +struct IndicatifCompat { + inner: D, + spinners: MultiProgress, +} - Logger::root(drain, slog::o!("version" => dfx_version_str())) +impl Drain for IndicatifCompat { + type Ok = D::Ok; + type Err = D::Err; + fn log(&self, record: &Record, values: &OwnedKVList) -> Result { + self.spinners.suspend(|| self.inner.log(record, values)) + } + #[inline] + fn is_enabled(&self, level: Level) -> bool { + self.inner.is_enabled(level) + } } diff --git a/src/dfx/src/lib/manifest.rs b/src/dfx/src/lib/manifest.rs index 8cf36b99e2..d78eaf680c 100644 --- a/src/dfx/src/lib/manifest.rs +++ b/src/dfx/src/lib/manifest.rs @@ -2,11 +2,12 @@ use crate::lib::error::{DfxError, DfxResult}; use crate::{error_invalid_argument, error_invalid_data}; use anyhow::Context; use fn_error_context::context; -use indicatif::{ProgressBar, ProgressDrawTarget}; use semver::Version; use serde::{Deserialize, Deserializer}; use std::collections::BTreeMap; +use super::environment::Environment; + fn parse_semver<'de, D>(version: &str) -> Result where D: Deserializer<'de>, @@ -60,6 +61,7 @@ pub fn is_upgrade_necessary(latest_version: Option<&Version>, current: &Version) #[context("Failed to fetch latest version.")] pub fn get_latest_version( + env: &dyn Environment, release_root: &str, timeout: Option, ) -> DfxResult { @@ -69,11 +71,7 @@ pub fn get_latest_version( .join("manifest.json") .map_err(|e| error_invalid_argument!("invalid manifest URL: {}", e))?; - let b = ProgressBar::new_spinner(); - b.set_draw_target(ProgressDrawTarget::stderr()); - - b.set_message("Checking for latest dfx version..."); - b.enable_steady_tick(80); + let b = env.new_spinner("Checking for latest dfx version...".into()); let client = match timeout { Some(timeout) => reqwest::blocking::Client::builder().timeout(timeout), @@ -104,6 +102,8 @@ pub fn get_latest_version( #[cfg(test)] mod tests { + use crate::lib::environment::test_env::TestEnv; + use super::*; const MANIFEST: &str = r#"{ @@ -135,19 +135,20 @@ mod tests { #[test] fn test_get_latest_version() { let _ = env_logger::try_init(); + let env = TestEnv; let _m = mockito::mock("GET", "/manifest.json") .with_status(200) .with_header("content-type", "application/json") .with_body(MANIFEST) .create(); - let latest_version = get_latest_version(&mockito::server_url(), None); + let latest_version = get_latest_version(&env, &mockito::server_url(), None); assert_eq!(latest_version.unwrap(), Version::parse("0.4.1").unwrap()); let _m = mockito::mock("GET", "/manifest.json") .with_status(200) .with_header("content-type", "application/json") .with_body("Not a valid JSON object") .create(); - let latest_version = get_latest_version(&mockito::server_url(), None); + let latest_version = get_latest_version(&env, &mockito::server_url(), None); assert!(latest_version.is_err()); } } diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 87b13b42ad..f4f1412cef 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -56,24 +56,35 @@ impl Canister { } } - pub fn prebuild(&self, pool: &CanisterPool, build_config: &BuildConfig) -> DfxResult { - self.builder.prebuild(pool, &self.info, build_config) + pub fn prebuild( + &self, + env: &dyn Environment, + pool: &CanisterPool, + build_config: &BuildConfig, + ) -> DfxResult { + self.builder.prebuild(env, pool, &self.info, build_config) } pub fn build( &self, + env: &dyn Environment, pool: &CanisterPool, build_config: &BuildConfig, ) -> DfxResult<&BuildOutput> { - let output = self.builder.build(pool, &self.info, build_config)?; + let output = self.builder.build(env, pool, &self.info, build_config)?; // Ignore the old output, and return a reference. let _ = self.output.replace(Some(output)); Ok(self.get_build_output().unwrap()) } - pub fn postbuild(&self, pool: &CanisterPool, build_config: &BuildConfig) -> DfxResult { - self.builder.postbuild(pool, &self.info, build_config) + pub fn postbuild( + &self, + env: &dyn Environment, + pool: &CanisterPool, + build_config: &BuildConfig, + ) -> DfxResult { + self.builder.postbuild(env, pool, &self.info, build_config) } pub fn get_name(&self) -> &str { @@ -106,12 +117,13 @@ impl Canister { #[context("Failed while trying to generate type declarations for '{}'.", self.info.get_name())] pub fn generate( &self, + env: &dyn Environment, logger: &Logger, pool: &CanisterPool, build_config: &BuildConfig, ) -> DfxResult { self.builder - .generate(logger, pool, &self.info, build_config) + .generate(env, logger, pool, &self.info, build_config) } #[context("Failed to post-process wasm of canister '{}'.", self.info.get_name())] @@ -534,6 +546,7 @@ impl CanisterPool { #[context("Failed to build dependencies graph for canister pool.")] fn build_dependencies_graph( &self, + env: &dyn Environment, canisters_to_build: Vec<&Canister>, ) -> DfxResult> { let mut graph: DiGraph = DiGraph::new(); @@ -546,6 +559,7 @@ impl CanisterPool { /// /// Returns the index of the canister's graph node. fn add_canister_and_dependencies_to_graph( + env: &dyn Environment, canister_pool: &CanisterPool, canister: &Canister, graph: &mut DiGraph, @@ -565,7 +579,7 @@ impl CanisterPool { let deps = canister .builder - .get_dependencies(canister_pool, &canister.info)?; + .get_dependencies(env, canister_pool, &canister.info)?; for dependency_id in deps { let dependency = canister_id_to_canister.get(&dependency_id).ok_or_else(|| { @@ -576,6 +590,7 @@ impl CanisterPool { ))) })?; let dependency_index = add_canister_and_dependencies_to_graph( + env, canister_pool, dependency, graph, @@ -596,6 +611,7 @@ impl CanisterPool { .collect::>(); for canister in canisters_to_build { add_canister_and_dependencies_to_graph( + env, self, canister, &mut graph, @@ -679,20 +695,27 @@ impl CanisterPool { Ok(()) } - fn step_prebuild(&self, build_config: &BuildConfig, canister: &Canister) -> DfxResult<()> { - canister.prebuild(self, build_config) + fn step_prebuild( + &self, + env: &dyn Environment, + build_config: &BuildConfig, + canister: &Canister, + ) -> DfxResult<()> { + canister.prebuild(env, self, build_config) } fn step_build<'a>( &self, + env: &dyn Environment, build_config: &BuildConfig, canister: &'a Canister, ) -> DfxResult<&'a BuildOutput> { - canister.build(self, build_config) + canister.build(env, self, build_config) } fn step_postbuild( &self, + env: &dyn Environment, build_config: &BuildConfig, canister: &Canister, build_output: &BuildOutput, @@ -703,7 +726,7 @@ impl CanisterPool { build_canister_js(&canister.canister_id(), &canister.info)?; - canister.postbuild(self, build_config) + canister.postbuild(env, self, build_config) } fn step_postbuild_all( @@ -729,6 +752,7 @@ impl CanisterPool { #[context("Failed while trying to build all canisters in the canister pool.")] pub fn build( &self, + env: &dyn Environment, log: &Logger, build_config: &BuildConfig, ) -> DfxResult>> { @@ -736,7 +760,7 @@ impl CanisterPool { .map_err(|e| DfxError::new(BuildError::PreBuildAllStepFailed(Box::new(e))))?; let canisters_to_build = self.canisters_to_build(build_config); - let graph = self.build_dependencies_graph(canisters_to_build.clone())?; + let graph = self.build_dependencies_graph(env, canisters_to_build.clone())?; let nodes = petgraph::algo::toposort(&graph, None).map_err(|cycle| { let message = match graph.node_weight(cycle.node_id()) { Some(canister_id) => match self.get_canister_info(canister_id) { @@ -768,7 +792,7 @@ impl CanisterPool { continue; } result.push( - self.step_prebuild(build_config, canister) + self.step_prebuild(env, build_config, canister) .map_err(|e| { BuildError::PreBuildStepFailed( *canister_id, @@ -777,7 +801,7 @@ impl CanisterPool { ) }) .and_then(|_| { - self.step_build(build_config, canister).map_err(|e| { + self.step_build(env, build_config, canister).map_err(|e| { BuildError::BuildStepFailed( *canister_id, canister.get_name().to_string(), @@ -786,7 +810,7 @@ impl CanisterPool { }) }) .and_then(|o| { - self.step_postbuild(build_config, canister, o) + self.step_postbuild(env, build_config, canister, o) .map_err(|e| { BuildError::PostBuildStepFailed( *canister_id, @@ -809,9 +833,14 @@ impl CanisterPool { /// Build all canisters, failing with the first that failed the build. Will return /// nothing if all succeeded. #[context("Failed while trying to build all canisters.")] - pub async fn build_or_fail(&self, log: &Logger, build_config: &BuildConfig) -> DfxResult<()> { + pub async fn build_or_fail( + &self, + env: &dyn Environment, + log: &Logger, + build_config: &BuildConfig, + ) -> DfxResult<()> { self.download(build_config).await?; - let outputs = self.build(log, build_config)?; + let outputs = self.build(env, log, build_config)?; for output in outputs { output.map_err(DfxError::new)?; diff --git a/src/dfx/src/lib/operations/canister/create_canister.rs b/src/dfx/src/lib/operations/canister/create_canister.rs index b8ffadaee4..bebff3aa84 100644 --- a/src/dfx/src/lib/operations/canister/create_canister.rs +++ b/src/dfx/src/lib/operations/canister/create_canister.rs @@ -46,7 +46,6 @@ pub async fn create_canister( subnet_selection: &mut SubnetSelectionType, ) -> DfxResult { let log = env.get_logger(); - info!(log, "Creating canister {}...", canister_name); let config = env.get_config_or_anyhow()?; let config_interface = config.get_config(); @@ -151,6 +150,7 @@ The command line value will be used.", } }; + let spinner = env.new_spinner(format!("Creating canister {canister_name}...").into()); let agent = env.get_agent(); let cid = match call_sender { CallSender::SelectedId => { @@ -180,6 +180,7 @@ The command line value will be used.", create_with_wallet(agent, &wallet_id, with_cycles, settings, subnet_selection).await } }?; + spinner.finish_and_clear(); let canister_id = cid.to_text(); info!( log, diff --git a/src/dfx/src/lib/operations/canister/deploy_canisters.rs b/src/dfx/src/lib/operations/canister/deploy_canisters.rs index b3689e7722..4b40fba0af 100644 --- a/src/dfx/src/lib/operations/canister/deploy_canisters.rs +++ b/src/dfx/src/lib/operations/canister/deploy_canisters.rs @@ -306,7 +306,7 @@ async fn build_canisters( BuildConfig::from_config(config, env.get_network_descriptor().is_playground())? .with_canisters_to_build(canisters_to_build.into()) .with_env_file(env_file); - canister_pool.build_or_fail(log, &build_config).await?; + canister_pool.build_or_fail(env, log, &build_config).await?; Ok(canister_pool) } diff --git a/src/dfx/src/lib/operations/canister/install_canister.rs b/src/dfx/src/lib/operations/canister/install_canister.rs index 78738ab751..d7e6c1c094 100644 --- a/src/dfx/src/lib/operations/canister/install_canister.rs +++ b/src/dfx/src/lib/operations/canister/install_canister.rs @@ -431,7 +431,7 @@ fn check_stable_compatibility( })?; let cache = env.get_cache(); let output = cache - .get_binary_command("moc")? + .get_binary_command(env, "moc")? .arg("--stable-compatible") .arg(&deployed_stable_path) .arg(stable_path) diff --git a/src/dfx/src/lib/package_arguments.rs b/src/dfx/src/lib/package_arguments.rs index ae57a5072c..772585c270 100644 --- a/src/dfx/src/lib/package_arguments.rs +++ b/src/dfx/src/lib/package_arguments.rs @@ -1,6 +1,7 @@ +use crate::config::cache::VersionCache; use crate::lib::error::{BuildError, DfxError, DfxResult}; +use crate::Environment; use anyhow::{anyhow, bail}; -use dfx_core::config::cache::Cache; use fn_error_context::context; use std::path::Path; use std::process::Command; @@ -12,13 +13,14 @@ pub type PackageArguments = Vec; #[context("Failed to load package arguments.")] pub fn load( - cache: &dyn Cache, + env: &dyn Environment, + cache: &VersionCache, packtool: &Option, project_root: &Path, ) -> DfxResult { if packtool.is_none() { let stdlib_path = cache - .get_binary_command_path("base")? + .get_binary_command_path(env, "base")? .into_os_string() .into_string() .map_err(|_| anyhow!("Path contains invalid Unicode data."))?; diff --git a/src/dfx/src/lib/progress_bar.rs b/src/dfx/src/lib/progress_bar.rs index 1c4bf31650..ccb7fccd90 100644 --- a/src/dfx/src/lib/progress_bar.rs +++ b/src/dfx/src/lib/progress_bar.rs @@ -1,5 +1,6 @@ -use indicatif::{ProgressBar as IndicatifProgressBar, ProgressDrawTarget}; -use std::borrow::Cow; +#![allow(clippy::disallowed_types)] +use indicatif::{MultiProgress, ProgressBar as IndicatifProgressBar}; +use std::{borrow::Cow, time::Duration}; pub struct ProgressBar { bar: Option, @@ -24,12 +25,11 @@ macro_rules! forward_fn_impl { } impl ProgressBar { - pub fn new_spinner(message: Cow<'static, str>) -> Self { + pub fn new_spinner(message: Cow<'static, str>, set: &MultiProgress) -> Self { let progress_bar = IndicatifProgressBar::new_spinner(); - progress_bar.set_draw_target(ProgressDrawTarget::stderr()); - + set.add(progress_bar.clone()); progress_bar.set_message(message); - progress_bar.enable_steady_tick(80); + progress_bar.enable_steady_tick(Duration::from_millis(80)); ProgressBar { bar: Some(progress_bar), diff --git a/src/dfx/src/main.rs b/src/dfx/src/main.rs index fbdbdeece0..390192a60f 100644 --- a/src/dfx/src/main.rs +++ b/src/dfx/src/main.rs @@ -10,6 +10,7 @@ use clap::{ArgAction, CommandFactory, Parser}; use dfx_core::config::project_templates; use dfx_core::extension::installed::InstalledExtensionManifests; use dfx_core::extension::manager::ExtensionManager; +use indicatif::MultiProgress; use std::collections::HashMap; use std::ffi::OsString; use std::path::PathBuf; @@ -54,7 +55,7 @@ pub struct CliOpts { /// Setup a logger with the proper configuration, based on arguments. /// Returns a topple of whether or not to have a progress bar, and a logger. -fn setup_logging(opts: &CliOpts) -> (i64, slog::Logger) { +fn setup_logging(opts: &CliOpts) -> (i64, slog::Logger, MultiProgress) { // Create a logger with our argument matches. let verbose_level = opts.verbose as i64 - opts.quiet as i64; @@ -63,8 +64,8 @@ fn setup_logging(opts: &CliOpts) -> (i64, slog::Logger) { "file" => LoggingMode::File(PathBuf::from(opts.logfile.as_deref().unwrap_or("log.txt"))), _ => LoggingMode::Stderr, }; - - (verbose_level, create_root_logger(verbose_level, mode)) + let (logger, spinners) = create_root_logger(verbose_level, mode); + (verbose_level, logger, spinners) } fn print_error_and_diagnosis(log_level: Option, err: Error, error_diagnosis: DiagnosedError) { @@ -157,13 +158,14 @@ fn inner_main(log_level: &mut Option) -> DfxResult { return commands::exec_without_env(cli_opts.command); } - let (verbose_level, log) = setup_logging(&cli_opts); + let (verbose_level, log, spinners) = setup_logging(&cli_opts); *log_level = Some(verbose_level); let identity = cli_opts.identity; let effective_canister_id = cli_opts.provisional_create_canister_effective_canister_id; let env = EnvironmentImpl::new(em)? .with_logger(log) + .with_spinners(spinners) .with_identity_override(identity) .with_verbose_level(verbose_level) .with_effective_canister_id(effective_canister_id);