fix: do not build remote canisters

CHANGELOG.md

@@ -180,6 +180,12 @@ The Replica returned an error: code 1, message: "Canister requested a compute al
 
 ### fix: For `dfx start --clean --background`, the background process no longer cleans a second time.
 
+### fix: do not build or generate remote canisters
+
+Canisters that specify a remote id for the network that's getting built falsely had their build steps run, blocking some normal use patterns of `dfx deploy`.
+Canisters with a remote id specified no longer get built.
+The same applies to `dfx generate`.
+
 ### refactor: Move replica URL functions into a module for reuse
 
 The running replica port and url are generally useful information. Previously the code to get the URL was embedded in the network proxy code. This moves it out into a library for reuse.

e2e/assets/remote/envvar/dfx.json

@@ -0,0 +1,21 @@
+{
+    "canisters": {
+        "basic": {
+            "type": "custom",
+            "candid": "main.did",
+            "wasm": "main.wasm",
+            "build": "bash -c 'echo \"CANISTER_ID_remote: $CANISTER_ID_remote\"'",
+            "dependencies": [
+                "remote"
+            ]
+        },
+        "remote": {
+            "main": "remote.mo",
+            "remote": {
+                "id": {
+                    "actuallylocal": "qoctq-giaaa-aaaaa-aaaea-cai"
+                }
+            }
+        }
+    }
+}

e2e/tests-dfx/remote.bash

@@ -137,7 +137,7 @@ teardown() {
     assert_match "Canister 'remote' is a remote canister on network 'actuallylocal', and cannot be installed from here."
 }
 
