Repository: Enselic/cargo-public-api (rust)

Issue:
Add option to force git checkouts
I see from the `--diff-git-checkouts` option description that inability to do `git checkout` on a dirty tree is a feature. However, there are cases where it's not desirable. In our case rust build script produces some artefacts that are supposed to be checked in the repo. So, when the tool checks out the first commit and generates the docs the tree is not clean and the tools fails to checkout the second commit.

Would you accept a PR implementing an additional `--force-git-checkouts` option?

Pull request:
Add option `--force-git-checkouts` to force git checkouts
Closes #172 

Patch:
diff --git a/cargo-public-api/src/git_utils.rs b/cargo-public-api/src/git_utils.rs
index 2251f23..651cb4c 100644
--- a/cargo-public-api/src/git_utils.rs
+++ b/cargo-public-api/src/git_utils.rs
@@ -7,7 +7,7 @@ use anyhow::{anyhow, Context, Result};
 
 /// Synchronously do a `git checkout` of `commit`.
 /// Returns the name of the original branch/commit.
-pub fn git_checkout(commit: &str, git_root: &Path, quiet: bool) -> Result<String> {
+pub fn git_checkout(commit: &str, git_root: &Path, quiet: bool, force: bool) -> Result<String> {
     let original_branch = current_branch_or_commit(&git_root)?;
 
     let mut command = Command::new("git");
@@ -16,6 +16,9 @@ pub fn git_checkout(commit: &str, git_root: &Path, quiet: bool) -> Result<String
     if quiet {
         command.arg("--quiet");
     }
+    if force {
+        command.arg("--force");
+    }
     if command.spawn()?.wait()?.success() {
         Ok(original_branch)
     } else {
diff --git a/cargo-public-api/src/main.rs b/cargo-public-api/src/main.rs
index 517d8f0..f9d3687 100644
--- a/cargo-public-api/src/main.rs
+++ b/cargo-public-api/src/main.rs
@@ -70,6 +70,11 @@ pub struct Args {
     #[clap(long, min_values = 2, max_values = 2)]
     diff_git_checkouts: Option<Vec<String>>,
 
+    /// Raise this flag to discard working tree changes during git checkouts when
+    /// `--diff-git-checkouts` is used.
+    #[clap(long)]
+    force_git_checkouts: bool,
+
     /// Usage: --diff-rustdoc-json <RUSTDOC_JSON_PATH_1> <RUSTDOC_JSON_PATH_2>
     ///
     /// Diff the public API across two different rustdoc JSON files.
@@ -318,7 +323,12 @@ fn print_diff(args: &Args, old: Vec<PublicItem>, new: Vec<PublicItem>) -> Result
 impl PostProcessing {
     fn perform(&self, args: &Args) -> Result<()> {
         if let Some(branch_to_restore) = &self.branch_to_restore {
-            git_utils::git_checkout(branch_to_restore, &args.git_root()?, !args.verbose)?;
+            git_utils::git_checkout(
+                branch_to_restore,
+                &args.git_root()?,
+                !args.verbose,
+                args.force_git_checkouts,
+            )?;
         }
 
         check_diff(args, &self.diff_to_check)
@@ -366,6 +376,7 @@ fn collect_public_api_from_commit(
             commit,
             &args.git_root()?,
             !args.verbose,
+            args.force_git_checkouts,
         )?)
     } else {
         None


Test patch:
diff --git a/cargo-public-api/tests/cargo-public-api-bin-tests.rs b/cargo-public-api/tests/cargo-public-api-bin-tests.rs
index d45e4ed..83e8961 100644
--- a/cargo-public-api/tests/cargo-public-api-bin-tests.rs
+++ b/cargo-public-api/tests/cargo-public-api-bin-tests.rs
@@ -26,6 +26,24 @@ use test_utils::rustdoc_json_path_for_crate;
 #[path = "../src/git_utils.rs"] // Say NO to copy-paste!
 mod git_utils;
 
+fn create_test_repo_with_dirty_git_tree() -> TestRepo {
+    let test_repo = TestRepo::new();
+
+    // Make the tree dirty by appending a comment to src/lib.rs
+    let mut lib_rs_path = test_repo.path.path().to_owned();
+    lib_rs_path.push("src/lib.rs");
+
+    let mut lib_rs = OpenOptions::new()
+        .write(true)
+        .append(true)
+        .open(&lib_rs_path)
+        .unwrap();
+
+    writeln!(lib_rs, "// Make git tree dirty").unwrap();
+
+    test_repo
+}
+
 // FIXME: This tests is ignored in CI due to some unknown issue with windows
 #[test]
 #[cfg_attr(all(target_family = "windows", in_ci), ignore)]
@@ -151,7 +169,7 @@ fn diff_public_items_detached_head() {
 
     // Detach HEAD
     let path = test_repo.path();
-    git_utils::git_checkout("v0.1.1", path, true).unwrap();
+    git_utils::git_checkout("v0.1.1", path, true, false).unwrap();
     assert_eq!(None, git_utils::current_branch(path).unwrap());
     let before = git_utils::current_commit(path).unwrap();
 
@@ -173,19 +191,7 @@ fn diff_public_items_detached_head() {
 #[test]
 #[cfg_attr(target_family = "windows", ignore)]
 fn diff_public_items_with_dirty_tree_fails() {
-    let test_repo = TestRepo::new();
-
-    // Make the tree dirty by appending a comment to src/lib.rs
-    let mut lib_rs_path = test_repo.path.path().to_owned();
-    lib_rs_path.push("src/lib.rs");
-
-    let mut lib_rs = OpenOptions::new()
-        .write(true)
-        .append(true)
-        .open(&lib_rs_path)
-        .unwrap();
-
-    writeln!(lib_rs, "// Make git tree dirty").unwrap();
+    let test_repo = create_test_repo_with_dirty_git_tree();
 
     // Make sure diffing does not destroy uncommitted data!
     let mut cmd = Command::cargo_bin("cargo-public-api").unwrap();
@@ -201,6 +207,25 @@ fn diff_public_items_with_dirty_tree_fails() {
         .failure();
 }
 
+/// Test that diffing succeedes if the git tree is dirty and
+/// `force-git-checkout` option is specified.
+#[test]
+#[cfg_attr(target_family = "windows", ignore)]
+fn diff_public_items_with_dirty_tree_succeedes_with_force_option() {
+    let test_repo = create_test_repo_with_dirty_git_tree();
+
+    let mut cmd = Command::cargo_bin("cargo-public-api").unwrap();
+    cmd.current_dir(&test_repo.path);
+    cmd.arg("--color=never");
+    cmd.arg("--diff-git-checkouts");
+    cmd.arg("v0.2.0");
+    cmd.arg("v0.3.0");
+    cmd.arg("--force-git-checkouts");
+    cmd.assert()
+        .stdout_or_bless("./tests/expected-output/example_api_diff_v0.2.0_to_v0.3.0.txt")
+        .success();
+}
+
 #[test]
 fn deny_when_not_diffing() {
     let mut cmd = TestCmd::new();