From e258e1113f00ac44206395ce51639b37000d9eb6 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 14 May 2024 15:47:44 -0700 Subject: [PATCH 1/9] test this branch --- .github/workflows/e2e.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 738ae32f12..743b1d19dc 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -3,6 +3,7 @@ on: push: branches: - master + - ens/sdk-1638-less-dfx-start-clean pull_request: concurrency: From 24f3ca54a8d0db4012eb5cf68846f3b4d7fae35f Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Thu, 23 May 2024 09:21:18 -0700 Subject: [PATCH 2/9] checkpoint checkpoint checkpoint checkpoint checkpoint clippy, cleanup --- Cargo.lock | 1 + e2e/tests-dfx/start.bash | 51 ++++++-- src/dfx-core/Cargo.toml | 1 + .../config/model/local_server_descriptor.rs | 29 ++++- src/dfx-core/src/config/model/mod.rs | 1 + .../src/config/model/settings_digest.rs | 110 +++++++++++++++++ src/dfx/src/commands/start.rs | 115 +++++++++++++++--- 7 files changed, 281 insertions(+), 27 deletions(-) create mode 100644 src/dfx-core/src/config/model/settings_digest.rs diff --git a/Cargo.lock b/Cargo.lock index 320651d790..2f4f31b7b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1617,6 +1617,7 @@ dependencies = [ "semver", "serde", "serde_json", + "sha2 0.10.8", "slog", "tar", "tempfile", diff --git a/e2e/tests-dfx/start.bash b/e2e/tests-dfx/start.bash index 9aae68219a..9b8fda48fe 100644 --- a/e2e/tests-dfx/start.bash +++ b/e2e/tests-dfx/start.bash @@ -12,6 +12,43 @@ teardown() { standard_teardown } +@test "start and stop with different options" { + dfx_start --artificial-delay 101 + dfx_stop + + # notice: no need to --clean + dfx_start --artificial-delay 102 + dfx_stop +} + +@test "project networks still need --clean" { + dfx_new hello + define_project_network + + dfx_start --artificial-delay 101 + dfx_stop + + dfx_start --artificial-delay 102 + dfx_stop +} + +@test "stop and start with other options does not disrupt projects" { + dfx_start --artificial-delay 101 + + dfx_new p1 + assert_command dfx deploy + CANISTER_ID="$(dfx canister id p1_backend)" + + assert_command dfx stop + dfx_start --artificial-delay 102 + assert_command dfx stop + + dfx_start --artificial-delay 101 + + assert_command dfx canister id p1_backend + assert_eq "$CANISTER_ID" +} + @test "start and stop outside project" { dfx_start @@ -447,18 +484,14 @@ teardown() { assert_match "Hello, World! from DFINITY" } -@test "modifying networks.json requires --clean on restart" { +@test "modifying networks.json does not require --clean on restart" { [[ "$USE_POCKETIC" ]] && skip "skipped for pocketic: --force" dfx_start dfx stop assert_command dfx_start dfx stop jq -n '.local.replica.log_level="warning"' > "$E2E_NETWORKS_JSON" - assert_command_fail dfx_start - assert_contains "The network state can't be reused with this configuration. Rerun with \`--clean\`." - assert_command dfx_start --force - dfx stop - assert_command dfx_start --clean + assert_command dfx_start } @test "project-local networks require --clean if dfx.json was updated" { @@ -480,8 +513,11 @@ teardown() { assert_command dfx_start --clean } -@test "flags count as configuration modification and require --clean" { +@test "flags count as configuration modification and require --clean for a project network" { [[ "$USE_POCKETIC" ]] && skip "skipped for pocketic: --artificial-delay" + dfx_new + define_project_network + dfx start --background dfx stop assert_command_fail dfx start --artificial-delay 100 --background @@ -493,6 +529,7 @@ teardown() { assert_command_fail dfx start --background assert_contains "The network state can't be reused with this configuration. Rerun with \`--clean\`." assert_command dfx start --force --background + dfx stop } @test "dfx start then ctrl-c won't hang and panic but stop actors quickly" { diff --git a/src/dfx-core/Cargo.toml b/src/dfx-core/Cargo.toml index 0b189ecca3..680b674f52 100644 --- a/src/dfx-core/Cargo.toml +++ b/src/dfx-core/Cargo.toml @@ -35,6 +35,7 @@ sec1 = { workspace = true, features = ["std"] } semver = { workspace = true, features = ["serde"] } serde.workspace = true serde_json.workspace = true +sha2.workspace = true slog = { workspace = true, features = ["max_level_trace"] } tar.workspace = true tempfile.workspace = true diff --git a/src/dfx-core/src/config/model/local_server_descriptor.rs b/src/dfx-core/src/config/model/local_server_descriptor.rs index 31beb4b759..848cab2dce 100644 --- a/src/dfx-core/src/config/model/local_server_descriptor.rs +++ b/src/dfx-core/src/config/model/local_server_descriptor.rs @@ -30,6 +30,7 @@ pub struct LocalServerDescriptor { /// $HOME/.local/share/dfx/network/local /// $APPDATA/dfx/network/local pub data_directory: PathBuf, + pub settings_digest: Option, pub bind_address: SocketAddr, @@ -62,9 +63,11 @@ impl LocalServerDescriptor { scope: LocalNetworkScopeDescriptor, legacy_pid_path: Option, ) -> Result { + let settings_digest = None; let bind_address = to_socket_addr(&bind).map_err(ParseBindAddressFailed)?; Ok(LocalServerDescriptor { data_directory, + settings_digest, bind_address, bitcoin, canister_http, @@ -78,7 +81,7 @@ impl LocalServerDescriptor { /// The contents of this file are different for each `dfx start --clean` /// or `dfx start` when the network data directory doesn't already exist pub fn network_id_path(&self) -> PathBuf { - self.data_directory.join("network-id") + self.data_dir_by_settings_digest().join("network-id") } /// This file contains the pid of the process started with `dfx start` @@ -164,9 +167,21 @@ impl LocalServerDescriptor { path.exists().then(|| load_json_file(&path)).transpose() } + pub fn data_dir_by_settings_digest(&self) -> PathBuf { + if self.scope == LocalNetworkScopeDescriptor::Project { + self.data_directory.clone() + } else { + let settings_digest = self + .settings_digest + .as_ref() + .expect("settings_digest must be set"); + self.data_directory.join(settings_digest) + } + } + /// The top-level directory holding state for the replica. pub fn state_dir(&self) -> PathBuf { - self.data_directory.join("state") + self.data_dir_by_settings_digest().join("state") } /// The replicated state of the replica. @@ -182,7 +197,8 @@ impl LocalServerDescriptor { /// This file contains the effective config the replica was started with. pub fn effective_config_path(&self) -> PathBuf { - self.data_directory.join("replica-effective-config.json") + self.data_dir_by_settings_digest() + .join("replica-effective-config.json") } } @@ -224,6 +240,13 @@ impl LocalServerDescriptor { }; Self { proxy, ..self } } + + pub fn with_settings_digest(self, settings_digest: String) -> Self { + Self { + settings_digest: Some(settings_digest), + ..self + } + } } impl LocalServerDescriptor { diff --git a/src/dfx-core/src/config/model/mod.rs b/src/dfx-core/src/config/model/mod.rs index 413203fa8f..2a6ec1ae6e 100644 --- a/src/dfx-core/src/config/model/mod.rs +++ b/src/dfx-core/src/config/model/mod.rs @@ -1,4 +1,5 @@ pub mod bitcoin_adapter; +pub mod settings_digest; pub mod canister_http_adapter; pub mod canister_id_store; pub mod dfinity; diff --git a/src/dfx-core/src/config/model/settings_digest.rs b/src/dfx-core/src/config/model/settings_digest.rs new file mode 100644 index 0000000000..45e64a1c8c --- /dev/null +++ b/src/dfx-core/src/config/model/settings_digest.rs @@ -0,0 +1,110 @@ +use crate::config::model::dfinity::{ReplicaLogLevel, ReplicaSubnetType}; +use crate::config::model::local_server_descriptor::LocalServerDescriptor; +use candid::Deserialize; +use serde::Serialize; +use sha2::{Digest, Sha256}; +use std::borrow::Cow; + +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] +#[serde(tag = "type", rename_all = "snake_case")] +enum HttpHandlerPortSetting { + Port { + port: u16, + }, + #[default] + WritePortToPath, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] +struct HttpHandlerSettings { + pub port: HttpHandlerPortSetting, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] +struct BtcAdapterSettings { + pub enabled: bool, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] +struct CanisterHttpAdapterSettings { + pub enabled: bool, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq)] +struct ReplicaSettings { + pub http_handler: HttpHandlerSettings, + pub subnet_type: ReplicaSubnetType, + pub btc_adapter: BtcAdapterSettings, + pub canister_http_adapter: CanisterHttpAdapterSettings, + pub log_level: ReplicaLogLevel, + pub artificial_delay: u32, + pub use_old_metering: bool, +} + +#[derive(Serialize, Deserialize, PartialEq, Eq)] +#[serde(tag = "type", rename_all = "snake_case")] +enum BackendSettings<'a> { + Replica { config: Cow<'a, ReplicaSettings> }, +} + +#[derive(Serialize, Deserialize, PartialEq, Eq)] +struct Settings<'a> { + pub ic_repo_commit: String, + #[serde(flatten)] + pub backend: BackendSettings<'a>, +} + +pub fn get_settings_digest( + ic_repo_commit: &str, + local_server_descriptor: &LocalServerDescriptor, + use_old_metering: bool, + artificial_delay: u32, +) -> String { + let backend = + get_replica_backend_settings(local_server_descriptor, use_old_metering, artificial_delay); + let settings = Settings { + ic_repo_commit: ic_repo_commit.into(), + backend, + }; + let normalized = serde_json::to_string_pretty(&settings).unwrap(); + let hash: Vec = Sha256::digest(normalized).to_vec(); + hex::encode(hash) +} + +fn get_replica_backend_settings( + local_server_descriptor: &LocalServerDescriptor, + use_old_metering: bool, + artificial_delay: u32, +) -> BackendSettings { + let http_handler = HttpHandlerSettings { + port: if let Some(port) = local_server_descriptor.replica.port { + HttpHandlerPortSetting::Port { port } + } else { + HttpHandlerPortSetting::WritePortToPath + }, + }; + let btc_adapter = BtcAdapterSettings { + enabled: local_server_descriptor.bitcoin.enabled, + }; + let canister_http_adapter = CanisterHttpAdapterSettings { + enabled: local_server_descriptor.canister_http.enabled, + }; + let replica_settings = ReplicaSettings { + http_handler, + subnet_type: local_server_descriptor + .replica + .subnet_type + .unwrap_or_default(), + btc_adapter, + canister_http_adapter, + log_level: local_server_descriptor + .replica + .log_level + .unwrap_or_default(), + artificial_delay, + use_old_metering, + }; + BackendSettings::Replica { + config: Cow::Owned(replica_settings), + } +} diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 8f48ce3d01..402afb6199 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -16,7 +16,10 @@ use crate::util::get_reusable_socket_addr; use actix::Recipient; use anyhow::{anyhow, bail, Context, Error}; use clap::{ArgAction, Parser}; -use dfx_core::config::model::local_server_descriptor::LocalServerDescriptor; +use dfx_core::config::model::settings_digest::get_settings_digest; +use dfx_core::config::model::local_server_descriptor::{ + LocalNetworkScopeDescriptor, LocalServerDescriptor, +}; use dfx_core::config::model::network_descriptor::NetworkDescriptor; use dfx_core::config::model::replica_config::{CachedConfig, ReplicaConfig}; use dfx_core::config::model::{bitcoin_adapter, canister_http_adapter}; @@ -26,7 +29,6 @@ use fn_error_context::context; use os_str_bytes::{OsStrBytes, OsStringBytes}; use slog::{info, warn, Logger}; use std::fs; -use std::fs::create_dir_all; use std::io::Read; use std::net::SocketAddr; use std::path::{Path, PathBuf}; @@ -164,6 +166,7 @@ pub fn exec( } else { Some(env.get_logger().clone()) }; + let network_descriptor = create_network_descriptor( project_config, env.get_networks_config(), @@ -181,9 +184,21 @@ pub fn exec( bitcoin_node, enable_canister_http, domain, + use_old_metering, + artificial_delay, )?; let local_server_descriptor = network_descriptor.local_server_descriptor()?; + + let subnet_type = local_server_descriptor + .replica + .subnet_type + .unwrap_or_default(); + let log_level = local_server_descriptor + .replica + .log_level + .unwrap_or_default(); + let pid_file_path = local_server_descriptor.dfx_pid_path(); check_previous_process_running(local_server_descriptor)?; @@ -196,18 +211,17 @@ pub fn exec( let (frontend_url, address_and_port) = frontend_address(local_server_descriptor, background)?; - let network_temp_dir = local_server_descriptor.data_directory.clone(); - create_dir_all(&network_temp_dir).with_context(|| { - format!( - "Failed to create network temp directory {}.", - network_temp_dir.to_string_lossy() - ) - })?; + dfx_core::fs::create_dir_all(&local_server_descriptor.data_dir_by_settings_digest())?; if !local_server_descriptor.network_id_path().exists() { write_network_id(local_server_descriptor)?; } + if let LocalNetworkScopeDescriptor::Shared { network_id_path } = &local_server_descriptor.scope + { + dfx_core::fs::copy(&local_server_descriptor.network_id_path(), network_id_path)?; + } + clean_older_state_dirs(local_server_descriptor)?; let state_root = local_server_descriptor.state_dir(); let btc_adapter_socket_holder_path = local_server_descriptor.btc_adapter_socket_holder_path(); @@ -229,6 +243,8 @@ pub fn exec( let previous_config_path = local_server_descriptor.effective_config_path(); + dfx_core::fs::create_dir_all(&local_server_descriptor.data_dir_by_settings_digest())?; + // dfx info replica-port will read these port files to find out which port to use, // so we need to make sure only one has a valid port in it. let replica_config_dir = local_server_descriptor.replica_configuration_dir(); @@ -285,14 +301,6 @@ pub fn exec( let canister_http_socket_path = canister_http_adapter_config .as_ref() .and_then(|cfg| cfg.get_socket_path()); - let subnet_type = local_server_descriptor - .replica - .subnet_type - .unwrap_or_default(); - let log_level = local_server_descriptor - .replica - .log_level - .unwrap_or_default(); let proxy_domains = local_server_descriptor.proxy.domain.clone().into_vec(); @@ -420,6 +428,72 @@ pub fn exec( Ok(()) } +fn clean_older_state_dirs(local_server_descriptor: &LocalServerDescriptor) -> DfxResult { + let data_dir = &local_server_descriptor.data_directory; + let settings_digest = local_server_descriptor.settings_digest.as_ref().unwrap(); + let directories_to_keep = 10; + if !data_dir.is_dir() { + return Ok(()); + } + let mut state_dirs = fs::read_dir(data_dir) + .with_context(|| format!("Failed to read state directory {}", data_dir.display()))? + .filter_map(|e| match e { + Ok(entry) if entry.path().is_dir() => Some(Ok(entry.path())), + Ok(_) => None, + Err(e) => Some(Err(e)), + }) + .collect::, _>>()?; + + state_dirs.retain(|p| { + p.file_name() + .map(|f| { + let filename: String = f.to_string_lossy().into(); + filename != *settings_digest + }) + .unwrap_or(true) + }); + // sort by modification time + state_dirs.sort_by_key(|p| p.metadata().map(|m| m.modified().unwrap()).unwrap()); + // keep the last 'directories_to_keep' directories + state_dirs = state_dirs + .iter() + .rev() + .skip(directories_to_keep) + .cloned() + .collect(); + // remove the rest + for dir in state_dirs { + fs::remove_dir_all(&dir) + .with_context(|| format!("Failed to remove state directory {}", dir.display()))?; + } + Ok(()) +} + +// #[derive(Serialize, Deserialize, PartialEq, Eq)] +// #[serde(tag = "type", rename_all = "snake_case")] +// #[allow(clippy::large_enum_variant)] +// pub enum CachedReplicaConfig<'a> { +// Replica { config: Cow<'a, ReplicaConfig> }, +// } +// +// #[derive(Serialize, Deserialize, PartialEq, Eq)] +// pub struct CachedConfig<'a> { +// pub replica_rev: String, +// #[serde(flatten)] +// pub config: CachedReplicaConfig<'a>, +// } +// +// impl<'a> CachedConfig<'a> { +// pub fn replica(config: &'a ReplicaConfig) -> Self { +// Self { +// replica_rev: replica_rev().into(), +// config: CachedReplicaConfig::Replica { +// config: Cow::Borrowed(config), +// }, +// } +// } +// } + pub fn apply_command_line_parameters( logger: &Logger, network_descriptor: NetworkDescriptor, @@ -429,6 +503,8 @@ pub fn apply_command_line_parameters( bitcoin_nodes: Vec, enable_canister_http: bool, domain: Vec, + use_old_metering: bool, + artificial_delay: u32, ) -> DfxResult { if enable_canister_http { warn!( @@ -465,6 +541,11 @@ pub fn apply_command_line_parameters( local_server_descriptor = local_server_descriptor.with_proxy_domains(domain) } + let settings_digest = + get_settings_digest(replica_rev(), &local_server_descriptor, use_old_metering, artificial_delay); + + local_server_descriptor = local_server_descriptor.with_settings_digest(settings_digest); + Ok(NetworkDescriptor { local_server_descriptor: Some(local_server_descriptor), ..network_descriptor From fad76fbecf65e979ccf9e1c6dbab7f741570e095 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 24 May 2024 11:10:39 -0700 Subject: [PATCH 3/9] checkpoint --- .../config/model/local_server_descriptor.rs | 6 +++ .../src/config/model/replica_config.rs | 4 +- .../src/config/model/settings_digest.rs | 13 ++++-- src/dfx-core/src/fs/mod.rs | 1 + src/dfx-core/src/json/mod.rs | 1 + src/dfx/src/commands/start.rs | 41 +++++++------------ 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/dfx-core/src/config/model/local_server_descriptor.rs b/src/dfx-core/src/config/model/local_server_descriptor.rs index 848cab2dce..1f0c2c578c 100644 --- a/src/dfx-core/src/config/model/local_server_descriptor.rs +++ b/src/dfx-core/src/config/model/local_server_descriptor.rs @@ -27,6 +27,7 @@ pub struct LocalServerDescriptor { /// The data directory is one of the following: /// /.dfx/network/local /// $HOME/Library/Application Support/org.dfinity.dfx/network/local + /// $HOME/Library/Application Support/org.dfinity.dfx/network/local /// $HOME/.local/share/dfx/network/local /// $APPDATA/dfx/network/local pub data_directory: PathBuf, @@ -197,6 +198,11 @@ impl LocalServerDescriptor { /// This file contains the effective config the replica was started with. pub fn effective_config_path(&self) -> PathBuf { + self.data_directory + .join("replica-effective-config.json") + } + + pub fn effective_config_path_by_settings_digest(&self) -> PathBuf { self.data_dir_by_settings_digest() .join("replica-effective-config.json") } diff --git a/src/dfx-core/src/config/model/replica_config.rs b/src/dfx-core/src/config/model/replica_config.rs index b36ee42932..297dec29d1 100644 --- a/src/dfx-core/src/config/model/replica_config.rs +++ b/src/dfx-core/src/config/model/replica_config.rs @@ -184,7 +184,7 @@ impl HttpHandlerConfig { } } -#[derive(Serialize, Deserialize, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] #[serde(tag = "type", rename_all = "snake_case")] #[allow(clippy::large_enum_variant)] pub enum CachedReplicaConfig<'a> { @@ -192,7 +192,7 @@ pub enum CachedReplicaConfig<'a> { PocketIc, } -#[derive(Serialize, Deserialize, PartialEq, Eq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] pub struct CachedConfig<'a> { pub replica_rev: String, #[serde(flatten)] diff --git a/src/dfx-core/src/config/model/settings_digest.rs b/src/dfx-core/src/config/model/settings_digest.rs index 45e64a1c8c..5ee03a5daf 100644 --- a/src/dfx-core/src/config/model/settings_digest.rs +++ b/src/dfx-core/src/config/model/settings_digest.rs @@ -44,7 +44,8 @@ struct ReplicaSettings { #[derive(Serialize, Deserialize, PartialEq, Eq)] #[serde(tag = "type", rename_all = "snake_case")] enum BackendSettings<'a> { - Replica { config: Cow<'a, ReplicaSettings> }, + Replica { settings: Cow<'a, ReplicaSettings> }, + PocketIc, } #[derive(Serialize, Deserialize, PartialEq, Eq)] @@ -59,9 +60,13 @@ pub fn get_settings_digest( local_server_descriptor: &LocalServerDescriptor, use_old_metering: bool, artificial_delay: u32, + pocketic: bool, ) -> String { - let backend = - get_replica_backend_settings(local_server_descriptor, use_old_metering, artificial_delay); + let backend = if pocketic { + BackendSettings::PocketIc + } else { + get_replica_backend_settings(local_server_descriptor, use_old_metering, artificial_delay) + }; let settings = Settings { ic_repo_commit: ic_repo_commit.into(), backend, @@ -105,6 +110,6 @@ fn get_replica_backend_settings( use_old_metering, }; BackendSettings::Replica { - config: Cow::Owned(replica_settings), + settings: Cow::Owned(replica_settings), } } diff --git a/src/dfx-core/src/fs/mod.rs b/src/dfx-core/src/fs/mod.rs index 022516e15e..fc75115110 100644 --- a/src/dfx-core/src/fs/mod.rs +++ b/src/dfx-core/src/fs/mod.rs @@ -16,6 +16,7 @@ pub fn canonicalize(path: &Path) -> Result { } pub fn copy(from: &Path, to: &Path) -> Result { + eprintln!("copy from: {} to: {}", from.display(), to.display()); std::fs::copy(from, to).map_err(|err| { FsError::new(CopyFileFailed( Box::new(from.to_path_buf()), diff --git a/src/dfx-core/src/json/mod.rs b/src/dfx-core/src/json/mod.rs index 1241a7a0cf..0b9b66373d 100644 --- a/src/dfx-core/src/json/mod.rs +++ b/src/dfx-core/src/json/mod.rs @@ -17,6 +17,7 @@ pub fn load_json_file serde::de::Deserialize<'a>>( } pub fn save_json_file(path: &Path, value: &T) -> Result<(), StructuredFileError> { + eprintln!("save json file to path: {:?}", path.display()); let content = serde_json::to_string_pretty(&value) .map_err(|err| SerializeJsonFileFailed(Box::new(path.to_path_buf()), err))?; crate::fs::write(path, content).map_err(WriteJsonFileFailed) diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 402afb6199..c8edeb484e 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -186,6 +186,7 @@ pub fn exec( domain, use_old_metering, artificial_delay, + pocketic, )?; let local_server_descriptor = network_descriptor.local_server_descriptor()?; @@ -219,6 +220,10 @@ pub fn exec( if let LocalNetworkScopeDescriptor::Shared { network_id_path } = &local_server_descriptor.scope { dfx_core::fs::copy(&local_server_descriptor.network_id_path(), network_id_path)?; + let effective_config_path_by_settings_digest = local_server_descriptor.effective_config_path_by_settings_digest(); + if effective_config_path_by_settings_digest.exists() { + dfx_core::fs::copy(&effective_config_path_by_settings_digest, &local_server_descriptor.effective_config_path())?; + } } clean_older_state_dirs(local_server_descriptor)?; @@ -338,9 +343,15 @@ pub fn exec( CachedConfig::replica(&replica_config, replica_rev().into()) }; - if !clean && !force && previous_config_path.exists() { + let is_shared_network = matches!(&local_server_descriptor.scope, LocalNetworkScopeDescriptor::Shared { .. }); + if is_shared_network { + save_json_file(&local_server_descriptor.effective_config_path_by_settings_digest(), &effective_config) + .context("Failed to write replica configuration")?; + } else if !clean && !force && previous_config_path.exists() { let previous_config = load_json_file(&previous_config_path) .context("Failed to read replica configuration. Rerun with `--clean`.")?; + eprintln!("Previous configuration: {:?}", previous_config); + eprintln!("Effective configuration: {:?}", effective_config); if !effective_config.can_share_state(&previous_config) { bail!( "The network state can't be reused with this configuration. Rerun with `--clean`." @@ -469,31 +480,6 @@ fn clean_older_state_dirs(local_server_descriptor: &LocalServerDescriptor) -> Df Ok(()) } -// #[derive(Serialize, Deserialize, PartialEq, Eq)] -// #[serde(tag = "type", rename_all = "snake_case")] -// #[allow(clippy::large_enum_variant)] -// pub enum CachedReplicaConfig<'a> { -// Replica { config: Cow<'a, ReplicaConfig> }, -// } -// -// #[derive(Serialize, Deserialize, PartialEq, Eq)] -// pub struct CachedConfig<'a> { -// pub replica_rev: String, -// #[serde(flatten)] -// pub config: CachedReplicaConfig<'a>, -// } -// -// impl<'a> CachedConfig<'a> { -// pub fn replica(config: &'a ReplicaConfig) -> Self { -// Self { -// replica_rev: replica_rev().into(), -// config: CachedReplicaConfig::Replica { -// config: Cow::Borrowed(config), -// }, -// } -// } -// } - pub fn apply_command_line_parameters( logger: &Logger, network_descriptor: NetworkDescriptor, @@ -505,6 +491,7 @@ pub fn apply_command_line_parameters( domain: Vec, use_old_metering: bool, artificial_delay: u32, + pocketic: bool, ) -> DfxResult { if enable_canister_http { warn!( @@ -542,7 +529,7 @@ pub fn apply_command_line_parameters( } let settings_digest = - get_settings_digest(replica_rev(), &local_server_descriptor, use_old_metering, artificial_delay); + get_settings_digest(replica_rev(), &local_server_descriptor, use_old_metering, artificial_delay, pocketic); local_server_descriptor = local_server_descriptor.with_settings_digest(settings_digest); From 85166f59965134e588db5dee088509ffd04c1c37 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 24 May 2024 11:14:00 -0700 Subject: [PATCH 4/9] checkpoint --- e2e/tests-dfx/start.bash | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/e2e/tests-dfx/start.bash b/e2e/tests-dfx/start.bash index 9b8fda48fe..d64f64c021 100644 --- a/e2e/tests-dfx/start.bash +++ b/e2e/tests-dfx/start.bash @@ -26,10 +26,9 @@ teardown() { define_project_network dfx_start --artificial-delay 101 - dfx_stop + dfx stop - dfx_start --artificial-delay 102 - dfx_stop + assert_command_fail dfx_start --artificial-delay 102 } @test "stop and start with other options does not disrupt projects" { From 507a8bef88b6da1553e774357633c74ac66d2b75 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 24 May 2024 11:20:31 -0700 Subject: [PATCH 5/9] disable some tests for pocketic --- e2e/tests-dfx/start.bash | 3 +++ 1 file changed, 3 insertions(+) diff --git a/e2e/tests-dfx/start.bash b/e2e/tests-dfx/start.bash index d64f64c021..f176e61b57 100644 --- a/e2e/tests-dfx/start.bash +++ b/e2e/tests-dfx/start.bash @@ -13,6 +13,7 @@ teardown() { } @test "start and stop with different options" { + [[ "$USE_POCKETIC" ]] && skip "skipped for pocketic: artificial delay, and clean required" dfx_start --artificial-delay 101 dfx_stop @@ -22,6 +23,7 @@ teardown() { } @test "project networks still need --clean" { + [[ "$USE_POCKETIC" ]] && skip "skipped for pocketic: artificial delay" dfx_new hello define_project_network @@ -32,6 +34,7 @@ teardown() { } @test "stop and start with other options does not disrupt projects" { + [[ "$USE_POCKETIC" ]] && skip "skipped for pocketic: artificial delay" dfx_start --artificial-delay 101 dfx_new p1 From 7993cd003a69c6494becd9d93777166dcd58baeb Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 24 May 2024 15:47:33 -0700 Subject: [PATCH 6/9] cleanup / refine --- e2e/tests-dfx/start.bash | 1 - .../config/model/local_server_descriptor.rs | 4 +- src/dfx-core/src/config/model/mod.rs | 2 +- src/dfx-core/src/fs/mod.rs | 1 - src/dfx-core/src/json/mod.rs | 1 - src/dfx/src/commands/start.rs | 88 +++++++++++-------- 6 files changed, 52 insertions(+), 45 deletions(-) diff --git a/e2e/tests-dfx/start.bash b/e2e/tests-dfx/start.bash index f176e61b57..53069fa858 100644 --- a/e2e/tests-dfx/start.bash +++ b/e2e/tests-dfx/start.bash @@ -531,7 +531,6 @@ teardown() { assert_command_fail dfx start --background assert_contains "The network state can't be reused with this configuration. Rerun with \`--clean\`." assert_command dfx start --force --background - dfx stop } @test "dfx start then ctrl-c won't hang and panic but stop actors quickly" { diff --git a/src/dfx-core/src/config/model/local_server_descriptor.rs b/src/dfx-core/src/config/model/local_server_descriptor.rs index 1f0c2c578c..9eb5cd6ad9 100644 --- a/src/dfx-core/src/config/model/local_server_descriptor.rs +++ b/src/dfx-core/src/config/model/local_server_descriptor.rs @@ -27,7 +27,6 @@ pub struct LocalServerDescriptor { /// The data directory is one of the following: /// /.dfx/network/local /// $HOME/Library/Application Support/org.dfinity.dfx/network/local - /// $HOME/Library/Application Support/org.dfinity.dfx/network/local /// $HOME/.local/share/dfx/network/local /// $APPDATA/dfx/network/local pub data_directory: PathBuf, @@ -198,8 +197,7 @@ impl LocalServerDescriptor { /// This file contains the effective config the replica was started with. pub fn effective_config_path(&self) -> PathBuf { - self.data_directory - .join("replica-effective-config.json") + self.data_directory.join("replica-effective-config.json") } pub fn effective_config_path_by_settings_digest(&self) -> PathBuf { diff --git a/src/dfx-core/src/config/model/mod.rs b/src/dfx-core/src/config/model/mod.rs index 2a6ec1ae6e..2014e4ae34 100644 --- a/src/dfx-core/src/config/model/mod.rs +++ b/src/dfx-core/src/config/model/mod.rs @@ -1,5 +1,4 @@ pub mod bitcoin_adapter; -pub mod settings_digest; pub mod canister_http_adapter; pub mod canister_id_store; pub mod dfinity; @@ -7,3 +6,4 @@ pub mod extension_canister_type; pub mod local_server_descriptor; pub mod network_descriptor; pub mod replica_config; +pub mod settings_digest; diff --git a/src/dfx-core/src/fs/mod.rs b/src/dfx-core/src/fs/mod.rs index fc75115110..022516e15e 100644 --- a/src/dfx-core/src/fs/mod.rs +++ b/src/dfx-core/src/fs/mod.rs @@ -16,7 +16,6 @@ pub fn canonicalize(path: &Path) -> Result { } pub fn copy(from: &Path, to: &Path) -> Result { - eprintln!("copy from: {} to: {}", from.display(), to.display()); std::fs::copy(from, to).map_err(|err| { FsError::new(CopyFileFailed( Box::new(from.to_path_buf()), diff --git a/src/dfx-core/src/json/mod.rs b/src/dfx-core/src/json/mod.rs index 0b9b66373d..1241a7a0cf 100644 --- a/src/dfx-core/src/json/mod.rs +++ b/src/dfx-core/src/json/mod.rs @@ -17,7 +17,6 @@ pub fn load_json_file serde::de::Deserialize<'a>>( } pub fn save_json_file(path: &Path, value: &T) -> Result<(), StructuredFileError> { - eprintln!("save json file to path: {:?}", path.display()); let content = serde_json::to_string_pretty(&value) .map_err(|err| SerializeJsonFileFailed(Box::new(path.to_path_buf()), err))?; crate::fs::write(path, content).map_err(WriteJsonFileFailed) diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index c8edeb484e..7a90cfb704 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -16,19 +16,21 @@ use crate::util::get_reusable_socket_addr; use actix::Recipient; use anyhow::{anyhow, bail, Context, Error}; use clap::{ArgAction, Parser}; -use dfx_core::config::model::settings_digest::get_settings_digest; -use dfx_core::config::model::local_server_descriptor::{ - LocalNetworkScopeDescriptor, LocalServerDescriptor, +use dfx_core::{ + config::model::{ + bitcoin_adapter, canister_http_adapter, + local_server_descriptor::{LocalNetworkScopeDescriptor, LocalServerDescriptor}, + network_descriptor::NetworkDescriptor, + replica_config::{CachedConfig, ReplicaConfig}, + settings_digest::get_settings_digest, + }, + fs, + json::{load_json_file, save_json_file}, + network::provider::{create_network_descriptor, LocalBindDetermination}, }; -use dfx_core::config::model::network_descriptor::NetworkDescriptor; -use dfx_core::config::model::replica_config::{CachedConfig, ReplicaConfig}; -use dfx_core::config::model::{bitcoin_adapter, canister_http_adapter}; -use dfx_core::json::{load_json_file, save_json_file}; -use dfx_core::network::provider::{create_network_descriptor, LocalBindDetermination}; use fn_error_context::context; use os_str_bytes::{OsStrBytes, OsStringBytes}; use slog::{info, warn, Logger}; -use std::fs; use std::io::Read; use std::net::SocketAddr; use std::path::{Path, PathBuf}; @@ -191,15 +193,6 @@ pub fn exec( let local_server_descriptor = network_descriptor.local_server_descriptor()?; - let subnet_type = local_server_descriptor - .replica - .subnet_type - .unwrap_or_default(); - let log_level = local_server_descriptor - .replica - .log_level - .unwrap_or_default(); - let pid_file_path = local_server_descriptor.dfx_pid_path(); check_previous_process_running(local_server_descriptor)?; @@ -212,17 +205,21 @@ pub fn exec( let (frontend_url, address_and_port) = frontend_address(local_server_descriptor, background)?; - dfx_core::fs::create_dir_all(&local_server_descriptor.data_dir_by_settings_digest())?; + fs::create_dir_all(&local_server_descriptor.data_dir_by_settings_digest())?; if !local_server_descriptor.network_id_path().exists() { write_network_id(local_server_descriptor)?; } if let LocalNetworkScopeDescriptor::Shared { network_id_path } = &local_server_descriptor.scope { - dfx_core::fs::copy(&local_server_descriptor.network_id_path(), network_id_path)?; - let effective_config_path_by_settings_digest = local_server_descriptor.effective_config_path_by_settings_digest(); + fs::copy(&local_server_descriptor.network_id_path(), network_id_path)?; + let effective_config_path_by_settings_digest = + local_server_descriptor.effective_config_path_by_settings_digest(); if effective_config_path_by_settings_digest.exists() { - dfx_core::fs::copy(&effective_config_path_by_settings_digest, &local_server_descriptor.effective_config_path())?; + fs::copy( + &effective_config_path_by_settings_digest, + &local_server_descriptor.effective_config_path(), + )?; } } @@ -248,8 +245,6 @@ pub fn exec( let previous_config_path = local_server_descriptor.effective_config_path(); - dfx_core::fs::create_dir_all(&local_server_descriptor.data_dir_by_settings_digest())?; - // dfx info replica-port will read these port files to find out which port to use, // so we need to make sure only one has a valid port in it. let replica_config_dir = local_server_descriptor.replica_configuration_dir(); @@ -306,6 +301,14 @@ pub fn exec( let canister_http_socket_path = canister_http_adapter_config .as_ref() .and_then(|cfg| cfg.get_socket_path()); + let subnet_type = local_server_descriptor + .replica + .subnet_type + .unwrap_or_default(); + let log_level = local_server_descriptor + .replica + .log_level + .unwrap_or_default(); let proxy_domains = local_server_descriptor.proxy.domain.clone().into_vec(); @@ -343,15 +346,19 @@ pub fn exec( CachedConfig::replica(&replica_config, replica_rev().into()) }; - let is_shared_network = matches!(&local_server_descriptor.scope, LocalNetworkScopeDescriptor::Shared { .. }); + let is_shared_network = matches!( + &local_server_descriptor.scope, + LocalNetworkScopeDescriptor::Shared { .. } + ); if is_shared_network { - save_json_file(&local_server_descriptor.effective_config_path_by_settings_digest(), &effective_config) - .context("Failed to write replica configuration")?; + save_json_file( + &local_server_descriptor.effective_config_path_by_settings_digest(), + &effective_config, + ) + .context("Failed to write replica configuration")?; } else if !clean && !force && previous_config_path.exists() { let previous_config = load_json_file(&previous_config_path) .context("Failed to read replica configuration. Rerun with `--clean`.")?; - eprintln!("Previous configuration: {:?}", previous_config); - eprintln!("Effective configuration: {:?}", effective_config); if !effective_config.can_share_state(&previous_config) { bail!( "The network state can't be reused with this configuration. Rerun with `--clean`." @@ -440,9 +447,10 @@ pub fn exec( } fn clean_older_state_dirs(local_server_descriptor: &LocalServerDescriptor) -> DfxResult { - let data_dir = &local_server_descriptor.data_directory; - let settings_digest = local_server_descriptor.settings_digest.as_ref().unwrap(); let directories_to_keep = 10; + let settings_digest = local_server_descriptor.settings_digest.as_ref().unwrap(); + + let data_dir = &local_server_descriptor.data_directory; if !data_dir.is_dir() { return Ok(()); } @@ -463,19 +471,18 @@ fn clean_older_state_dirs(local_server_descriptor: &LocalServerDescriptor) -> Df }) .unwrap_or(true) }); - // sort by modification time + + // keep the X most recent directories state_dirs.sort_by_key(|p| p.metadata().map(|m| m.modified().unwrap()).unwrap()); - // keep the last 'directories_to_keep' directories state_dirs = state_dirs .iter() .rev() .skip(directories_to_keep) .cloned() .collect(); - // remove the rest + for dir in state_dirs { - fs::remove_dir_all(&dir) - .with_context(|| format!("Failed to remove state directory {}", dir.display()))?; + fs::remove_dir_all(&dir)?; } Ok(()) } @@ -528,8 +535,13 @@ pub fn apply_command_line_parameters( local_server_descriptor = local_server_descriptor.with_proxy_domains(domain) } - let settings_digest = - get_settings_digest(replica_rev(), &local_server_descriptor, use_old_metering, artificial_delay, pocketic); + let settings_digest = get_settings_digest( + replica_rev(), + &local_server_descriptor, + use_old_metering, + artificial_delay, + pocketic, + ); local_server_descriptor = local_server_descriptor.with_settings_digest(settings_digest); From d211401aa9e732cd42184c90cd7b959cd71003cb Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 28 May 2024 12:53:52 -0700 Subject: [PATCH 7/9] refine --- src/dfx/src/commands/start.rs | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 7a90cfb704..cc96cfefb4 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -454,26 +454,22 @@ fn clean_older_state_dirs(local_server_descriptor: &LocalServerDescriptor) -> Df if !data_dir.is_dir() { return Ok(()); } - let mut state_dirs = fs::read_dir(data_dir) - .with_context(|| format!("Failed to read state directory {}", data_dir.display()))? + let mut state_dirs = fs::read_dir(data_dir)? .filter_map(|e| match e { - Ok(entry) if entry.path().is_dir() => Some(Ok(entry.path())), + Ok(entry) if is_candidate_state_dir(&entry.path(), settings_digest) => { + Some(Ok(entry.path())) + } Ok(_) => None, Err(e) => Some(Err(e)), }) .collect::, _>>()?; - state_dirs.retain(|p| { - p.file_name() - .map(|f| { - let filename: String = f.to_string_lossy().into(); - filename != *settings_digest - }) - .unwrap_or(true) - }); - // keep the X most recent directories - state_dirs.sort_by_key(|p| p.metadata().map(|m| m.modified().unwrap()).unwrap()); + state_dirs.sort_by_cached_key(|p| { + p.metadata() + .map(|m| m.modified().unwrap_or(SystemTime::UNIX_EPOCH)) + .unwrap_or(SystemTime::UNIX_EPOCH) + }); state_dirs = state_dirs .iter() .rev() @@ -487,6 +483,17 @@ fn clean_older_state_dirs(local_server_descriptor: &LocalServerDescriptor) -> Df Ok(()) } +fn is_candidate_state_dir(path: &Path, settings_digest: &str) -> bool { + path.is_dir() + && path + .file_name() + .map(|f| { + let filename: String = f.to_string_lossy().into(); + filename != *settings_digest + }) + .unwrap_or(true) +} + pub fn apply_command_line_parameters( logger: &Logger, network_descriptor: NetworkDescriptor, From 2c3db462b30aa7e911778bb9e95375801abcbb58 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 29 May 2024 10:49:09 -0700 Subject: [PATCH 8/9] cleanup, pocketic, changelog --- CHANGELOG.md | 43 +++++++++++++++++++++++++ e2e/tests-dfx/shared_local_network.bash | 10 ++++++ src/dfx/src/commands/start.rs | 24 +++----------- 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c245308b39..56dc27118e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,49 @@ # UNRELEASED +### feat: `dfx start` for the shared local network stores replica state files in unique directories by options + +The state files for different replica versions are often incompatible, +so `dfx start` requires the `--clean` argument in order to reset data when +using different replica versions or different replica options. + +For the local shared network, dfx now stores replica state files in different +directories, split up by replica version and options. + +As an example, you'll be able to do things like this going forward: +```bash +dfx +0.21.0 start +(cd project1 && dfx deploy && dfx canister call ...) +dfx stop + +dfx +0.22.0 start +# notice --clean is not required. +# even if --clean were passed, the canisters for project1 would be unaffected. +(cd project2 && dfx deploy) +# project1 won't be affected unless you call dfx in its directory +dfx stop + +dfx +0.21.0 start +# the canisters are still deployed +(cd project1 && dfx canister call ...) +``` + +Prior to this change, the second `dfx start` would have had to include `--clean`, +which would have reset the state of the shared local network, affecting all projects. + +This also means `dfx start` for the shared local network won't ever require you to pass `--clean`. + +`dfx start` will delete old replica state directories. At present, it retains the 10 most recently used. + +This doesn't apply to project-specific networks, and it doesn't apply with `--pocketic`. + +It doesn't apply to project-specific networks because the project's canister ids would +reset anyway on first access. If you run `dfx start` in a project directory where dfx.json +defines the local network, you'll still be prompted to run with `--clean` if using a +different replica version or different replica options. + +It doesn't apply to `--pocketic` because PocketIC does not yet persist any data. + # 0.20.2 ### fix: `dfx canister delete` fails diff --git a/e2e/tests-dfx/shared_local_network.bash b/e2e/tests-dfx/shared_local_network.bash index 52044fee2f..053d63be1d 100644 --- a/e2e/tests-dfx/shared_local_network.bash +++ b/e2e/tests-dfx/shared_local_network.bash @@ -12,6 +12,16 @@ teardown() { standard_teardown } +@test "shared local network always requires --clean with pocketic" { + [[ "$USE_POCKETIC" ]] || skip "specific to dfx start --pocketic" + + assert_command dfx_start + assert_command dfx stop + + assert_command_fail dfx start --pocketic + assert_contains "The network state can't be reused with this configuration. Rerun with \`--clean\`" +} + @test "dfx start creates no files in the current directory when run from an empty directory" { dfx_start assert_command find . diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index cc96cfefb4..ddb173a0e6 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -248,12 +248,7 @@ pub fn exec( // dfx info replica-port will read these port files to find out which port to use, // so we need to make sure only one has a valid port in it. let replica_config_dir = local_server_descriptor.replica_configuration_dir(); - fs::create_dir_all(&replica_config_dir).with_context(|| { - format!( - "Failed to create replica config directory {}.", - replica_config_dir.display() - ) - })?; + fs::create_dir_all(&replica_config_dir)?; let replica_port_path = empty_writable_path(local_server_descriptor.replica_port_path())?; let pocketic_port_path = empty_writable_path(local_server_descriptor.pocketic_port_path())?; @@ -275,14 +270,7 @@ pub fn exec( local_server_descriptor.describe(env.get_logger()); write_pid(&pid_file_path); - std::fs::write(&webserver_port_path, address_and_port.port().to_string()).with_context( - || { - format!( - "Failed to write webserver port file {}.", - webserver_port_path.to_string_lossy() - ) - }, - )?; + fs::write(&webserver_port_path, address_and_port.port().to_string())?; let btc_adapter_config = configure_btc_adapter_if_enabled( local_server_descriptor, @@ -350,12 +338,11 @@ pub fn exec( &local_server_descriptor.scope, LocalNetworkScopeDescriptor::Shared { .. } ); - if is_shared_network { + if is_shared_network && !pocketic { save_json_file( &local_server_descriptor.effective_config_path_by_settings_digest(), &effective_config, - ) - .context("Failed to write replica configuration")?; + )?; } else if !clean && !force && previous_config_path.exists() { let previous_config = load_json_file(&previous_config_path) .context("Failed to read replica configuration. Rerun with `--clean`.")?; @@ -365,8 +352,7 @@ pub fn exec( ) } } - save_json_file(&previous_config_path, &effective_config) - .context("Failed to write replica configuration")?; + save_json_file(&previous_config_path, &effective_config)?; let network_descriptor = network_descriptor.clone(); From 9ccf727ce814ac47587b1cd98758513112be7060 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 29 May 2024 11:41:58 -0700 Subject: [PATCH 9/9] revert branch build --- .github/workflows/e2e.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 743b1d19dc..738ae32f12 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -3,7 +3,6 @@ on: push: branches: - master - - ens/sdk-1638-less-dfx-start-clean pull_request: concurrency: