fix: Build the canister dependency graph only for the requested canisters

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 # UNRELEASED
 
+### fix: Allow canisters to be deployed even if unrelated canisters in dfx.json are malformed
+
 ### feat!: enable cycles ledger support unconditionally
 
 ### chore!: removed `unsafe-eval` CSP from default starter template

e2e/tests-dfx/deploy.bash

@@ -150,6 +150,17 @@ teardown() {
 @test "deploy succeeds when specify canister ID in dfx.json" {
   dfx_start
   jq '.canisters.hello_backend.specified_id="n5n4y-3aaaa-aaaaa-p777q-cai"' dfx.json | sponge dfx.json
+  cat dfx.json
+  assert_command dfx deploy hello_backend
+  assert_command dfx canister id hello_backend
+  assert_match n5n4y-3aaaa-aaaaa-p777q-cai
+}
+
+@test "deploy succeeds when specify canister ID in dfx.json even if other canisters are malformed" {
+  dfx_start
+  jq '.canisters.hello_backend.specified_id="n5n4y-3aaaa-aaaaa-p777q-cai"' dfx.json | sponge dfx.json
+  # add a malformed canister
+  jq '.canisters += {"malformed": {"remote": {"id": {"local": "hptcf-emaaa-aaaaa-qaawq-cai"}}}}' dfx.json | sponge dfx.json
   assert_command dfx deploy hello_backend
   assert_command dfx canister id hello_backend
   assert_match n5n4y-3aaaa-aaaaa-p777q-cai

src/dfx/src/lib/canister_info/motoko.rs

@@ -63,7 +63,7 @@ impl CanisterInfoFactory for MotokoCanisterInfo {
         );
         let main_path = info
             .get_main_file()
-            .context("`main` attribute is required on Motoko canisters in dfx.json")?;
+            .context("`main` attribute is required on Motoko canisters in dfx.json (and Motoko is the default canister type if not otherwise specified)")?;
         let input_path = workspace_root.join(main_path);
         let output_root = info.get_output_root().to_path_buf();
         let output_wasm_path = output_root.join(name).with_extension("wasm");

src/dfx/src/lib/models/canister.rs

@@ -527,29 +527,80 @@ impl CanisterPool {
         &self.logger
     }
 
+    /// Builds a dependency graph for the given canisters.
+    /// Only canisters in `canisters_to_build` and their dependencies will be
+    /// included in the graph.
     #[context("Failed to build dependencies graph for canister pool.")]
-    fn build_dependencies_graph(&self) -> DfxResult<DiGraph<CanisterId, ()>> {
+    fn build_dependencies_graph(
+        &self,
+        canisters_to_build: Vec<&Canister>,
+    ) -> DfxResult<DiGraph<CanisterId, ()>> {
         let mut graph: DiGraph<CanisterId, ()> = DiGraph::new();
-        let mut id_set: BTreeMap<CanisterId, NodeIndex<u32>> = BTreeMap::new();
-
-        // Add all the canisters as nodes.
-        for canister in &self.canisters {
-            let canister_id = canister.info.get_canister_id()?;
-            id_set.insert(canister_id, graph.add_node(canister_id));
-        }
 
-        // Add all the edges.
-        for canister in &self.canisters {
+        /// Recursive function that does the actual work of adding the canister
+        /// and its dependencies to the graph.
+        /// This is separate from the top-level function so that the top-level
+        /// function can be called without worrying about the implementation
+        /// details.
+        ///
+        /// Returns the index of the canister's graph node.
+        fn add_canister_and_dependencies_to_graph(
+            canister_pool: &CanisterPool,
+            canister: &Canister,
+            graph: &mut DiGraph<CanisterId, ()>,
+            canister_id_to_canister: &BTreeMap<CanisterId, &Canister>,
+            canister_id_to_index: &mut BTreeMap<CanisterId, NodeIndex<u32>>,
+        ) -> DfxResult<NodeIndex> {
             let canister_id = canister.canister_id();
-            let canister_info = &canister.info;
-            let deps = canister.builder.get_dependencies(self, canister_info)?;
-            if let Some(node_ix) = id_set.get(&canister_id) {
-                for d in deps {
-                    if let Some(dep_ix) = id_set.get(&d) {
-                        graph.add_edge(*node_ix, *dep_ix, ());
-                    }
-                }
+
+            // If this canister has already been visited, return its index. Its
+            // dependencies were already added on a previous visit.
+            if let Some(node_ix) = canister_id_to_index.get(&canister_id) {
+                return Ok(*node_ix);
             }
+            // Otherwise, add it to the graph.
+            let node_ix = graph.add_node(canister_id);
+            canister_id_to_index.insert(canister_id, node_ix);
+
+            let deps = canister
+                .builder
+                .get_dependencies(canister_pool, &canister.info)?;
+
+            for dependency_id in deps {
+                let dependency = canister_id_to_canister.get(&dependency_id).ok_or_else(|| {
+                    DfxError::new(BuildError::DependencyError(format!(
+                        "Canister '{}' depends on canister '{}' which does not exist.",
+                        canister.info.get_name(),
+                        dependency_id.to_text()
+                    )))
+                })?;
+                let dependency_index = add_canister_and_dependencies_to_graph(
+                    canister_pool,
+                    dependency,
+                    graph,
+                    canister_id_to_canister,
+                    canister_id_to_index,
+                )?;
+                graph.add_edge(node_ix, dependency_index, ());
+            }
+
+            Ok(node_ix)
+        }
+
+        let mut canister_id_to_index: BTreeMap<CanisterId, NodeIndex<u32>> = BTreeMap::new();
+        let canister_id_to_canister = self
+            .canisters
+            .iter()
+            .map(|c| (c.canister_id(), c.as_ref()))
+            .collect::<BTreeMap<CanisterId, &Canister>>();
+        for canister in canisters_to_build {
+            add_canister_and_dependencies_to_graph(
+                self,
+                canister,
+                &mut graph,
+                &canister_id_to_canister,
+                &mut canister_id_to_index,
+            )?;
         }
 
         // Verify the graph has no cycles.
@@ -691,7 +742,8 @@ impl CanisterPool {
         self.step_prebuild_all(log, build_config)
             .map_err(|e| DfxError::new(BuildError::PreBuildAllStepFailed(Box::new(e))))?;
 
-        let graph = self.build_dependencies_graph()?;
+        let canisters_to_build = self.canisters_to_build(build_config);
+        let graph = self.build_dependencies_graph(canisters_to_build.clone())?;
         let nodes = petgraph::algo::toposort(&graph, None).map_err(|cycle| {
             let message = match graph.node_weight(cycle.node_id()) {
                 Some(canister_id) => match self.get_canister_info(canister_id) {
@@ -839,14 +891,15 @@ impl CanisterPool {
         Ok(())
     }
 
-    pub fn canisters_to_build(&self, build_config: &BuildConfig) -> Vec<&Arc<Canister>> {
+    pub fn canisters_to_build(&self, build_config: &BuildConfig) -> Vec<&Canister> {
         if let Some(canister_names) = &build_config.canisters_to_build {
             self.canisters
                 .iter()
                 .filter(|can| canister_names.contains(&can.info.get_name().to_string()))
+                .map(Arc::as_ref)
                 .collect()
         } else {
-            self.canisters.iter().collect()
+            self.canisters.iter().map(Arc::as_ref).collect()
         }
     }
 }