Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: output .env file is now relative to project root, not cwd #3435

Merged
merged 1 commit into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading