From a1f2f5a53f1aa32b63d66629c2f22d84aae5f4a9 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 22 Nov 2022 08:50:52 +0800 Subject: [PATCH 1/5] Add error message when `cargo fix` on an empty repo Signed-off-by: hi-rustin --- src/cargo/ops/fix.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index d143beb7719..4192193c775 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -154,6 +154,14 @@ fn check_version_control(config: &Config, opts: &FixOptions) -> CargoResult<()> if let Ok(repo) = git2::Repository::discover(config.cwd()) { let mut repo_opts = git2::StatusOptions::new(); repo_opts.include_ignored(false); + if repo.is_empty()? { + bail!( + "no commits found in the git repository, and \ + `cargo fix` can potentially perform destructive changes; if you'd \ + like to suppress this error pass `--allow-dirty`, `--allow-staged`, \ + or commit your changes" + ) + } for status in repo.statuses(Some(&mut repo_opts))?.iter() { if let Some(path) = status.path() { match status.status() { From 89b8a8bb1083578a02dd501f4bfb669a7c84a399 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 22 Nov 2022 09:05:21 +0800 Subject: [PATCH 2/5] Suppress error with `--allow-dirty` and add a test Signed-off-by: hi-rustin --- crates/cargo-test-support/src/git.rs | 14 ++++++++++++++ src/cargo/ops/fix.rs | 4 ++-- tests/testsuite/fix.rs | 23 +++++++++++++++++++++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/crates/cargo-test-support/src/git.rs b/crates/cargo-test-support/src/git.rs index 18c4646b37d..cb2ea38a8c0 100644 --- a/crates/cargo-test-support/src/git.rs +++ b/crates/cargo-test-support/src/git.rs @@ -175,6 +175,20 @@ where (git_project, repo) } +/// Create a new git repository with a project. +/// Returns both the Project and the git Repository but does not commit. +pub fn new_repo_without_add_and_commit(name: &str, callback: F) -> (Project, git2::Repository) +where + F: FnOnce(ProjectBuilder) -> ProjectBuilder, +{ + let mut git_project = project().at(name); + git_project = callback(git_project); + let git_project = git_project.build(); + + let repo = init(&git_project.root()); + (git_project, repo) +} + /// Add all files in the working directory to the git index. pub fn add(repo: &git2::Repository) { // FIXME(libgit2/libgit2#2514): apparently, `add_all` will add all submodules diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 4192193c775..40297ec3f11 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -154,11 +154,11 @@ fn check_version_control(config: &Config, opts: &FixOptions) -> CargoResult<()> if let Ok(repo) = git2::Repository::discover(config.cwd()) { let mut repo_opts = git2::StatusOptions::new(); repo_opts.include_ignored(false); - if repo.is_empty()? { + if repo.is_empty()? && !opts.allow_dirty { bail!( "no commits found in the git repository, and \ `cargo fix` can potentially perform destructive changes; if you'd \ - like to suppress this error pass `--allow-dirty`, `--allow-staged`, \ + like to suppress this error pass `--allow-dirty`, \ or commit your changes" ) } diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index b56a89f47bf..790d105d74b 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -771,6 +771,25 @@ commit the changes to these files: p.cargo("fix --allow-staged").run(); } +#[cargo_test] +fn errors_on_empty_repo() { + let (p, _) = + git::new_repo_without_add_and_commit("foo", |p| p.file("src/lib.rs", "pub fn foo() {}")); + + p.cargo("fix") + .with_status(101) + .with_stderr( + "\ +error: no commits found in the git repository, \ +and `cargo fix` can potentially perform destructive changes; \ +if you'd like to suppress this error pass `--allow-dirty`, \ +or commit your changes +", + ) + .run(); + p.cargo("fix --allow-dirty").run(); +} + #[cargo_test] fn does_not_warn_about_clean_working_directory() { let p = git::new("foo", |p| p.file("src/lib.rs", "pub fn foo() {}")); @@ -1715,7 +1734,7 @@ fn fix_with_run_cargo_in_proc_macros() { "src/lib.rs", r#" use proc_macro::*; - + #[proc_macro] pub fn foo(_input: TokenStream) -> TokenStream { let output = std::process::Command::new(env!("CARGO")) @@ -1725,7 +1744,7 @@ fn fix_with_run_cargo_in_proc_macros() { eprintln!("{}", std::str::from_utf8(&output.stderr).unwrap()); println!("{}", std::str::from_utf8(&output.stdout).unwrap()); "".parse().unwrap() - } + } "#, ) .file( From 25f7d4b09a5a42666264cc61ab50aea95703a3f2 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 22 Nov 2022 18:56:52 +0800 Subject: [PATCH 3/5] Remove new_repo_without_add_and_commit Signed-off-by: hi-rustin --- crates/cargo-test-support/src/git.rs | 14 -------------- tests/testsuite/fix.rs | 8 +++++--- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/crates/cargo-test-support/src/git.rs b/crates/cargo-test-support/src/git.rs index cb2ea38a8c0..18c4646b37d 100644 --- a/crates/cargo-test-support/src/git.rs +++ b/crates/cargo-test-support/src/git.rs @@ -175,20 +175,6 @@ where (git_project, repo) } -/// Create a new git repository with a project. -/// Returns both the Project and the git Repository but does not commit. -pub fn new_repo_without_add_and_commit(name: &str, callback: F) -> (Project, git2::Repository) -where - F: FnOnce(ProjectBuilder) -> ProjectBuilder, -{ - let mut git_project = project().at(name); - git_project = callback(git_project); - let git_project = git_project.build(); - - let repo = init(&git_project.root()); - (git_project, repo) -} - /// Add all files in the working directory to the git index. pub fn add(repo: &git2::Repository) { // FIXME(libgit2/libgit2#2514): apparently, `add_all` will add all submodules diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 790d105d74b..897433cb000 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -2,7 +2,7 @@ use cargo::core::Edition; use cargo_test_support::compare::assert_match_exact; -use cargo_test_support::git; +use cargo_test_support::git::{self, init}; use cargo_test_support::paths::{self, CargoPathExt}; use cargo_test_support::registry::{Dependency, Package}; use cargo_test_support::tools; @@ -773,8 +773,10 @@ commit the changes to these files: #[cargo_test] fn errors_on_empty_repo() { - let (p, _) = - git::new_repo_without_add_and_commit("foo", |p| p.file("src/lib.rs", "pub fn foo() {}")); + let mut git_project = project().at("foo"); + git_project = git_project.file("src/lib.rs", "pub fn foo() {}"); + let p = git_project.build(); + let _ = init(&p.root()); p.cargo("fix") .with_status(101) From 3189dc3b0673b551339ea9172980450e80f21b17 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Sat, 26 Nov 2022 19:37:19 +0800 Subject: [PATCH 4/5] Include untracked files Signed-off-by: hi-rustin --- src/cargo/ops/fix.rs | 9 +-------- tests/testsuite/fix.rs | 15 ++++++++++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 40297ec3f11..2b8c55b41c7 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -154,14 +154,7 @@ fn check_version_control(config: &Config, opts: &FixOptions) -> CargoResult<()> if let Ok(repo) = git2::Repository::discover(config.cwd()) { let mut repo_opts = git2::StatusOptions::new(); repo_opts.include_ignored(false); - if repo.is_empty()? && !opts.allow_dirty { - bail!( - "no commits found in the git repository, and \ - `cargo fix` can potentially perform destructive changes; if you'd \ - like to suppress this error pass `--allow-dirty`, \ - or commit your changes" - ) - } + repo_opts.include_untracked(true); for status in repo.statuses(Some(&mut repo_opts))?.iter() { if let Some(path) = status.path() { match status.status() { diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 897433cb000..984972a4a70 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -772,7 +772,7 @@ commit the changes to these files: } #[cargo_test] -fn errors_on_empty_repo() { +fn errors_about_untracked_files() { let mut git_project = project().at("foo"); git_project = git_project.file("src/lib.rs", "pub fn foo() {}"); let p = git_project.build(); @@ -782,10 +782,15 @@ fn errors_on_empty_repo() { .with_status(101) .with_stderr( "\ -error: no commits found in the git repository, \ -and `cargo fix` can potentially perform destructive changes; \ -if you'd like to suppress this error pass `--allow-dirty`, \ -or commit your changes +error: the working directory of this package has uncommitted changes, \ +and `cargo fix` can potentially perform destructive changes; if you'd \ +like to suppress this error pass `--allow-dirty`, `--allow-staged`, or \ +commit the changes to these files: + + * Cargo.toml (dirty) + * src/ (dirty) + + ", ) .run(); From 21b2fe98a4076381d173c67c563b7093458f9643 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Sat, 26 Nov 2022 20:15:25 +0800 Subject: [PATCH 5/5] Fix fix_in_existing_repo_weird_ignore broken test Signed-off-by: hi-rustin --- tests/testsuite/fix.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 984972a4a70..0b88519f5a0 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -1431,7 +1431,7 @@ fn fix_in_existing_repo_weird_ignore() { let p = git::new("foo", |project| { project .file("src/lib.rs", "") - .file(".gitignore", "foo\ninner\n") + .file(".gitignore", "foo\ninner\nCargo.lock\ntarget\n") .file("inner/file", "") });