From 93bf4b7d39cca18d55615b5d5289b657db7463ab Mon Sep 17 00:00:00 2001 From: Prithvi Shahi <50885601+p-shahi@users.noreply.github.com> Date: Tue, 1 Sep 2020 17:55:27 -0700 Subject: [PATCH] feat: build granularity (#974) - `dfx build` now optionally takes a `canister_name` and optionally takes `--all` (`--all` and `canister_name` are mutually exclusive) - builds direct and transitive dependencies specified in `dfx.json` i.e. if dfx.json has `"dependencies": ["{canister_B}"]` for a `canister_A`, `dfx build canister_A` builds `canister_A` and `canister_B` # Breaking Changes - removes `dfx build --skip-frontend` as it is no longer required --- .../transitive_deps_canisters/a/main.mo | 5 + .../transitive_deps_canisters/b/main.mo | 5 + .../transitive_deps_canisters/c/main.mo | 5 + .../transitive_deps_canisters/d/main.mo | 5 + .../assets/transitive_deps_canisters/dfx.json | 38 ++++++ .../transitive_deps_canisters/e/main.mo | 5 + .../transitive_deps_canisters/patch.bash | 1 + e2e/bats/build_granular.bash | 96 +++++++++++++++ e2e/bats/create.bash | 2 +- e2e/bats/frontend.bash | 4 +- src/dfx/src/commands/build.rs | 28 +++-- src/dfx/src/lib/builders/assets.rs | 73 +++++------ src/dfx/src/lib/builders/mod.rs | 9 -- src/dfx/src/lib/message.rs | 3 +- src/dfx/src/lib/models/canister.rs | 115 ++++++++++++++---- 15 files changed, 304 insertions(+), 90 deletions(-) create mode 100644 e2e/bats/assets/transitive_deps_canisters/a/main.mo create mode 100644 e2e/bats/assets/transitive_deps_canisters/b/main.mo create mode 100644 e2e/bats/assets/transitive_deps_canisters/c/main.mo create mode 100644 e2e/bats/assets/transitive_deps_canisters/d/main.mo create mode 100644 e2e/bats/assets/transitive_deps_canisters/dfx.json create mode 100644 e2e/bats/assets/transitive_deps_canisters/e/main.mo create mode 100644 e2e/bats/assets/transitive_deps_canisters/patch.bash create mode 100644 e2e/bats/build_granular.bash diff --git a/e2e/bats/assets/transitive_deps_canisters/a/main.mo b/e2e/bats/assets/transitive_deps_canisters/a/main.mo new file mode 100644 index 0000000000..29d2bfc68f --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/a/main.mo @@ -0,0 +1,5 @@ +actor { + public func greet(name : Text) : async Text { + return "Namaste, " # name # "!"; + }; +}; diff --git a/e2e/bats/assets/transitive_deps_canisters/b/main.mo b/e2e/bats/assets/transitive_deps_canisters/b/main.mo new file mode 100644 index 0000000000..7decd559a3 --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/b/main.mo @@ -0,0 +1,5 @@ +actor { + public func greet(name : Text) : async Text { + return "Hola, " # name # "!"; + }; +}; diff --git a/e2e/bats/assets/transitive_deps_canisters/c/main.mo b/e2e/bats/assets/transitive_deps_canisters/c/main.mo new file mode 100644 index 0000000000..59e65defaa --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/c/main.mo @@ -0,0 +1,5 @@ +actor { + public func greet(name : Text) : async Text { + return "Hello, " # name # "!"; + }; +}; diff --git a/e2e/bats/assets/transitive_deps_canisters/d/main.mo b/e2e/bats/assets/transitive_deps_canisters/d/main.mo new file mode 100644 index 0000000000..59e65defaa --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/d/main.mo @@ -0,0 +1,5 @@ +actor { + public func greet(name : Text) : async Text { + return "Hello, " # name # "!"; + }; +}; diff --git a/e2e/bats/assets/transitive_deps_canisters/dfx.json b/e2e/bats/assets/transitive_deps_canisters/dfx.json new file mode 100644 index 0000000000..eef29a38cd --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/dfx.json @@ -0,0 +1,38 @@ +{ + "canisters": { + "canister_a": { + "main": "./a/main.mo", + "type": "motoko" + }, + "canister_b": { + "dependencies": ["canister_a"], + "main": "./b/main.mo", + "type": "motoko" + }, + "canister_c": { + "dependencies": ["canister_b"], + "main": "./c/main.mo", + "type": "motoko" + }, + "canister_d": { + "dependencies": ["canister_e"], + "main": "./d/main.mo", + "type": "motoko" + }, + "canister_e": { + "dependencies": ["canister_d"], + "main": "./e/main.mo", + "type": "motoko" + } + }, + "defaults": { + "build": { + "packtool": "" + } + }, + "networks": { + "local": { + "bind": "127.0.0.1:8000" + } + } +} diff --git a/e2e/bats/assets/transitive_deps_canisters/e/main.mo b/e2e/bats/assets/transitive_deps_canisters/e/main.mo new file mode 100644 index 0000000000..59e65defaa --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/e/main.mo @@ -0,0 +1,5 @@ +actor { + public func greet(name : Text) : async Text { + return "Hello, " # name # "!"; + }; +}; diff --git a/e2e/bats/assets/transitive_deps_canisters/patch.bash b/e2e/bats/assets/transitive_deps_canisters/patch.bash new file mode 100644 index 0000000000..525625eedc --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/patch.bash @@ -0,0 +1 @@ +# Do nothing diff --git a/e2e/bats/build_granular.bash b/e2e/bats/build_granular.bash new file mode 100644 index 0000000000..a56e6d4dda --- /dev/null +++ b/e2e/bats/build_granular.bash @@ -0,0 +1,96 @@ +#!/usr/bin/env bats + +load utils/_ + +setup() { + # We want to work from a temporary directory, different for every test. + cd $(mktemp -d -t dfx-e2e-XXXXXXXX) + + dfx_new +} + +teardown() { + dfx_stop +} + +@test "direct dependencies are built" { + dfx_start + dfx canister create --all + #specify build for only assets_canister + dfx build e2e_project_assets + + #validate direct dependency built and is callable + assert_command dfx canister install e2e_project + assert_command dfx canister call e2e_project greet World +} + +@test "transitive dependencies are built" { + install_asset transitive_deps_canisters + dfx_start + dfx canister create --all + #install of tertiary dependency canister will fail since its not built + assert_command_fail dfx canister install canister_a + #specify build for primary canister + dfx build canister_c + + #validate tertiary transitive dependency is built and callable + assert_command dfx canister install canister_a + assert_command dfx canister call canister_a greet World + assert_match '("Namaste, World!")' +} + +@test "unspecified dependencies are not built" { + dfx_start + dfx canister create --all + # only build motoko canister + dfx build e2e_project + # validate assets canister wasn't built and can't be installed + assert_command_fail dfx canister install e2e_project_assets + assert_match "No such file or directory" +} + + +@test "manual build of specified canisters succeeds" { + install_asset assetscanister + + dfx_start + dfx canister create e2e_project + dfx build e2e_project + assert_command dfx canister install e2e_project + assert_command dfx canister call e2e_project greet World + + assert_command_fail dfx canister install e2e_project_assets + assert_match "Cannot find canister id. Please issue 'dfx canister create e2e_project_assets'." + dfx canister create e2e_project_assets + dfx build e2e_project_assets + dfx canister install e2e_project_assets + + assert_command dfx canister call --query e2e_project_assets retrieve '("binary/noise.txt")' + assert_eq '(vec { 184; 1; 32; 128; 10; 119; 49; 50; 32; 0; 120; 121; 10; 75; 76; 11; 10; 106; 107; })' + + assert_command dfx canister call --query e2e_project_assets retrieve '("text-with-newlines.txt")' + assert_eq '(vec { 99; 104; 101; 114; 114; 105; 101; 115; 10; 105; 116; 39; 115; 32; 99; 104; 101; 114; 114; 121; 32; 115; 101; 97; 115; 111; 110; 10; 67; 72; 69; 82; 82; 73; 69; 83; })' + +} + +@test "cyclic dependencies are detected" { + install_asset transitive_deps_canisters + dfx_start + dfx canister create --all + assert_command dfx build canister_e + assert_match "Possible circular dependency detected during evaluation of canister_d's dependency on canister_e." +} + +@test "the all flag builds everything" { + dfx_start + dfx canister create --all + assert_command dfx build --all + assert_command dfx canister install --all +} + + +@test "the all flags conflicts with canister name" { + dfx_start + dfx canister create --all + assert_command_fail dfx build e2e_project --all +} diff --git a/e2e/bats/create.bash b/e2e/bats/create.bash index 00c289ec78..780dfe7a23 100644 --- a/e2e/bats/create.bash +++ b/e2e/bats/create.bash @@ -28,7 +28,7 @@ teardown() { @test "build fails without create" { dfx_start assert_command_fail dfx build - assert_match "Cannot find canister id. Please issue 'dfx canister create e2e_project'" + assert_match "Cannot find canister id." } @test "build fails if all canisters in project are not created" { diff --git a/e2e/bats/frontend.bash b/e2e/bats/frontend.bash index c242347596..9a37e9b5ab 100644 --- a/e2e/bats/frontend.bash +++ b/e2e/bats/frontend.bash @@ -16,7 +16,7 @@ teardown() { @test "dfx start serves a frontend" { dfx_start dfx canister create --all - dfx build --skip-frontend + dfx build e2e_project sleep 1 assert_command curl http://localhost:8000 # 8000 = default port. @@ -31,7 +31,7 @@ teardown() { cat <<<$(jq '.networks.local.bind="127.0.0.1:12345"' dfx.json) >dfx.json dfx canister create --all - dfx build --skip-frontend + dfx build e2e_project assert_command curl http://localhost:12345 # 8000 = default port. assert_match "" diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index c162a60c0f..a23b4eb482 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -11,10 +11,18 @@ pub fn construct() -> App<'static, 'static> { SubCommand::with_name("build") .about(UserMessage::BuildCanister.to_str()) .arg( - Arg::with_name("skip-frontend") - .long("skip-frontend") - .takes_value(false) - .help(UserMessage::SkipFrontend.to_str()), + Arg::with_name("canister_name") + .takes_value(true) + .conflicts_with("all") + .help(UserMessage::BuildCanisterName.to_str()) + .required(false), + ) + .arg( + Arg::with_name("all") + .long("all") + .conflicts_with("canister_name") + .help(UserMessage::BuildAll.to_str()) + .takes_value(false), ) .arg( Arg::with_name("check") @@ -45,8 +53,12 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { env.get_cache().install()?; let build_mode_check = args.is_present("check"); - // First build. - let canister_pool = CanisterPool::load(&env, build_mode_check)?; + + // Option can be None in which case --all was specified + let some_canister = args.value_of("canister_name"); + + // Get pool of canisters to build + let canister_pool = CanisterPool::load(&env, build_mode_check, some_canister)?; // Create canisters on the replica and associate canister ids locally. if args.is_present("check") { @@ -66,9 +78,7 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { slog::info!(logger, "Building canisters..."); canister_pool.build_or_fail( - BuildConfig::from_config(&config)? - .with_skip_frontend(args.is_present("skip-frontend")) - .with_build_mode_check(build_mode_check), + BuildConfig::from_config(&config)?.with_build_mode_check(build_mode_check), )?; Ok(()) diff --git a/src/dfx/src/lib/builders/assets.rs b/src/dfx/src/lib/builders/assets.rs index 9807c30c1f..5b6b93fec9 100644 --- a/src/dfx/src/lib/builders/assets.rs +++ b/src/dfx/src/lib/builders/assets.rs @@ -104,43 +104,36 @@ impl CanisterBuilder for AssetsBuilder { info: &CanisterInfo, config: &BuildConfig, ) -> DfxResult { - if !config.skip_frontend { - let deps = match info.get_extra_value("dependencies") { - None => vec![], - Some(v) => Vec::::deserialize(v).map_err(|_| { - DfxError::Unknown(String::from("Field 'dependencies' is of the wrong type")) - })?, - }; - let dependencies = deps - .iter() - .map(|name| { - pool.get_first_canister_with_name(name) - .map(|c| c.canister_id()) - .map_or_else( - || Err(DfxError::UnknownCanisterNamed(name.clone())), - DfxResult::Ok, - ) - }) - .collect::>>()?; - - build_frontend( - pool.get_logger(), - info.get_workspace_root(), - &config.network_name, - dependencies, - pool, - )?; - } + let deps = match info.get_extra_value("dependencies") { + None => vec![], + Some(v) => Vec::::deserialize(v).map_err(|_| { + DfxError::Unknown(String::from("Field 'dependencies' is of the wrong type")) + })?, + }; + let dependencies = deps + .iter() + .map(|name| { + pool.get_first_canister_with_name(name) + .map(|c| c.canister_id()) + .map_or_else( + || Err(DfxError::UnknownCanisterNamed(name.clone())), + DfxResult::Ok, + ) + }) + .collect::>>()?; - let assets_canister_info = info.as_info::()?; - if !config.skip_frontend { - assets_canister_info.assert_source_paths()?; - } - copy_assets( + build_frontend( pool.get_logger(), - &assets_canister_info, - config.skip_frontend, + info.get_workspace_root(), + &config.network_name, + dependencies, + pool, )?; + + let assets_canister_info = info.as_info::()?; + assets_canister_info.assert_source_paths()?; + + copy_assets(pool.get_logger(), &assets_canister_info)?; Ok(()) } } @@ -170,20 +163,16 @@ fn delete_output_directory( Ok(()) } -fn copy_assets( - logger: &slog::Logger, - assets_canister_info: &AssetsCanisterInfo, - skip_frontend: bool, -) -> DfxResult { +fn copy_assets(logger: &slog::Logger, assets_canister_info: &AssetsCanisterInfo) -> DfxResult { let source_paths = assets_canister_info.get_source_paths(); let output_assets_path = assets_canister_info.get_output_assets_path(); for source_path in source_paths { - // If we skip-frontend and the source doesn't exist, we ignore it. - if skip_frontend && !source_path.exists() { + // If the source doesn't exist, we ignore it. + if !source_path.exists() { slog::warn!( logger, - r#"Skip copying "{}" because --skip-frontend was used."#, + r#"Source path "{}" does not exist."#, source_path.to_string_lossy() ); diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index 0fbe176f1d..f5ec0daa53 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -78,7 +78,6 @@ pub trait CanisterBuilder { #[derive(Clone)] pub struct BuildConfig { profile: Profile, - pub skip_frontend: bool, pub build_mode_check: bool, pub network_name: String, @@ -98,20 +97,12 @@ impl BuildConfig { Ok(BuildConfig { network_name, profile: config_intf.profile.unwrap_or(Profile::Debug), - skip_frontend: false, build_mode_check: false, build_root: build_root.clone(), idl_root: build_root.join("idl/"), }) } - pub fn with_skip_frontend(self, skip_frontend: bool) -> Self { - Self { - skip_frontend, - ..self - } - } - pub fn with_build_mode_check(self, build_mode_check: bool) -> Self { Self { build_mode_check, diff --git a/src/dfx/src/lib/message.rs b/src/dfx/src/lib/message.rs index 5da12a0f34..6ec9504804 100644 --- a/src/dfx/src/lib/message.rs +++ b/src/dfx/src/lib/message.rs @@ -88,8 +88,9 @@ user_message!( RequestId => "Specifies the request identifier. The request identifier is an hexadecimal string starting with 0x.", // dfx build + BuildAll => "Builds all canisters configured in dfx.json.", + BuildCanisterName => "Specifies the canister name. Either this or the --all flag are required.", BuildCanister => "Builds all or specific canisters from the code in your project. By default, all canisters are built.", - SkipFrontend => "Skip building the frontend, only build the canisters.", BuildCheck => "Build canisters without creating them. This can be used to check that canisters build ok.", CanisterComputeNetwork => "Override the compute network to connect to. By default uses the local network.", diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 4c4ac34d9b..622f27df79 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -1,4 +1,5 @@ use crate::config::cache::Cache; +use crate::config::dfinity::Config; use crate::lib::builders::{ BuildConfig, BuildOutput, BuilderPool, CanisterBuilder, IdlBuildOutput, WasmBuildOutput, }; @@ -9,7 +10,8 @@ use crate::lib::models::canister_id_store::CanisterIdStore; use crate::util::assets; use ic_types::principal::Principal as CanisterId; use petgraph::graph::{DiGraph, NodeIndex}; -use rand::{thread_rng, RngCore}; +use rand::{thread_rng, Rng, RngCore}; +use serde::Deserialize; use slog::Logger; use std::cell::RefCell; use std::collections::BTreeMap; @@ -90,41 +92,103 @@ pub struct CanisterPool { cache: Arc, } +struct PoolConstructHelper<'a> { + config: &'a Config, + builder_pool: BuilderPool, + canister_id_store: CanisterIdStore, + generate_cid: bool, + canisters_map: &'a mut Vec>, + visited_map: BTreeMap, + logger: &'a Logger, +} + impl CanisterPool { + fn insert(canister_name: &str, pool_helper: &mut PoolConstructHelper<'_>) -> DfxResult<()> { + let canister_id = match pool_helper.canister_id_store.find(canister_name) { + Some(canister_id) => Some(canister_id), + None if pool_helper.generate_cid => Some(Canister::generate_random_canister_id()?), + _ => None, + }; + let info = CanisterInfo::load(pool_helper.config, canister_name, canister_id)?; + + if let Some(builder) = pool_helper.builder_pool.get(&info) { + pool_helper + .canisters_map + .insert(0, Arc::new(Canister::new(info, builder))); + pool_helper + .visited_map + .insert(canister_name.to_owned(), thread_rng().gen::()); + Ok(()) + } else { + Err(DfxError::CouldNotFindBuilderForCanister( + info.get_name().to_string(), + )) + } + } + + fn insert_with_dependencies( + canister_name: &str, + pool_helper: &mut PoolConstructHelper<'_>, + ) -> DfxResult<()> { + //insert this canister + CanisterPool::insert(canister_name, pool_helper)?; + + // recursively fetch direct and transitive dependencies + let canister_id = pool_helper.canister_id_store.get(canister_name)?; + let info = CanisterInfo::load(pool_helper.config, canister_name, Some(canister_id))?; + let deps = match info.get_extra_value("dependencies") { + None => vec![], + Some(v) => Vec::::deserialize(v).map_err(|_| { + DfxError::Unknown(String::from("Field 'dependencies' is of the wrong type")) + })?, + }; + + for canister in deps.iter() { + if !pool_helper.visited_map.contains_key(&canister.to_owned()) { + CanisterPool::insert_with_dependencies(canister, pool_helper)?; + } else { + slog::warn!( + pool_helper.logger, + "Possible circular dependency detected during evaluation of {}'s dependency on {}.", + &canister_name.to_owned(), + &canister.to_owned(), + ); + } + } + Ok(()) + } + pub fn load( env: &dyn Environment, - provide_random_canister_id_if_missing: bool, + generate_cid: bool, + some_canister: Option<&str>, ) -> DfxResult { let logger = env.get_logger().new(slog::o!()); let config = env .get_config() .ok_or(DfxError::CommandMustBeRunInAProject)?; - let canisters = config.get_config().canisters.as_ref().ok_or_else(|| { - DfxError::Unknown("No canisters in the configuration file.".to_string()) - })?; - let builder_pool = BuilderPool::new(env)?; let mut canisters_map = Vec::new(); - let canister_id_store = CanisterIdStore::for_env(env)?; - - for (key, _value) in canisters.iter() { - let canister_id = match canister_id_store.find(key) { - Some(canister_id) => Some(canister_id), - None if provide_random_canister_id_if_missing => { - Some(Canister::generate_random_canister_id()?) - } - _ => None, - }; - - let info = CanisterInfo::load(&config, &key, canister_id)?; - - if let Some(builder) = builder_pool.get(&info) { - canisters_map.push(Arc::new(Canister::new(info, builder))); - } else { - return Err(DfxError::CouldNotFindBuilderForCanister( - info.get_name().to_string(), - )); + let mut pool_helper = PoolConstructHelper { + config: &config, + builder_pool: BuilderPool::new(env)?, + canister_id_store: CanisterIdStore::for_env(env)?, + generate_cid, + canisters_map: &mut canisters_map, + visited_map: BTreeMap::new(), + logger: env.get_logger(), + }; + + if let Some(canister_name) = some_canister { + CanisterPool::insert_with_dependencies(canister_name, &mut pool_helper)?; + } else { + // insert all canisters configured in dfx.json + let canisters = config.get_config().canisters.as_ref().ok_or_else(|| { + DfxError::Unknown("No canisters in the configuration file.".to_string()) + })?; + for (key, _value) in canisters.iter() { + CanisterPool::insert(key, &mut pool_helper)?; } } @@ -296,7 +360,6 @@ impl CanisterPool { &self, build_config: BuildConfig, ) -> DfxResult>> { - // check for canister ids before building self.step_prebuild_all(&build_config).map_err(|e| { DfxError::BuildError(BuildErrorKind::PrebuildAllStepFailed(Box::new(e))) })?;