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

Feat/init overwrite #1477

Merged
merged 7 commits into from
Jul 23, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions FULL_HELP_DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ Initialize a Soroban project with an example contract
* `-f`, `--frontend-template <FRONTEND_TEMPLATE>` — An optional flag to pass in a url for a frontend template repository.

Default value: ``
* `-o`, `--overwrite` — Overwrite all existing files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add the short option -o, opened a PR removing it and why is mentioned there:




Expand Down
1 change: 1 addition & 0 deletions cmd/soroban-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,4 @@ ureq = { version = "2.9.1", features = ["json"] }
assert_cmd = "2.0.4"
assert_fs = "1.0.7"
predicates = "2.1.5"
walkdir = "2.5.0"
155 changes: 124 additions & 31 deletions cmd/soroban-cli/src/commands/contract/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ pub struct Cmd {
long_help = "An optional flag to pass in a url for a frontend template repository."
)]
pub frontend_template: String,

#[arg(short, long, long_help = "Overwrite all existing files.")]
Copy link
Member

@leighmcculloch leighmcculloch Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the intent that the command will overwrite the entire directory such as remove the directory contents first, then start writing? Or is the intent that the command will overwrite individual files also included in the template and leave any additional files as they are?

The term 'all' may introduce some ambiguity to the scope of what is removed, and given the destructive nature of this command we should try and limit the ambiguity.

The term 'any' may limit the ambiguity, although not perfectly.

Suggested change
#[arg(short, long, long_help = "Overwrite all existing files.")]
#[arg(short, long, long_help = "Overwrite any existing files.")]

pub overwrite: bool,
}

