fixes for paths on Windows

Cargo.toml

@@ -76,6 +76,7 @@ maplit = "1.0.2"
 minus = { version = "5.6.1", features = ["dynamic_output", "search"] }
 num_cpus = "1.16.0"
 once_cell = "1.19.0"
+path-slash = "0.2.1"
 pest = "2.7.11"
 pest_derive = "2.7.11"
 pollster = "0.3.0"

cli/Cargo.toml

@@ -74,6 +74,7 @@ jj-lib = { workspace = true }
 maplit = { workspace = true }
 minus = { workspace = true }
 once_cell = { workspace = true }
+path-slash = { workspace = true }
 pest = { workspace = true }
 pest_derive = { workspace = true }
 pollster = { workspace = true }

cli/src/commands/git/clone.rs

@@ -16,7 +16,7 @@ use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::{fs, io};
 
-use jj_lib::git::{self, GitFetchError, GitFetchStats};
+use jj_lib::git::{self, sanitize_git_url_if_path, GitFetchError, GitFetchStats};
 use jj_lib::repo::Repo;
 use jj_lib::str_util::StringPattern;
 use jj_lib::workspace::Workspace;
@@ -47,23 +47,6 @@ pub struct GitCloneArgs {
     colocate: bool,
 }
 
-fn absolute_git_source(cwd: &Path, source: &str) -> String {
-    // Git appears to turn URL-like source to absolute path if local git directory
-    // exits, and fails because '$PWD/https' is unsupported protocol. Since it would
-    // be tedious to copy the exact git (or libgit2) behavior, we simply assume a
-    // source containing ':' is a URL, SSH remote, or absolute path with Windows
-    // drive letter.
-    if !source.contains(':') && Path::new(source).exists() {
-        // It's less likely that cwd isn't utf-8, so just fall back to original source.
-        cwd.join(source)
-            .into_os_string()
-            .into_string()
-            .unwrap_or_else(|_| source.to_owned())
-    } else {
-        source.to_owned()
-    }
-}
-
 fn clone_destination_for_source(source: &str) -> Option<&str> {
     let destination = source.strip_suffix(".git").unwrap_or(source);
     let destination = destination.strip_suffix('/').unwrap_or(destination);
@@ -89,7 +72,7 @@ pub fn cmd_git_clone(
         return Err(cli_error("--at-op is not respected"));
     }
     let remote_name = "origin";
-    let source = absolute_git_source(command.cwd(), &args.source);
+    let source = sanitize_git_url_if_path(command.cwd(), &args.source);
     let wc_path_str = args
         .destination
         .as_deref()

cli/src/commands/git/remote/add.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use jj_lib::git;
+use jj_lib::git::{self, sanitize_git_url_if_path};
 use jj_lib::repo::Repo;
 
 use crate::cli_util::CommandHelper;
@@ -37,6 +37,10 @@ pub fn cmd_git_remote_add(
     let workspace_command = command.workspace_helper(ui)?;
     let repo = workspace_command.repo();
     let git_repo = get_git_repo(repo.store())?;
-    git::add_remote(&git_repo, &args.remote, &args.url)?;
+    git::add_remote(
+        &git_repo,
+        &args.remote,
+        &sanitize_git_url_if_path(command.cwd(), &args.url),
+    )?;
     Ok(())
 }

cli/src/commands/git/remote/set_url.rs

@@ -37,6 +37,6 @@ pub fn cmd_git_remote_set_url(
     let workspace_command = command.workspace_helper(ui)?;
     let repo = workspace_command.repo();
     let git_repo = get_git_repo(repo.store())?;
-    git::set_remote_url(&git_repo, &args.remote, &args.url)?;
+    git::set_remote_url(&git_repo, &args.remote, &args.url, command.cwd())?;
     Ok(())
 }

cli/tests/common/mod.rs

@@ -14,9 +14,11 @@
 
 use std::cell::RefCell;
 use std::collections::HashMap;
+use std::fs;
 use std::path::{Path, PathBuf};
 
 use itertools::Itertools as _;
+use path_slash::PathBufExt;
 use regex::{Captures, Regex};
 use tempfile::TempDir;
 
@@ -53,7 +55,7 @@ impl Default for TestEnvironment {
         testutils::hermetic_libgit2();
 
         let tmp_dir = testutils::new_temp_dir();
-        let env_root = tmp_dir.path().canonicalize().unwrap();
+        let env_root = dunce::canonicalize(tmp_dir.path()).unwrap();
         let home_dir = env_root.join("home");
         std::fs::create_dir(&home_dir).unwrap();
         let config_dir = env_root.join("config");
@@ -320,13 +322,20 @@ impl TestEnvironment {
     pub fn normalize_output(&self, text: &str) -> String {
         let text = text.replace("jj.exe", "jj");
         let regex = Regex::new(&format!(
-            r"{}(\S+)",
-            regex::escape(&self.env_root.display().to_string())
+            r"({}|{}|{})(\S+)",
+            regex::escape(&self.env_root.display().to_string()),
+            regex::escape(&self.env_root.to_slash_lossy()),
+            regex::escape(
+                &fs::canonicalize(&self.env_root)
+                    .unwrap()
+                    .display()
+                    .to_string()
+            )
         ))
         .unwrap();
         regex
             .replace_all(&text, |caps: &Captures| {
-                format!("$TEST_ENV{}", caps[1].replace('\\', "/"))
+                format!("$TEST_ENV{}", caps[2].replace('\\', "/"))
             })
             .to_string()
     }

lib/Cargo.toml

@@ -52,6 +52,7 @@ itertools = { workspace = true }
 jj-lib-proc-macros = { workspace = true }
 maplit = { workspace = true }
 once_cell = { workspace = true }
+path-slash = { workspace = true }
 pest = { workspace = true }
 pest_derive = { workspace = true }
 pollster = { workspace = true }

lib/src/git.rs

@@ -18,11 +18,12 @@ use std::borrow::Cow;
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::default::Default;
 use std::io::Read;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::{fmt, str};
 
 use git2::Oid;
 use itertools::Itertools;
+use path_slash::PathBufExt;
 use tempfile::NamedTempFile;
 use thiserror::Error;
 
@@ -1143,10 +1144,43 @@ pub fn rename_remote(
     Ok(())
 }
 
+fn looks_like_a_path_to_git(source: &str) -> bool {
+    source.chars().nth(1) == Some(':') || // Looks like a Windows C:... path
+    // Match Git's behavior, the URL syntax [is only recognized if there are no
+    // slashes before the first
+    // colon](https://git-scm.com/docs/git-clone#_git_urls)
+    !source
+        .split_once("/")  
+        .map_or(source, |(first, _)| first)
+        .contains(":")
+}
+
+pub fn sanitize_git_url_if_path(cwd: &Path, source: &str) -> String {
+    // Git appears to turn URL-like source to absolute path if local git directory
+    // exits, and fails because '$PWD/https' is unsupported protocol. Since it would
+    // be tedious to copy the exact git (or libgit2) behavior, we simply assume a
+    // source containing ':' is a URL, SSH remote, or absolute path with Windows
+    // drive letter.
+    if looks_like_a_path_to_git(source) && Path::new(source).exists() {
+        // TODO: This won't work for Windows UNC path or (less importantly) if the
+        // original source is a non-UTF Windows path. For Windows UNC paths, we
+        // could use dunce, though see also https://gitlab.com/kornelski/dunce/-/issues/7
+        // TODO: double-check about UNC paths; what does Git do?
+        cwd.join(source).to_slash().map_or_else(
+            // It's less likely that cwd isn't utf-8, so just fall back to original source.
+            || source.to_owned(),
+            |s| s.to_string(),
+        )
+    } else {
+        source.to_owned()
+    }
+}
+
 pub fn set_remote_url(
     git_repo: &git2::Repository,
     remote_name: &str,
     new_remote_url: &str,
+    cwd: &Path,
 ) -> Result<(), GitRemoteManagementError> {
     if remote_name == REMOTE_NAME_FOR_LOCAL_GIT_REPO {
         return Err(GitRemoteManagementError::RemoteReservedForLocalGitRepo);
@@ -1164,7 +1198,7 @@ pub fn set_remote_url(
     })?;
 
     git_repo
-        .remote_set_url(remote_name, new_remote_url)
+        .remote_set_url(remote_name, &sanitize_git_url_if_path(cwd, new_remote_url))
         .map_err(GitRemoteManagementError::InternalGitError)?;
     Ok(())
 }