From a2b4bd239f8ee51227c971674a5eb645c1d8e74c Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Wed, 29 Jun 2022 21:55:20 -0700
Subject: [PATCH] cli: check that clone target is an empty dir if it exists

We used to check only if the destination already had a `.jj/`
directory. This patch changes that to check that the destination is an
empty directory.

Closes #399.
---
 src/commands.rs         | 14 +++++++++++++-
 tests/test_git_clone.rs | 20 ++++++++------------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/commands.rs b/src/commands.rs
index f1a598496..95028695e 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -4964,6 +4964,14 @@ fn clone_destination_for_source(source: &str) -> Option<&str> {
         .map(|(_, name)| name)
 }
 
+fn is_empty_dir(path: &Path) -> bool {
+    if let Ok(mut entries) = path.read_dir() {
+        entries.next().is_none()
+    } else {
+        false
+    }
+}
+
 fn cmd_git_clone(
     ui: &mut Ui,
     command: &CommandHelper,
@@ -4986,7 +4994,11 @@ fn cmd_git_clone(
         })?;
     let wc_path = ui.cwd().join(wc_path_str);
     if wc_path.exists() {
-        assert!(wc_path.is_dir());
+        if !is_empty_dir(&wc_path) {
+            return Err(CommandError::UserError(
+                "Destination path exists and is not an empty directory".to_string(),
+            ));
+        }
     } else {
         fs::create_dir(&wc_path).unwrap();
     }
diff --git a/tests/test_git_clone.rs b/tests/test_git_clone.rs
index 18cf6021e..d5c84ca9f 100644
--- a/tests/test_git_clone.rs
+++ b/tests/test_git_clone.rs
@@ -61,24 +61,20 @@ fn test_git_clone() {
     // Try cloning into an existing workspace
     let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "clone"]);
     insta::assert_snapshot!(stderr.replace(test_env.env_root().join("clone").to_str().unwrap(), "<dest>"), @r###"
-    Error: The target repo already exists
+    Error: Destination path exists and is not an empty directory
     "###);
 
     // Try cloning into an existing file
     std::fs::write(test_env.env_root().join("file"), "contents").unwrap();
-    // TODO: This shouldn't crash (i.e. not exit code 101)
-    test_env
-        .jj_cmd(test_env.env_root(), &["git", "clone", "source", "file"])
-        .assert()
-        .code(101);
+    let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "file"]);
+    insta::assert_snapshot!(stderr.replace(test_env.env_root().join("file").to_str().unwrap(), "<dest>"), @r###"
+    Error: Destination path exists and is not an empty directory
+    "###);
 
     // Try cloning into non-empty, non-workspace directory
     std::fs::remove_dir_all(test_env.env_root().join("clone").join(".jj")).unwrap();
-    // TODO: This should fail (https://github.com/martinvonz/jj/issues/399)
-    let stdout = test_env.jj_cmd_success(test_env.env_root(), &["git", "clone", "source", "clone"]);
-    insta::assert_snapshot!(stdout.replace(test_env.env_root().join("clone").to_str().unwrap(), "<dest>"), @r###"
-    Fetching into new repo in "<dest>"
-    Working copy now at: b56015e38065 (no description set)
-    Added 1 files, modified 0 files, removed 0 files
+    let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["git", "clone", "source", "clone"]);
+    insta::assert_snapshot!(stderr.replace(test_env.env_root().join("clone").to_str().unwrap(), "<dest>"), @r###"
+    Error: Destination path exists and is not an empty directory
     "###);
 }