From 97b6eb26849650f975af0a2b34763694a16601c8 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 9 Aug 2023 12:27:48 +0900
Subject: [PATCH] cli: clean up .git directory if "jj git clone --colocate"
 failed

---
 cli/src/commands/git.rs     | 18 +++++++++++-------
 cli/tests/test_git_clone.rs |  6 ++----
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs
index 263e9d588..d0fa8f1e0 100644
--- a/cli/src/commands/git.rs
+++ b/cli/src/commands/git.rs
@@ -1,11 +1,11 @@
 use std::collections::HashSet;
-use std::fs;
 use std::io::{Read, Write};
 use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use std::sync::Mutex;
 use std::time::Instant;
+use std::{fs, io};
 
 use clap::{ArgGroup, Subcommand};
 use itertools::Itertools;
@@ -418,13 +418,17 @@ fn cmd_git_clone(
     if clone_result.is_err() {
         // Canonicalize because fs::remove_dir_all() doesn't seem to like e.g.
         // `/some/path/.`
-        if let Err(err) = fs::remove_dir_all(canonical_wc_path.join(".jj")).and_then(|_| {
-            if !wc_path_existed {
-                fs::remove_dir(&canonical_wc_path)
-            } else {
-                Ok(())
+        let clean_up_dirs = || -> io::Result<()> {
+            fs::remove_dir_all(canonical_wc_path.join(".jj"))?;
+            if args.colocate {
+                fs::remove_dir_all(canonical_wc_path.join(".git"))?;
             }
-        }) {
+            if !wc_path_existed {
+                fs::remove_dir(&canonical_wc_path)?;
+            }
+            Ok(())
+        };
+        if let Err(err) = clean_up_dirs() {
             writeln!(
                 ui.warning(),
                 "Failed to clean up {}: {}",
diff --git a/cli/tests/test_git_clone.rs b/cli/tests/test_git_clone.rs
index 2bd5bf6c6..d0ac81f0e 100644
--- a/cli/tests/test_git_clone.rs
+++ b/cli/tests/test_git_clone.rs
@@ -219,11 +219,9 @@ fn test_git_clone_colocate() {
     Fetching into new repo in "$TEST_ENV/failed"
     "###);
     insta::assert_snapshot!(stderr, @r###"
-    Failed to clean up $TEST_ENV/failed: Directory not empty (os error 39)
     Error: could not find repository from '$TEST_ENV/bad'; class=Repository (6)
     "###);
-    // FIXME: assert!(!test_env.env_root().join("failed").exists());
-    std::fs::remove_dir_all(test_env.env_root().join("failed")).unwrap();
+    assert!(!test_env.env_root().join("failed").exists());
 
     // Failed clone shouldn't remove the existing destination directory
     std::fs::create_dir(test_env.env_root().join("failed")).unwrap();
@@ -243,7 +241,7 @@ fn test_git_clone_colocate() {
     Error: could not find repository from '$TEST_ENV/bad'; class=Repository (6)
     "###);
     assert!(test_env.env_root().join("failed").exists());
-    // FIXME: assert!(!test_env.env_root().join("failed").join(".git").exists());
+    assert!(!test_env.env_root().join("failed").join(".git").exists());
     assert!(!test_env.env_root().join("failed").join(".jj").exists());
 
     // Failed clone (if attempted) shouldn't remove the existing workspace