Spofford/deploylib

Cargo.toml

@@ -60,6 +60,7 @@ ic-management-canister-types = { version = "0.7.1" }
 ic-utils = { version = "0.47.0" }
 icp = { path = "crates/icp" }
 icp-canister-interfaces = { path = "crates/icp-canister-interfaces" }
+icp-deploy-canister = { path = "crates/icp-deploy-canister" }
 icp-sync-plugin = { path = "crates/icp-sync-plugin" }
 ic-identity-hsm = "0.47.0"
 icrc-ledger-types = "0.1.10"

crates/icp-cli/Cargo.toml

@@ -43,6 +43,7 @@ ic-management-canister-types.workspace = true
 ic-utils.workspace = true
 icp-canister-interfaces.workspace = true
 icp = { workspace = true, features = ["clap"] }
+icp-deploy-canister.workspace = true
 icrc-ledger-types.workspace = true
 indicatif.workspace = true
 itertools.workspace = true

crates/icp-cli/src/commands/canister/install.rs

@@ -10,14 +10,13 @@ use icp::fs;
 use icp::prelude::*;
 use tracing::{info, warn};
 
+use icp_deploy_canister::install_canister_resolved;
+
 use crate::{
     commands::args::{self, ArgsOpt},
     operations::{
         candid_compat::{CandidCompatibility, check_candid_compatibility},
-        install::{
-            WasmMemoryPersistenceOpt, install_canister, is_eop_canister,
-            resolve_install_mode_and_status,
-        },
+        install::{WasmMemoryPersistenceOpt, is_eop_canister, resolve_install_mode_and_status},
     },
 };
 
@@ -184,16 +183,19 @@ pub(crate) async fn exec(ctx: &Context, args: &InstallArgs) -> Result<(), anyhow
         }
     }
 
-    install_canister(
-        &agent,
-        args.proxy,
-        &canister_id,
+    let wmp = args
+        .wasm_memory_persistence
+        .map(WasmMemoryPersistenceOpt::to_ic);
+    install_canister_resolved(
         &canister_display,
+        canister_id,
         &wasm,
         install_mode,
         status,
         init_args_bytes.as_deref(),
-        args.wasm_memory_persistence,
+        wmp,
+        &agent,
+        args.proxy,
     )
     .await?;
 

crates/icp-cli/src/commands/deploy.rs

@@ -479,17 +479,16 @@ pub(crate) async fn exec(ctx: &Context, args: &DeployArgs) -> Result<(), anyhow:
             .into_iter()
             .collect();
 
-        let pkg_cache = ctx.dirs.package_cache()?;
+        let resolver = ctx.resource_resolver()?;
         sync_many(
-            ctx.syncer.clone(),
             agent.clone(),
+            resolver,
             sync_canisters,
             environment_selection.name().to_owned(),
             env.network.name.clone(),
             canister_ids,
             args.proxy,
             ctx.debug,
-            &pkg_cache,
         )
         .await?;
     }

crates/icp-cli/src/commands/sync.rs

@@ -8,8 +8,10 @@ use icp::identity::IdentitySelection;
 use std::collections::BTreeMap;
 use tracing::info;
 
+use icp::Canister;
+
 use crate::{
-    operations::{proxy_management, sync::sync_many},
+    operations::{binding_env_vars::set_binding_env_vars_many, proxy_management, sync::sync_many},
     options::{EnvironmentOpt, IdentityOpt},
 };
 
@@ -122,17 +124,33 @@ pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), anyhow::E
         .into_iter()
         .collect();
 
-    let pkg_cache = ctx.dirs.package_cache()?;
+    // Apply the generated `PUBLIC_CANISTER_ID:*` environment variables before
+    // syncing. `deploy` does this, but standalone `icp sync` previously did not,
+    // so a synced canister could run against stale/absent binding ids.
+    let target_canisters: Vec<(Principal, Canister)> = sync_canisters
+        .iter()
+        .map(|(cid, _, info)| (*cid, info.clone()))
+        .collect();
+    set_binding_env_vars_many(
+        agent.clone(),
+        args.proxy,
+        environment_selection.name(),
+        target_canisters,
+        canister_ids.clone(),
+        ctx.debug,
+    )
+    .await?;
+
+    let resolver = ctx.resource_resolver()?;
     sync_many(
-        ctx.syncer.clone(),
         agent,
+        resolver,
         sync_canisters,
         environment_selection.name().to_owned(),
         env.network.name.clone(),
         canister_ids,
         args.proxy,
         ctx.debug,
-        &pkg_cache,
     )
     .await?;
 

crates/icp-cli/src/operations/binding_env_vars.rs

@@ -1,29 +1,15 @@
 use std::collections::{BTreeMap, HashSet};
+use std::sync::Arc;
 
 use futures::{StreamExt, stream::FuturesOrdered};
 use ic_agent::{Agent, export::Principal};
-use ic_management_canister_types::{CanisterSettings, EnvironmentVariable, UpdateSettingsArgs};
 use icp::Canister;
+use icp_deploy_canister::{SyncCanisterError, apply_binding_env_vars};
 use snafu::Snafu;
 use tracing::error;
 
 use crate::progress::{ProgressManager, ProgressManagerSettings};
 
