Skip to content

Commit

Permalink
feat: streamline dfx new output (#4073)
Browse files Browse the repository at this point in the history
- add some spinners
- add some debug logs
- remove welcome message
- check project template compatibility before writing anything

Fixes: https://dfinity.atlassian.net/browse/SDK-1940
  • Loading branch information
ericswanson-dfinity authored Jan 23, 2025
1 parent 45251d9 commit b04e931
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 56 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
Due to the incompatibility between the APIs on the replica port and the PocketIC port, `dfx info replica-port`
no longer works with PocketIC, and the PocketIC port is provided by a new command, `dfx info pocketic-config-port`.

### feat: streamlined `dfx new` output

### test: adds playwright tests for `dfx new` project frontends

The first of a suite of baseline tests to automate testing starter projects. Makes sure that sveltekit, react, vue, and vanilla frontends are compatible with other dfx or asset canister changes.
Expand Down
5 changes: 5 additions & 0 deletions e2e/tests-dfx/new.bash
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ teardown() {
assert_eq "motoko"
}

@test "checks for frontend test compatibility before writing base files" {
assert_command_fail dfx new broken --type rust --no-frontend --extras frontend-tests
assert_directory_not_exists broken
}

@test "frontend templates apply successfully" {
for frontend in sveltekit vue react vanilla simple-assets none; do
assert_command dfx new e2e_${frontend/-/_} --frontend $frontend
Expand Down
112 changes: 60 additions & 52 deletions src/dfx/src/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ use dialoguer::{FuzzySelect, MultiSelect};
use fn_error_context::context;
use indicatif::HumanBytes;
use semver::Version;
use slog::{info, trace, warn, Logger};
use slog::{debug, error, info, trace, warn, Logger};
use std::collections::{BTreeMap, HashMap};
use std::io::{self, IsTerminal, Read};
use std::path::{Path, PathBuf};
use std::process::{Command, ExitStatus, Stdio};
use std::process::{Command, Output, Stdio};
use std::time::Duration;
use tar::Archive;
use walkdir::WalkDir;
Expand Down Expand Up @@ -200,7 +200,7 @@ pub fn init_git(log: &Logger, project_name: &Path) -> DfxResult {
.status();

if init_status.is_ok() && init_status.unwrap().success() {
info!(log, "Initializing git repository...");
debug!(log, "Initializing git repository");
std::process::Command::new("git")
.arg("add")
.current_dir(project_name)
Expand Down Expand Up @@ -328,7 +328,9 @@ fn scaffold_frontend_code(
variables: &BTreeMap<String, String>,
) -> DfxResult {
let log = env.get_logger();
let spinner = env.new_spinner("Checking for node".into());
let node_installed = program_installed(program::NODE);
spinner.set_message("Checking for npm".into());
let npm_installed = program_installed(program::NPM);

let project_name_str = project_name
Expand All @@ -340,9 +342,11 @@ fn scaffold_frontend_code(
let js_agent_version = if let Some(v) = agent_version {
v.clone()
} else {
spinner.set_message("Getting agent-js version from npm".into());
get_agent_js_version_from_npm(AGENT_JS_DEFAULT_INSTALL_DIST_TAG)
.map_err(|err| anyhow!("Cannot execute npm: {}", err))?
};
spinner.finish_and_clear();

let mut variables = variables.clone();
variables.insert("js_agent_version".to_string(), js_agent_version);
Expand All @@ -368,6 +372,7 @@ fn scaffold_frontend_code(
run_post_create_command(env, project_name, frontend, &variables)?;
}
} else {
spinner.finish_and_clear();
if !node_installed {
warn!(
log,
Expand Down Expand Up @@ -450,11 +455,6 @@ pub fn exec(env: &dyn Environment, mut opts: NewOpts) -> DfxResult {

DiskBasedCache::install(&env.get_cache().version_str())?;

info!(
log,
r#"Creating new project "{}"..."#,
project_name.display()
);
if dry_run {
warn!(
log,
Expand Down Expand Up @@ -499,14 +499,7 @@ pub fn exec(env: &dyn Environment, mut opts: NewOpts) -> DfxResult {
("ic_commit".to_string(), replica_rev().to_string()),
]);

write_files_from_entries(
log,
&mut assets::new_project_base_files().context("Failed to get base project archive.")?,
project_name,
dry_run,
&variables,
)?;

debug!(log, "Gathering project templates");
let frontend: Option<ProjectTemplate> =
if opts.no_frontend || matches!(opts.frontend.as_ref(), Some(s) if s == "none") {
None
Expand Down Expand Up @@ -542,6 +535,16 @@ pub fn exec(env: &dyn Environment, mut opts: NewOpts) -> DfxResult {
};

let requirements = get_requirements(&backend, frontend.as_ref(), &extras)?;

debug!(log, "Writing base files");
write_files_from_entries(
log,
&mut assets::new_project_base_files().context("Failed to get base project archive.")?,
project_name,
dry_run,
&variables,
)?;

for requirement in &requirements {
write_project_template_resources(log, requirement, project_name, dry_run, &variables)?;
}
Expand All @@ -568,6 +571,7 @@ pub fn exec(env: &dyn Environment, mut opts: NewOpts) -> DfxResult {
// If on mac, we should validate that XCode toolchain was installed.
#[cfg(target_os = "macos")]
{
debug!(log, "Checking if xcode is installed");
let mut should_git = true;
if let Ok(code) = Command::new("xcode-select")
.arg("-p")
Expand Down Expand Up @@ -602,19 +606,7 @@ pub fn exec(env: &dyn Environment, mut opts: NewOpts) -> DfxResult {
run_post_create_command(env, project_name, requirement, &variables)?;
}
}

// Print welcome message.
info!(
log,
"===============================================================================
Welcome to the internet computer developer community!
To learn more before you start coding, check out the developer docs and samples:
- Documentation: https://internetcomputer.org/docs/current/developer-docs
- Samples: https://internetcomputer.org/samples
==============================================================================="
);
info!(log, r#"Created new project "{}""#, project_name.display());

Ok(())
}
Expand Down Expand Up @@ -668,49 +660,64 @@ fn run_post_create_command(

for command in &project_template.post_create {
let command = replace_variables(command.clone(), variables);
debug!(env.get_logger(), "Running command: {}", &command);
let mut cmd = direct_or_shell_command(&command, root)?;

let spinner = project_template
.post_create_spinner_message
.as_ref()
.map(|msg| env.new_spinner(msg.clone().into()));

let status = cmd
.stderr(Stdio::inherit())
.stdout(Stdio::inherit())
.status()
let output = cmd
.stderr(Stdio::piped())
.stdout(Stdio::piped())
.output()
.with_context(|| {
format!(
"Failed to run post-create command '{}' for project template '{}.",
"Failed to run post-create command '{}' for project template '{}'.",
&command, &project_template.name
)
});

if let Some(spinner) = spinner {
let message = match status {
Ok(status) if status.success() => "Done.",
_ => "Failed.",
};
spinner.finish_with_message(message.into());
spinner.finish_and_clear();
}

if let Ok(output) = &output {
if !output.status.success() {
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);

let msg = format!(
"Post-create command '{}' failed.\n--- stdout ---\n{}\n--- stderr ---\n{}",
&command, stdout, stderr
);
if project_template.post_create_failure_warning.is_some() {
warn!(log, "{}", msg);
} else {
error!(log, "{}", msg);
}
}
}

if let Some(warning) = &project_template.post_create_failure_warning {
warn_on_post_create_error(log, status, &command, warning);
warn_on_post_create_error(log, output, &command, warning);
} else {
fail_on_post_create_error(command, status)?;
fail_on_post_create_error(command, output)?;
}
}
Ok(())
}

fn warn_on_post_create_error(
log: &Logger,
status: Result<ExitStatus, Error>,
output: Result<Output, Error>,
command: &str,
warning: &str,
) {
match status {
Ok(status) if status.success() => {}
Ok(status) => match status.code() {
match output {
Ok(output) if output.status.success() => {}
Ok(output) => match output.status.code() {
Some(code) => {
warn!(
log,
Expand All @@ -730,13 +737,10 @@ fn warn_on_post_create_error(
}
}

fn fail_on_post_create_error(
command: String,
status: Result<ExitStatus, Error>,
) -> Result<(), Error> {
let status = status?;
if !status.success() {
match status.code() {
fn fail_on_post_create_error(command: String, output: Result<Output, Error>) -> Result<(), Error> {
let output = output?;
if !output.status.success() {
match output.status.code() {
Some(code) => {
bail!("Post-create command '{command}' failed with exit code {code}.")
}
Expand All @@ -753,6 +757,10 @@ fn write_project_template_resources(
dry_run: bool,
variables: &BTreeMap<String, String>,
) -> DfxResult {
debug!(
logger,
"Writing files for project template: {}", template.name
);
match &template.resource_location {
ResourceLocation::Bundled { get_archive_fn } => {
let mut resources = get_archive_fn()?;
Expand Down
6 changes: 5 additions & 1 deletion src/dfx/src/config/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,11 @@ pub fn install_version(v: &str, force: bool) -> Result<PathBuf, InstallCacheErro

if dfx_core::fs::rename(temp_p.as_path(), &p).is_ok() {
if let Some(b) = b {
b.finish_with_message(format!("Installed dfx {} to cache.", v));
if force {
b.finish_with_message(format!("Installed dfx {} to cache.", v));
} else {
b.finish_and_clear();
}
}
} else {
dfx_core::fs::remove_dir_all(temp_p.as_path())?;
Expand Down
1 change: 0 additions & 1 deletion src/dfx/src/lib/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ pub fn get_latest_version(
let manifest_url = url
.join("manifest.json")
.map_err(|e| error_invalid_argument!("invalid manifest URL: {}", e))?;
println!("Fetching manifest {}", manifest_url);

let b = ProgressBar::new_spinner();
b.set_draw_target(ProgressDrawTarget::stderr());
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/lib/progress_bar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl ProgressBar {
}

forward_fn_impl!(finish_and_clear);
forward_fn_impl!(finish_with_message, message: Cow<'static, str>);
forward_fn_impl!(set_message, message: Cow<'static, str>);

pub fn discard() -> Self {
ProgressBar { bar: None }
Expand Down
3 changes: 2 additions & 1 deletion src/dfx/src/lib/project/templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const NPM_INSTALL_SPINNER_MESSAGE: &str = "Installing node dependencies...";
const NPM_INSTALL_FAILURE_WARNING: &str =
"An error occurred. See the messages above for more details.";
const CARGO_UPDATE_FAILURE_MESSAGE: &str = "You will need to run it yourself (or a similar command like `cargo vendor`), because `dfx build` will use the --locked flag with Cargo.";
const CARGO_UPDATE_SPINNER_MESSAGE: &str = "Updating cargo lockfile...";

pub fn builtin_templates() -> Vec<ProjectTemplate> {
let motoko = ProjectTemplate {
Expand All @@ -32,7 +33,7 @@ pub fn builtin_templates() -> Vec<ProjectTemplate> {
category: ProjectTemplateCategory::Backend,
post_create: vec!["cargo update".to_string()],
post_create_failure_warning: Some(CARGO_UPDATE_FAILURE_MESSAGE.to_string()),
post_create_spinner_message: None,
post_create_spinner_message: Some(CARGO_UPDATE_SPINNER_MESSAGE.to_string()),
requirements: vec![],
sort_order: 1,
};
Expand Down

0 comments on commit b04e931

Please sign in to comment.