From ed07098fb5be682d705ebefdc26b2d9a6d360ee4 Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Tue, 25 Aug 2020 12:38:55 -0700 Subject: [PATCH 1/9] Initial changes --- e2e/bats/assetscanister.bash | 2 +- e2e/bats/base.bash | 4 +- e2e/bats/basic-project.bash | 8 ++-- e2e/bats/bootstrap.bash | 2 +- e2e/bats/build.bash | 26 +++++----- e2e/bats/call.bash | 2 +- e2e/bats/create.bash | 4 +- e2e/bats/frontend.bash | 4 +- e2e/bats/id.bash | 2 +- e2e/bats/identity.bash | 2 +- e2e/bats/install.bash | 6 +-- e2e/bats/lifecycle.bash | 2 +- e2e/bats/network.bash | 4 +- e2e/bats/packtool.bash | 10 ++-- e2e/bats/print.bash | 2 +- e2e/bats/provider.bash | 2 +- e2e/bats/usage.bash | 4 +- src/dfx/src/commands/build.rs | 21 ++++++++- src/dfx/src/lib/models/canister.rs | 76 +++++++++++++++++++++--------- 19 files changed, 116 insertions(+), 67 deletions(-) diff --git a/e2e/bats/assetscanister.bash b/e2e/bats/assetscanister.bash index 99d0775538..55c93b66bf 100644 --- a/e2e/bats/assetscanister.bash +++ b/e2e/bats/assetscanister.bash @@ -18,7 +18,7 @@ teardown() { dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install e2e_project_assets assert_command dfx canister call --query e2e_project_assets retrieve '("binary/noise.txt")' diff --git a/e2e/bats/base.bash b/e2e/bats/base.bash index 4143532fa0..eaee6012a9 100644 --- a/e2e/bats/base.bash +++ b/e2e/bats/base.bash @@ -18,7 +18,7 @@ teardown() { dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install e2e_project assert_command dfx canister call --query e2e_project is_digit '("5")' @@ -34,6 +34,6 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match 'import error, package "base" not defined' } diff --git a/e2e/bats/basic-project.bash b/e2e/bats/basic-project.bash index b84fe66b27..afe30bff7f 100644 --- a/e2e/bats/basic-project.bash +++ b/e2e/bats/basic-project.bash @@ -18,7 +18,7 @@ teardown() { install_asset greet dfx_start dfx canister create --all - dfx build + dfx build --all # INSTALL_REQUEST_ID=$(dfx canister install hello --async) # dfx canister request-status $INSTALL_REQUEST_ID dfx canister install hello @@ -43,7 +43,7 @@ teardown() { install_asset counter dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install hello assert_command dfx canister call hello read @@ -87,7 +87,7 @@ teardown() { install_asset counter_idl dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install --all assert_command dfx canister call hello inc '(42,false,"testzZ",vec{1;2;3},opt record{head=42; tail=opt record{head=+43; tail=null}}, variant { cons=record{ 42; variant { cons=record{43; variant { nil }} } } })' @@ -98,7 +98,7 @@ teardown() { install_asset matrix_multiply dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install --all assert_command dfx canister call hello multiply '(vec{vec{1;2};vec{3;4};vec{5;6}},vec{vec{1;2;3};vec{4;5;6}})' diff --git a/e2e/bats/bootstrap.bash b/e2e/bats/bootstrap.bash index 8a08648aac..2f0b6e83f2 100644 --- a/e2e/bats/bootstrap.bash +++ b/e2e/bats/bootstrap.bash @@ -15,7 +15,7 @@ teardown() { @test "bootstrap fetches candid file" { dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install hello ID=$(dfx canister id hello) diff --git a/e2e/bats/build.bash b/e2e/bats/build.bash index c6a8676897..c0e61cbf66 100644 --- a/e2e/bats/build.bash +++ b/e2e/bats/build.bash @@ -18,7 +18,7 @@ teardown() { install_asset invalid dfx_start dfx canister create --all - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match "syntax error" } @@ -26,7 +26,7 @@ teardown() { install_asset import dfx_start dfx canister create --all - assert_command dfx build + assert_command dfx build --all dfx canister install --all assert_command dfx canister call e2e_project greet World assert_match "10World" @@ -35,7 +35,7 @@ teardown() { @test "build succeeds on default project" { dfx_start dfx canister create --all - assert_command dfx build + assert_command dfx build --all } # TODO: Before Tungsten, we need to update this test for code with inter-canister calls. @@ -43,9 +43,9 @@ teardown() { @test "build twice produces the same wasm binary" { dfx_start dfx canister create --all - assert_command dfx build + assert_command dfx build --all cp .dfx/local/canisters/e2e_project/e2e_project.wasm ./old.wasm - assert_command dfx build + assert_command dfx build --all assert_command diff .dfx/local/canisters/e2e_project/e2e_project.wasm ./old.wasm } @@ -53,7 +53,7 @@ teardown() { install_asset warning dfx_start dfx canister create --all - assert_command dfx build + assert_command dfx build --all assert_match "warning, this pattern consuming type" } @@ -61,7 +61,7 @@ teardown() { install_asset import_error dfx_start dfx canister create --all - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match 'import error, canister alias "random" not defined' } @@ -69,7 +69,7 @@ teardown() { dfx_start dfx config canisters.e2e_project.type unknown_canister_type dfx canister create --all - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match "CouldNotFindBuilderForCanister" } @@ -77,7 +77,7 @@ teardown() { dfx_start install_asset custom_canister dfx canister create --all - assert_command dfx build + assert_command dfx build --all assert_match "CUSTOM_CANISTER_BUILD_DONE" dfx canister install --all @@ -87,7 +87,7 @@ teardown() { @test "build succeeds with network parameter" { dfx_start dfx canister --network local create --all - assert_command dfx build --network local + assert_command dfx build --network local --all } @test "build succeeds when requested network is configured" { @@ -95,13 +95,13 @@ teardown() { assert_command dfx config networks.tungsten.providers '[ "http://127.0.0.1:8000" ]' assert_command dfx canister --network tungsten create --all - assert_command dfx build --network tungsten + assert_command dfx build --network tungsten --all } @test "build output for local network is in expected directory" { dfx_start dfx canister create --all - assert_command dfx build + assert_command dfx build --all assert_command ls .dfx/local/canisters/e2e_project/ assert_command ls .dfx/local/canisters/e2e_project/e2e_project.wasm } @@ -110,7 +110,7 @@ teardown() { dfx_start assert_command dfx config networks.tungsten.providers '[ "http://127.0.0.1:8000" ]' dfx canister --network tungsten create --all - assert_command dfx build --network tungsten + assert_command dfx build --network tungsten --all assert_command ls .dfx/tungsten/canisters/e2e_project/ assert_command ls .dfx/tungsten/canisters/e2e_project/e2e_project.wasm } diff --git a/e2e/bats/call.bash b/e2e/bats/call.bash index 1005a44cc0..adcfb882e9 100644 --- a/e2e/bats/call.bash +++ b/e2e/bats/call.bash @@ -15,7 +15,7 @@ teardown() { install_asset greet dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install hello assert_command dfx canister call $(dfx canister id hello) greet '("Names are difficult")' assert_match '("Hello, Names are difficult!")' diff --git a/e2e/bats/create.bash b/e2e/bats/create.bash index 00c289ec78..936b19d490 100644 --- a/e2e/bats/create.bash +++ b/e2e/bats/create.bash @@ -27,14 +27,14 @@ teardown() { @test "build fails without create" { dfx_start - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match "Cannot find canister id. Please issue 'dfx canister create e2e_project'" } @test "build fails if all canisters in project are not created" { dfx_start assert_command dfx canister create e2e_project - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match "Cannot find canister id. Please issue 'dfx canister create e2e_project_assets'" } diff --git a/e2e/bats/frontend.bash b/e2e/bats/frontend.bash index c242347596..ad633d8e7e 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 --skip-frontend --all 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 --skip-frontend --all assert_command curl http://localhost:12345 # 8000 = default port. assert_match "" diff --git a/e2e/bats/id.bash b/e2e/bats/id.bash index 03f58a2740..794563e94b 100644 --- a/e2e/bats/id.bash +++ b/e2e/bats/id.bash @@ -15,7 +15,7 @@ teardown() { install_asset id dfx_start dfx canister create --all - dfx build + dfx build --all assert_command dfx canister id e2e_project assert_match $(cat .dfx/local/canister_ids.json | jq -r .e2e_project.local) } diff --git a/e2e/bats/identity.bash b/e2e/bats/identity.bash index 1d6aa0dec4..de36f2af5f 100644 --- a/e2e/bats/identity.bash +++ b/e2e/bats/identity.bash @@ -17,7 +17,7 @@ teardown() { install_asset identity dfx_start dfx canister create --all - assert_command dfx build + assert_command dfx build --all assert_command dfx canister install --all ID_CALL=$(dfx canister call e2e_project fromCall) diff --git a/e2e/bats/install.bash b/e2e/bats/install.bash index a1354a2ca3..c7bc45beef 100644 --- a/e2e/bats/install.bash +++ b/e2e/bats/install.bash @@ -26,7 +26,7 @@ teardown() { @test "install succeeds when --all is provided" { dfx_start dfx canister create --all - dfx build + dfx build --all assert_command dfx canister install --all @@ -36,7 +36,7 @@ teardown() { @test "install succeeds with network name" { dfx_start dfx canister create --all - dfx build + dfx build --all assert_command dfx canister --network local install --all @@ -46,7 +46,7 @@ teardown() { @test "install fails with network name that is not in dfx.json" { dfx_start dfx canister create --all - dfx build + dfx build --all assert_command_fail dfx canister --network nosuch install --all diff --git a/e2e/bats/lifecycle.bash b/e2e/bats/lifecycle.bash index 95190732ff..e759c253a4 100644 --- a/e2e/bats/lifecycle.bash +++ b/e2e/bats/lifecycle.bash @@ -15,7 +15,7 @@ teardown() { install_asset greet dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install hello assert_command dfx canister status hello assert_match "Canister hello's status is Running." diff --git a/e2e/bats/network.bash b/e2e/bats/network.bash index 9a3abe0f6e..0cadab3e5d 100644 --- a/e2e/bats/network.bash +++ b/e2e/bats/network.bash @@ -64,7 +64,7 @@ teardown() { @test "failure message does not include network if for local network" { dfx_start - assert_command_fail dfx build --network local + assert_command_fail dfx build --network local --all assert_match "Cannot find canister id. Please issue 'dfx canister create e2e_project" } @@ -73,6 +73,6 @@ teardown() { assert_command dfx config networks.tungsten.providers '[ "http://127.0.0.1:8000" ]' - assert_command_fail dfx build --network tungsten + assert_command_fail dfx build --network tungsten --all assert_match "Cannot find canister id. Please issue 'dfx canister --network tungsten create e2e_project" } diff --git a/e2e/bats/packtool.bash b/e2e/bats/packtool.bash index 7e5cbd8256..457ff7bad6 100644 --- a/e2e/bats/packtool.bash +++ b/e2e/bats/packtool.bash @@ -18,7 +18,7 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match 'import error, package "(rate|describe)" not defined' } @@ -28,7 +28,7 @@ teardown() { dfx_start dfx canister create --all - dfx build + dfx build --all } @test "project calls dependencies made available by packtool" { @@ -37,7 +37,7 @@ teardown() { dfx_start dfx canister create --all - dfx build + dfx build --all dfx canister install e2e_project assert_command dfx canister call e2e_project rate '("rust")' @@ -53,7 +53,7 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match 'Failed to invoke the package tool' assert_match 'no-such-command.*that.*command.*cannot.*be.*invoked' assert_match 'No such file or directory \(os error 2\)' @@ -65,7 +65,7 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match 'Package tool.*reported an error' assert_match 'sh.*command-that-fails.bash' assert_match 'exit code: 3' diff --git a/e2e/bats/print.bash b/e2e/bats/print.bash index 974897a6e4..fad8611c25 100644 --- a/e2e/bats/print.bash +++ b/e2e/bats/print.bash @@ -19,7 +19,7 @@ teardown() { install_asset print dfx_start 2>stderr.txt dfx canister create --all - dfx build + dfx build --all dfx canister install e2e_project dfx canister call e2e_project hello run cat stderr.txt diff --git a/e2e/bats/provider.bash b/e2e/bats/provider.bash index 7a3ff58258..792f0becf3 100644 --- a/e2e/bats/provider.bash +++ b/e2e/bats/provider.bash @@ -19,7 +19,7 @@ teardown() { # flakiness (replica start time). @test "provider flag can be passed in" { skip "refactor this test to replace client.bash" - dfx build + dfx build --all # Start a replica manually on a specific port. $(dfx cache show)/replica --config ' diff --git a/e2e/bats/usage.bash b/e2e/bats/usage.bash index 0e7b6ff32e..714a6d2405 100644 --- a/e2e/bats/usage.bash +++ b/e2e/bats/usage.bash @@ -24,12 +24,12 @@ teardown() { # Make sure we're in an empty directory. cd $(mktemp -d -t dfx-e2e-XXXXXXXX) - assert_command_fail dfx build + assert_command_fail dfx build --all assert_match "must be run in a project" dfx new t --no-frontend cd t dfx_start dfx canister create --all - assert_command dfx build + assert_command dfx build --all } diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index c162a60c0f..723f8b95a7 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -10,6 +10,19 @@ use clap::{App, Arg, ArgMatches, SubCommand}; pub fn construct() -> App<'static, 'static> { SubCommand::with_name("build") .about(UserMessage::BuildCanister.to_str()) + .arg( + Arg::with_name("canister_name") + .takes_value(true) + .help(UserMessage::CreateCanisterName.to_str()) + .required(false), + ) + .arg( + Arg::with_name("all") + .long("all") + .required_unless("canister_name") + .help(UserMessage::CreateAll.to_str()) + .takes_value(false), + ) .arg( Arg::with_name("skip-frontend") .long("skip-frontend") @@ -45,8 +58,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 whole pool of canisters configured in dfx.json + 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") { diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 4c4ac34d9b..9a25606f34 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -10,6 +10,7 @@ use crate::util::assets; use ic_types::principal::Principal as CanisterId; use petgraph::graph::{DiGraph, NodeIndex}; use rand::{thread_rng, RngCore}; +use serde::Deserialize; use slog::Logger; use std::cell::RefCell; use std::collections::BTreeMap; @@ -91,40 +92,70 @@ pub struct CanisterPool { } impl CanisterPool { + fn insert( + env: &dyn Environment, + generate_cid: bool, + canister_name: &str, + canisters_map: &mut Vec>, + ) -> DfxResult<()> { + let builder_pool = BuilderPool::new(env)?; + let canister_id_store = CanisterIdStore::for_env(env)?; + let config = env + .get_config() + .ok_or(DfxError::CommandMustBeRunInAProject)?; + + let canister_id = match canister_id_store.find(canister_name) { + Some(canister_id) => Some(canister_id), + None if generate_cid => Some(Canister::generate_random_canister_id()?), + _ => None, + }; + let info = CanisterInfo::load(&config, canister_name, canister_id)?; + + if let Some(builder) = builder_pool.get(&info) { + canisters_map.insert(0, Arc::new(Canister::new(info, builder))); + Ok(()) + } else { + Err(DfxError::CouldNotFindBuilderForCanister( + info.get_name().to_string(), + )) + } + } + 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, + if let Some(canister_name) = some_canister { + CanisterPool::insert(env, generate_cid, canister_name, &mut canisters_map)?; + + // get direct dependencies + let canister_id = CanisterIdStore::for_env(env)?.get(canister_name)?; + let info = CanisterInfo::load(&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")) + })?, }; - 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(), - )); + for canister in deps.iter() { + CanisterPool::insert(env, generate_cid, canister, &mut canisters_map)?; + } + } 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(env, generate_cid, key, &mut canisters_map)?; } } @@ -297,6 +328,7 @@ impl CanisterPool { build_config: BuildConfig, ) -> DfxResult>> { // check for canister ids before building + // THIS DOES NOTHING self.step_prebuild_all(&build_config).map_err(|e| { DfxError::BuildError(BuildErrorKind::PrebuildAllStepFailed(Box::new(e))) })?; From 41ee453888a79f988dbdb861f8b133785355acec Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Mon, 31 Aug 2020 11:58:35 -0700 Subject: [PATCH 2/9] update test --- e2e/bats/create.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/bats/create.bash b/e2e/bats/create.bash index 936b19d490..2ea121133e 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 --all - 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" { From d525d5b9bdc107de73cff1e5345939847c3306d2 Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Mon, 31 Aug 2020 13:21:19 -0700 Subject: [PATCH 3/9] add e2e/bats/build-granular.bash --- e2e/bats/build-granular.bash | 60 ++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 e2e/bats/build-granular.bash diff --git a/e2e/bats/build-granular.bash b/e2e/bats/build-granular.bash new file mode 100644 index 0000000000..170311ee32 --- /dev/null +++ b/e2e/bats/build-granular.bash @@ -0,0 +1,60 @@ +#!/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 "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; })' + +} From 91f7d4cbce2d0a3fb8f3a3df5e5a048e78a243b3 Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Mon, 31 Aug 2020 13:36:37 -0700 Subject: [PATCH 4/9] remove --skip-frontend --- e2e/bats/frontend.bash | 4 ++-- src/dfx/src/commands/build.rs | 11 ++--------- src/dfx/src/lib/builders/assets.rs | 17 +++++++++-------- src/dfx/src/lib/builders/mod.rs | 16 ++++++++-------- src/dfx/src/lib/message.rs | 5 +++-- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/e2e/bats/frontend.bash b/e2e/bats/frontend.bash index ad633d8e7e..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 --all + 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 --all + 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 723f8b95a7..587d3904dc 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -13,22 +13,16 @@ pub fn construct() -> App<'static, 'static> { .arg( Arg::with_name("canister_name") .takes_value(true) - .help(UserMessage::CreateCanisterName.to_str()) + .help(UserMessage::BuildCanisterName.to_str()) .required(false), ) .arg( Arg::with_name("all") .long("all") .required_unless("canister_name") - .help(UserMessage::CreateAll.to_str()) + .help(UserMessage::BuildAll.to_str()) .takes_value(false), ) - .arg( - Arg::with_name("skip-frontend") - .long("skip-frontend") - .takes_value(false) - .help(UserMessage::SkipFrontend.to_str()), - ) .arg( Arg::with_name("check") .long("check") @@ -84,7 +78,6 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { canister_pool.build_or_fail( BuildConfig::from_config(&config)? - .with_skip_frontend(args.is_present("skip-frontend")) .with_build_mode_check(build_mode_check), )?; diff --git a/src/dfx/src/lib/builders/assets.rs b/src/dfx/src/lib/builders/assets.rs index 9807c30c1f..745c7064fd 100644 --- a/src/dfx/src/lib/builders/assets.rs +++ b/src/dfx/src/lib/builders/assets.rs @@ -104,7 +104,7 @@ impl CanisterBuilder for AssetsBuilder { info: &CanisterInfo, config: &BuildConfig, ) -> DfxResult { - if !config.skip_frontend { + // if !config.skip_frontend { let deps = match info.get_extra_value("dependencies") { None => vec![], Some(v) => Vec::::deserialize(v).map_err(|_| { @@ -130,16 +130,16 @@ impl CanisterBuilder for AssetsBuilder { dependencies, pool, )?; - } + // } let assets_canister_info = info.as_info::()?; - if !config.skip_frontend { + // if !config.skip_frontend { assets_canister_info.assert_source_paths()?; - } + // } copy_assets( pool.get_logger(), &assets_canister_info, - config.skip_frontend, + // config.skip_frontend, )?; Ok(()) } @@ -173,14 +173,15 @@ fn delete_output_directory( fn copy_assets( logger: &slog::Logger, assets_canister_info: &AssetsCanisterInfo, - skip_frontend: bool, + // skip_frontend: bool, ) -> 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 skip_frontend && !source_path.exists() { + if !source_path.exists() { slog::warn!( logger, r#"Skip copying "{}" because --skip-frontend was used."#, diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index 0fbe176f1d..6e5ee38059 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -78,7 +78,7 @@ pub trait CanisterBuilder { #[derive(Clone)] pub struct BuildConfig { profile: Profile, - pub skip_frontend: bool, + // pub skip_frontend: bool, pub build_mode_check: bool, pub network_name: String, @@ -98,19 +98,19 @@ impl BuildConfig { Ok(BuildConfig { network_name, profile: config_intf.profile.unwrap_or(Profile::Debug), - skip_frontend: false, + // 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_skip_frontend(self, skip_frontend: bool) -> Self { + // Self { + // skip_frontend, + // ..self + // } + // } pub fn with_build_mode_check(self, build_mode_check: bool) -> Self { Self { diff --git a/src/dfx/src/lib/message.rs b/src/dfx/src/lib/message.rs index 5da12a0f34..82ed0c31d0 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 - 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.", + 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.", + BuildAll => "Builds all canisters configured in dfx.json.", 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.", From 54ca271dd3bff84763c154b60c28283a67b7c389 Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Mon, 31 Aug 2020 13:52:44 -0700 Subject: [PATCH 5/9] fmt clippy and other misc --- src/dfx/src/commands/build.rs | 3 +- src/dfx/src/lib/builders/assets.rs | 70 +++++++++++++----------------- src/dfx/src/lib/builders/mod.rs | 9 ---- src/dfx/src/lib/models/canister.rs | 2 - 4 files changed, 30 insertions(+), 54 deletions(-) diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index 587d3904dc..0c6cdca093 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -77,8 +77,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_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 745c7064fd..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,21 +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 the source doesn't exist, we ignore it. - // if skip_frontend && !source_path.exists() { 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 6e5ee38059..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/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 9a25606f34..8d8fbf8091 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -327,8 +327,6 @@ impl CanisterPool { &self, build_config: BuildConfig, ) -> DfxResult>> { - // check for canister ids before building - // THIS DOES NOTHING self.step_prebuild_all(&build_config).map_err(|e| { DfxError::BuildError(BuildErrorKind::PrebuildAllStepFailed(Box::new(e))) })?; From c3ea617d0db6ec860113e73970602ad15e5fb6a0 Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Mon, 31 Aug 2020 16:49:11 -0700 Subject: [PATCH 6/9] build transitive deps and some cleanup --- .../transitive_deps_canisters/a/main.mo | 5 ++ .../transitive_deps_canisters/b/main.mo | 5 ++ .../transitive_deps_canisters/c/main.mo | 5 ++ .../assets/transitive_deps_canisters/dfx.json | 28 +++++++ .../transitive_deps_canisters/patch.bash | 1 + ...uild-granular.bash => build_granular.bash} | 14 ++++ src/dfx/src/commands/build.rs | 3 +- src/dfx/src/lib/models/canister.rs | 83 +++++++++++-------- 8 files changed, 110 insertions(+), 34 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/dfx.json create mode 100644 e2e/bats/assets/transitive_deps_canisters/patch.bash rename e2e/bats/{build-granular.bash => build_granular.bash} (77%) 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/dfx.json b/e2e/bats/assets/transitive_deps_canisters/dfx.json new file mode 100644 index 0000000000..f78fe85c31 --- /dev/null +++ b/e2e/bats/assets/transitive_deps_canisters/dfx.json @@ -0,0 +1,28 @@ +{ + "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" + } + }, + "defaults": { + "build": { + "packtool": "" + } + }, + "networks": { + "local": { + "bind": "127.0.0.1:8000" + } + } +} 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 similarity index 77% rename from e2e/bats/build-granular.bash rename to e2e/bats/build_granular.bash index 170311ee32..56d5296a73 100644 --- a/e2e/bats/build-granular.bash +++ b/e2e/bats/build_granular.bash @@ -24,6 +24,20 @@ teardown() { 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 diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index 0c6cdca093..332b74ab44 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -13,6 +13,7 @@ pub fn construct() -> App<'static, 'static> { .arg( Arg::with_name("canister_name") .takes_value(true) + .required_unless("all") .help(UserMessage::BuildCanisterName.to_str()) .required(false), ) @@ -56,7 +57,7 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { // Option can be None in which case --all was specified let some_canister = args.value_of("canister_name"); - // Get whole pool of canisters configured in dfx.json + // 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. diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 8d8fbf8091..bdcd311f82 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, }; @@ -91,28 +92,27 @@ pub struct CanisterPool { cache: Arc, } -impl CanisterPool { - fn insert( - env: &dyn Environment, - generate_cid: bool, - canister_name: &str, - canisters_map: &mut Vec>, - ) -> DfxResult<()> { - let builder_pool = BuilderPool::new(env)?; - let canister_id_store = CanisterIdStore::for_env(env)?; - let config = env - .get_config() - .ok_or(DfxError::CommandMustBeRunInAProject)?; +struct PoolConstructHelper<'a> { + config: &'a Config, + builder_pool: BuilderPool, + canister_id_store: CanisterIdStore, + generate_cid: bool, + canisters_map: &'a mut Vec>, +} - let canister_id = match canister_id_store.find(canister_name) { +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 generate_cid => Some(Canister::generate_random_canister_id()?), + None if pool_helper.generate_cid => Some(Canister::generate_random_canister_id()?), _ => None, }; - let info = CanisterInfo::load(&config, canister_name, canister_id)?; + let info = CanisterInfo::load(pool_helper.config, canister_name, canister_id)?; - if let Some(builder) = builder_pool.get(&info) { - canisters_map.insert(0, Arc::new(Canister::new(info, builder))); + if let Some(builder) = pool_helper.builder_pool.get(&info) { + pool_helper + .canisters_map + .insert(0, Arc::new(Canister::new(info, builder))); Ok(()) } else { Err(DfxError::CouldNotFindBuilderForCanister( @@ -121,6 +121,29 @@ impl CanisterPool { } } + 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() { + CanisterPool::insert_with_dependencies(canister, pool_helper)?; + } + Ok(()) + } + pub fn load( env: &dyn Environment, generate_cid: bool, @@ -133,29 +156,23 @@ impl CanisterPool { let mut canisters_map = Vec::new(); + 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, + }; + if let Some(canister_name) = some_canister { - CanisterPool::insert(env, generate_cid, canister_name, &mut canisters_map)?; - - // get direct dependencies - let canister_id = CanisterIdStore::for_env(env)?.get(canister_name)?; - let info = CanisterInfo::load(&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() { - CanisterPool::insert(env, generate_cid, canister, &mut canisters_map)?; - } + 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(env, generate_cid, key, &mut canisters_map)?; + CanisterPool::insert(key, &mut pool_helper)?; } } From ceea151bb88d24239758886c453c7852452e7755 Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Mon, 31 Aug 2020 19:37:10 -0700 Subject: [PATCH 7/9] warn on cyclic dep and prevent stack overflow on recursion --- .../transitive_deps_canisters/d/main.mo | 5 +++++ .../assets/transitive_deps_canisters/dfx.json | 10 ++++++++++ .../transitive_deps_canisters/e/main.mo | 5 +++++ e2e/bats/build_granular.bash | 8 ++++++++ src/dfx/src/lib/models/canister.rs | 20 +++++++++++++++++-- 5 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 e2e/bats/assets/transitive_deps_canisters/d/main.mo create mode 100644 e2e/bats/assets/transitive_deps_canisters/e/main.mo 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 index f78fe85c31..eef29a38cd 100644 --- a/e2e/bats/assets/transitive_deps_canisters/dfx.json +++ b/e2e/bats/assets/transitive_deps_canisters/dfx.json @@ -13,6 +13,16 @@ "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": { 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/build_granular.bash b/e2e/bats/build_granular.bash index 56d5296a73..7408709e6c 100644 --- a/e2e/bats/build_granular.bash +++ b/e2e/bats/build_granular.bash @@ -72,3 +72,11 @@ teardown() { 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." +} diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index bdcd311f82..622f27df79 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -10,7 +10,7 @@ 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; @@ -98,6 +98,8 @@ struct PoolConstructHelper<'a> { canister_id_store: CanisterIdStore, generate_cid: bool, canisters_map: &'a mut Vec>, + visited_map: BTreeMap, + logger: &'a Logger, } impl CanisterPool { @@ -113,6 +115,9 @@ impl CanisterPool { 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( @@ -139,7 +144,16 @@ impl CanisterPool { }; for canister in deps.iter() { - CanisterPool::insert_with_dependencies(canister, pool_helper)?; + 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(()) } @@ -162,6 +176,8 @@ impl CanisterPool { 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 { From e17511d1843857e4c793971b2032077a39f819ff Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Tue, 1 Sep 2020 12:25:56 -0700 Subject: [PATCH 8/9] remove --all requirement --- e2e/bats/assetscanister.bash | 2 +- e2e/bats/base.bash | 4 ++-- e2e/bats/basic-project.bash | 8 ++++---- e2e/bats/bootstrap.bash | 2 +- e2e/bats/build.bash | 26 +++++++++++++------------- e2e/bats/call.bash | 2 +- e2e/bats/create.bash | 4 ++-- e2e/bats/id.bash | 2 +- e2e/bats/identity.bash | 2 +- e2e/bats/install.bash | 6 +++--- e2e/bats/lifecycle.bash | 2 +- e2e/bats/network.bash | 4 ++-- e2e/bats/packtool.bash | 10 +++++----- e2e/bats/print.bash | 2 +- e2e/bats/provider.bash | 2 +- e2e/bats/usage.bash | 4 ++-- src/dfx/src/commands/build.rs | 8 -------- src/dfx/src/lib/message.rs | 3 +-- 18 files changed, 42 insertions(+), 51 deletions(-) diff --git a/e2e/bats/assetscanister.bash b/e2e/bats/assetscanister.bash index 55c93b66bf..99d0775538 100644 --- a/e2e/bats/assetscanister.bash +++ b/e2e/bats/assetscanister.bash @@ -18,7 +18,7 @@ teardown() { dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install e2e_project_assets assert_command dfx canister call --query e2e_project_assets retrieve '("binary/noise.txt")' diff --git a/e2e/bats/base.bash b/e2e/bats/base.bash index eaee6012a9..4143532fa0 100644 --- a/e2e/bats/base.bash +++ b/e2e/bats/base.bash @@ -18,7 +18,7 @@ teardown() { dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install e2e_project assert_command dfx canister call --query e2e_project is_digit '("5")' @@ -34,6 +34,6 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match 'import error, package "base" not defined' } diff --git a/e2e/bats/basic-project.bash b/e2e/bats/basic-project.bash index afe30bff7f..b84fe66b27 100644 --- a/e2e/bats/basic-project.bash +++ b/e2e/bats/basic-project.bash @@ -18,7 +18,7 @@ teardown() { install_asset greet dfx_start dfx canister create --all - dfx build --all + dfx build # INSTALL_REQUEST_ID=$(dfx canister install hello --async) # dfx canister request-status $INSTALL_REQUEST_ID dfx canister install hello @@ -43,7 +43,7 @@ teardown() { install_asset counter dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install hello assert_command dfx canister call hello read @@ -87,7 +87,7 @@ teardown() { install_asset counter_idl dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install --all assert_command dfx canister call hello inc '(42,false,"testzZ",vec{1;2;3},opt record{head=42; tail=opt record{head=+43; tail=null}}, variant { cons=record{ 42; variant { cons=record{43; variant { nil }} } } })' @@ -98,7 +98,7 @@ teardown() { install_asset matrix_multiply dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install --all assert_command dfx canister call hello multiply '(vec{vec{1;2};vec{3;4};vec{5;6}},vec{vec{1;2;3};vec{4;5;6}})' diff --git a/e2e/bats/bootstrap.bash b/e2e/bats/bootstrap.bash index 2f0b6e83f2..8a08648aac 100644 --- a/e2e/bats/bootstrap.bash +++ b/e2e/bats/bootstrap.bash @@ -15,7 +15,7 @@ teardown() { @test "bootstrap fetches candid file" { dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install hello ID=$(dfx canister id hello) diff --git a/e2e/bats/build.bash b/e2e/bats/build.bash index c0e61cbf66..c6a8676897 100644 --- a/e2e/bats/build.bash +++ b/e2e/bats/build.bash @@ -18,7 +18,7 @@ teardown() { install_asset invalid dfx_start dfx canister create --all - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match "syntax error" } @@ -26,7 +26,7 @@ teardown() { install_asset import dfx_start dfx canister create --all - assert_command dfx build --all + assert_command dfx build dfx canister install --all assert_command dfx canister call e2e_project greet World assert_match "10World" @@ -35,7 +35,7 @@ teardown() { @test "build succeeds on default project" { dfx_start dfx canister create --all - assert_command dfx build --all + assert_command dfx build } # TODO: Before Tungsten, we need to update this test for code with inter-canister calls. @@ -43,9 +43,9 @@ teardown() { @test "build twice produces the same wasm binary" { dfx_start dfx canister create --all - assert_command dfx build --all + assert_command dfx build cp .dfx/local/canisters/e2e_project/e2e_project.wasm ./old.wasm - assert_command dfx build --all + assert_command dfx build assert_command diff .dfx/local/canisters/e2e_project/e2e_project.wasm ./old.wasm } @@ -53,7 +53,7 @@ teardown() { install_asset warning dfx_start dfx canister create --all - assert_command dfx build --all + assert_command dfx build assert_match "warning, this pattern consuming type" } @@ -61,7 +61,7 @@ teardown() { install_asset import_error dfx_start dfx canister create --all - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match 'import error, canister alias "random" not defined' } @@ -69,7 +69,7 @@ teardown() { dfx_start dfx config canisters.e2e_project.type unknown_canister_type dfx canister create --all - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match "CouldNotFindBuilderForCanister" } @@ -77,7 +77,7 @@ teardown() { dfx_start install_asset custom_canister dfx canister create --all - assert_command dfx build --all + assert_command dfx build assert_match "CUSTOM_CANISTER_BUILD_DONE" dfx canister install --all @@ -87,7 +87,7 @@ teardown() { @test "build succeeds with network parameter" { dfx_start dfx canister --network local create --all - assert_command dfx build --network local --all + assert_command dfx build --network local } @test "build succeeds when requested network is configured" { @@ -95,13 +95,13 @@ teardown() { assert_command dfx config networks.tungsten.providers '[ "http://127.0.0.1:8000" ]' assert_command dfx canister --network tungsten create --all - assert_command dfx build --network tungsten --all + assert_command dfx build --network tungsten } @test "build output for local network is in expected directory" { dfx_start dfx canister create --all - assert_command dfx build --all + assert_command dfx build assert_command ls .dfx/local/canisters/e2e_project/ assert_command ls .dfx/local/canisters/e2e_project/e2e_project.wasm } @@ -110,7 +110,7 @@ teardown() { dfx_start assert_command dfx config networks.tungsten.providers '[ "http://127.0.0.1:8000" ]' dfx canister --network tungsten create --all - assert_command dfx build --network tungsten --all + assert_command dfx build --network tungsten assert_command ls .dfx/tungsten/canisters/e2e_project/ assert_command ls .dfx/tungsten/canisters/e2e_project/e2e_project.wasm } diff --git a/e2e/bats/call.bash b/e2e/bats/call.bash index adcfb882e9..1005a44cc0 100644 --- a/e2e/bats/call.bash +++ b/e2e/bats/call.bash @@ -15,7 +15,7 @@ teardown() { install_asset greet dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install hello assert_command dfx canister call $(dfx canister id hello) greet '("Names are difficult")' assert_match '("Hello, Names are difficult!")' diff --git a/e2e/bats/create.bash b/e2e/bats/create.bash index 2ea121133e..780dfe7a23 100644 --- a/e2e/bats/create.bash +++ b/e2e/bats/create.bash @@ -27,14 +27,14 @@ teardown() { @test "build fails without create" { dfx_start - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match "Cannot find canister id." } @test "build fails if all canisters in project are not created" { dfx_start assert_command dfx canister create e2e_project - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match "Cannot find canister id. Please issue 'dfx canister create e2e_project_assets'" } diff --git a/e2e/bats/id.bash b/e2e/bats/id.bash index 794563e94b..03f58a2740 100644 --- a/e2e/bats/id.bash +++ b/e2e/bats/id.bash @@ -15,7 +15,7 @@ teardown() { install_asset id dfx_start dfx canister create --all - dfx build --all + dfx build assert_command dfx canister id e2e_project assert_match $(cat .dfx/local/canister_ids.json | jq -r .e2e_project.local) } diff --git a/e2e/bats/identity.bash b/e2e/bats/identity.bash index de36f2af5f..1d6aa0dec4 100644 --- a/e2e/bats/identity.bash +++ b/e2e/bats/identity.bash @@ -17,7 +17,7 @@ teardown() { install_asset identity dfx_start dfx canister create --all - assert_command dfx build --all + assert_command dfx build assert_command dfx canister install --all ID_CALL=$(dfx canister call e2e_project fromCall) diff --git a/e2e/bats/install.bash b/e2e/bats/install.bash index c7bc45beef..a1354a2ca3 100644 --- a/e2e/bats/install.bash +++ b/e2e/bats/install.bash @@ -26,7 +26,7 @@ teardown() { @test "install succeeds when --all is provided" { dfx_start dfx canister create --all - dfx build --all + dfx build assert_command dfx canister install --all @@ -36,7 +36,7 @@ teardown() { @test "install succeeds with network name" { dfx_start dfx canister create --all - dfx build --all + dfx build assert_command dfx canister --network local install --all @@ -46,7 +46,7 @@ teardown() { @test "install fails with network name that is not in dfx.json" { dfx_start dfx canister create --all - dfx build --all + dfx build assert_command_fail dfx canister --network nosuch install --all diff --git a/e2e/bats/lifecycle.bash b/e2e/bats/lifecycle.bash index e759c253a4..95190732ff 100644 --- a/e2e/bats/lifecycle.bash +++ b/e2e/bats/lifecycle.bash @@ -15,7 +15,7 @@ teardown() { install_asset greet dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install hello assert_command dfx canister status hello assert_match "Canister hello's status is Running." diff --git a/e2e/bats/network.bash b/e2e/bats/network.bash index 0cadab3e5d..9a3abe0f6e 100644 --- a/e2e/bats/network.bash +++ b/e2e/bats/network.bash @@ -64,7 +64,7 @@ teardown() { @test "failure message does not include network if for local network" { dfx_start - assert_command_fail dfx build --network local --all + assert_command_fail dfx build --network local assert_match "Cannot find canister id. Please issue 'dfx canister create e2e_project" } @@ -73,6 +73,6 @@ teardown() { assert_command dfx config networks.tungsten.providers '[ "http://127.0.0.1:8000" ]' - assert_command_fail dfx build --network tungsten --all + assert_command_fail dfx build --network tungsten assert_match "Cannot find canister id. Please issue 'dfx canister --network tungsten create e2e_project" } diff --git a/e2e/bats/packtool.bash b/e2e/bats/packtool.bash index 457ff7bad6..7e5cbd8256 100644 --- a/e2e/bats/packtool.bash +++ b/e2e/bats/packtool.bash @@ -18,7 +18,7 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match 'import error, package "(rate|describe)" not defined' } @@ -28,7 +28,7 @@ teardown() { dfx_start dfx canister create --all - dfx build --all + dfx build } @test "project calls dependencies made available by packtool" { @@ -37,7 +37,7 @@ teardown() { dfx_start dfx canister create --all - dfx build --all + dfx build dfx canister install e2e_project assert_command dfx canister call e2e_project rate '("rust")' @@ -53,7 +53,7 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match 'Failed to invoke the package tool' assert_match 'no-such-command.*that.*command.*cannot.*be.*invoked' assert_match 'No such file or directory \(os error 2\)' @@ -65,7 +65,7 @@ teardown() { dfx_start dfx canister create --all - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match 'Package tool.*reported an error' assert_match 'sh.*command-that-fails.bash' assert_match 'exit code: 3' diff --git a/e2e/bats/print.bash b/e2e/bats/print.bash index ac8c172d8c..a49c78cc41 100644 --- a/e2e/bats/print.bash +++ b/e2e/bats/print.bash @@ -19,7 +19,7 @@ teardown() { install_asset print dfx_start 2>stderr.txt dfx canister create --all - dfx build --all + dfx build dfx canister install e2e_project dfx canister call e2e_project hello run tail -2 stderr.txt diff --git a/e2e/bats/provider.bash b/e2e/bats/provider.bash index 792f0becf3..7a3ff58258 100644 --- a/e2e/bats/provider.bash +++ b/e2e/bats/provider.bash @@ -19,7 +19,7 @@ teardown() { # flakiness (replica start time). @test "provider flag can be passed in" { skip "refactor this test to replace client.bash" - dfx build --all + dfx build # Start a replica manually on a specific port. $(dfx cache show)/replica --config ' diff --git a/e2e/bats/usage.bash b/e2e/bats/usage.bash index 714a6d2405..0e7b6ff32e 100644 --- a/e2e/bats/usage.bash +++ b/e2e/bats/usage.bash @@ -24,12 +24,12 @@ teardown() { # Make sure we're in an empty directory. cd $(mktemp -d -t dfx-e2e-XXXXXXXX) - assert_command_fail dfx build --all + assert_command_fail dfx build assert_match "must be run in a project" dfx new t --no-frontend cd t dfx_start dfx canister create --all - assert_command dfx build --all + assert_command dfx build } diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index 332b74ab44..7020354ed5 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -13,17 +13,9 @@ pub fn construct() -> App<'static, 'static> { .arg( Arg::with_name("canister_name") .takes_value(true) - .required_unless("all") .help(UserMessage::BuildCanisterName.to_str()) .required(false), ) - .arg( - Arg::with_name("all") - .long("all") - .required_unless("canister_name") - .help(UserMessage::BuildAll.to_str()) - .takes_value(false), - ) .arg( Arg::with_name("check") .long("check") diff --git a/src/dfx/src/lib/message.rs b/src/dfx/src/lib/message.rs index 82ed0c31d0..75704e96d8 100644 --- a/src/dfx/src/lib/message.rs +++ b/src/dfx/src/lib/message.rs @@ -89,8 +89,7 @@ user_message!( // dfx build 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.", - BuildAll => "Builds all canisters configured in dfx.json.", + BuildCanister => "Builds all or specific canisters from the code in your project. By default, all canisters are built.", 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.", From 4f4fcc4ec34c9056618bdf4e37822bbe46bb1537 Mon Sep 17 00:00:00 2001 From: Prithvi Shahi Date: Tue, 1 Sep 2020 13:49:57 -0700 Subject: [PATCH 9/9] add back --all this time with conflicts_with --- e2e/bats/build_granular.bash | 14 ++++++++++++++ src/dfx/src/commands/build.rs | 8 ++++++++ src/dfx/src/lib/message.rs | 1 + 3 files changed, 23 insertions(+) diff --git a/e2e/bats/build_granular.bash b/e2e/bats/build_granular.bash index 7408709e6c..a56e6d4dda 100644 --- a/e2e/bats/build_granular.bash +++ b/e2e/bats/build_granular.bash @@ -80,3 +80,17 @@ teardown() { 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/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index 7020354ed5..a23b4eb482 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -13,9 +13,17 @@ pub fn construct() -> App<'static, 'static> { .arg( 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") .long("check") diff --git a/src/dfx/src/lib/message.rs b/src/dfx/src/lib/message.rs index 75704e96d8..6ec9504804 100644 --- a/src/dfx/src/lib/message.rs +++ b/src/dfx/src/lib/message.rs @@ -88,6 +88,7 @@ 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.", BuildCheck => "Build canisters without creating them. This can be used to check that canisters build ok.",