-use super::proxy::UpdateOrProxyError;
-use super::proxy_management;
-
-#[derive(Debug, Snafu)]
-pub enum BindingEnvVarsOperationError {
-    #[snafu(display("Could not find canister id(s) for {} in environment '{environment}'. Make sure they are created first", canister_names.join(", ")))]
-    CanisterNotCreated {
-        environment: String,
-        canister_names: Vec<String>,
-    },
-
-    #[snafu(transparent)]
-    UpdateOrProxy { source: UpdateOrProxyError },
-}
-
 #[derive(Debug, Snafu)]
 #[snafu(display("Canister(s) {names:?} failed to update environment variables."))]
 pub struct SetBindingEnvVarsManyError {
@@ -34,50 +20,15 @@ pub struct SetBindingEnvVarsManyError {
 struct BindingEnvVarsFailure {
     canister_name: String,
     canister_id: Principal,
-    error: BindingEnvVarsOperationError,
-}
-
-pub(crate) async fn set_env_vars_for_canister(
-    agent: &Agent,
-    proxy: Option<Principal>,
-    canister_id: &Principal,
-    canister_info: &Canister,
-    binding_vars: &[(String, String)],
-) -> Result<(), BindingEnvVarsOperationError> {
-    let mut environment_variables = canister_info
-        .settings
-        .environment_variables
-        .to_owned()
-        .unwrap_or_default();
-
-    // inject the ids of the other canisters
-    for (k, v) in binding_vars.iter() {
-        environment_variables.insert(k.to_string(), v.to_string());
-    }
-
-    let environment_variables = environment_variables
-        .into_iter()
-        .map(|(name, value)| EnvironmentVariable { name, value })
-        .collect::<Vec<_>>();
-
-    proxy_management::update_settings(
-        agent,
-        proxy,
-        UpdateSettingsArgs {
-            canister_id: *canister_id,
-            settings: CanisterSettings {
-                environment_variables: Some(environment_variables),
-                ..Default::default()
-            },
-            sender_canister_version: None,
-        },
-    )
-    .await?;
-
-    Ok(())
+    error: SyncCanisterError,
 }
 
-/// Orchestrates setting environment variables for multiple canisters with progress tracking
+/// Orchestrates setting environment variables for multiple canisters with progress tracking.
+///
+/// The per-canister work (computing the generated `PUBLIC_CANISTER_ID:*`
+/// bindings, merging with manifest env vars, and applying them) lives in
+/// `icp_deploy_canister::apply_binding_env_vars`; this wrapper only adds the
+/// missing-id precheck and progress display.
 pub(crate) async fn set_binding_env_vars_many(
     agent: Agent,
     proxy: Option<Principal>,
@@ -86,20 +37,14 @@ pub(crate) async fn set_binding_env_vars_many(
     canister_list: BTreeMap<String, Principal>,
     debug: bool,
 ) -> Result<(), SetBindingEnvVarsManyError> {
-    // Check that all the canisters in this environment have an id
-    // We need to have all the ids to generate environment variables
-    // for the bindings
+    // Check that all the canisters in this environment have an id: we need all
+    // ids to generate the binding environment variables.
     let canisters_with_ids: HashSet<&String> = canister_list.keys().collect();
 
-    let all_canister_names: Vec<String> = target_canisters
+    let missing_canisters: Vec<String> = target_canisters
         .iter()
         .map(|(_, info)| info.name.clone())
-        .collect();
-
-    let missing_canisters: Vec<String> = all_canister_names
-        .iter()
-        .filter(|c| !canisters_with_ids.contains(*c))
-        .map(|c| c.to_string())
+        .filter(|c| !canisters_with_ids.contains(c))
         .collect();
 
     if !missing_canisters.is_empty() {
@@ -116,38 +61,22 @@ pub(crate) async fn set_binding_env_vars_many(
         .fail();
     }
 
+    let canister_list = Arc::new(canister_list);
+
     let mut futs = FuturesOrdered::new();
     let progress_manager = ProgressManager::new(ProgressManagerSettings { hidden: debug });
 
     for (cid, info) in target_canisters {
         let pb = progress_manager.create_progress_bar(&info.name);
         let canister_name = info.name.clone();
-
-        // Each canister receives only the ids it is wired to (its own project's
-        // canisters by their local names, plus any declared dependencies under
-        // their aliases), resolved to the ids that exist in this environment.
-        // A project without dependencies wires every canister to every sibling,
-        // reproducing the previous flat behavior.
-        let binding_vars: Vec<(String, String)> = info
-            .bindings
-            .iter()
-            .filter_map(|(env_name, referenced_key)| {
-                canister_list.get(referenced_key).map(|principal| {
-                    (
-                        format!("PUBLIC_CANISTER_ID:{env_name}"),
-                        principal.to_text(),
-                    )
-                })
-            })
-            .collect();
+        let agent = agent.clone();
+        let canister_list = canister_list.clone();
 
         let settings_fn = {
-            let agent = agent.clone();
             let pb = pb.clone();
-
             async move {
                 pb.set_message("Updating environment variables...");
-                set_env_vars_for_canister(&agent, proxy, &cid, &info, &binding_vars).await
+                apply_binding_env_vars(&info, cid, &canister_list, &agent, proxy).await
             }
         };
 
@@ -160,7 +89,6 @@ pub(crate) async fn set_binding_env_vars_many(
             )
             .await;
 
-            // Map error to include canister context for deferred printing
             result.map_err(|error| BindingEnvVarsFailure {
                 canister_name,
                 canister_id: cid,