Skip to content

Commit

Permalink
chore: clean up dfx canister create output (#4080)
Browse files Browse the repository at this point in the history
* change canister creation to spinners

* keep spinners and logs from interfering with each other

* Spinner for wallet creation
  • Loading branch information
adamspofford-dfinity authored and rikonor committed Feb 3, 2025
1 parent 3319c39 commit f79b3fc
Show file tree
Hide file tree
Showing 31 changed files with 365 additions and 183 deletions.
31 changes: 22 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
too-many-arguments-threshold = 16
too-many-arguments-threshold = 16

[[disallowed-types]]
path = "indicatif::ProgressBar"
reason = "use env.new_spinner() for log compatibility"
4 changes: 2 additions & 2 deletions e2e/tests-dfx/deploy.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
18 changes: 2 additions & 16 deletions src/dfx-core/src/config/cache.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,15 @@
#[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;
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<bool, IsCacheInstalledError>;
fn delete(&self) -> Result<(), DeleteCacheError>;
fn get_binary_command_path(
&self,
binary_name: &str,
) -> Result<PathBuf, GetBinaryCommandPathError>;
fn get_binary_command(
&self,
binary_name: &str,
) -> Result<std::process::Command, GetBinaryCommandPathError>;
}

pub fn get_cache_root() -> Result<PathBuf, GetCacheRootError> {
let cache_root = std::env::var_os("DFX_CACHE_ROOT");
// dirs-next is not used for *nix to preserve existing paths
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions src/dfx/src/actors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ pub fn start_btc_adapter_actor(
shutdown_controller: Addr<ShutdownController>,
btc_adapter_pid_file_path: PathBuf,
) -> DfxResult<Recipient<BtcAdapterReadySubscribe>> {
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,
Expand All @@ -69,7 +71,7 @@ pub fn start_canister_http_adapter_actor(
) -> DfxResult<Recipient<CanisterHttpAdapterReadySubscribe>> {
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,
Expand Down Expand Up @@ -133,8 +135,8 @@ pub fn start_replica_actor(
canister_http_adapter_ready_subscribe: Option<Recipient<CanisterHttpAdapterReadySubscribe>>,
) -> DfxResult<Addr<Replica>> {
// 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();
Expand Down Expand Up @@ -169,7 +171,7 @@ pub fn start_pocketic_proxy_actor(
pocketic_proxy_pid_path: PathBuf,
pocketic_proxy_port_path: PathBuf,
) -> DfxResult<Addr<PocketIcProxy>> {
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,
Expand All @@ -190,7 +192,7 @@ pub fn start_pocketic_actor(
shutdown_controller: Addr<ShutdownController>,
pocketic_port_path: PathBuf,
) -> DfxResult<Addr<PocketIc>> {
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
Expand Down
6 changes: 3 additions & 3 deletions src/dfx/src/commands/build.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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(())
}
4 changes: 2 additions & 2 deletions src/dfx/src/commands/cache/install.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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(())
}
4 changes: 2 additions & 2 deletions src/dfx/src/commands/extension/install.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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());
Expand Down
8 changes: 4 additions & 4 deletions src/dfx/src/commands/generate.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(())
Expand Down
9 changes: 3 additions & 6 deletions src/dfx/src/commands/language_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/dfx/src/commands/new.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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!(
Expand Down
Loading

0 comments on commit f79b3fc

Please sign in to comment.