From 79c6786d643bee3177543f9409278d28a6b972a6 Mon Sep 17 00:00:00 2001 From: Eric Swanson <64809312+ericswanson-dfinity@users.noreply.github.com> Date: Tue, 7 Nov 2023 12:15:05 -0800 Subject: [PATCH] fix: output .env file is now relative to project root, not cwd (#3435) dfx.json can specify `output_env_file` (the default for new projects is `".env"`), and some commands accept `--output-env-file .env` on the command line. If `dfx deploy`, `dfx build`, or `dfx canister install` were executed in a subdirectory of a project, they would create/read this file in that subdirectory, rather than the same directory as dfx.json (the project root). With this change, the location of the env file is taken to be relative to the project root, and furthermore must be contained within the project directory. Also fixed three places that could output "No such file or directory (OS error 2)" without telling which path wasn't found. Fixes: https://dfinity.atlassian.net/browse/SDK-1028 --- CHANGELOG.md | 6 ++++ e2e/tests-dfx/dotenv.bash | 37 ++++++++++++++++++++++++ src/dfx-core/src/config/model/dfinity.rs | 30 +++++++++++++++++++ src/dfx-core/src/error/config.rs | 16 ++++++++++ src/dfx/src/commands/build.rs | 4 +-- src/dfx/src/commands/canister/install.rs | 8 ++--- src/dfx/src/commands/deploy.rs | 4 +-- src/dfx/src/lib/builders/mod.rs | 8 ++--- 8 files changed, 97 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55e6e868d5..03fe44d2ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ # UNRELEASED +=== fix: output_env_file is now considered relative to project root + +The .env file location, whether specified as `output_env_file` in dfx.json +or `--output-env-file ` on the commandline, is now considered relative +to the project root, rather than relative to the current working directory. + === feat: Read dfx canister install argument from a file Enables passing large arguments that cannot be passed directly in the command line using the `--argument-file` flag. For example `dfx canister install --argument-file ./my/argument/file.txt my_canister_name`. diff --git a/e2e/tests-dfx/dotenv.bash b/e2e/tests-dfx/dotenv.bash index 1f6f2ab755..2f92e07d0e 100644 --- a/e2e/tests-dfx/dotenv.bash +++ b/e2e/tests-dfx/dotenv.bash @@ -14,6 +14,43 @@ teardown() { standard_teardown } + +@test "puts .env in project root" { + dfx_start + jq '.canisters["e2e_project_backend"].post_install="echo post install backend"' dfx.json | sponge dfx.json + jq '.canisters["e2e_project_frontend"].post_install="echo post install frontend"' dfx.json | sponge dfx.json + + mkdir subdir + mkdir subdir/canister-install-all subdir/canister-install-single + mkdir subdir/build-all subdir/build-single + mkdir subdir/deploy-single subdir/deploy-all + dfx canister create --all + ( cd subdir/build-single && dfx build e2e_project_frontend ) + ( cd subdir/build-all && dfx build --all ) + ( cd subdir/canister-install-single && dfx canister install e2e_project_backend ) + dfx canister uninstall-code e2e_project_backend + ( cd subdir/canister-install-all && dfx canister install --all ) + rm -rf .dfx + ( cd subdir/deploy-single && dfx deploy e2e_project_backend) + ( cd subdir/deploy-all && dfx deploy ) + + assert_command find . -name .env + assert_eq "./.env" +} + +@test "the output_env_file must be contained within project" { + dfx_start + mkdir ../outside + + assert_command_fail dfx deploy --output-env-file nonexistent/.env + assert_contains "failed to canonicalize output_env_file" + assert_contains "working-dir/e2e_project/nonexistent: No such file or directory" + assert_command_fail dfx deploy --output-env-file /etc/passwd + assert_contains "The output_env_file must be a relative path, but is /etc/passwd" + assert_command_fail dfx deploy --output-env-file ../outside/.env + assert_match "The output_env_file must be within the project root, but is .*/working-dir/e2e_project/../outside/.env" +} + @test "writes environment variables to .env" { dfx_start dfx canister create --all diff --git a/src/dfx-core/src/config/model/dfinity.rs b/src/dfx-core/src/config/model/dfinity.rs index e67d9a7c38..17c8b1d8a7 100644 --- a/src/dfx-core/src/config/model/dfinity.rs +++ b/src/dfx-core/src/config/model/dfinity.rs @@ -3,6 +3,7 @@ use crate::config::directories::get_user_dfx_config_dir; use crate::config::model::bitcoin_adapter::BitcoinAdapterLogLevel; use crate::config::model::canister_http_adapter::HttpAdapterLogLevel; +use crate::error::config::GetOutputEnvFileError; use crate::error::dfx_config::AddDependenciesError::CanisterCircularDependency; use crate::error::dfx_config::GetCanisterNamesWithDependenciesError::AddDependenciesFailed; use crate::error::dfx_config::GetComputeAllocationError::GetComputeAllocationFailed; @@ -1013,6 +1014,35 @@ impl Config { ) } + // returns the path to the output env file if any, guaranteed to be + // a child relative to the project root + pub fn get_output_env_file( + &self, + from_cmdline: Option, + ) -> Result, GetOutputEnvFileError> { + from_cmdline + .or(self.config.output_env_file.clone()) + .map(|p| { + if p.is_relative() { + let p = self.get_project_root().join(p); + + // cannot canonicalize a path that doesn't exist, but the parent should exist + let env_parent = + crate::fs::parent(&p).map_err(GetOutputEnvFileError::Parent)?; + let env_parent = crate::fs::canonicalize(&env_parent) + .map_err(GetOutputEnvFileError::Canonicalize)?; + if !env_parent.starts_with(self.get_project_root()) { + Err(GetOutputEnvFileError::OutputEnvFileMustBeInProjectRoot(p)) + } else { + Ok(self.get_project_root().join(p)) + } + } else { + Err(GetOutputEnvFileError::OutputEnvFileMustBeRelative(p)) + } + }) + .transpose() + } + pub fn save(&self) -> Result<(), StructuredFileError> { save_json_file(&self.path, &self.json) } diff --git a/src/dfx-core/src/error/config.rs b/src/dfx-core/src/error/config.rs index 1121332e42..1658ba575e 100644 --- a/src/dfx-core/src/error/config.rs +++ b/src/dfx-core/src/error/config.rs @@ -1,5 +1,6 @@ use crate::error::fs::FsError; use crate::error::get_user_home::GetUserHomeError; +use std::path::PathBuf; use thiserror::Error; #[derive(Error, Debug)] @@ -13,3 +14,18 @@ pub enum ConfigError { #[error("Failed to determine shared network data directory: {0}")] DetermineSharedNetworkDirectoryFailed(GetUserHomeError), } + +#[derive(Error, Debug)] +pub enum GetOutputEnvFileError { + #[error("failed to canonicalize output_env_file")] + Canonicalize(#[source] FsError), + + #[error("The output_env_file must be within the project root, but is {}", .0.display())] + OutputEnvFileMustBeInProjectRoot(PathBuf), + + #[error("The output_env_file must be a relative path, but is {}", .0.display())] + OutputEnvFileMustBeRelative(PathBuf), + + #[error(transparent)] + Parent(FsError), +} diff --git a/src/dfx/src/commands/build.rs b/src/dfx/src/commands/build.rs index d9abf3412b..22d58f3dcf 100644 --- a/src/dfx/src/commands/build.rs +++ b/src/dfx/src/commands/build.rs @@ -40,9 +40,7 @@ pub fn exec(env: &dyn Environment, opts: CanisterBuildOpts) -> DfxResult { // Read the config. let config = env.get_config_or_anyhow()?; - let env_file = opts - .output_env_file - .or_else(|| config.get_config().output_env_file.clone()); + let env_file = config.get_output_env_file(opts.output_env_file)?; // Check the cache. This will only install the cache if there isn't one installed // already. diff --git a/src/dfx/src/commands/canister/install.rs b/src/dfx/src/commands/canister/install.rs index 304c5eab69..0b01dd243d 100644 --- a/src/dfx/src/commands/canister/install.rs +++ b/src/dfx/src/commands/canister/install.rs @@ -155,9 +155,7 @@ pub async fn exec( } else { let canister_info = canister_info?; let config = config.unwrap(); - let env_file = opts - .output_env_file - .or_else(|| config.get_config().output_env_file.clone()); + let env_file = config.get_output_env_file(opts.output_env_file)?; let idl_path = canister_info.get_constructor_idl_path(); let init_type = get_candid_init_type(&idl_path); let install_args = || blob_from_arguments(arguments, None, arg_type, &init_type); @@ -182,9 +180,7 @@ pub async fn exec( } else if opts.all { // Install all canisters. let config = env.get_config_or_anyhow()?; - let env_file = opts - .output_env_file - .or_else(|| config.get_config().output_env_file.clone()); + let env_file = config.get_output_env_file(opts.output_env_file)?; if let Some(canisters) = &config.get_config().canisters { for canister in canisters.keys() { if pull_canisters_in_config.contains_key(canister) { diff --git a/src/dfx/src/commands/deploy.rs b/src/dfx/src/commands/deploy.rs index 7a429dcd85..5058ecb88f 100644 --- a/src/dfx/src/commands/deploy.rs +++ b/src/dfx/src/commands/deploy.rs @@ -115,9 +115,7 @@ pub fn exec(env: &dyn Environment, opts: DeployOpts) -> DfxResult { .map_err(|err| anyhow!(err)) .context("Failed to parse InstallMode.")?; let config = env.get_config_or_anyhow()?; - let env_file = opts - .output_env_file - .or_else(|| config.get_config().output_env_file.clone()); + let env_file = config.get_output_env_file(opts.output_env_file)?; let with_cycles = opts.with_cycles; diff --git a/src/dfx/src/lib/builders/mod.rs b/src/dfx/src/lib/builders/mod.rs index cfa2934da2..0a553ef1ff 100644 --- a/src/dfx/src/lib/builders/mod.rs +++ b/src/dfx/src/lib/builders/mod.rs @@ -448,7 +448,7 @@ fn write_environment_variables(vars: &[Env<'_>], write_path: &Path) -> DfxResult // the section is correctly formed let end_pos = end_pos + END_TAG.len() + start_pos + START_TAG.len(); existing_file.replace_range(start_pos..end_pos, &write_string); - fs::write(write_path, existing_file)?; + dfx_core::fs::write(write_path, existing_file)?; return Ok(()); } else { // the file has been edited, so we don't know how much to delete, so we append instead @@ -456,10 +456,10 @@ fn write_environment_variables(vars: &[Env<'_>], write_path: &Path) -> DfxResult } // append to the existing file existing_file.push_str(&write_string); - fs::write(write_path, existing_file)?; + dfx_core::fs::write(write_path, existing_file)?; } else { // no existing file, okay to clobber - fs::write(write_path, write_string)?; + dfx_core::fs::write(write_path, write_string)?; } Ok(()) } @@ -501,7 +501,7 @@ impl BuildConfig { idl_root: canister_root.join("idl/"), // TODO: possibly move to `network_root.join("idl/")` lsp_root: network_root.join("lsp/"), canisters_to_build: None, - env_file: config_intf.output_env_file.clone(), + env_file: config.get_output_env_file(None)?, }) }