fn possible_example_values() -> ValueParser {
Expand Down Expand Up @@ -89,7 +92,12 @@ impl Cmd {
println!("ℹ️ Initializing project at {}", self.project_path);
let project_path = Path::new(&self.project_path);

init(project_path, &self.frontend_template, &self.with_example)?;
init(
project_path,
&self.frontend_template,
&self.with_example,
self.overwrite,
)?;

Ok(())
}
Expand All @@ -103,13 +111,14 @@ fn init(
project_path: &Path,
frontend_template: &str,
with_examples: &[String],
overwrite: bool,
) -> Result<(), Error> {
// create a project dir, and copy the contents of the base template (contract-init-template) into it
create_dir_all(project_path).map_err(|e| {
eprintln!("Error creating new project directory: {project_path:?}");
e
})?;
copy_template_files(project_path)?;
copy_template_files(project_path, overwrite)?;

if !check_internet_connection() {
println!("⚠️ It doesn't look like you're connected to the internet. We're still able to initialize a new project, but additional examples and the frontend template will not be included.");
Expand All @@ -127,7 +136,7 @@ fn init(
clone_repo(frontend_template, fe_template_dir.path())?;

// copy the frontend template files into the project
copy_frontend_files(fe_template_dir.path(), project_path)?;
copy_frontend_files(fe_template_dir.path(), project_path, overwrite)?;
}

// if there are --with-example flags, include the example contracts
Expand All @@ -142,17 +151,17 @@ fn init(
clone_repo(SOROBAN_EXAMPLES_URL, examples_dir.path())?;

// copy the example contracts into the project
copy_example_contracts(examples_dir.path(), project_path, with_examples)?;
copy_example_contracts(examples_dir.path(), project_path, with_examples, overwrite)?;
}

Ok(())
}

fn copy_template_files(project_path: &Path) -> Result<(), Error> {
fn copy_template_files(project_path: &Path, overwrite: bool) -> Result<(), Error> {
for item in TemplateFiles::iter() {
let mut to = project_path.join(item.as_ref());

if file_exists(&to) {
let exists = file_exists(&to);
if exists && !overwrite {
println!(
"ℹ️ Skipped creating {} as it already exists",
&to.to_string_lossy()
Expand Down Expand Up @@ -184,7 +193,11 @@ fn copy_template_files(project_path: &Path) -> Result<(), Error> {
to = project_path.join(item_parent_path).join("Cargo.toml");
}

println!("➕ Writing {}", &to.to_string_lossy());
if exists {
println!("🔄 Overwriting {}", &to.to_string_lossy());
} else {
println!("➕ Writing {}", &to.to_string_lossy());
}
write(&to, file_contents).map_err(|e| {
eprintln!("Error writing file: {to:?}");
e
Expand All @@ -193,7 +206,7 @@ fn copy_template_files(project_path: &Path) -> Result<(), Error> {
Ok(())
}

fn copy_contents(from: &Path, to: &Path) -> Result<(), Error> {
fn copy_contents(from: &Path, to: &Path, overwrite: bool) -> Result<(), Error> {
let contents_to_exclude_from_copy = [
".git",
".github",
Expand Down Expand Up @@ -223,24 +236,26 @@ fn copy_contents(from: &Path, to: &Path) -> Result<(), Error> {
eprintln!("Error creating directory: {new_path:?}");
e
})?;
copy_contents(&path, &new_path)?;
copy_contents(&path, &new_path, overwrite)?;
} else {
if file_exists(&new_path) {
if new_path.to_string_lossy().contains(".gitignore") {
append_contents(&path, &new_path)?;
}
if new_path.to_string_lossy().contains("README.md") {
let exists = file_exists(&new_path);
let new_path_str = new_path.to_string_lossy();
if exists {
let append =
new_path_str.contains(".gitignore") || new_path_str.contains("README.md");
if append {
append_contents(&path, &new_path)?;
}

println!(
"ℹ️ Skipped creating {} as it already exists",
&new_path.to_string_lossy()
);
continue;
if overwrite && !append {
println!("🔄 Overwriting {new_path_str}");
} else {
println!("ℹ️ Skipped creating {new_path_str} as it already exists");
continue;
}
} else {
println!("➕ Writing {new_path_str}");
}

println!("➕ Writing {}", &new_path.to_string_lossy());
copy(&path, &new_path).map_err(|e| {
eprintln!(
"Error copying from {:?} to {:?}",
Expand Down Expand Up @@ -302,7 +317,12 @@ fn clone_repo(from_url: &str, to_path: &Path) -> Result<(), Error> {
Ok(())
}

fn copy_example_contracts(from: &Path, to: &Path, contracts: &[String]) -> Result<(), Error> {
fn copy_example_contracts(
from: &Path,
to: &Path,
contracts: &[String],
overwrite: bool,
) -> Result<(), Error> {
let project_contracts_path = to.join("contracts");
for contract in contracts {
println!("ℹ️ Initializing example contract: {contract}");
Expand All @@ -315,7 +335,7 @@ fn copy_example_contracts(from: &Path, to: &Path, contracts: &[String]) -> Resul
e
})?;

copy_contents(&from_contract_path, &to_contract_path)?;
copy_contents(&from_contract_path, &to_contract_path, overwrite)?;
edit_contract_cargo_file(&to_contract_path)?;
}

Expand Down Expand Up @@ -354,9 +374,9 @@ fn edit_contract_cargo_file(contract_path: &Path) -> Result<(), Error> {
Ok(())
}

fn copy_frontend_files(from: &Path, to: &Path) -> Result<(), Error> {
fn copy_frontend_files(from: &Path, to: &Path, overwrite: bool) -> Result<(), Error> {
println!("ℹ️ Initializing with frontend template");
copy_contents(from, to)?;
copy_contents(from, to, overwrite)?;
edit_package_json_files(to)
}

Expand Down Expand Up @@ -447,7 +467,13 @@ fn get_merged_file_delimiter(file_path: &Path) -> String {
#[cfg(test)]
mod tests {
use itertools::Itertools;
use std::fs::{self, read_to_string};
use std::{
collections::HashMap,
fs::{self, read_to_string},
path::PathBuf,
time::SystemTime,
};
use walkdir::WalkDir;

use super::*;

Expand All @@ -458,7 +484,8 @@ mod tests {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join(TEST_PROJECT_NAME);
let with_examples = vec![];
init(project_dir.as_path(), "", &with_examples).unwrap();
let overwrite = false;
init(project_dir.as_path(), "", &with_examples, overwrite).unwrap();

assert_base_template_files_exist(&project_dir);
assert_default_hello_world_contract_files_exist(&project_dir);
Expand All @@ -476,7 +503,8 @@ mod tests {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join(TEST_PROJECT_NAME);
let with_examples = ["alloc".to_owned()];
init(project_dir.as_path(), "", &with_examples).unwrap();
let overwrite = false;
init(project_dir.as_path(), "", &with_examples, overwrite).unwrap();

assert_base_template_files_exist(&project_dir);
assert_default_hello_world_contract_files_exist(&project_dir);
Expand All @@ -499,7 +527,8 @@ mod tests {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join("project");
let with_examples = ["account".to_owned(), "atomic_swap".to_owned()];
init(project_dir.as_path(), "", &with_examples).unwrap();
let overwrite = false;
init(project_dir.as_path(), "", &with_examples, overwrite).unwrap();

assert_base_template_files_exist(&project_dir);
assert_default_hello_world_contract_files_exist(&project_dir);
Expand All @@ -523,7 +552,8 @@ mod tests {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join("project");
let with_examples = ["invalid_example".to_owned(), "atomic_swap".to_owned()];
assert!(init(project_dir.as_path(), "", &with_examples,).is_err());
let overwrite = false;
assert!(init(project_dir.as_path(), "", &with_examples, overwrite).is_err());

temp_dir.close().unwrap();
}
Expand All @@ -533,10 +563,12 @@ mod tests {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join(TEST_PROJECT_NAME);
let with_examples = vec![];
let overwrite = false;
init(
project_dir.as_path(),
"https://github.com/stellar/soroban-astro-template",
&with_examples,
overwrite,
)
.unwrap();

Expand All @@ -556,15 +588,73 @@ mod tests {
temp_dir.close().unwrap();
}

#[test]
fn test_init_with_overwrite() {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join(TEST_PROJECT_NAME);
let with_examples = vec![];

// First initialization
init(
project_dir.as_path(),
"https://github.com/stellar/soroban-astro-template",
&with_examples,
false,
)
.unwrap();

// Get initial modification times
let initial_mod_times = get_mod_times(&project_dir);

// Second initialization with overwrite
init(
project_dir.as_path(),
"https://github.com/stellar/soroban-astro-template",
&with_examples,
true, // overwrite = true
)
.unwrap();

// Get new modification times
let new_mod_times = get_mod_times(&project_dir);

// Compare modification times
for (path, initial_time) in initial_mod_times {
let new_time = new_mod_times.get(&path).expect("File should still exist");
assert!(
new_time > &initial_time,
"File {} should have a later modification time",
path.display()
);
}

temp_dir.close().unwrap();
}

fn get_mod_times(dir: &Path) -> HashMap<PathBuf, SystemTime> {
let mut mod_times = HashMap::new();
for entry in WalkDir::new(dir) {
let entry = entry.unwrap();
if entry.file_type().is_file() {
let path = entry.path().to_owned();
let metadata = fs::metadata(&path).unwrap();
mod_times.insert(path, metadata.modified().unwrap());
}
}
mod_times
}

#[test]
fn test_init_from_within_an_existing_project() {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join("./");
let with_examples = vec![];
let overwrite = false;
init(
project_dir.as_path(),
"https://github.com/stellar/soroban-astro-template",
&with_examples,
overwrite,
)
.unwrap();

Expand All @@ -591,10 +681,12 @@ mod tests {
let temp_dir = tempfile::tempdir().unwrap();
let project_dir = temp_dir.path().join(TEST_PROJECT_NAME);
let with_examples = vec![];
let overwrite = false;
init(
project_dir.as_path(),
"https://github.com/stellar/soroban-astro-template",
&with_examples,
overwrite,
)
.unwrap();

Expand All @@ -603,6 +695,7 @@ mod tests {
project_dir.as_path(),
"https://github.com/stellar/soroban-astro-template",
&with_examples,
overwrite,
)
.unwrap();

Expand Down
Loading