From ce74e5d90e7d788948acdefb7170adf6578ba555 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 26 Mar 2024 11:33:45 -0700 Subject: [PATCH 1/3] refactor: removing PullCanisterInfo --- src/dfx/src/lib/builders/mod.rs | 7 +++- src/dfx/src/lib/builders/pull.rs | 34 +++++++++++------- src/dfx/src/lib/canister_info.rs | 43 +++++++++++++++++++--- src/dfx/src/lib/canister_info/pull.rs | 52 --------------------------- src/dfx/src/lib/models/canister.rs | 10 ++++-- 5 files changed, 73 insertions(+), 73 deletions(-) delete mode 100644 src/dfx/src/lib/canister_info/pull.rs diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index df33e387d2..ffa23252d3 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -28,6 +28,7 @@ mod pull; mod rust; pub use custom::custom_download; +pub use pull::get_pull_build_output; #[derive(Debug)] pub enum WasmBuildOutput { @@ -148,7 +149,11 @@ pub trait CanisterBuilder { ) })?; - let did_from_build = self.get_candid_path(pool, info, config)?; + let did_from_build = match info.get_common_output_idl_path() { + Some(p) => p, + None => self.get_candid_path(pool, info, config)?, + }; + if !did_from_build.exists() { bail!( "Candid file: {} doesn't exist.", diff --git a/src/dfx/src/lib/builders/pull.rs b/src/dfx/src/lib/builders/pull.rs index 0da205d7b7..689941b3ce 100644 --- a/src/dfx/src/lib/builders/pull.rs +++ b/src/dfx/src/lib/builders/pull.rs @@ -1,11 +1,11 @@ use crate::lib::builders::{ BuildConfig, BuildOutput, CanisterBuilder, IdlBuildOutput, WasmBuildOutput, }; -use crate::lib::canister_info::pull::PullCanisterInfo; -use crate::lib::canister_info::CanisterInfo; +use crate::lib::canister_info::{CanisterInfo, PullInfo}; use crate::lib::environment::Environment; use crate::lib::error::DfxResult; use crate::lib::models::canister::CanisterPool; +use anyhow::anyhow; use candid::Principal as CanisterId; use fn_error_context::context; use slog::o; @@ -43,23 +43,33 @@ impl CanisterBuilder for PullBuilder { canister_info: &CanisterInfo, _config: &BuildConfig, ) -> DfxResult { - let pull_info = canister_info.as_info::()?; - Ok(BuildOutput { - canister_id: *pull_info.get_canister_id(), - // It's impossible to know if the downloaded wasm is gzip or not with only the info in `dfx.json`. - wasm: WasmBuildOutput::None, - idl: IdlBuildOutput::File(pull_info.get_output_idl_path().to_path_buf()), - }) + unreachable!("call get_pull_build_output directly"); } + #[context("Failed to get candid path for pull canister '{}'.", info.get_name())] fn get_candid_path( &self, _pool: &CanisterPool, info: &CanisterInfo, _config: &BuildConfig, ) -> DfxResult { - let pull_info = info.as_info::()?; - let output_idl_path = pull_info.get_output_idl_path(); - Ok(output_idl_path.to_path_buf()) + unreachable!("pull canister must provide common_output_idl_path") } } + +pub fn get_pull_build_output( + canister_info: &CanisterInfo, + pull_info: &PullInfo, +) -> DfxResult { + let canister_id = *pull_info.get_canister_id(); + let output_idl_path = canister_info + .get_common_output_idl_path() + .ok_or_else(|| anyhow!("no common output idl path"))?; + + Ok(BuildOutput { + canister_id, + // It's impossible to know if the downloaded wasm is gzip or not with only the info in `dfx.json`. + wasm: WasmBuildOutput::None, + idl: IdlBuildOutput::File(output_idl_path), + }) +} diff --git a/src/dfx/src/lib/canister_info.rs b/src/dfx/src/lib/canister_info.rs index 0e1e4bf173..cd3633e65d 100644 --- a/src/dfx/src/lib/canister_info.rs +++ b/src/dfx/src/lib/canister_info.rs @@ -17,9 +17,8 @@ use std::path::{Path, PathBuf}; pub mod assets; pub mod custom; pub mod motoko; -pub mod pull; pub mod rust; -use self::pull::PullCanisterInfo; +use crate::lib::deps::get_candid_path_in_project; use assets::AssetsCanisterInfo; use custom::CustomCanisterInfo; use motoko::MotokoCanisterInfo; @@ -59,6 +58,14 @@ pub struct CanisterInfo { pull_dependencies: Vec<(String, CanisterId)>, gzip: bool, init_arg: Option, + + pull: Option, + common_output_idl_path: Option, +} + +#[derive(Debug)] +pub struct PullInfo { + id: Principal, } impl CanisterInfo { @@ -133,7 +140,15 @@ impl CanisterInfo { let output_root = build_root.join(name); + let mut pull = None; + let mut common_output_idl_path = None; + let type_specific = canister_config.type_specific.clone(); + if let CanisterTypeProperties::Pull { id } = type_specific { + common_output_idl_path = Some(get_candid_path_in_project(workspace_root, &id)); + + pull = Some(PullInfo { id }); + } let args = match &canister_config.args { Some(args) if !args.is_empty() => canister_config.args.clone(), @@ -167,6 +182,8 @@ impl CanisterInfo { pull_dependencies, gzip, init_arg, + pull, + common_output_idl_path, }; Ok(canister_info) @@ -294,6 +311,10 @@ impl CanisterInfo { /// /// To be separated into service.did and init_args. pub fn get_output_idl_path(&self) -> Option { + if let Some(common_output_idl_path) = &self.common_output_idl_path { + return Some(common_output_idl_path.clone()); + } + match &self.type_specific { CanisterTypeProperties::Motoko { .. } => self .as_info::() @@ -307,9 +328,7 @@ impl CanisterInfo { CanisterTypeProperties::Rust { .. } => self .as_info::() .map(|x| x.get_output_idl_path().to_path_buf()), - CanisterTypeProperties::Pull { .. } => self - .as_info::() - .map(|x| x.get_output_idl_path().to_path_buf()), + CanisterTypeProperties::Pull { .. } => unreachable!(), } .ok() .or_else(|| self.remote_candid.clone()) @@ -367,4 +386,18 @@ impl CanisterInfo { pub fn get_init_arg(&self) -> Option<&str> { self.init_arg.as_deref() } + + pub fn get_pull_info(&self) -> Option<&PullInfo> { + self.pull.as_ref() + } + + pub fn get_common_output_idl_path(&self) -> Option { + self.common_output_idl_path.as_ref().cloned() + } +} + +impl PullInfo { + pub fn get_canister_id(&self) -> &Principal { + &self.id + } } diff --git a/src/dfx/src/lib/canister_info/pull.rs b/src/dfx/src/lib/canister_info/pull.rs deleted file mode 100644 index ddcb6ef2a0..0000000000 --- a/src/dfx/src/lib/canister_info/pull.rs +++ /dev/null @@ -1,52 +0,0 @@ -use crate::lib::canister_info::{CanisterInfo, CanisterInfoFactory}; -use crate::lib::deps::get_candid_path_in_project; -use crate::lib::error::DfxResult; -use anyhow::bail; -use candid::Principal; -use dfx_core::config::model::dfinity::CanisterTypeProperties; -use std::path::{Path, PathBuf}; - -pub struct PullCanisterInfo { - name: String, - canister_id: Principal, - output_idl_path: PathBuf, -} - -impl PullCanisterInfo { - pub fn get_name(&self) -> &str { - &self.name - } - - pub fn get_canister_id(&self) -> &Principal { - &self.canister_id - } - - pub fn get_output_idl_path(&self) -> &Path { - self.output_idl_path.as_path() - } -} - -impl CanisterInfoFactory for PullCanisterInfo { - fn create(info: &CanisterInfo) -> DfxResult { - let name = info.get_name().to_string(); - let canister_id = { - if let CanisterTypeProperties::Pull { id } = info.type_specific.clone() { - id - } else { - bail!( - "Attempted to construct a pull canister from a type:{} canister config", - info.type_specific.name() - ); - } - }; - - let workspace_root = info.get_workspace_root().to_path_buf(); - let output_idl_path = get_candid_path_in_project(&workspace_root, &canister_id); - - Ok(Self { - name, - canister_id, - output_idl_path, - }) - } -} diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index 21a4ea9db6..dafc81939a 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -1,6 +1,6 @@ use crate::lib::builders::{ - custom_download, BuildConfig, BuildOutput, BuilderPool, CanisterBuilder, IdlBuildOutput, - WasmBuildOutput, + custom_download, get_pull_build_output, BuildConfig, BuildOutput, BuilderPool, CanisterBuilder, + IdlBuildOutput, WasmBuildOutput, }; use crate::lib::canister_info::CanisterInfo; use crate::lib::environment::Environment; @@ -65,7 +65,11 @@ impl Canister { pool: &CanisterPool, build_config: &BuildConfig, ) -> DfxResult<&BuildOutput> { - let output = self.builder.build(pool, &self.info, build_config)?; + let output = if let Some(pull_info) = self.info.get_pull_info() { + get_pull_build_output(&self.info, pull_info) + } else { + self.builder.build(pool, &self.info, build_config) + }?; // Ignore the old output, and return a reference. let _ = self.output.replace(Some(output)); From e85143dfd9f6ac5b862f8144161acd9a913e2d2d Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 26 Mar 2024 11:35:24 -0700 Subject: [PATCH 2/3] run e2e on branch --- .github/workflows/e2e.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index c3ca449d7c..ffa1da4e1b 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -3,6 +3,7 @@ on: push: branches: - master + - ens/poc-pull-info-custom-canister pull_request: concurrency: From d0cefe26afc6a8008e8050d3d71075ac66cc3e61 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 26 Mar 2024 12:18:37 -0700 Subject: [PATCH 3/3] actually remove PullBuilder --- src/dfx/src/lib/builders/custom.rs | 7 +++- src/dfx/src/lib/builders/mod.rs | 2 +- src/dfx/src/lib/builders/pull.rs | 56 +----------------------------- src/dfx/src/lib/canister_info.rs | 8 ++++- src/dfx/src/lib/models/canister.rs | 10 ++---- 5 files changed, 18 insertions(+), 65 deletions(-) diff --git a/src/dfx/src/lib/builders/custom.rs b/src/dfx/src/lib/builders/custom.rs index 4114b69609..e2f2a4379d 100644 --- a/src/dfx/src/lib/builders/custom.rs +++ b/src/dfx/src/lib/builders/custom.rs @@ -1,5 +1,6 @@ use crate::lib::builders::{ - BuildConfig, BuildOutput, CanisterBuilder, IdlBuildOutput, WasmBuildOutput, + get_pull_build_output, BuildConfig, BuildOutput, CanisterBuilder, IdlBuildOutput, + WasmBuildOutput, }; use crate::lib::canister_info::custom::CustomCanisterInfo; use crate::lib::canister_info::CanisterInfo; @@ -101,6 +102,10 @@ impl CanisterBuilder for CustomBuilder { info: &CanisterInfo, config: &BuildConfig, ) -> DfxResult { + if let Some(pull_info) = info.get_pull_info() { + return get_pull_build_output(info, pull_info); + } + let CustomBuilderExtra { input_candid_url: _, candid, diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index ffa23252d3..d6fd442447 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -525,7 +525,7 @@ impl BuilderPool { ("custom", Arc::new(custom::CustomBuilder::new(env)?)), ("motoko", Arc::new(motoko::MotokoBuilder::new(env)?)), ("rust", Arc::new(rust::RustBuilder::new(env)?)), - ("pull", Arc::new(pull::PullBuilder::new(env)?)), + // ("pull", Arc::new(pull::PullBuilder::new(env)?)), ]); Ok(Self { builders }) diff --git a/src/dfx/src/lib/builders/pull.rs b/src/dfx/src/lib/builders/pull.rs index 689941b3ce..31f44157c1 100644 --- a/src/dfx/src/lib/builders/pull.rs +++ b/src/dfx/src/lib/builders/pull.rs @@ -1,61 +1,7 @@ -use crate::lib::builders::{ - BuildConfig, BuildOutput, CanisterBuilder, IdlBuildOutput, WasmBuildOutput, -}; +use crate::lib::builders::{BuildOutput, IdlBuildOutput, WasmBuildOutput}; use crate::lib::canister_info::{CanisterInfo, PullInfo}; -use crate::lib::environment::Environment; use crate::lib::error::DfxResult; -use crate::lib::models::canister::CanisterPool; use anyhow::anyhow; -use candid::Principal as CanisterId; -use fn_error_context::context; -use slog::o; -use std::path::PathBuf; - -pub struct PullBuilder { - _logger: slog::Logger, -} - -impl PullBuilder { - #[context("Failed to create PullBuilder.")] - pub fn new(env: &dyn Environment) -> DfxResult { - Ok(Self { - _logger: env.get_logger().new(o! { - "module" => "pull" - }), - }) - } -} - -impl CanisterBuilder for PullBuilder { - #[context("Failed to get dependencies for canister '{}'.", info.get_name())] - fn get_dependencies( - &self, - _pool: &CanisterPool, - info: &CanisterInfo, - ) -> DfxResult> { - Ok(vec![]) - } - - #[context("Failed to build Pull canister '{}'.", canister_info.get_name())] - fn build( - &self, - _pool: &CanisterPool, - canister_info: &CanisterInfo, - _config: &BuildConfig, - ) -> DfxResult { - unreachable!("call get_pull_build_output directly"); - } - - #[context("Failed to get candid path for pull canister '{}'.", info.get_name())] - fn get_candid_path( - &self, - _pool: &CanisterPool, - info: &CanisterInfo, - _config: &BuildConfig, - ) -> DfxResult { - unreachable!("pull canister must provide common_output_idl_path") - } -} pub fn get_pull_build_output( canister_info: &CanisterInfo, diff --git a/src/dfx/src/lib/canister_info.rs b/src/dfx/src/lib/canister_info.rs index cd3633e65d..8872fd3ba9 100644 --- a/src/dfx/src/lib/canister_info.rs +++ b/src/dfx/src/lib/canister_info.rs @@ -9,6 +9,7 @@ use dfx_core::config::model::dfinity::{ CanisterDeclarationsConfig, CanisterMetadataSection, CanisterTypeProperties, Config, Pullable, WasmOptLevel, }; +use dfx_core::json::structure::SerdeVec; use dfx_core::network::provider::get_network_context; use dfx_core::util; use fn_error_context::context; @@ -143,11 +144,16 @@ impl CanisterInfo { let mut pull = None; let mut common_output_idl_path = None; - let type_specific = canister_config.type_specific.clone(); + let mut type_specific = canister_config.type_specific.clone(); if let CanisterTypeProperties::Pull { id } = type_specific { common_output_idl_path = Some(get_candid_path_in_project(workspace_root, &id)); pull = Some(PullInfo { id }); + type_specific = CanisterTypeProperties::Custom { + wasm: "".to_string(), + build: SerdeVec::Many(vec![]), + candid: "".to_string(), + } } let args = match &canister_config.args { diff --git a/src/dfx/src/lib/models/canister.rs b/src/dfx/src/lib/models/canister.rs index dafc81939a..21a4ea9db6 100644 --- a/src/dfx/src/lib/models/canister.rs +++ b/src/dfx/src/lib/models/canister.rs @@ -1,6 +1,6 @@ use crate::lib::builders::{ - custom_download, get_pull_build_output, BuildConfig, BuildOutput, BuilderPool, CanisterBuilder, - IdlBuildOutput, WasmBuildOutput, + custom_download, BuildConfig, BuildOutput, BuilderPool, CanisterBuilder, IdlBuildOutput, + WasmBuildOutput, }; use crate::lib::canister_info::CanisterInfo; use crate::lib::environment::Environment; @@ -65,11 +65,7 @@ impl Canister { pool: &CanisterPool, build_config: &BuildConfig, ) -> DfxResult<&BuildOutput> { - let output = if let Some(pull_info) = self.info.get_pull_info() { - get_pull_build_output(&self.info, pull_info) - } else { - self.builder.build(pool, &self.info, build_config) - }?; + let output = self.builder.build(pool, &self.info, build_config)?; // Ignore the old output, and return a reference. let _ = self.output.replace(Some(output));