diff --git a/CHANGELOG.md b/CHANGELOG.md index c245308b39..9484ad25ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ # UNRELEASED +### feat: rebuild only necessary canisters + +Cache `get_imports()` (renamed to `add_imports()`) results. + +Read only those `--print-deps` dependencies that are necessary to read. + +Don't compile canisters for which all dependencies are elder than the `.wasm` file. +This results in big compilation speedups. + +### feat: specify canisters not to deploy + +`"deploy": false` canister option makes it not to deploy, unless explicitly specified on the command line. + +### chore: Improve help text of `dfx identity new` to include which characters are valid in identity names + # 0.20.2 ### fix: `dfx canister delete` fails diff --git a/docs/cli-reference/dfx-build.mdx b/docs/cli-reference/dfx-build.mdx index 0e58b8f43d..90a660e0ec 100644 --- a/docs/cli-reference/dfx-build.mdx +++ b/docs/cli-reference/dfx-build.mdx @@ -10,6 +10,8 @@ Note that you can only run this command from within the project directory struct The `dfx build` command looks for the source code to compile using the information you have configured under the `canisters` section in the `dfx.json` configuration file. +For compilation speed reasons, `dfx build` (and `dfx deploy`) don't recompile canisters, all dependencies of which are elder than the existing Candid file (from the previous compilation). Moreover, the Candid (`.did`) file is updated only when strictly necessary (that is on public interface change). This makes dependent canisters not recompile when a dependency canister changes without interface change. + ## Basic usage ``` bash diff --git a/docs/cli-reference/dfx-deploy.mdx b/docs/cli-reference/dfx-deploy.mdx index 72573176c5..6853c57785 100644 --- a/docs/cli-reference/dfx-deploy.mdx +++ b/docs/cli-reference/dfx-deploy.mdx @@ -4,7 +4,7 @@ import { MarkdownChipRow } from "/src/components/Chip/MarkdownChipRow"; -Use the `dfx deploy` command to register, build, and deploy a dapp on the local canister execution environment, on the IC or on a specified testnet. By default, all canisters defined in the project `dfx.json` configuration file are deployed. +Use the `dfx deploy` command to register, build, and deploy a dapp on the local canister execution environment, on the IC or on a specified testnet. By default, all canisters defined in the project `dfx.json` configuration file are deployed, except of the canisters with `"deploy": false` option. This command simplifies the developer workflow by enabling you to run one command instead of running the following commands as separate steps: diff --git a/docs/dfx-json-schema.json b/docs/dfx-json-schema.json index 1cbf3f3470..73ef4d28fa 100644 --- a/docs/dfx-json-schema.json +++ b/docs/dfx-json-schema.json @@ -369,6 +369,12 @@ "type": "string" } }, + "deploy": { + "title": "Deploy", + "description": "`false` value means not to deploy this canister unless it's explicitly specified in the command line (supposed use: canister that are created by other canisters).", + "default": true, + "type": "boolean" + }, "frontend": { "title": "Force Frontend URL", "description": "Mostly unused. If this value is not null, a frontend URL is displayed after deployment even if the canister type is not 'asset'.", diff --git a/e2e/assets/add_dependency/a.mo b/e2e/assets/add_dependency/a.mo new file mode 100644 index 0000000000..8599f4e34a --- /dev/null +++ b/e2e/assets/add_dependency/a.mo @@ -0,0 +1,5 @@ +import _ "canister:b" + +actor { + public shared func f() {}; +} \ No newline at end of file diff --git a/e2e/assets/add_dependency/b.mo b/e2e/assets/add_dependency/b.mo new file mode 100644 index 0000000000..2f669ccd60 --- /dev/null +++ b/e2e/assets/add_dependency/b.mo @@ -0,0 +1,3 @@ +actor { + public shared func g() {}; +} \ No newline at end of file diff --git a/e2e/assets/add_dependency/dfx.json b/e2e/assets/add_dependency/dfx.json new file mode 100644 index 0000000000..2581ea52a3 --- /dev/null +++ b/e2e/assets/add_dependency/dfx.json @@ -0,0 +1,14 @@ +{ + "canisters": { + "a": { + "main": "a.mo", + "dependencies": ["c"] + }, + "b": { + "main": "b.mo" + }, + "c": { + "main": "b.mo" + } + } +} \ No newline at end of file diff --git a/e2e/assets/add_dependency/dfx_corrected.json b/e2e/assets/add_dependency/dfx_corrected.json new file mode 100644 index 0000000000..f6171e9eba --- /dev/null +++ b/e2e/assets/add_dependency/dfx_corrected.json @@ -0,0 +1,11 @@ +{ + "canisters": { + "a": { + "main": "a.mo", + "dependencies": ["b"] + }, + "b": { + "main": "b.mo" + } + } +} \ No newline at end of file diff --git a/e2e/assets/broken_canister_dep/a.mo b/e2e/assets/broken_canister_dep/a.mo new file mode 100644 index 0000000000..8599f4e34a --- /dev/null +++ b/e2e/assets/broken_canister_dep/a.mo @@ -0,0 +1,5 @@ +import _ "canister:b" + +actor { + public shared func f() {}; +} \ No newline at end of file diff --git a/e2e/assets/broken_canister_dep/dfx.json b/e2e/assets/broken_canister_dep/dfx.json new file mode 100644 index 0000000000..5d0b42eaa6 --- /dev/null +++ b/e2e/assets/broken_canister_dep/dfx.json @@ -0,0 +1,8 @@ +{ + "canisters": { + "a": { + "type": "motoko", + "main": "a.mo" + } + } +} \ No newline at end of file diff --git a/e2e/assets/make_like/README.txt b/e2e/assets/make_like/README.txt new file mode 100644 index 0000000000..014d9afb98 --- /dev/null +++ b/e2e/assets/make_like/README.txt @@ -0,0 +1 @@ +Sources are put into src/ to check how it behaves with subdirectories. \ No newline at end of file diff --git a/e2e/assets/make_like/dfx.json b/e2e/assets/make_like/dfx.json new file mode 100644 index 0000000000..564f3de8c2 --- /dev/null +++ b/e2e/assets/make_like/dfx.json @@ -0,0 +1,19 @@ +{ + "version": 1, + "canisters": { + "dependency": { + "main": "src/dependency.mo" + }, + "dependent": { + "main": "src/dependent.mo", + "dependencies": [ + "dependency" + ] + } + }, + "networks": { + "local": { + "bind": "127.0.0.1:8000" + } + } +} \ No newline at end of file diff --git a/e2e/assets/make_like/src/dependency.mo b/e2e/assets/make_like/src/dependency.mo new file mode 100644 index 0000000000..8be304ae1d --- /dev/null +++ b/e2e/assets/make_like/src/dependency.mo @@ -0,0 +1,7 @@ +import L "lib"; + +actor { + public shared func greet(name: Text) : async Text { + return "Hello, " # name # "!"; + } +} \ No newline at end of file diff --git a/e2e/assets/make_like/src/dependency_altered.mo b/e2e/assets/make_like/src/dependency_altered.mo new file mode 100644 index 0000000000..11d3ed6f57 --- /dev/null +++ b/e2e/assets/make_like/src/dependency_altered.mo @@ -0,0 +1,10 @@ +import L "lib"; + +actor { + public shared func greet(name: Text) : async Text { + return "Hello, " # name # "!"; + }; + + public shared func anotherFunction() : async () { + }; +} \ No newline at end of file diff --git a/e2e/assets/make_like/src/dependent.mo b/e2e/assets/make_like/src/dependent.mo new file mode 100644 index 0000000000..1aab542d75 --- /dev/null +++ b/e2e/assets/make_like/src/dependent.mo @@ -0,0 +1,8 @@ +import L "lib"; +import D "canister:dependency"; + +actor { + public shared func greet(name : Text) : async Text { + return "Hello, " # name # "!"; + }; +}; diff --git a/e2e/assets/make_like/src/lib.mo b/e2e/assets/make_like/src/lib.mo new file mode 100644 index 0000000000..b1facbe3eb --- /dev/null +++ b/e2e/assets/make_like/src/lib.mo @@ -0,0 +1 @@ +module {} \ No newline at end of file diff --git a/e2e/assets/wrong_ids/dfx.json b/e2e/assets/wrong_ids/dfx.json new file mode 100644 index 0000000000..14a8fbbc4f --- /dev/null +++ b/e2e/assets/wrong_ids/dfx.json @@ -0,0 +1,8 @@ +{ + "canisters": { + "pst": { + "main": "src/a.mo", + "type": "motoko" + } + } +} \ No newline at end of file diff --git a/e2e/assets/wrong_ids/src/a.mo b/e2e/assets/wrong_ids/src/a.mo new file mode 100644 index 0000000000..19c6fd745c --- /dev/null +++ b/e2e/assets/wrong_ids/src/a.mo @@ -0,0 +1 @@ +actor A {}; diff --git a/e2e/tests-dfx/add_dependency.bash b/e2e/tests-dfx/add_dependency.bash new file mode 100644 index 0000000000..1e17e49230 --- /dev/null +++ b/e2e/tests-dfx/add_dependency.bash @@ -0,0 +1,28 @@ +#!/usr/bin/env bats + +load ../utils/_ + +setup() { + standard_setup + + install_asset add_dependency +} + +teardown() { + dfx_stop + + standard_teardown +} + +# Check that attempt to compile before correcting dependencies does not break further compilation. +@test "compiles after correcting a dependency" { + install_asset base + + dfx_start + + assert_command_fail dfx deploy + + cp dfx_corrected.json dfx.json + + assert_command dfx deploy +} diff --git a/e2e/tests-dfx/broken_canister_dep.bash b/e2e/tests-dfx/broken_canister_dep.bash new file mode 100644 index 0000000000..c1c7ee6689 --- /dev/null +++ b/e2e/tests-dfx/broken_canister_dep.bash @@ -0,0 +1,23 @@ +#!/usr/bin/env bats + +load ../utils/_ + +setup() { + standard_setup + + install_asset broken_canister_dep +} + +teardown() { + dfx_stop + + standard_teardown +} + +@test "crash on a broken canister dependency" { + dfx_start + + assert_command_fail dfx deploy + + assert_not_contains "panicked at" +} \ No newline at end of file diff --git a/e2e/tests-dfx/make_like.bash b/e2e/tests-dfx/make_like.bash new file mode 100644 index 0000000000..dee421173a --- /dev/null +++ b/e2e/tests-dfx/make_like.bash @@ -0,0 +1,170 @@ +#!/usr/bin/env bats + +load ../utils/_ +# load ../utils/cycles-ledger + +setup() { + standard_setup + + install_asset make_like +} + +teardown() { + dfx_stop + + standard_teardown +} + +@test "trying to break dependency compiling: deploy" { + dfx_start + + assert_command dfx deploy -vv dependent + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_contains 'Installing code for canister dependent' + assert_contains 'Installing code for canister dependency' + + touch src/dependent.mo + assert_command dfx deploy -vv dependent + assert_contains '/src/dependent.mo"' + assert_not_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + touch src/dependency.mo + assert_command dfx deploy -vv dependent + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + touch src/dependency.mo + assert_command dfx deploy -vv dependency + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_not_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + assert_command dfx deploy -vv dependent + assert_not_contains '/src/dependent.mo"' + assert_not_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + touch src/lib.mo + assert_command dfx deploy -vv dependent + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + touch src/lib.mo + assert_command dfx deploy -vv dependency + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_not_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + touch src/lib.mo + assert_command dfx deploy -vv + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + touch src/dependency.mo + assert_command dfx deploy -vv + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + touch src/dependent.mo + assert_command dfx deploy -vv + assert_contains '/src/dependent.mo"' + assert_not_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' + + cp src/dependency_altered.mo src/dependency.mo + assert_command dfx deploy -vv + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + assert_contains 'Upgrading code for canister dependent' + assert_contains 'Upgrading code for canister dependency' +} + +@test "trying to break dependency compiling: build" { + dfx_start + + assert_command dfx canister create dependency + assert_command dfx canister create dependent + assert_command dfx build -vv dependent + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + touch src/dependent.mo + assert_command dfx build -vv dependent + assert_contains '/src/dependent.mo"' + assert_not_contains "/src/dependency.mo" + + touch src/dependency.mo + assert_command dfx build -vv dependent + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + touch src/dependency.mo + assert_command dfx build -vv dependency + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + assert_command dfx build -vv dependent + assert_not_contains '/src/dependent.mo"' + assert_not_contains "/src/dependency.mo" + + touch src/lib.mo + assert_command dfx build -vv dependent + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + touch src/lib.mo + assert_command dfx build -vv dependency + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + touch src/lib.mo + assert_command dfx build -vv + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + touch src/dependency.mo + assert_command dfx build -vv + assert_not_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + touch src/dependent.mo + assert_command dfx build -vv + assert_contains '/src/dependent.mo"' + assert_not_contains "/src/dependency.mo" + + cp src/dependency_altered.mo src/dependency.mo + assert_command dfx build -vv + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" +} + +@test "mix build and deploy" { + dfx_start + + assert_command dfx canister create dependency + assert_command dfx canister create dependent + assert_command dfx build -vv + assert_contains '/src/dependent.mo"' + assert_contains "/src/dependency.mo" + + assert_command dfx deploy -vv dependent + assert_not_contains '/src/dependent.mo"' + assert_not_contains "/src/dependency.mo" + assert_contains 'Installing code for canister dependent' + assert_contains 'Installing code for canister dependency' +} diff --git a/e2e/tests-dfx/wrong_ids.bash b/e2e/tests-dfx/wrong_ids.bash new file mode 100644 index 0000000000..f61e33bccd --- /dev/null +++ b/e2e/tests-dfx/wrong_ids.bash @@ -0,0 +1,23 @@ +#!/usr/bin/env bats + +load ../utils/_ + +setup() { + standard_setup + + install_asset wrong_ids +} + +teardown() { + dfx_stop + + standard_teardown +} + +@test "deploy causes wrong ids" { + dfx_start + + assert_command dfx deploy -vv pst + assert_command ls .dfx/local/lsp + assert_contains '-cai.did' +} \ No newline at end of file diff --git a/src/dfx-core/src/config/cache.rs b/src/dfx-core/src/config/cache.rs index b365524935..e75048ea66 100644 --- a/src/dfx-core/src/config/cache.rs +++ b/src/dfx-core/src/config/cache.rs @@ -4,6 +4,7 @@ use crate::error::cache::CacheError; #[cfg(not(windows))] use crate::foundation::get_user_home; use semver::Version; +use std::fmt::Debug; use std::path::PathBuf; pub trait Cache { @@ -14,6 +15,12 @@ pub trait Cache { fn get_binary_command(&self, binary_name: &str) -> Result; } +impl Debug for dyn Cache { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "[cache version {}]", self.version_str()) + } +} + 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-core/src/config/model/dfinity.rs b/src/dfx-core/src/config/model/dfinity.rs index 199250148f..10e9ab691a 100644 --- a/src/dfx-core/src/config/model/dfinity.rs +++ b/src/dfx-core/src/config/model/dfinity.rs @@ -257,6 +257,11 @@ pub struct ConfigCanistersCanister { #[serde(default)] pub dependencies: Vec, + /// # Deploy + /// `false` value means not to deploy this canister unless it's explicitly specified in the command line (supposed use: canister that are created by other canisters). + #[serde(default = "default_true")] + pub deploy: bool, + /// # Force Frontend URL /// Mostly unused. /// If this value is not null, a frontend URL is displayed after deployment even if the canister type is not 'asset'. @@ -327,6 +332,10 @@ pub struct ConfigCanistersCanister { pub init_arg_file: Option, } +fn default_true() -> bool { + true +} + #[derive(Clone, Debug, Serialize, JsonSchema)] #[serde(tag = "type", rename_all = "snake_case")] pub enum CanisterTypeProperties { @@ -965,7 +974,7 @@ impl ConfigInterface { .wasm_memory_limit) } - fn get_canister_config( + pub fn get_canister_config( &self, canister_name: &str, ) -> Result<&ConfigCanistersCanister, GetCanisterConfigError> { diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index 2957c453d3..5d7529b145 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -33,8 +33,8 @@ pub struct CanisterBuildOpts { network: NetworkOpt, } -pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { - let env = create_agent_environment(env, opts.network.to_network_name())?; +pub fn exec(env1: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { + let env = create_agent_environment(env1, opts.network.to_network_name())?; let logger = env.get_logger(); @@ -54,16 +54,6 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { .get_canister_names_with_dependencies(opts.canister_name.as_deref())?; let canisters_to_load = add_canisters_with_ids(&required_canisters, &env, &config); - let canisters_to_build = required_canisters - .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. @@ -78,7 +68,13 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { let store = env.get_canister_id_store()?; for canister in canister_pool.get_canister_list() { let canister_name = canister.get_name(); - store.get(canister_name)?; + if config + .get_config() + .get_canister_config(canister_name)? + .deploy + { + store.get(canister_name)?; + } } } @@ -88,9 +84,23 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { let build_config = BuildConfig::from_config(&config, env.get_network_descriptor().is_playground())? .with_build_mode_check(build_mode_check) - .with_canisters_to_build(canisters_to_build) + .with_canisters_to_build(if let Some(canister) = opts.canister_name { + vec![canister] // hacky + } else { + config + .get_config() + .get_canister_names_with_dependencies(None)? + .into_iter() + .filter(|canister_name| { + !config + .get_config() + .is_remote_canister(canister_name, &env.get_network_descriptor().name) + .unwrap_or(false) + }) + .collect::>() + }) .with_env_file(env_file); - runtime.block_on(canister_pool.build_or_fail(logger, &build_config))?; + runtime.block_on(canister_pool.build_or_fail(env1, logger, &build_config))?; Ok(()) } diff --git a/src/dfx/src/commands/generate.rs b/src/dfx/src/commands/generate.rs index 17992ef765..438c31d373 100644 --- a/src/dfx/src/commands/generate.rs +++ b/src/dfx/src/commands/generate.rs @@ -20,8 +20,8 @@ pub struct GenerateOpts { network: Option, } -pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult { - let env = create_anonymous_agent_environment(env, None)?; +pub fn exec(env1: &dyn Environment, opts: GenerateOpts) -> DfxResult { + let env = create_anonymous_agent_environment(env1, None)?; let log = env.get_logger(); // Read the config. @@ -66,7 +66,7 @@ pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult { .with_canisters_to_build(canisters_to_generate); if build_config - .canisters_to_build + .user_specified_canisters .as_ref() .map(|v| !v.is_empty()) .unwrap_or(false) @@ -74,7 +74,7 @@ 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(env1, log, &build_config))?; } for canister in canister_pool_load.canisters_to_build(&generate_config) { diff --git a/src/dfx/src/lib/builders/assets.rs b/src/dfx/src/lib/builders/assets.rs index 5860cbb8a0..90910b1396 100644 --- a/src/dfx/src/lib/builders/assets.rs +++ b/src/dfx/src/lib/builders/assets.rs @@ -77,6 +77,7 @@ impl CanisterBuilder for AssetsBuilder { #[context("Failed to get dependencies for canister '{}'.", info.get_name())] fn get_dependencies( &self, + _env: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, ) -> DfxResult> { @@ -86,6 +87,7 @@ impl CanisterBuilder for AssetsBuilder { #[context("Failed to build asset canister '{}'.", info.get_name())] fn build( &self, + _env: &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 5abbc5f2ac..e7399d0eed 100644 --- a/src/dfx/src/lib/builders/custom.rs +++ b/src/dfx/src/lib/builders/custom.rs @@ -88,6 +88,7 @@ impl CanisterBuilder for CustomBuilder { #[context("Failed to get dependencies for canister '{}'.", info.get_name())] fn get_dependencies( &self, + _env: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, ) -> DfxResult> { @@ -97,6 +98,7 @@ impl CanisterBuilder for CustomBuilder { #[context("Failed to build custom canister {}.", info.get_name())] fn build( &self, + _env: &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 866f7cd1ff..0baac29ac3 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -3,19 +3,24 @@ use crate::lib::canister_info::CanisterInfo; use crate::lib::environment::Environment; use crate::lib::error::{BuildError, DfxError, DfxResult}; use crate::lib::models::canister::CanisterPool; -use anyhow::{bail, Context}; +use crate::lib::models::canister::Import; +use anyhow::{anyhow, bail, Context}; use candid::Principal as CanisterId; use candid_parser::utils::CandidSource; +use dfx_core::config::cache::Cache; use dfx_core::config::model::dfinity::{Config, Profile}; use dfx_core::network::provider::get_network_context; use dfx_core::util; use fn_error_context::context; use handlebars::Handlebars; +use petgraph::visit::Bfs; +use slog::trace; +use slog::Logger; use std::borrow::Cow; use std::collections::BTreeMap; use std::ffi::OsStr; use std::fmt::Write; -use std::fs; +use std::fs::{self, metadata}; use std::io::Read; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -29,6 +34,8 @@ mod rust; pub use custom::custom_download; +use super::canister_info::motoko::MotokoCanisterInfo; + #[derive(Debug)] pub enum WasmBuildOutput { // Wasm(Vec), @@ -57,6 +64,7 @@ pub trait CanisterBuilder { /// list. fn get_dependencies( &self, + _env: &dyn Environment, _pool: &CanisterPool, _info: &CanisterInfo, ) -> DfxResult> { @@ -76,6 +84,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, @@ -221,6 +230,215 @@ pub trait CanisterBuilder { Ok(()) } + /// TODO: It is called too many times. It caches data in `env.imports`, but better not to call repeatedly anyway. + #[context("Failed to find imports for canister '{}'.", info.get_name())] + fn read_dependencies( + &self, + env: &dyn Environment, + pool: &CanisterPool, + info: &CanisterInfo, + cache: &dyn Cache, + ) -> DfxResult { + #[context("Failed recursive dependency detection at {}.", parent)] + fn read_dependencies_recursive( + env: &dyn Environment, + cache: &dyn Cache, + pool: &CanisterPool, + parent: &Import, + ) -> DfxResult { + if env.get_imports().borrow().nodes().contains_key(parent) { + // The item and its descendants are already in the graph. + return Ok(()); + } + let parent_node_index = env.get_imports().borrow_mut().update_node(parent); + + let file = match parent { + Import::Canister(parent_name) => { + let parent_canister = pool.get_first_canister_with_name(parent_name) + .ok_or_else(|| anyhow!("No such canister {}", parent_name))?; + let parent_canister_info = parent_canister.get_info(); + if parent_canister_info.is_motoko() { + let motoko_info = parent_canister + .get_info() + .as_info::() + .context("Getting Motoko info")?; + Some( + motoko_info + .get_main_path() + .canonicalize() + .with_context(|| { + format!( + "Canonicalizing Motoko path {}", + motoko_info.get_main_path().to_string_lossy() + ) + })?, + ) + } else { + for child in parent_canister_info.get_dependencies() { + read_dependencies_recursive( + env, + cache, + pool, + &Import::Canister(child.clone()), + )?; + + let child_node = Import::Canister(child.clone()); + let child_node_index = + env.get_imports().borrow_mut().update_node(&child_node); + env.get_imports().borrow_mut().update_edge( + parent_node_index, + child_node_index, + (), + ); + } + return Ok(()); + } + } + Import::FullPath(path) => Some(path.clone()), + _ => None, + }; + if let Some(file) = file { + let mut command = cache + .get_binary_command("moc") + .context("Getting binary command \"moc\"")?; + let command = command.arg("--print-deps").arg(file); + let output = command + .output() + .with_context(|| format!("Error executing {:#?}", command))?; + let output = String::from_utf8_lossy(&output.stdout); + + for line in output.lines() { + let child = Import::try_from(line).context("Failed to create MotokoImport.")?; + match &child { + Import::Canister(_) | Import::FullPath(_) => { + read_dependencies_recursive(env, cache, pool, &child)? + } + _ => {} + } + let child_node_index = env.get_imports().borrow_mut().update_node(&child); + env.get_imports().borrow_mut().update_edge( + parent_node_index, + child_node_index, + (), + ); + } + } + + Ok(()) + } + + read_dependencies_recursive( + env, + cache, + pool, + &Import::Canister(info.get_name().to_string()), + )?; + + Ok(()) + } + + fn should_build( + &self, + env: &dyn Environment, + pool: &CanisterPool, + canister_info: &CanisterInfo, + cache: &dyn Cache, + logger: &Logger, + ) -> DfxResult { + if !canister_info.is_motoko() { + return Ok(true); + } + + let output_wasm_path = canister_info.get_output_wasm_path(); + + self.read_dependencies(env, pool, canister_info, cache)?; + + // Check that one of the dependencies is newer than the target: + if let Ok(wasm_file_metadata) = metadata(output_wasm_path) { + let wasm_file_time = match wasm_file_metadata.modified() { + Ok(wasm_file_time) => wasm_file_time, + Err(_) => { + return Ok(true); // need to compile + } + }; + let imports = env.get_imports().borrow(); + let start = if let Some(node_index) = imports + .nodes() + .get(&Import::Canister(canister_info.get_name().to_string())) + { + *node_index + } else { + panic!("programming error"); + }; + let mut import_iter = Bfs::new(&imports.graph(), start); + let mut top_level = true; // link to our main Canister with `.wasm` + loop { + if let Some(import) = import_iter.next(&imports.graph()) { + let top_level_cur = top_level; + top_level = false; + let subnode = &imports.graph()[import]; + if top_level_cur { + assert!( + matches!(subnode, Import::Canister(_)), + "the top-level import must be a canister" + ); + } + let imported_file = match subnode { + Import::Canister(canister_name) => { + if let Some(canister) = + pool.get_first_canister_with_name(canister_name.as_str()) + { + let main_file = if top_level_cur { + if let Some(main_file) = canister.get_info().get_main_file() { + canister + .get_info() + .get_workspace_root() + .join(main_file) + .canonicalize()? + } else { + continue; + } + } else { + canister.get_info().get_service_idl_path() + }; + Some(main_file) + } else { + None + } + } + Import::Ic(_canister_id) => { + continue; + } + Import::Lib(_path) => { + // Skip libs, all changes by package managers don't modify existing directories but create new ones. + continue; + } + Import::FullPath(full_path) => Some(full_path.clone()), + }; + if let Some(imported_file) = imported_file { + let imported_file_metadata = + metadata(&imported_file).with_context(|| { + format!("Getting metadata of {}", imported_file.to_string_lossy()) + })?; + let imported_file_time = imported_file_metadata.modified()?; + if imported_file_time > wasm_file_time { + break; + }; + }; + } else { + trace!( + logger, + "Canister {} already compiled.", + canister_info.get_name() + ); + return Ok(false); + } + } + }; + + Ok(true) + } + /// Get the path to the provided candid file for the canister. /// No need to guarantee the file exists, as the caller will handle that. fn get_candid_path( @@ -398,7 +616,11 @@ pub fn get_and_write_environment_variables<'a>( (Borrowed("DFX_NETWORK"), Borrowed(network_name.as_ref())), ]; for dep in dependencies { - let canister = pool.get_canister(dep).unwrap(); + let canister = if let Some(canister) = pool.get_canister(dep) { + canister + } else { + continue; + }; if let Some(candid_path) = canister.get_info().get_remote_candid_if_remote() { vars.push(( Owned(format!( @@ -422,13 +644,16 @@ pub fn get_and_write_environment_variables<'a>( } } for canister in pool.get_canister_list() { - vars.push(( - Owned(format!( - "CANISTER_ID_{}", - canister.get_name().replace('-', "_").to_ascii_uppercase(), - )), - Owned(canister.canister_id().to_text().into()), - )); + // Don't try to add `deploy: false` canisters: + if let Some(canister_id) = canister.get_info().get_canister_id_option() { + vars.push(( + Owned(format!( + "CANISTER_ID_{}", + canister.get_name().replace('-', "_").to_ascii_uppercase(), + )), + Owned(canister_id.to_text().into()), + )); + } } if let Ok(id) = info.get_canister_id() { vars.push((Borrowed("CANISTER_ID"), Owned(format!("{}", id).into()))); @@ -493,9 +718,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 only a subset of canisters should be built, then user_specified_canisters contains these canisters' names. /// If all canisters should be built, then this is None. - pub canisters_to_build: Option>, + pub user_specified_canisters: Option>, /// If environment variables should be output to a `.env` file, `env_file` is set to its path. pub env_file: Option, } @@ -516,7 +741,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, + user_specified_canisters: None, env_file: config.get_output_env_file(None)?, }) } @@ -530,7 +755,7 @@ impl BuildConfig { pub fn with_canisters_to_build(self, canisters: Vec) -> Self { Self { - canisters_to_build: Some(canisters), + user_specified_canisters: Some(canisters), ..self } } diff --git a/src/dfx/src/lib/builders/motoko.rs b/src/dfx/src/lib/builders/motoko.rs index 344d1519dc..5660868f86 100644 --- a/src/dfx/src/lib/builders/motoko.rs +++ b/src/dfx/src/lib/builders/motoko.rs @@ -6,7 +6,7 @@ use crate::lib::canister_info::CanisterInfo; use crate::lib::environment::Environment; use crate::lib::error::{BuildError, DfxError, DfxResult}; use crate::lib::metadata::names::{CANDID_ARGS, CANDID_SERVICE}; -use crate::lib::models::canister::CanisterPool; +use crate::lib::models::canister::{Canister, CanisterPool, Import}; use crate::lib::package_arguments::{self, PackageArguments}; use crate::util::assets::management_idl; use anyhow::Context; @@ -15,9 +15,8 @@ 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}; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::convert::TryFrom; -use std::fmt::Debug; use std::path::{Path, PathBuf}; use std::process::Output; use std::sync::Arc; @@ -41,74 +40,48 @@ impl MotokoBuilder { } } -#[context("Failed to find imports for canister at '{}'.", info.get_main_path().display())] -fn get_imports(cache: &dyn Cache, info: &MotokoCanisterInfo) -> DfxResult> { - #[context("Failed recursive dependency detection at {}.", file.display())] - fn get_imports_recursive( - cache: &dyn Cache, - file: &Path, - result: &mut BTreeSet, - ) -> DfxResult { - if result.contains(&MotokoImport::Relative(file.to_path_buf())) { - return Ok(()); - } - - result.insert(MotokoImport::Relative(file.to_path_buf())); - - let mut command = cache.get_binary_command("moc")?; - let command = command.arg("--print-deps").arg(file); - let output = command - .output() - .with_context(|| format!("Error executing {:#?}", command))?; - let output = String::from_utf8_lossy(&output.stdout); - - for line in output.lines() { - let import = MotokoImport::try_from(line).context("Failed to create MotokoImport.")?; - match import { - MotokoImport::Relative(path) => { - get_imports_recursive(cache, path.as_path(), result)?; - } - _ => { - result.insert(import); - } - } - } - - Ok(()) - } - - let mut result = BTreeSet::new(); - get_imports_recursive(cache, info.get_main_path(), &mut result)?; - - Ok(result) -} - impl CanisterBuilder for MotokoBuilder { #[context("Failed to get dependencies for canister '{}'.", info.get_name())] fn get_dependencies( &self, + env: &dyn Environment, pool: &CanisterPool, info: &CanisterInfo, ) -> DfxResult> { - let motoko_info = info.as_info::()?; - let imports = get_imports(self.cache.as_ref(), &motoko_info)?; - - Ok(imports - .iter() - .filter_map(|import| { - if let MotokoImport::Canister(name) = import { - pool.get_first_canister_with_name(name) - } else { - None - } - }) - .map(|canister| canister.canister_id()) - .collect()) + self.read_dependencies(env, pool, info, self.cache.as_ref())?; + + let imports = env.get_imports().borrow(); + let graph = imports.graph(); + match petgraph::algo::toposort(graph, None) { + Ok(order) => { + Ok(order + .into_iter() + .filter_map(|id| match graph.node_weight(id) { + Some(Import::Canister(name)) => { + pool.get_first_canister_with_name(name.as_str()) // TODO: a little inefficient + } + _ => None, + }) + .map(|canister| canister.canister_id()) + .collect()) + } + Err(err) => { + let message = match graph.node_weight(err.node_id()) { + Some(Import::Canister(name)) => name, + _ => "", + }; + return Err(DfxError::new(BuildError::DependencyError(format!( + "Found circular dependency: {}", + message + )))); + } + } } #[context("Failed to build Motoko canister '{}'.", canister_info.get_name())] fn build( &self, + env: &dyn Environment, pool: &CanisterPool, canister_info: &CanisterInfo, config: &BuildConfig, @@ -116,11 +89,21 @@ impl CanisterBuilder for MotokoBuilder { let motoko_info = canister_info.as_info::()?; let profile = config.profile; let input_path = motoko_info.get_main_path(); - let output_wasm_path = motoko_info.get_output_wasm_path(); + let output_wasm_path = canister_info.get_output_wasm_path(); + // Map from name to principal (for our dependencies): + // TODO: It is better to use the auto-constructed graph for Motoko dependencies, + // but we need to transfer the value of `subgraph` from `build_order()` to here somehow, so needing to change code structure. let id_map = pool .get_canister_list() .iter() + .filter(|&c| { + canister_info + .get_dependencies() + .iter() + .map(|s| s.as_str()) + .any(|name| name == c.get_name()) // TODO: slow + }) .map(|c| (c.get_name().to_string(), c.canister_id().to_text())) .collect(); @@ -130,21 +113,39 @@ impl CanisterBuilder for MotokoBuilder { motoko_info.get_output_root().to_string_lossy() ) })?; - let cache = &self.cache; let idl_dir_path = &config.idl_root; std::fs::create_dir_all(idl_dir_path) .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)? - .contains(&MotokoImport::Ic("aaaaa-aa".to_string())) + if env + .get_imports() + .borrow() + .nodes() + .contains_key(&Import::Ic("aaaaa-aa".to_string())) { let management_idl_path = idl_dir_path.join("aaaaa-aa.did"); dfx_core::fs::write(management_idl_path, management_idl()?)?; } + let cache = &self.cache; + let package_arguments = package_arguments::load(cache.as_ref(), motoko_info.get_packtool())?; + let mut package_arguments_map = BTreeMap::<&str, &str>::new(); + { + // block + let mut i = 0; + while i + 3 <= package_arguments.len() { + if package_arguments[i] == "--package" { + package_arguments_map + .insert(&package_arguments[i + 1], &package_arguments[i + 2]); + i += 3; + } else { + i += 1; + } + } + } let moc_arguments = match motoko_info.get_args() { Some(args) => [ @@ -182,11 +183,17 @@ impl CanisterBuilder for MotokoBuilder { }; motoko_compile(&self.logger, cache.as_ref(), ¶ms)?; + // for `deploy: false` canisters. + let canister_id = if let Some(canister_id) = canister_info.get_canister_id_option() { + canister_id + } else { + Canister::generate_random_canister_id()? + }; + Ok(BuildOutput { - canister_id: canister_info - .get_canister_id() - .expect("Could not find canister ID."), - wasm: WasmBuildOutput::File(motoko_info.get_output_wasm_path().to_path_buf()), + // duplicate code + canister_id, + wasm: WasmBuildOutput::File(canister_info.get_output_wasm_path().to_path_buf()), idl: IdlBuildOutput::File(motoko_info.get_output_idl_path().to_path_buf()), }) } @@ -259,15 +266,7 @@ fn motoko_compile(logger: &Logger, cache: &dyn Cache, params: &MotokoParams<'_>) Ok(()) } -#[derive(Debug, PartialOrd, Ord, PartialEq, Eq)] -enum MotokoImport { - Canister(String), - Ic(String), - Lib(String), - Relative(PathBuf), -} - -impl TryFrom<&str> for MotokoImport { +impl TryFrom<&str> for Import { type Error = DfxError; fn try_from(line: &str) -> Result { @@ -294,9 +293,9 @@ impl TryFrom<&str> for MotokoImport { } let (prefix, name) = url.split_at(index + 1); match prefix { - "canister:" => MotokoImport::Canister(name.to_owned()), - "ic:" => MotokoImport::Ic(name.to_owned()), - "mo:" => MotokoImport::Lib(name.to_owned()), + "canister:" => Import::Canister(name.to_owned()), + "ic:" => Import::Ic(name.to_owned()), + "mo:" => Import::Lib(name.to_owned()), _ => { return Err(DfxError::new(BuildError::DependencyError(format!( "Unknown import {}", @@ -314,7 +313,7 @@ impl TryFrom<&str> for MotokoImport { path.display() )))); }; - MotokoImport::Relative(path) + Import::FullPath(path) } None => { return Err(DfxError::new(BuildError::DependencyError(format!( diff --git a/src/dfx/src/lib/builders/pull.rs b/src/dfx/src/lib/builders/pull.rs index 0da205d7b7..36098e0b93 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, + _env: &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, + _env: &dyn Environment, _pool: &CanisterPool, canister_info: &CanisterInfo, _config: &BuildConfig, diff --git a/src/dfx/src/lib/builders/rust.rs b/src/dfx/src/lib/builders/rust.rs index ad715bb567..7268930baf 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, + _env: &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, @@ -75,7 +77,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, diff --git a/src/dfx/src/lib/canister_info.rs b/src/dfx/src/lib/canister_info.rs index 89a9eb8b1f..d7a3e33241 100644 --- a/src/dfx/src/lib/canister_info.rs +++ b/src/dfx/src/lib/canister_info.rs @@ -62,6 +62,7 @@ pub struct CanisterInfo { tech_stack: Option, gzip: bool, init_arg: Option, + output_wasm_path: PathBuf, init_arg_file: Option, } @@ -151,6 +152,8 @@ impl CanisterInfo { let init_arg = canister_config.init_arg.clone(); let init_arg_file = canister_config.init_arg_file.clone(); + let output_wasm_path = output_root.join(name).with_extension("wasm"); + let canister_info = CanisterInfo { name: name.to_string(), declarations_config, @@ -173,6 +176,7 @@ impl CanisterInfo { pull_dependencies, gzip, init_arg, + output_wasm_path, init_arg_file, }; @@ -207,6 +211,9 @@ impl CanisterInfo { pub fn get_output_root(&self) -> &Path { &self.output_root } + pub fn get_output_wasm_path(&self) -> &Path { + self.output_wasm_path.as_path() + } #[context("Failed to get canister id for '{}'.", self.name)] pub fn get_canister_id(&self) -> DfxResult { @@ -222,6 +229,10 @@ impl CanisterInfo { } } + pub fn get_canister_id_option(&self) -> Option { + self.canister_id + } + pub fn get_dependencies(&self) -> &[String] { &self.dependencies } diff --git a/src/dfx/src/lib/canister_info/motoko.rs b/src/dfx/src/lib/canister_info/motoko.rs index f187a8765d..a8fb49171e 100644 --- a/src/dfx/src/lib/canister_info/motoko.rs +++ b/src/dfx/src/lib/canister_info/motoko.rs @@ -8,7 +8,6 @@ pub struct MotokoCanisterInfo { input_path: PathBuf, output_root: PathBuf, - output_wasm_path: PathBuf, output_idl_path: PathBuf, output_stable_path: PathBuf, output_did_js_path: PathBuf, @@ -23,9 +22,6 @@ impl MotokoCanisterInfo { pub fn get_main_path(&self) -> &Path { self.input_path.as_path() } - pub fn get_output_wasm_path(&self) -> &Path { - self.output_wasm_path.as_path() - } pub fn get_output_idl_path(&self) -> &Path { self.output_idl_path.as_path() } @@ -80,7 +76,6 @@ impl CanisterInfoFactory for MotokoCanisterInfo { Ok(MotokoCanisterInfo { input_path, output_root, - output_wasm_path, output_idl_path, output_stable_path, output_did_js_path, diff --git a/src/dfx/src/lib/environment.rs b/src/dfx/src/lib/environment.rs index 8036254c6f..55417fbb91 100644 --- a/src/dfx/src/lib/environment.rs +++ b/src/dfx/src/lib/environment.rs @@ -25,6 +25,9 @@ use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use super::graph::graph_nodes_map::GraphWithNodesMap; +use super::models::canister::Import; + pub trait Environment { fn get_cache(&self) -> Arc; fn get_config(&self) -> Result>, LoadDfxConfigError>; @@ -84,6 +87,8 @@ pub trait Environment { self.get_config()?, ) } + + fn get_imports(&self) -> &RefCell>; } pub enum ProjectConfig { @@ -108,6 +113,10 @@ pub struct EnvironmentImpl { effective_canister_id: Option, extension_manager: ExtensionManager, + + /// Graph currently read imports and their children, not necessarily the entire graph of all imports. + /// Invariant: with each node contains all its descendants. + imports: RefCell>, } impl EnvironmentImpl { @@ -125,6 +134,7 @@ impl EnvironmentImpl { identity_override: None, effective_canister_id: None, extension_manager, + imports: RefCell::new(GraphWithNodesMap::new()), }) } @@ -263,6 +273,10 @@ impl Environment for EnvironmentImpl { fn get_extension_manager(&self) -> &ExtensionManager { &self.extension_manager } + + fn get_imports(&self) -> &RefCell> { + &self.imports + } } pub struct AgentEnvironment<'a> { @@ -270,6 +284,7 @@ pub struct AgentEnvironment<'a> { agent: Agent, network_descriptor: NetworkDescriptor, identity_manager: IdentityManager, + imports: RefCell>, effective_canister_id: Option, } @@ -309,6 +324,7 @@ impl<'a> AgentEnvironment<'a> { agent: create_agent(logger, url, identity, timeout)?, network_descriptor: network_descriptor.clone(), identity_manager, + imports: RefCell::new(GraphWithNodesMap::new()), effective_canister_id, }) } @@ -393,6 +409,10 @@ impl<'a> Environment for AgentEnvironment<'a> { fn get_extension_manager(&self) -> &ExtensionManager { self.backend.get_extension_manager() } + + fn get_imports(&self) -> &RefCell> { + &self.imports + } } #[context("Failed to create agent with url {}.", url)] diff --git a/src/dfx/src/lib/error/build.rs b/src/dfx/src/lib/error/build.rs index 67c65f4baa..b47c203844 100644 --- a/src/dfx/src/lib/error/build.rs +++ b/src/dfx/src/lib/error/build.rs @@ -8,9 +8,8 @@ pub enum BuildError { #[error("The pre-build all step failed")] PreBuildAllStepFailed(#[source] Box), - #[error("The post-build all step failed")] - PostBuildAllStepFailed(#[source] Box), - + // #[error("The post-build all step failed")] + // PostBuildAllStepFailed(#[source] Box), #[error("The pre-build step failed for canister '{0}' ({1})")] PreBuildStepFailed(Principal, String, #[source] Box), diff --git a/src/dfx/src/lib/graph/graph_nodes_map.rs b/src/dfx/src/lib/graph/graph_nodes_map.rs new file mode 100644 index 0000000000..c588ea1d3c --- /dev/null +++ b/src/dfx/src/lib/graph/graph_nodes_map.rs @@ -0,0 +1,54 @@ +// TODO: Integrate it into `petgraph` library. + +use std::collections::HashMap; +use std::hash::Hash; + +use petgraph::{ + graph::EdgeIndex, + graph::IndexType, + graph::{DefaultIx, NodeIndex}, + Directed, EdgeType, Graph, +}; + +pub struct GraphWithNodesMap { + graph: Graph, + nodes: HashMap>, +} + +impl GraphWithNodesMap { + pub fn graph(&self) -> &Graph { + &self.graph + } + pub fn nodes(&self) -> &HashMap> { + &self.nodes + } +} + +impl GraphWithNodesMap +where + Ty: EdgeType, + Ix: IndexType, +{ + pub fn update_node(&mut self, weight: &N) -> NodeIndex + where + N: Eq + Hash + Clone, + { + // TODO: Get rid of two `clone`s (apparently, requires data stucture change). + *self + .nodes + .entry(weight.clone()) + .or_insert_with(|| self.graph.add_node(weight.clone())) + } + pub fn update_edge(&mut self, a: NodeIndex, b: NodeIndex, weight: E) -> EdgeIndex { + self.graph.update_edge(a, b, weight) + } +} + +impl GraphWithNodesMap { + pub fn new() -> Self { + Self { + graph: Graph::new(), + nodes: HashMap::new(), + } + } +} diff --git a/src/dfx/src/lib/graph/mod.rs b/src/dfx/src/lib/graph/mod.rs new file mode 100644 index 0000000000..74efc67f82 --- /dev/null +++ b/src/dfx/src/lib/graph/mod.rs @@ -0,0 +1,2 @@ +pub mod graph_nodes_map; +pub mod traverse_filtered; diff --git a/src/dfx/src/lib/graph/traverse_filtered.rs b/src/dfx/src/lib/graph/traverse_filtered.rs new file mode 100644 index 0000000000..825e594970 --- /dev/null +++ b/src/dfx/src/lib/graph/traverse_filtered.rs @@ -0,0 +1,69 @@ +// TODO: Somebody, adopt this code (and BFS) to `petgraph`. +use petgraph::{data::DataMap, visit::IntoNeighborsDirected}; + +use crate::lib::error::DfxResult; + +pub struct DfsFiltered {} + +impl DfsFiltered { + pub fn new() -> Self { + Self {} + } + + // FIXME: For certain input graphs, the number of variants grows exponentially. + pub fn traverse2( + &mut self, + graph: G, + mut predicate: P, + mut call: C, + node_id: NodeId, + ) -> DfxResult<()> + where + C: FnMut(&NodeId, &NodeId) -> DfxResult<()>, + G: IntoNeighborsDirected + DataMap, + NodeId: Copy + Eq, + P: FnMut(&NodeId) -> DfxResult, + { + Self::traverse2_recursive(graph, &mut predicate, &mut call, node_id, &mut Vec::new()) + } + + fn traverse2_recursive( + graph: G, + predicate: &mut P, + call: &mut C, + node_id: NodeId, + ancestors: &mut Vec, + ) -> DfxResult<()> + where + C: FnMut(&NodeId, &NodeId) -> DfxResult<()>, + G: IntoNeighborsDirected + DataMap, + NodeId: Copy + Eq, + P: FnMut(&NodeId) -> DfxResult, + NodeId: Copy + Eq, + { + let ancestor_id = ancestors + .iter() + .rev() + .find_map(|&id| -> Option> { + match predicate(&id) { + Ok(true) => Some(Ok(id)), + Ok(false) => None, + Err(err) => Some(Err(err)), + } + }) + .transpose()?; + if predicate(&node_id)? { + if let Some(ancestor_id) = ancestor_id { + assert!(ancestor_id != node_id); + call(&ancestor_id, &node_id)?; + } + } + ancestors.push(node_id); + for subnode_id in graph.neighbors(node_id) { + Self::traverse2_recursive(graph, predicate, call, subnode_id, ancestors)?; + } + ancestors.pop(); + + Ok(()) + } +} diff --git a/src/dfx/src/lib/mod.rs b/src/dfx/src/lib/mod.rs index c1d33bc68f..dc179fb4d4 100644 --- a/src/dfx/src/lib/mod.rs +++ b/src/dfx/src/lib/mod.rs @@ -8,6 +8,7 @@ pub mod diagnosis; pub mod environment; pub mod error; pub mod error_code; +pub mod graph; pub mod ic_attributes; pub mod identity; pub mod info; diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index aa4d75a5f6..7d28f1219a 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -5,6 +5,8 @@ use crate::lib::builders::{ use crate::lib::canister_info::CanisterInfo; use crate::lib::environment::Environment; use crate::lib::error::{BuildError, DfxError, DfxResult}; +use crate::lib::graph::graph_nodes_map::GraphWithNodesMap; +use crate::lib::graph::traverse_filtered::DfsFiltered; use crate::lib::metadata::dfx::DfxMetadata; use crate::lib::metadata::names::{CANDID_ARGS, CANDID_SERVICE, DFX}; use crate::lib::wasm::file::{compress_bytes, read_wasm_module}; @@ -12,6 +14,7 @@ use crate::util::assets; use anyhow::{anyhow, bail, Context}; use candid::Principal as CanisterId; use candid_parser::utils::CandidSource; +use dfx_core::config::cache::Cache; use dfx_core::config::model::canister_id_store::CanisterIdStore; use dfx_core::config::model::dfinity::{ CanisterMetadataSection, Config, MetadataVisibility, TechStack, WasmOptLevel, @@ -20,15 +23,18 @@ use fn_error_context::context; use ic_wasm::metadata::{add_metadata, remove_metadata, Kind}; use ic_wasm::optimize::OptLevel; use itertools::Itertools; -use petgraph::graph::{DiGraph, NodeIndex}; +use petgraph::algo::toposort; +use petgraph::graph::NodeIndex; +use petgraph::visit::Bfs; use rand::{thread_rng, RngCore}; use slog::{error, info, trace, warn, Logger}; use std::cell::RefCell; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::convert::TryFrom; use std::ffi::OsStr; +use std::fmt::Display; use std::io::Read; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::Arc; @@ -38,8 +44,9 @@ use std::sync::Arc; /// Once an instance of a canister is built it is immutable. So for comparing /// two canisters one can use their ID. pub struct Canister { - info: CanisterInfo, - builder: Arc, + // TODO: Two below `pubs` are a hack. + pub info: CanisterInfo, + pub builder: Arc, output: RefCell>, } unsafe impl Send for Canister {} @@ -62,10 +69,11 @@ impl Canister { 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)); @@ -330,7 +338,7 @@ impl Canister { // - LSP_ROOT/CANISTER_ID.did let mut targets = vec![]; targets.push(self.info.get_service_idl_path()); - let canister_id = self.canister_id(); + let canister_id = build_output.canister_id; targets.push( build_config .idl_root @@ -349,7 +357,10 @@ impl Canister { continue; } dfx_core::fs::composite::ensure_parent_dir_exists(&target)?; - dfx_core::fs::write(&target, &service_did)?; + if !target.exists() || dfx_core::fs::read_to_string(&target)? != service_did { + // TODO: Make atomic operation. + dfx_core::fs::write(&target, &service_did)?; + } dfx_core::fs::set_permissions_readwrite(&target)?; } @@ -435,6 +446,28 @@ fn check_valid_subtype(compiled_idl_path: &Path, specified_idl_path: &Path) -> D Ok(()) } +/// Used mainly for Motoko +/// +/// TODO: Copying this type uses `String.clone()` what may be inefficient. +#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] +pub enum Import { + Canister(String), + Ic(String), + Lib(String), // TODO: Unused, because package manager never update existing files (but create new dirs) + FullPath(PathBuf), +} + +impl Display for Import { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Canister(name) => write!(f, "canister {}", name), + Self::Ic(principal) => write!(f, "principal {}", principal), + Self::Lib(name) => write!(f, "library {}", name), + Self::FullPath(file) => write!(f, "file {}", file.to_string_lossy()), + } + } +} + /// A canister pool is a list of canisters. pub struct CanisterPool { canisters: Vec>, @@ -499,7 +532,7 @@ impl CanisterPool { pub fn get_canister(&self, canister_id: &CanisterId) -> Option<&Canister> { for c in &self.canisters { let info = &c.info; - if Some(canister_id) == info.get_canister_id().ok().as_ref() { + if Some(canister_id) == info.get_canister_id_option().as_ref() { return Some(c); } } @@ -510,6 +543,7 @@ impl CanisterPool { self.canisters.iter().map(|c| c.as_ref()).collect() } + #[allow(unused)] // TODO pub fn get_canister_info(&self, canister_id: &CanisterId) -> Option<&CanisterInfo> { self.get_canister(canister_id).map(|c| &c.info) } @@ -527,65 +561,180 @@ impl CanisterPool { &self.logger } + /// Build only dependencies relevant for `user_specified_canisters`. + /// + /// TODO: Probably shouldn't be `pub`. #[context("Failed to build dependencies graph for canister pool.")] - fn build_dependencies_graph(&self) -> DfxResult> { - let mut graph: DiGraph = DiGraph::new(); - let mut id_set: BTreeMap> = BTreeMap::new(); - - // Add all the canisters as nodes. + pub fn build_canister_dependencies_graph( + &self, + env: &dyn Environment, + toplevel_canisters: &[&Canister], + cache: &dyn Cache, + ) -> DfxResult> { for canister in &self.canisters { - let canister_id = canister.info.get_canister_id()?; - id_set.insert(canister_id, graph.add_node(canister_id)); + // a little inefficient + let contains = toplevel_canisters + .iter() + .map(|canister| canister.get_info().get_name()) + .contains(&canister.get_info().get_name()); + if contains { + canister + .builder + .read_dependencies(env, self, canister.get_info(), cache)?; // TODO: It is called multiple times during the flow. + } } - // Add all the edges. - for canister in &self.canisters { - let canister_id = canister.canister_id(); - let canister_info = &canister.info; - let deps = canister.builder.get_dependencies(self, canister_info)?; - if let Some(node_ix) = id_set.get(&canister_id) { - for d in deps { - if let Some(dep_ix) = id_set.get(&d) { - graph.add_edge(*node_ix, *dep_ix, ()); - } + let imports = env.get_imports().borrow(); + let source_graph = imports.graph(); + let source_ids = imports.nodes(); + let start: Vec<_> = toplevel_canisters + .iter() + .map(|canister| Import::Canister(canister.get_name().to_string())) + .collect(); + let start_nodes: Vec<_> = start + .into_iter() + .filter_map(|node| { + if let Some(&id) = source_ids.get(&node) { + Some(id) + } else { + None } - } + }) + .collect(); + // Transform the graph of file dependencies to graph of canister dependencies. + // For this do DFS for each of `start`. + let mut dest_graph: GraphWithNodesMap = GraphWithNodesMap::new(); + for start_node in start_nodes.into_iter() { + let mut filtered_dfs = DfsFiltered::new(); + + // Add start node: + let start = source_graph.node_weight(start_node).unwrap(); + let start_name = match start { + Import::Canister(name) => name, + _ => { + panic!("programming error"); + } + }; + dest_graph.update_node(start_name); + + filtered_dfs.traverse2( + source_graph, + |&s| { + let source_id = source_graph.node_weight(s); + if let Some(Import::Canister(_)) = source_id { + Ok(true) + } else { + Ok(false) + } + }, + |&source_parent_id, &source_child_id| { + let parent = source_graph.node_weight(source_parent_id).unwrap(); + let parent_name = match parent { + Import::Canister(name) => name, + _ => { + panic!("programming error"); + } + }; + + let child = source_graph.node_weight(source_child_id).unwrap(); + let child_name = match child { + Import::Canister(name) => name, + _ => { + panic!("programming error"); + } + }; + + let dest_parent_id = dest_graph.update_node(parent_name); + let dest_child_id = dest_graph.update_node(child_name); + dest_graph.update_edge(dest_parent_id, dest_child_id, ()); + + Ok(()) + }, + start_node, + )?; } - // Verify the graph has no cycles. - if let Err(err) = petgraph::algo::toposort(&graph, None) { - let message = match graph.node_weight(err.node_id()) { - Some(canister_id) => match self.get_canister_info(canister_id) { - Some(info) => info.get_name().to_string(), - None => format!("<{}>", canister_id.to_text()), - }, - None => "".to_string(), - }; - Err(DfxError::new(BuildError::DependencyError(format!( - "Found circular dependency: {}", - message - )))) + Ok(dest_graph) + } + + fn canister_dependencies( + &self, + env: &dyn Environment, + toplevel_canisters: &[&Canister], + ) -> Vec> { + let iter = toplevel_canisters + .iter() + .flat_map(|canister| { + // TODO: Is `unwrap` on the next line legit? + let parent_node = *env + .get_imports() + .borrow() + .nodes() + .get(&Import::Canister(canister.get_name().to_owned())) + .unwrap(); + let imports = env.get_imports().borrow(); + let neighbors = imports.graph().neighbors(parent_node); + neighbors + .filter_map(|id| { + imports.nodes().iter().find_map(move |(k, v)| { + if v == &id { + Some(k.clone()) + } else { + None + } + }) + }) // TODO: slow + .filter_map(|import| { + if let Import::Canister(name) = import { + self.get_first_canister_with_name(&name) + .map(|canister| (name, canister)) + } else { + None + } + }) + .collect::>() + }) + .collect::>(); // eliminate duplicates + iter.values().cloned().collect() + } + + #[context("Failed step_prebuild_all.")] + fn step_prebuild_all(&self, build_config: &BuildConfig) -> DfxResult<()> { + // cargo audit + if self + .canisters_to_build(build_config) + .iter() + .any(|can| can.info.is_rust()) + { + self.run_cargo_audit()?; } else { - Ok(graph) + trace!( + self.logger, + "No canister of type 'rust' found. Not trying to run 'cargo audit'." + ) } + + Ok(()) } - #[context("Failed step_prebuild_all.")] - fn step_prebuild_all(&self, log: &Logger, build_config: &BuildConfig) -> DfxResult<()> { + fn step_prebuild( + &self, + env: &dyn Environment, + build_config: &BuildConfig, + canister: &Canister, + ) -> DfxResult<()> { + canister.prebuild(self, build_config)?; // moc expects all .did files of dependencies to be in with name .did. - // Because some canisters don't get built these .did files have to be copied over manually. - for canister in self.canisters.iter().filter(|c| { - build_config - .canisters_to_build - .as_ref() - .map(|cans| !cans.iter().contains(&c.get_name().to_string())) - .unwrap_or(false) - }) { + + // Copy .did files into this temporary directory. + let log = self.get_logger(); + for canister in self.canister_dependencies(env, &[canister]) { let maybe_from = if let Some(remote_candid) = canister.info.get_remote_candid() { Some(remote_candid) } else { canister.info.get_output_idl_path() }; + // TODO: It tries to copy non-existing files (not yet compiled canisters..) if let Some(from) = maybe_from.as_ref() { if from.exists() { let to = build_config.idl_root.join(format!( @@ -618,159 +767,224 @@ impl CanisterPool { } } - // cargo audit - if self - .canisters_to_build(build_config) - .iter() - .any(|can| can.info.is_rust()) - { - self.run_cargo_audit()?; - } else { - trace!( - self.logger, - "No canister of type 'rust' found. Not trying to run 'cargo audit'." - ) - } - Ok(()) } - fn step_prebuild(&self, build_config: &BuildConfig, canister: &Canister) -> DfxResult<()> { - canister.prebuild(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, ) -> 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. + // TODO: The following `map` is a hack. + for canister in &self.canister_dependencies(env, &[&canister]) { + 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"); + + // Ignore errors (e.g. File Not Found). + let _ = std::fs::remove_file(idl_file_path); + } + canister.candid_post_process(self.get_logger(), build_config, build_output)?; canister.wasm_post_process(self.get_logger(), build_output)?; - build_canister_js(&canister.canister_id(), &canister.info)?; + build_canister_js(&build_output.canister_id, &canister.info)?; canister.postbuild(self, build_config) } - fn step_postbuild_all( + // fn step_postbuild_all( + // &self, + // _build_config: &BuildConfig, + // _order: &[CanisterId], + // ) -> DfxResult<()> { + // Ok(()) + // } + + // TODO: This function is called twice during deploy of a canister. + pub fn build_order( &self, - build_config: &BuildConfig, - _order: &[CanisterId], - ) -> 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_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"); + env: &dyn Environment, + toplevel_canisters: &[Arc], + ) -> DfxResult> { + trace!(env.get_logger(), "Building dependencies graph."); + // TODO: The following `map` is a hack. + let graph = self.build_canister_dependencies_graph( + env, + &toplevel_canisters + .iter() + .map(|canister| canister.as_ref()) + .collect::>(), + env.get_cache().as_ref(), + )?; // TODO: Can `clone` be eliminated? - // Ignore errors (e.g. File Not Found). - let _ = std::fs::remove_file(idl_file_path); + let toplevel_nodes: Vec = toplevel_canisters + .iter() + .map(|canister| -> DfxResult { + graph + .nodes() + .get(&canister.get_name().to_string()) + .copied() + .ok_or_else(|| anyhow!("No such canister {}.", canister.get_name())) + }) + .try_collect()?; + + // TODO: The following isn't very efficient. + + let mut reachable_nodes = HashMap::new(); + + for &start_node in toplevel_nodes.iter() { + let mut bfs = Bfs::new(&graph.graph(), start_node); // or `Dfs`, does not matter + while let Some(node) = bfs.next(&graph.graph()) { + reachable_nodes.insert(node, ()); + } } - Ok(()) + let subgraph = graph.graph().filter_map( + |node, _| { + if reachable_nodes.contains_key(&node) { + Some(node) + } else { + None + } + }, + |edge, _| Some(edge), + ); + + let nodes = toposort(&subgraph, None).map_err(|err| { + let node = graph.graph().node_weight(err.node_id()); + if let Some(node) = node { + anyhow!("Cycle in node dependencies: {}", node) + } else { + panic!("programming error"); + } + })?; + + let order: Vec = nodes + .iter() + .rev() // Reverse the order, as we have a dependency graph, we want to reverse indices. + .map(|idx| graph.graph().node_weight(*idx).unwrap().to_string()) + .collect(); + let log = env.get_logger(); + info!(log, "Build order: {}", order.join(" ")); + Ok(order) } /// Build all canisters, returning a vector of results of each builds. + /// + /// TODO: `log` can be got from `env`, can't it? #[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>> { - self.step_prebuild_all(log, build_config) - .map_err(|e| DfxError::new(BuildError::PreBuildAllStepFailed(Box::new(e))))?; - - let graph = self.build_dependencies_graph()?; - 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) { - Some(info) => info.get_name().to_string(), - None => format!("<{}>", canister_id.to_text()), - }, - None => "".to_string(), + ) -> DfxResult { + // TODO: The next statement is slow and confusing code. + let toplevel_canisters: Vec> = + if let Some(canisters) = build_config.user_specified_canisters.clone() { + self.canisters + .iter() + .filter(|c| canisters.contains(&c.get_name().to_string())) + .cloned() + .collect() + } else { + self.canisters.clone() }; - BuildError::DependencyError(format!("Found circular dependency: {}", message)) - })?; - let order: Vec = nodes - .iter() - .rev() // Reverse the order, as we have a dependency graph, we want to reverse indices. - .map(|idx| *graph.node_weight(*idx).unwrap()) - .collect(); + let order = self.build_order(env, &toplevel_canisters)?; - 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()) + self.step_prebuild_all(build_config) + .map_err(|e| DfxError::new(BuildError::PreBuildAllStepFailed(Box::new(e))))?; + + for canister_name in &order { + if let Some(canister) = self.get_first_canister_with_name(canister_name) { + trace!(log, "Building canister '{}'.", canister_name); + // TODO: + // } else { + // trace!(log, "Not building canister '{}'.", canister.get_name()); + // continue; + // } + if canister + .builder + .should_build( + env, + self, + &canister.info, + env.get_cache().as_ref(), + env.get_logger(), + ) + .with_context(|| { + format!("Checking whether to build canister {}", 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) + self.step_prebuild(env, build_config, canister.as_ref()) .map_err(|e| { BuildError::PreBuildStepFailed( - *canister_id, + canister.canister_id(), canister.get_name().to_string(), Box::new(e), ) }) .and_then(|_| { - self.step_build(build_config, canister).map_err(|e| { - BuildError::BuildStepFailed( - *canister_id, - canister.get_name().to_string(), - Box::new(e), - ) - }) + self.step_build(env, build_config, canister.as_ref()) + .map_err(|e| { + BuildError::BuildStepFailed( + canister.canister_id(), + canister.get_name().to_string(), + Box::new(e), + ) + }) }) - .and_then(|o| { - self.step_postbuild(build_config, canister, o) + .and_then(|o: &BuildOutput| { + self.step_postbuild(env, build_config, canister.as_ref(), o) .map_err(|e| { BuildError::PostBuildStepFailed( - *canister_id, + o.canister_id, canister.get_name().to_string(), Box::new(e), ) }) .map(|_| o) - }), - ); + })?; + } } } - self.step_postbuild_all(build_config, &order) - .map_err(|e| DfxError::new(BuildError::PostBuildAllStepFailed(Box::new(e))))?; + // self.step_postbuild_all(build_config, &order.iter().map(|name| { + // self.get_first_canister_with_name(name).unwrap().canister_id() // TODO: `unwrap()` + // }).collect::>()) + // .map_err(|e| DfxError::new(BuildError::PostBuildAllStepFailed(Box::new(e))))?; - Ok(result) + Ok(()) } /// Build all canisters, failing with the first that failed the build. Will return /// nothing if all succeeded. + /// + /// TODO: `log` can be got from `env`, can't it? #[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)?; - - for output in outputs { - output.map_err(DfxError::new)?; - } + self.build(env, log, build_config)?; Ok(()) } @@ -840,7 +1054,7 @@ impl CanisterPool { } pub fn canisters_to_build(&self, build_config: &BuildConfig) -> Vec<&Arc> { - if let Some(canister_names) = &build_config.canisters_to_build { + if let Some(canister_names) = &build_config.user_specified_canisters { self.canisters .iter() .filter(|can| canister_names.contains(&can.info.get_name().to_string())) diff --git a/src/dfx/src/lib/operations/canister/deploy_canisters.rs b/src/dfx/src/lib/operations/canister/deploy_canisters.rs index e36e477b15..1a31b3bb03 100644 --- a/src/dfx/src/lib/operations/canister/deploy_canisters.rs +++ b/src/dfx/src/lib/operations/canister/deploy_canisters.rs @@ -5,14 +5,12 @@ use crate::lib::environment::Environment; use crate::lib::error::DfxResult; use crate::lib::ic_attributes::CanisterSettings; use crate::lib::installers::assets::prepare_assets_for_proposal; -use crate::lib::models::canister::CanisterPool; +use crate::lib::models::canister::{Canister, CanisterPool}; use crate::lib::operations::canister::deploy_canisters::DeployMode::{ ComputeEvidence, ForceReinstallSingleCanister, NormalDeploy, PrepareForProposal, }; use crate::lib::operations::canister::motoko_playground::reserve_canister_with_playground; -use crate::lib::operations::canister::{ - all_project_canisters_with_ids, create_canister, install_canister::install_canister, -}; +use crate::lib::operations::canister::{create_canister, install_canister::install_canister}; use crate::util::clap::subnet_selection_opt::SubnetSelectionType; use anyhow::{anyhow, bail, Context}; use candid::Principal; @@ -25,9 +23,14 @@ use ic_utils::interfaces::management_canister::attributes::{ }; use ic_utils::interfaces::management_canister::builders::{InstallMode, WasmMemoryLimit}; use icrc_ledger_types::icrc1::account::Subaccount; +use itertools::Itertools; use slog::info; +// use core::slice::SlicePattern; use std::convert::TryFrom; use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use super::add_canisters_with_ids; #[derive(Eq, PartialEq, Debug, Clone)] pub enum DeployMode { @@ -63,7 +66,6 @@ pub async fn deploy_canisters( let config = env .get_config()? .ok_or_else(|| anyhow!("Cannot find dfx configuration file in the current working directory. Did you forget to create one?"))?; - let initial_canister_id_store = env.get_canister_id_store()?; let pull_canisters_in_config = config.get_config().get_pull_canisters()?; if let Some(canister_name) = some_canister { @@ -77,7 +79,14 @@ pub async fn deploy_canisters( let canisters_to_deploy = canister_with_dependencies(&config, some_canister)?; - let canisters_to_build = match deploy_mode { + let required_canisters = config + .get_config() + .get_canister_names_with_dependencies(some_canister)?; + let canisters_to_load = add_canisters_with_ids(&required_canisters, env, &config); + + let canister_pool = CanisterPool::load(env, false, &canisters_to_load)?; + + let toplevel_canisters = match deploy_mode { PrepareForProposal(canister_name) | ComputeEvidence(canister_name) => { vec![canister_name.clone()] } @@ -96,26 +105,56 @@ pub async fn deploy_canisters( }) .collect(), }; + let toplevel_canisters = toplevel_canisters + .into_iter() + .map(|name: String| -> DfxResult<_> { + canister_pool + .get_first_canister_with_name(name.as_str()) + .ok_or_else(|| { + anyhow!( + "A canister with the name '{}' was not found in the current project.", + name.clone() + ) + }) + }) + // .map(|v| &v) + .try_collect::, Vec>, _>()?; + let toplevel_canisters: &[Arc] = &toplevel_canisters; + + let order = canister_pool.build_order(env, toplevel_canisters)?; + let order_canisters = order + .iter() + .map(|name| canister_pool.get_first_canister_with_name(name).unwrap()) + .collect::>(); - let canisters_to_install: Vec = canisters_to_build + // Run this before calculating `canisters_to_install` to obtain canisters config. + let _new_canister_pool = CanisterPool::load(env, false, &order)?; // with newly registered canisters + + let canisters_to_install: &Vec = &order .clone() .into_iter() - .filter(|canister_name| !pull_canisters_in_config.contains_key(canister_name)) + .filter(|canister_name| { + !pull_canisters_in_config.contains_key(canister_name) + && //(some_canister == Some(canister_name) || // do deploy a canister that was explicitly specified + config.get_config().get_canister_config(canister_name).map_or( + true, |canister_config| canister_config.deploy) + }) .collect(); + let canister_id_store = env.get_canister_id_store()?; + if some_canister.is_some() { - info!(log, "Deploying: {}", canisters_to_install.join(" ")); } else { info!(log, "Deploying all canisters."); } - if canisters_to_deploy + if canisters_to_install .iter() - .any(|canister| initial_canister_id_store.find(canister).is_none()) + .any(|canister| canister_id_store.find(canister).is_none()) { register_canisters( env, - &canisters_to_deploy, - &initial_canister_id_store, + canisters_to_install, + &canister_id_store, with_cycles, specified_id_from_cli, call_sender, @@ -130,14 +169,16 @@ pub async fn deploy_canisters( info!(env.get_logger(), "All canisters have already been created."); } - let canisters_to_load = all_project_canisters_with_ids(env, &config); + // hack to load deployed canister IDs (such as of Rust canisters) + let new_canister_pool2 = CanisterPool::load(env, false, &order)?; // with newly registered canisters - let pool = build_canisters( + build_canisters( env, - &canisters_to_load, - &canisters_to_build, + order_canisters.as_slice(), + // toplevel_canisters, &config, env_file.clone(), + &new_canister_pool2, ) .await?; @@ -146,15 +187,15 @@ pub async fn deploy_canisters( let force_reinstall = matches!(deploy_mode, ForceReinstallSingleCanister(_)); install_canisters( env, - &canisters_to_install, - &initial_canister_id_store, + canisters_to_install, + &canister_id_store, &config, argument, argument_type, force_reinstall, upgrade_unchanged, call_sender, - pool, + new_canister_pool2, skip_consent, env_file.as_deref(), no_asset_upgrade, @@ -164,11 +205,10 @@ pub async fn deploy_canisters( info!(log, "Deployed canisters."); } PrepareForProposal(canister_name) => { - prepare_assets_for_commit(env, &initial_canister_id_store, &config, canister_name) - .await? + prepare_assets_for_commit(env, &canister_id_store, &config, canister_name).await? } ComputeEvidence(canister_name) => { - compute_evidence(env, &initial_canister_id_store, &config, canister_name).await? + compute_evidence(env, &canister_id_store, &config, canister_name).await? } } @@ -204,7 +244,7 @@ async fn register_canisters( ) -> DfxResult { let canisters_to_create = canister_names .iter() - .filter(|n| canister_id_store.find(n).is_none()) + .filter(|name| canister_id_store.find(name).is_none()) .cloned() .collect::>(); if canisters_to_create.is_empty() { @@ -289,22 +329,26 @@ async fn register_canisters( #[context("Failed to build all canisters.")] async fn build_canisters( env: &dyn Environment, - canisters_to_load: &[String], - canisters_to_build: &[String], + // canisters_to_load: &[String], + toplevel_canisters: &[Arc], config: &Config, env_file: Option, -) -> DfxResult { + canister_pool: &CanisterPool, +) -> DfxResult<()> { let log = env.get_logger(); info!(log, "Building canisters..."); - let build_mode_check = false; - let canister_pool = CanisterPool::load(env, build_mode_check, canisters_to_load)?; let build_config = BuildConfig::from_config(config, env.get_network_descriptor().is_playground())? - .with_canisters_to_build(canisters_to_build.into()) + .with_canisters_to_build( + toplevel_canisters + .iter() + .map(|canister| canister.get_name().to_string()) + .collect(), + ) // hack .with_env_file(env_file); - canister_pool.build_or_fail(log, &build_config).await?; - Ok(canister_pool) + canister_pool.build_or_fail(env, log, &build_config).await?; + Ok(()) } #[context("Failed while trying to install all canisters.")] diff --git a/src/dfx/src/lib/operations/canister/install_canister.rs b/src/dfx/src/lib/operations/canister/install_canister.rs index 3d3ff2a0e1..8d1b788414 100644 --- a/src/dfx/src/lib/operations/canister/install_canister.rs +++ b/src/dfx/src/lib/operations/canister/install_canister.rs @@ -17,7 +17,6 @@ use candid::Principal; use dfx_core::canister::{build_wallet_canister, install_canister_wasm, install_mode_to_prompt}; use dfx_core::cli::ask_for_consent; use dfx_core::config::model::canister_id_store::CanisterIdStore; -use dfx_core::config::model::network_descriptor::NetworkDescriptor; use dfx_core::identity::CallSender; use fn_error_context::context; use ic_agent::Agent; @@ -28,7 +27,9 @@ use ic_utils::Argument; use itertools::Itertools; use sha2::{Digest, Sha256}; use slog::{debug, info, warn}; +use std::borrow::Cow; use std::collections::HashSet; +use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -46,7 +47,7 @@ pub async fn install_canister( mode: Option, call_sender: &CallSender, upgrade_unchanged: bool, - pool: Option<&CanisterPool>, + _pool: Option<&CanisterPool>, // TODO: Remove. skip_consent: bool, env_file: Option<&Path>, no_asset_upgrade: bool, @@ -255,15 +256,30 @@ The command line value will be used.", info!(log, "Uploading assets to asset canister..."); post_install_store_assets(canister_info, agent, log).await?; } + let tmp; + let pool2 = { + let config = env.get_config_or_anyhow()?; + let canisters_to_load = all_project_canisters_with_ids(env, &config); + + tmp = CanisterPool::load(env, false, &canisters_to_load) + .context("Error collecting canisters for post-install task")?; + &tmp + }; + // TODO: Use scanned dependencies instead. + let dependencies = pool2 + .get_canister_list() + .iter() + .map(|can| can.canister_id()) + .collect_vec(); + let vars: Vec<(Cow<'static, str>, Cow)> = get_and_write_environment_variables( + canister_info, + &network.name, + pool2, + dependencies.as_slice(), + env_file, + )?; if !canister_info.get_post_install().is_empty() { - let config = env.get_config()?; - run_post_install_tasks( - env, - canister_info, - network, - pool, - env_file.or_else(|| config.as_ref()?.get_config().output_env_file.as_deref()), - )?; + run_post_install_tasks(canister_info, &vars)?; } Ok(()) @@ -402,31 +418,11 @@ fn check_stable_compatibility( #[context("Failed to run post-install tasks")] fn run_post_install_tasks( - env: &dyn Environment, canister: &CanisterInfo, - network: &NetworkDescriptor, - pool: Option<&CanisterPool>, - env_file: Option<&Path>, + vars: &Vec<(Cow<'static, str>, Cow)>, ) -> DfxResult { - let tmp; - let pool = match pool { - Some(pool) => pool, - None => { - let config = env.get_config_or_anyhow()?; - let canisters_to_load = all_project_canisters_with_ids(env, &config); - - tmp = CanisterPool::load(env, false, &canisters_to_load) - .context("Error collecting canisters for post-install task")?; - &tmp - } - }; - let dependencies = pool - .get_canister_list() - .iter() - .map(|can| can.canister_id()) - .collect_vec(); for task in canister.get_post_install() { - run_post_install_task(canister, task, network, pool, &dependencies, env_file)?; + run_post_install_task(canister, task, vars)?; } Ok(()) } @@ -435,10 +431,7 @@ fn run_post_install_tasks( fn run_post_install_task( canister: &CanisterInfo, task: &str, - network: &NetworkDescriptor, - pool: &CanisterPool, - dependencies: &[Principal], - env_file: Option<&Path>, + vars: &Vec<(Cow<'static, str>, Cow)>, ) -> DfxResult { let cwd = canister.get_workspace_root(); let words = shell_words::split(task) @@ -448,10 +441,8 @@ fn run_post_install_task( .map_err(|_| anyhow!("Cannot find command or file {}", &words[0]))?; let mut command = Command::new(canonicalized); command.args(&words[1..]); - let vars = - get_and_write_environment_variables(canister, &network.name, pool, dependencies, env_file)?; for (key, val) in vars { - command.env(&*key, val); + command.env(&**key, val); } command .current_dir(cwd)