From 27dd8a01ce5eb3dc52ea8c14962145c3ec5a4989 Mon Sep 17 00:00:00 2001 From: Denny Biasiolli Date: Mon, 19 Feb 2024 10:59:43 +0100 Subject: [PATCH 1/3] cli: using fs::create_dir_all in create_migrations_directory To recursively create a directory and all of its parent components if they are missing. --- diesel_cli/src/main.rs | 2 +- diesel_cli/tests/database_setup.rs | 33 +++++++++++++++++++++ diesel_cli/tests/support/project_builder.rs | 2 +- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index 045248292c85..b21e99ef7014 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -183,7 +183,7 @@ fn generate_completions_command(matches: &ArgMatches) { /// Returns a `DatabaseError::ProjectRootNotFound` if no Cargo.toml is found. fn create_migrations_directory(path: &Path) -> Result { println!("Creating migrations directory at: {}", path.display()); - fs::create_dir(path)?; + fs::create_dir_all(path)?; fs::File::create(path.join(".keep"))?; Ok(path.to_owned()) } diff --git a/diesel_cli/tests/database_setup.rs b/diesel_cli/tests/database_setup.rs index ffedc1630a20..947ee706b149 100644 --- a/diesel_cli/tests/database_setup.rs +++ b/diesel_cli/tests/database_setup.rs @@ -152,6 +152,39 @@ fn database_setup_respects_migration_dir_by_arg() { assert!(db.table_exists("users")); } +#[test] +fn database_setup_respects_migration_nested_dir_by_arg() { + let p = project("database_setup_respects_migration_nested_dir_by_arg") + .folder("foo/bar") + .build(); + let db = database(&p.database_url()); + + p.create_migration_in_directory( + "foo/bar", + "12345_create_users_table", + "CREATE TABLE users ( id INTEGER )", + Some("DROP TABLE users"), + None, + ); + + // sanity check + assert!(!db.exists()); + + let result = p + .command("database") + .arg("setup") + .arg("--migration-dir=foo/bar") + .run(); + + assert!(result.is_success(), "Result was unsuccessful {:?}", result); + assert!( + result.stdout().contains("Running migration 12345"), + "Unexpected stdout {}", + result.stdout() + ); + assert!(db.table_exists("users")); +} + #[test] fn database_setup_respects_migration_dir_by_env() { let p = project("database_setup_respects_migration_dir_by_env") diff --git a/diesel_cli/tests/support/project_builder.rs b/diesel_cli/tests/support/project_builder.rs index c1199b1aba17..5110e409c599 100644 --- a/diesel_cli/tests/support/project_builder.rs +++ b/diesel_cli/tests/support/project_builder.rs @@ -45,7 +45,7 @@ impl ProjectBuilder { File::create(tempdir.path().join("Cargo.toml")).unwrap(); for folder in self.folders { - fs::create_dir(tempdir.path().join(folder)).unwrap(); + fs::create_dir_all(tempdir.path().join(folder)).unwrap(); } for (file, contents) in self.files { From 02e0f7623bb9f47449efe120b433fcb229da469c Mon Sep 17 00:00:00 2001 From: Denny Biasiolli Date: Mon, 19 Feb 2024 13:01:09 +0100 Subject: [PATCH 2/3] cli: writing the custom migration dir to the config file --- diesel_cli/src/main.rs | 14 +++++++++++--- diesel_cli/tests/setup.rs | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index b21e99ef7014..77df4aba7a59 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -96,8 +96,8 @@ where #[tracing::instrument] fn run_setup_command(matches: &ArgMatches) -> Result<(), crate::errors::Error> { - create_config_file(matches)?; let migrations_dir = create_migrations_dir(matches)?; + create_config_file(matches, &migrations_dir)?; database::setup_database(matches, &migrations_dir)?; Ok(()) @@ -136,12 +136,20 @@ fn create_migrations_dir(matches: &ArgMatches) -> Result Result<(), crate::errors::Error> { +fn create_config_file( + matches: &ArgMatches, + migrations_dir: &Path, +) -> Result<(), crate::errors::Error> { use std::io::Write; let path = Config::file_path(matches); if !path.exists() { + let source_content = include_str!("default_files/diesel.toml").to_string(); + let modified_content = source_content.replace( + "dir = \"migrations\"", + &format!("dir = \"{}\"", migrations_dir.display()), + ); let mut file = fs::File::create(path)?; - file.write_all(include_bytes!("default_files/diesel.toml"))?; + file.write_all(modified_content.as_bytes())?; } Ok(()) diff --git a/diesel_cli/tests/setup.rs b/diesel_cli/tests/setup.rs index d0bab07b5b17..f5d3adbab6be 100644 --- a/diesel_cli/tests/setup.rs +++ b/diesel_cli/tests/setup.rs @@ -170,6 +170,22 @@ fn setup_works_with_migration_dir_by_arg() { assert!(p.has_file("foo")); } +#[test] +fn setup_writes_migration_dir_by_arg_to_config_file() { + let p = project("setup_writes_migration_dir_by_arg_to_config_file").build(); + + // make sure the project builder doesn't create it for us + assert!(!p.has_file("migrations")); + assert!(!p.has_file("foo")); + + let result = p.command("setup").arg("--migration-dir=foo").run(); + + assert!(result.is_success(), "Result was unsuccessful {:?}", result); + assert!(!p.has_file("migrations")); + assert!(p.has_file("foo")); + assert!(p.file_contents("diesel.toml").contains("dir = \"foo\"")); +} + #[test] fn setup_works_with_migration_dir_by_env() { let p = project("setup_works_with_migration_dir_by_env").build(); From 8a4db857a6ac8bd4ceb19b60b908336fab359454 Mon Sep 17 00:00:00 2001 From: Denny Biasiolli Date: Thu, 22 Feb 2024 14:49:25 +0100 Subject: [PATCH 3/3] cli: fixing windows paths in toml strings and adding a specific test for that --- diesel_cli/src/main.rs | 4 +++- diesel_cli/tests/setup.rs | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index 77df4aba7a59..a0ae06a971a3 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -144,9 +144,11 @@ fn create_config_file( let path = Config::file_path(matches); if !path.exists() { let source_content = include_str!("default_files/diesel.toml").to_string(); + // convert the path to a valid toml string (escaping backslashes on windows) + let migrations_dir_toml_string = migrations_dir.display().to_string().replace('\\', "\\\\"); let modified_content = source_content.replace( "dir = \"migrations\"", - &format!("dir = \"{}\"", migrations_dir.display()), + &format!("dir = \"{}\"", migrations_dir_toml_string), ); let mut file = fs::File::create(path)?; file.write_all(modified_content.as_bytes())?; diff --git a/diesel_cli/tests/setup.rs b/diesel_cli/tests/setup.rs index f5d3adbab6be..a65a0d39ff28 100644 --- a/diesel_cli/tests/setup.rs +++ b/diesel_cli/tests/setup.rs @@ -186,6 +186,25 @@ fn setup_writes_migration_dir_by_arg_to_config_file() { assert!(p.file_contents("diesel.toml").contains("dir = \"foo\"")); } +#[test] +#[cfg(windows)] +fn setup_writes_migration_dir_by_arg_to_config_file_win() { + let p = project("setup_writes_migration_dir_by_arg_to_config_file_win").build(); + + // make sure the project builder doesn't create it for us + assert!(!p.has_file("migrations")); + assert!(!p.has_file("foo")); + + let result = p.command("setup").arg("--migration-dir=foo\\bar").run(); + + assert!(result.is_success(), "Result was unsuccessful {:?}", result); + assert!(!p.has_file("migrations")); + assert!(p.has_file("foo")); + assert!(p + .file_contents("diesel.toml") + .contains("dir = \"foo\\\\bar\"")); +} + #[test] fn setup_works_with_migration_dir_by_env() { let p = project("setup_works_with_migration_dir_by_env").build();