diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aa10c2d5c..1100c4d26b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -180,6 +180,12 @@ The Replica returned an error: code 1, message: "Canister requested a compute al ### fix: For `dfx start --clean --background`, the background process no longer cleans a second time. +### fix: do not build or generate remote canisters + +Canisters that specify a remote id for the network that's getting built falsely had their build steps run, blocking some normal use patterns of `dfx deploy`. +Canisters with a remote id specified no longer get built. +The same applies to `dfx generate`. + ### refactor: Move replica URL functions into a module for reuse The running replica port and url are generally useful information. Previously the code to get the URL was embedded in the network proxy code. This moves it out into a library for reuse. diff --git a/e2e/assets/remote/envvar/dfx.json b/e2e/assets/remote/envvar/dfx.json new file mode 100644 index 0000000000..beb81e522a --- /dev/null +++ b/e2e/assets/remote/envvar/dfx.json @@ -0,0 +1,21 @@ +{ + "canisters": { + "basic": { + "type": "custom", + "candid": "main.did", + "wasm": "main.wasm", + "build": "bash -c 'echo \"CANISTER_ID_remote: $CANISTER_ID_remote\"'", + "dependencies": [ + "remote" + ] + }, + "remote": { + "main": "remote.mo", + "remote": { + "id": { + "actuallylocal": "qoctq-giaaa-aaaaa-aaaea-cai" + } + } + } + } +} \ No newline at end of file diff --git a/e2e/tests-dfx/remote.bash b/e2e/tests-dfx/remote.bash index 0f7691c7fd..03be8cd197 100644 --- a/e2e/tests-dfx/remote.bash +++ b/e2e/tests-dfx/remote.bash @@ -137,7 +137,7 @@ teardown() { assert_match "Canister 'remote' is a remote canister on network 'actuallylocal', and cannot be installed from here." } -@test "canister create --all and canister install --all skip remote canisters" { +@test "canister create --all, canister install --all, dfx generate skip remote canisters" { install_asset remote/actual dfx_start setup_actuallylocal_shared_network @@ -174,8 +174,12 @@ teardown() { assert_eq '("mock")' assert_command dfx canister create --all --network actuallylocal - assert_command dfx build --network actuallylocal + assert_command dfx build --network actuallylocal -vv + assert_match "Not building canister 'remote'" assert_command dfx canister install --all --network actuallylocal + assert_command dfx generate --network actuallylocal + assert_match "Generating type declarations for canister basic" + assert_not_match "Generating type declarations for canister remote" assert_command dfx canister call basic read_remote --network actuallylocal assert_eq '("this is data in the remote canister")' @@ -273,7 +277,8 @@ teardown() { assert_command dfx canister call remote which_am_i assert_eq '("mock")' - assert_command dfx deploy --network actuallylocal + assert_command dfx deploy --network actuallylocal -vv + assert_match "Not building canister 'remote'" assert_command dfx canister call basic read_remote --network actuallylocal assert_eq '("this is data in the remote canister")' @@ -297,3 +302,13 @@ teardown() { assert_command jq .remote canister_ids.json assert_eq "null" } + +@test "build step sets remote cid environment variable correctly" { + install_asset remote/envvar + install_asset wasm/identity # need to have some .did and .wasm files to point our custom canister to + dfx_start + setup_actuallylocal_shared_network + + assert_command dfx deploy --network actuallylocal -vv + assert_match "CANISTER_ID_remote: qoctq-giaaa-aaaaa-aaaea-cai" +} diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index 4ad81e8f36..5e12ef2f62 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -43,17 +43,26 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { let build_mode_check = opts.check; // Option can be None in which case --all was specified - let canister_names = config + let canisters_to_load = config .get_config() .get_canister_names_with_dependencies(opts.canister_name.as_deref())?; - - // Get pool of canisters to build - let canister_pool = CanisterPool::load(&env, build_mode_check, &canister_names)?; + let canisters_to_build = canisters_to_load + .clone() + .into_iter() + .filter(|canister_name| { + !config + .get_config() + .is_remote_canister(canister_name, &env.get_network_descriptor().name) + .unwrap_or(false) + }) + .collect(); + + let canister_pool = CanisterPool::load(&env, build_mode_check, &canisters_to_load)?; // Create canisters on the replica and associate canister ids locally. if build_mode_check { slog::warn!( - env.get_logger(), + logger, "Building canisters to check they build ok. Canister IDs might be hard coded." ); } else { @@ -69,8 +78,10 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { slog::info!(logger, "Building canisters..."); let runtime = Runtime::new().expect("Unable to create a runtime"); - let build_config = BuildConfig::from_config(&config)?.with_build_mode_check(build_mode_check); - runtime.block_on(canister_pool.build_or_fail(&build_config))?; + let build_config = BuildConfig::from_config(&config)? + .with_build_mode_check(build_mode_check) + .with_canisters_to_build(canisters_to_build); + runtime.block_on(canister_pool.build_or_fail(logger, &build_config))?; Ok(()) } diff --git a/src/dfx/src/commands/generate.rs b/src/dfx/src/commands/generate.rs index 942a346620..891643ddcd 100644 --- a/src/dfx/src/commands/generate.rs +++ b/src/dfx/src/commands/generate.rs @@ -7,6 +7,7 @@ use crate::lib::provider::create_agent_environment; use crate::NetworkOpt; use clap::Parser; +use slog::trace; use tokio::runtime::Runtime; /// Generate type declarations for canisters from the code in your project @@ -22,6 +23,7 @@ pub struct GenerateOpts { pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult { let env = create_agent_environment(env, opts.network.network)?; + let log = env.get_logger(); // Read the config. let config = env.get_config_or_anyhow()?; @@ -31,40 +33,60 @@ pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult { env.get_cache().install()?; // Option can be None which means generate types for all canisters - let canister_names = config + let canisters_to_load = config .get_config() .get_canister_names_with_dependencies(opts.canister_name.as_deref())?; + let canisters_to_generate = canisters_to_load + .clone() + .into_iter() + .filter(|canister_name| { + !config + .get_config() + .is_remote_canister(canister_name, &env.get_network_descriptor().name) + .unwrap_or(false) + }) + .collect(); - // Get pool of canisters to build - let canister_pool = CanisterPool::load(&env, false, &canister_names)?; + let canister_pool = CanisterPool::load(&env, false, &canisters_to_load)?; - // This is just to display an error if trying to generate before creating the canister. + // This is just to display an error if trying to generate before creating the canister(s). let store = CanisterIdStore::for_env(&env)?; + // If generate for motoko canister, build first - let mut build_before_generate = false; + let mut build_before_generate = Vec::new(); for canister in canister_pool.get_canister_list() { let canister_name = canister.get_name(); let canister_id = store.get(canister_name)?; if let Some(info) = canister_pool.get_canister_info(&canister_id) { if info.is_motoko() { - build_before_generate = true; + build_before_generate.push(canister_name.to_string()); + trace!( + log, + "Found Motoko canister '{}' - will have to build before generating IDL.", + canister_name + ); } } } - let build_config = BuildConfig::from_config(&config)?; + let build_config = + BuildConfig::from_config(&config)?.with_canisters_to_build(build_before_generate); + let generate_config = + BuildConfig::from_config(&config)?.with_canisters_to_build(canisters_to_generate); - if build_before_generate { - slog::info!( - env.get_logger(), - "Building canisters before generate for Motoko" - ); + if build_config + .canisters_to_build + .as_ref() + .map(|v| !v.is_empty()) + .unwrap_or(false) + { + 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_or_fail(&build_config))?; + runtime.block_on(canister_pool.build_or_fail(log, &build_config))?; } - for canister in canister_pool.get_canister_list() { - canister.generate(&canister_pool, &build_config)?; + for canister in canister_pool.canisters_to_build(&generate_config) { + canister.generate(&canister_pool, &generate_config)?; } Ok(()) diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index bba486ff4f..f955d28c9b 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -365,6 +365,9 @@ pub struct BuildConfig { pub lsp_root: PathBuf, /// The root for all build files. pub build_root: PathBuf, + /// If only a subset of canisters should be built, then canisters_to_build contains these canisters' names. + /// If all canisters should be built, then this is None. + pub canisters_to_build: Option>, } impl BuildConfig { @@ -382,6 +385,7 @@ impl BuildConfig { build_root: canister_root.clone(), idl_root: canister_root.join("idl/"), // TODO: possibly move to `network_root.join("idl/")` lsp_root: network_root.join("lsp/"), + canisters_to_build: None, }) } @@ -391,6 +395,13 @@ impl BuildConfig { ..self } } + + pub fn with_canisters_to_build(self, canisters: Vec) -> Self { + Self { + canisters_to_build: Some(canisters), + ..self + } + } } #[context("Failed to shrink wasm at {}.", &wasm_path.as_ref().display())] diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 80a1f9bfb5..946a9382a2 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -13,6 +13,7 @@ use crate::lib::wasm::metadata::add_candid_service_metadata; use anyhow::{anyhow, bail, Context}; use candid::Principal as CanisterId; use fn_error_context::context; +use itertools::Itertools; use petgraph::graph::{DiGraph, NodeIndex}; use rand::{thread_rng, RngCore}; use slog::{error, info, trace, warn, Logger}; @@ -238,8 +239,43 @@ impl CanisterPool { } #[context("Failed step_prebuild_all.")] - fn step_prebuild_all(&self, _build_config: &BuildConfig) -> DfxResult<()> { - if self.canisters.iter().any(|can| can.info.is_rust()) { + fn step_prebuild_all(&self, log: &Logger, build_config: &BuildConfig) -> DfxResult<()> { + // moc expects all .did files of dependencies to be in with name .did. + // Because remote canisters don't get built (and the did file not output in the right place) the .did files have to be copied over manually. + for canister in self.canisters.iter().filter(|c| c.info.is_remote()) { + let maybe_from = if let Some(remote_candid) = canister.info.get_remote_candid() { + Some(remote_candid) + } else { + canister.info.get_output_idl_path() + }; + if let Some(from) = maybe_from.as_ref() { + if from.exists() { + let to = build_config.idl_root.join(format!( + "{}.did", + canister.info.get_canister_id()?.to_text() + )); + if std::fs::copy(&from, &to).is_err() { + warn!( + log, + "Failed to copy canister candid from {} to {}. This may produce errors during the build.", + from.to_string_lossy(), + to.to_string_lossy() + ); + } + } else { + warn!(log, ".did file for canister '{}' does not exist at {}. This may result in errors during the build.", canister.get_name(), from.to_string_lossy()); + } + } else { + warn!(log, "Failed to find a configured .did file for canister '{}'. Not specifying that field may result in errors during the build.", canister.get_name()); + } + } + + // cargo audit + if self + .canisters_to_build(build_config) + .iter() + .any(|can| can.info.is_rust()) + { self.run_cargo_audit()?; } else { trace!( @@ -247,6 +283,7 @@ impl CanisterPool { "No canister of type 'rust' found. Not trying to run 'cargo audit'." ) } + Ok(()) } @@ -368,7 +405,7 @@ impl CanisterPool { ) -> DfxResult<()> { // We don't want to simply remove the whole directory, as in the future, // we may want to keep the IDL files downloaded from network. - for canister in &self.canisters { + for canister in self.canisters_to_build(build_config) { let idl_root = &build_config.idl_root; let canister_id = canister.canister_id(); let idl_file_path = idl_root.join(canister_id.to_text()).with_extension("did"); @@ -384,9 +421,10 @@ impl CanisterPool { #[context("Failed while trying to build all canisters in the canister pool.")] pub fn build( &self, + log: &Logger, build_config: &BuildConfig, ) -> DfxResult>> { - self.step_prebuild_all(build_config) + self.step_prebuild_all(log, build_config) .map_err(|e| DfxError::new(BuildError::PreBuildAllStepFailed(Box::new(e))))?; let graph = self.build_dependencies_graph()?; @@ -406,9 +444,20 @@ impl CanisterPool { .map(|idx| *graph.node_weight(*idx).unwrap()) .collect(); + let canisters_to_build = self.canisters_to_build(build_config); let mut result = Vec::new(); for canister_id in &order { if let Some(canister) = self.get_canister(canister_id) { + if canisters_to_build + .iter() + .map(|c| c.get_name()) + .contains(&canister.get_name()) + { + trace!(log, "Building canister '{}'.", canister.get_name()); + } else { + trace!(log, "Not building canister '{}'.", canister.get_name()); + continue; + } result.push( self.step_prebuild(build_config, canister) .map_err(|e| { @@ -451,9 +500,9 @@ 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, build_config: &BuildConfig) -> DfxResult<()> { + pub async fn build_or_fail(&self, log: &Logger, build_config: &BuildConfig) -> DfxResult<()> { self.download(build_config).await?; - let outputs = self.build(build_config)?; + let outputs = self.build(log, build_config)?; for output in outputs { output.map_err(DfxError::new)?; @@ -462,8 +511,8 @@ impl CanisterPool { Ok(()) } - async fn download(&self, _build_config: &BuildConfig) -> DfxResult { - for canister in &self.canisters { + async fn download(&self, build_config: &BuildConfig) -> DfxResult { + for canister in self.canisters_to_build(build_config) { let info = canister.get_info(); if info.is_custom() { @@ -525,6 +574,17 @@ impl CanisterPool { } Ok(()) } + + pub fn canisters_to_build(&self, build_config: &BuildConfig) -> Vec<&Arc> { + if let Some(canister_names) = &build_config.canisters_to_build { + self.canisters + .iter() + .filter(|can| canister_names.contains(&can.info.get_name().to_string())) + .collect() + } else { + self.canisters.iter().collect() + } + } } #[context("Failed to decode path to str.")] diff --git a/src/dfx/src/lib/operations/canister/deploy_canisters.rs b/src/dfx/src/lib/operations/canister/deploy_canisters.rs index aa175a14be..d8337860dc 100644 --- a/src/dfx/src/lib/operations/canister/deploy_canisters.rs +++ b/src/dfx/src/lib/operations/canister/deploy_canisters.rs @@ -43,7 +43,7 @@ pub async fn deploy_canisters( let network = env.get_network_descriptor(); - let canisters_to_build = canister_with_dependencies(&config, some_canister)?; + let canisters_to_load = canister_with_dependencies(&config, some_canister)?; let canisters_to_deploy = if force_reinstall { // don't force-reinstall the dependencies too. @@ -58,19 +58,17 @@ pub async fn deploy_canisters( None => bail!("The --mode=reinstall is only valid when deploying a single canister, because reinstallation destroys all data in the canister."), } } else { - canisters_to_build.clone() - }; - let canisters_to_deploy: Vec = canisters_to_deploy - .into_iter() - .filter(|canister_name| { - !matches!( - config + canisters_to_load + .clone() + .into_iter() + .filter(|canister_name| { + !config .get_config() - .get_remote_canister_id(canister_name, &network.name), - Ok(Some(_)) - ) - }) - .collect(); + .is_remote_canister(canister_name, &env.get_network_descriptor().name) + .unwrap_or(false) + }) + .collect() + }; if some_canister.is_some() { info!(log, "Deploying: {}", canisters_to_deploy.join(" ")); @@ -80,7 +78,7 @@ pub async fn deploy_canisters( register_canisters( env, - &canisters_to_build, + &canisters_to_deploy, &initial_canister_id_store, timeout, with_cycles, @@ -89,7 +87,7 @@ pub async fn deploy_canisters( ) .await?; - let pool = build_canisters(env, &canisters_to_build, &config).await?; + let pool = build_canisters(env, &canisters_to_load, &canisters_to_deploy, &config).await?; install_canisters( env, @@ -124,6 +122,7 @@ fn canister_with_dependencies( Ok(canister_names) } +/// Creates canisters that have not been created yet. #[context("Failed while trying to register all canisters.")] async fn register_canisters( env: &dyn Environment, @@ -193,16 +192,18 @@ async fn register_canisters( #[context("Failed to build call canisters.")] async fn build_canisters( env: &dyn Environment, - canister_names: &[String], + referenced_canisters: &[String], + canisters_to_build: &[String], config: &Config, ) -> DfxResult { - info!(env.get_logger(), "Building canisters..."); + let log = env.get_logger(); + info!(log, "Building canisters..."); let build_mode_check = false; - let canister_pool = CanisterPool::load(env, build_mode_check, canister_names)?; + let canister_pool = CanisterPool::load(env, build_mode_check, referenced_canisters)?; - canister_pool - .build_or_fail(&BuildConfig::from_config(config)?) - .await?; + let build_config = + BuildConfig::from_config(config)?.with_canisters_to_build(canisters_to_build.into()); + canister_pool.build_or_fail(log, &build_config).await?; Ok(canister_pool) }