-@test "canister create --all and canister install --all skip remote canisters" {
+@test "canister create --all, canister install --all, dfx generate skip remote canisters" {
     install_asset remote/actual
     dfx_start
     setup_actuallylocal_shared_network
@@ -174,8 +174,12 @@ teardown() {
     assert_eq '("mock")'
 
     assert_command dfx canister create --all --network actuallylocal
-    assert_command dfx build --network actuallylocal
+    assert_command dfx build --network actuallylocal -vv
+    assert_match "Not building canister 'remote'"
     assert_command dfx canister install --all --network actuallylocal
+    assert_command dfx generate --network actuallylocal
+    assert_match "Generating type declarations for canister basic"
+    assert_not_match "Generating type declarations for canister remote"
 
     assert_command dfx canister call basic read_remote --network actuallylocal
     assert_eq '("this is data in the remote canister")'
@@ -273,7 +277,8 @@ teardown() {
     assert_command dfx canister call remote which_am_i
     assert_eq '("mock")'
 
-    assert_command dfx deploy --network actuallylocal
+    assert_command dfx deploy --network actuallylocal -vv
+    assert_match "Not building canister 'remote'"
     assert_command dfx canister call basic read_remote --network actuallylocal
     assert_eq '("this is data in the remote canister")'
 
@@ -297,3 +302,13 @@ teardown() {
     assert_command jq .remote canister_ids.json
     assert_eq "null"
 }
+
+@test "build step sets remote cid environment variable correctly" {
+    install_asset remote/envvar
+    install_asset wasm/identity # need to have some .did and .wasm files to point our custom canister to
+    dfx_start
+    setup_actuallylocal_shared_network
+
+    assert_command dfx deploy --network actuallylocal -vv
+    assert_match "CANISTER_ID_remote: qoctq-giaaa-aaaaa-aaaea-cai"
+}

src/dfx/src/commands/build.rs

@@ -43,17 +43,26 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult {
     let build_mode_check = opts.check;
 
     // Option can be None in which case --all was specified
-    let canister_names = config
+    let canisters_to_load = config
         .get_config()
         .get_canister_names_with_dependencies(opts.canister_name.as_deref())?;
-
-    // Get pool of canisters to build
-    let canister_pool = CanisterPool::load(&env, build_mode_check, &canister_names)?;
+    let canisters_to_build = canisters_to_load
+        .clone()
+        .into_iter()
+        .filter(|canister_name| {
+            !config
+                .get_config()
+                .is_remote_canister(canister_name, &env.get_network_descriptor().name)
+                .unwrap_or(false)
+        })
+        .collect();
+
+    let canister_pool = CanisterPool::load(&env, build_mode_check, &canisters_to_load)?;
 
     // Create canisters on the replica and associate canister ids locally.
     if build_mode_check {
         slog::warn!(
-            env.get_logger(),
+            logger,
             "Building canisters to check they build ok. Canister IDs might be hard coded."
         );
     } else {
@@ -69,8 +78,10 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult {
     slog::info!(logger, "Building canisters...");
 
     let runtime = Runtime::new().expect("Unable to create a runtime");
-    let build_config = BuildConfig::from_config(&config)?.with_build_mode_check(build_mode_check);
-    runtime.block_on(canister_pool.build_or_fail(&build_config))?;
+    let build_config = BuildConfig::from_config(&config)?
+        .with_build_mode_check(build_mode_check)
+        .with_canisters_to_build(canisters_to_build);
+    runtime.block_on(canister_pool.build_or_fail(logger, &build_config))?;
 
     Ok(())
 }

src/dfx/src/commands/generate.rs

@@ -7,6 +7,7 @@ use crate::lib::provider::create_agent_environment;
 use crate::NetworkOpt;
 
 use clap::Parser;
+use slog::trace;
 use tokio::runtime::Runtime;
 
 /// Generate type declarations for canisters from the code in your project
@@ -22,6 +23,7 @@ pub struct GenerateOpts {
 
 pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult {
     let env = create_agent_environment(env, opts.network.network)?;
+    let log = env.get_logger();
 
     // Read the config.
     let config = env.get_config_or_anyhow()?;
@@ -31,40 +33,60 @@ pub fn exec(env: &dyn Environment, opts: GenerateOpts) -> DfxResult {
     env.get_cache().install()?;
 
     // Option can be None which means generate types for all canisters
-    let canister_names = config
+    let canisters_to_load = config
         .get_config()
         .get_canister_names_with_dependencies(opts.canister_name.as_deref())?;
+    let canisters_to_generate = canisters_to_load
+        .clone()
+        .into_iter()
+        .filter(|canister_name| {
+            !config
+                .get_config()
+                .is_remote_canister(canister_name, &env.get_network_descriptor().name)
+                .unwrap_or(false)
+        })
+        .collect();
 
-    // Get pool of canisters to build
-    let canister_pool = CanisterPool::load(&env, false, &canister_names)?;
+    let canister_pool = CanisterPool::load(&env, false, &canisters_to_load)?;
 
-    // This is just to display an error if trying to generate before creating the canister.
+    // This is just to display an error if trying to generate before creating the canister(s).
     let store = CanisterIdStore::for_env(&env)?;
+
     // If generate for motoko canister, build first
-    let mut build_before_generate = false;
+    let mut build_before_generate = Vec::new();
     for canister in canister_pool.get_canister_list() {
         let canister_name = canister.get_name();
         let canister_id = store.get(canister_name)?;
         if let Some(info) = canister_pool.get_canister_info(&canister_id) {
             if info.is_motoko() {
-                build_before_generate = true;
+                build_before_generate.push(canister_name.to_string());
+                trace!(
+                    log,
+                    "Found Motoko canister '{}' - will have to build before generating IDL.",
+                    canister_name
+                );
             }
         }
     }
 
-    let build_config = BuildConfig::from_config(&config)?;
+    let build_config =
+        BuildConfig::from_config(&config)?.with_canisters_to_build(build_before_generate);
+    let generate_config =
+        BuildConfig::from_config(&config)?.with_canisters_to_build(canisters_to_generate);
 
-    if build_before_generate {
-        slog::info!(
-            env.get_logger(),
-            "Building canisters before generate for Motoko"
-        );
+    if build_config
+        .canisters_to_build
+        .as_ref()
+        .map(|v| !v.is_empty())
+        .unwrap_or(false)
+    {
+        slog::info!(log, "Building canisters before generate for Motoko");
         let runtime = Runtime::new().expect("Unable to create a runtime");
-        runtime.block_on(canister_pool.build_or_fail(&build_config))?;
+        runtime.block_on(canister_pool.build_or_fail(log, &build_config))?;
     }
 
-    for canister in canister_pool.get_canister_list() {
-        canister.generate(&canister_pool, &build_config)?;
+    for canister in canister_pool.canisters_to_build(&generate_config) {
+        canister.generate(&canister_pool, &generate_config)?;
     }
 
     Ok(())

src/dfx/src/lib/builders/mod.rs

@@ -365,6 +365,9 @@ pub struct BuildConfig {
     pub lsp_root: PathBuf,
     /// The root for all build files.
     pub build_root: PathBuf,
+    /// If only a subset of canisters should be built, then canisters_to_build contains these canisters' names.
+    /// If all canisters should be built, then this is None.
+    pub canisters_to_build: Option<Vec<String>>,
 }
 
 impl BuildConfig {
@@ -382,6 +385,7 @@ impl BuildConfig {
             build_root: canister_root.clone(),
             idl_root: canister_root.join("idl/"), // TODO: possibly move to `network_root.join("idl/")`
             lsp_root: network_root.join("lsp/"),
+            canisters_to_build: None,
         })
     }
 
@@ -391,6 +395,13 @@ impl BuildConfig {
             ..self
         }
     }
+
+    pub fn with_canisters_to_build(self, canisters: Vec<String>) -> Self {
+        Self {
+            canisters_to_build: Some(canisters),
+            ..self
+        }
+    }
 }
 
 #[context("Failed to shrink wasm at {}.", &wasm_path.as_ref().display())]

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

@@ -13,6 +13,7 @@ use crate::lib::wasm::metadata::add_candid_service_metadata;
 use anyhow::{anyhow, bail, Context};
 use candid::Principal as CanisterId;
 use fn_error_context::context;
+use itertools::Itertools;
 use petgraph::graph::{DiGraph, NodeIndex};
 use rand::{thread_rng, RngCore};
 use slog::{error, info, trace, warn, Logger};
@@ -238,15 +239,51 @@ impl CanisterPool {
     }
 
     #[context("Failed step_prebuild_all.")]
-    fn step_prebuild_all(&self, _build_config: &BuildConfig) -> DfxResult<()> {
-        if self.canisters.iter().any(|can| can.info.is_rust()) {
+    fn step_prebuild_all(&self, log: &Logger, build_config: &BuildConfig) -> DfxResult<()> {
+        // moc expects all .did files of dependencies to be in <output_idl_path> with name <canister id>.did.
+        // Because remote canisters don't get built (and the did file not output in the right place) the .did files have to be copied over manually.
+        for canister in self.canisters.iter().filter(|c| c.info.is_remote()) {
+            let maybe_from = if let Some(remote_candid) = canister.info.get_remote_candid() {
+                Some(remote_candid)
+            } else {
+                canister.info.get_output_idl_path()
+            };
+            if let Some(from) = maybe_from.as_ref() {
+                if from.exists() {
+                    let to = build_config.idl_root.join(format!(
+                        "{}.did",
+                        canister.info.get_canister_id()?.to_text()
+                    ));
+                    if std::fs::copy(&from, &to).is_err() {
+                        warn!(
+                                    log,
+                                    "Failed to copy canister candid from {} to {}. This may produce errors during the build.",
+                                    from.to_string_lossy(),
+                                    to.to_string_lossy()
+                                );
+                    }
+                } else {
+                    warn!(log, ".did file for canister '{}' does not exist at {}. This may result in errors during the build.", canister.get_name(), from.to_string_lossy());
+                }
+            } else {
+                warn!(log, "Failed to find a configured .did file for canister '{}'. Not specifying that field may result in errors during the build.", canister.get_name());
+            }
+        }
+
+        // cargo audit
+        if self
+            .canisters_to_build(build_config)
+            .iter()
+            .any(|can| can.info.is_rust())
+        {
             self.run_cargo_audit()?;
         } else {
             trace!(
                 self.logger,
                 "No canister of type 'rust' found. Not trying to run 'cargo audit'."
             )
         }
+
         Ok(())
     }
 
@@ -368,7 +405,7 @@ impl CanisterPool {
     ) -> DfxResult<()> {
         // We don't want to simply remove the whole directory, as in the future,
         // we may want to keep the IDL files downloaded from network.
-        for canister in &self.canisters {
+        for canister in self.canisters_to_build(build_config) {
             let idl_root = &build_config.idl_root;
             let canister_id = canister.canister_id();
             let idl_file_path = idl_root.join(canister_id.to_text()).with_extension("did");
@@ -384,9 +421,10 @@ impl CanisterPool {
     #[context("Failed while trying to build all canisters in the canister pool.")]
     pub fn build(
         &self,
+        log: &Logger,
         build_config: &BuildConfig,
     ) -> DfxResult<Vec<Result<&BuildOutput, BuildError>>> {
-        self.step_prebuild_all(build_config)
+        self.step_prebuild_all(log, build_config)
             .map_err(|e| DfxError::new(BuildError::PreBuildAllStepFailed(Box::new(e))))?;
 
         let graph = self.build_dependencies_graph()?;
@@ -406,9 +444,20 @@ impl CanisterPool {
             .map(|idx| *graph.node_weight(*idx).unwrap())
             .collect();
 
+        let canisters_to_build = self.canisters_to_build(build_config);
         let mut result = Vec::new();
         for canister_id in &order {
             if let Some(canister) = self.get_canister(canister_id) {
+                if canisters_to_build
+                    .iter()
+                    .map(|c| c.get_name())
+                    .contains(&canister.get_name())
+                {
+                    trace!(log, "Building canister '{}'.", canister.get_name());
+                } else {
+                    trace!(log, "Not building canister '{}'.", canister.get_name());
+                    continue;
+                }
                 result.push(
                     self.step_prebuild(build_config, canister)
                         .map_err(|e| {
@@ -451,9 +500,9 @@ impl CanisterPool {
     /// Build all canisters, failing with the first that failed the build. Will return
     /// nothing if all succeeded.
     #[context("Failed while trying to build all canisters.")]
-    pub async fn build_or_fail(&self, build_config: &BuildConfig) -> DfxResult<()> {
+    pub async fn build_or_fail(&self, log: &Logger, build_config: &BuildConfig) -> DfxResult<()> {
         self.download(build_config).await?;
-        let outputs = self.build(build_config)?;
+        let outputs = self.build(log, build_config)?;
 
         for output in outputs {
             output.map_err(DfxError::new)?;
@@ -462,8 +511,8 @@ impl CanisterPool {
         Ok(())
     }
 
-    async fn download(&self, _build_config: &BuildConfig) -> DfxResult {
-        for canister in &self.canisters {
+    async fn download(&self, build_config: &BuildConfig) -> DfxResult {
+        for canister in self.canisters_to_build(build_config) {
             let info = canister.get_info();
 
             if info.is_custom() {
@@ -525,6 +574,17 @@ impl CanisterPool {
         }
         Ok(())
     }
+
+    pub fn canisters_to_build(&self, build_config: &BuildConfig) -> Vec<&Arc<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()))
+                .collect()
+        } else {
+            self.canisters.iter().collect()
+        }
+    }
 }
 
 #[context("Failed to decode path to str.")]