Skip to content

Commit

Permalink
fix: output .env file is now relative to project root, not cwd (#3435)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ericswanson-dfinity authored Nov 7, 2023
1 parent 12a8ea1 commit 79c6786
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 16 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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`.
Expand Down
37 changes: 37 additions & 0 deletions e2e/tests-dfx/dotenv.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions src/dfx-core/src/config/model/dfinity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<PathBuf>,
) -> Result<Option<PathBuf>, 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)
}
Expand Down
16 changes: 16 additions & 0 deletions src/dfx-core/src/error/config.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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),
}
4 changes: 1 addition & 3 deletions src/dfx/src/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 2 additions & 6 deletions src/dfx/src/commands/canister/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
8 changes: 4 additions & 4 deletions src/dfx/src/lib/builders/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,18 +448,18 @@ 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
}
}
// 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(())
}
Expand Down Expand Up @@ -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)?,
})
}

Expand Down

0 comments on commit 79c6786

Please sign in to comment.