Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master' into ens/sdk-1240-corrup…
Browse files Browse the repository at this point in the history
…t-cache-dir
  • Loading branch information
ericswanson-dfinity committed Nov 7, 2023
2 parents 565e959 + 79c6786 commit f19292b
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 @@ -10,6 +10,12 @@ dfx only looks for the existence of a cache version subdirectory to
determine whether it has been installed. The end result was that later
commands would fail when the cache did not contain expected files.

=== 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 f19292b

Please sign in to comment.