From b9d71a3551f24808966e8a57fb9fca60366a59a9 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 7 Nov 2023 11:01:45 -0800 Subject: [PATCH] fix: output .env file is now relative to project root, not cwd 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)?, }) }