repo, workspace: use dunce::canonicalize() to normalize paths

These paths may be printed, compared with user inputs, or passed to external
programs. It's probably better to avoid unusual "\\?\C:\" paths on Windows.

Fixes #5143

CHANGELOG.md

@@ -85,6 +85,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 * `jj config path --user` no longer creates new file at the default config path.
 
+* On Windows, workspace paths (printed by `jj root`) no longer use UNC-style
+  `\\?\` paths unless necessary.
+
 ## [0.24.0] - 2024-12-04
 
 ### Release highlights

cli/src/cli_util.rs

@@ -3643,7 +3643,7 @@ impl CliRunner {
         // `cwd` is canonicalized for consistency with `Workspace::workspace_root()` and
         // to easily compute relative paths between them.
         let cwd = env::current_dir()
-            .and_then(|cwd| cwd.canonicalize())
+            .and_then(dunce::canonicalize)
             .map_err(|_| {
                 user_error_with_hint(
                     "Could not determine current directory",
@@ -3692,7 +3692,7 @@ impl CliRunner {
         let maybe_workspace_loader = if let Some(path) = &args.global_args.repository {
             // TODO: maybe path should be canonicalized by WorkspaceLoader?
             let abs_path = cwd.join(path);
-            let abs_path = abs_path.canonicalize().unwrap_or(abs_path);
+            let abs_path = dunce::canonicalize(&abs_path).unwrap_or(abs_path);
             // Invalid -R path is an error. No need to proceed.
             let loader = self
                 .workspace_loader_factory

cli/src/commands/git/clone.rs

@@ -17,7 +17,6 @@ use std::io;
 use std::io::Write;
 use std::num::NonZeroU32;
 use std::path::Path;
-use std::path::PathBuf;
 
 use jj_lib::git;
 use jj_lib::git::GitFetchError;
@@ -128,8 +127,7 @@ pub fn cmd_git_clone(
 
     // Canonicalize because fs::remove_dir_all() doesn't seem to like e.g.
     // `/some/path/.`
-    let canonical_wc_path: PathBuf = wc_path
-        .canonicalize()
+    let canonical_wc_path = dunce::canonicalize(&wc_path)
         .map_err(|err| user_error_with_message(format!("Failed to create {wc_path_str}"), err))?;
     let clone_result = do_git_clone(
         ui,

cli/src/commands/git/init.rs

@@ -91,7 +91,7 @@ pub fn cmd_git_init(
     let cwd = command.cwd();
     let wc_path = cwd.join(&args.destination);
     let wc_path = file_util::create_or_reuse_dir(&wc_path)
-        .and_then(|_| wc_path.canonicalize())
+        .and_then(|_| dunce::canonicalize(wc_path))
         .map_err(|e| user_error_with_message("Failed to create workspace", e))?;
 
     do_init(

cli/src/commands/init.rs

@@ -62,7 +62,7 @@ pub(crate) fn cmd_init(
     let cwd = command.cwd();
     let wc_path = cwd.join(&args.destination);
     let wc_path = file_util::create_or_reuse_dir(&wc_path)
-        .and_then(|_| wc_path.canonicalize())
+        .and_then(|_| dunce::canonicalize(wc_path))
         .map_err(|e| user_error_with_message("Failed to create workspace", e))?;
 
     // Preserve existing behaviour where `jj init` is not able to create

cli/src/complete.rs

@@ -684,7 +684,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> {
     let mut raw_config = config_from_environment(default_config_layers());
     let ui = Ui::with_config(raw_config.as_ref()).expect("default config should be valid");
     let cwd = std::env::current_dir()
-        .and_then(|cwd| cwd.canonicalize())
+        .and_then(dunce::canonicalize)
         .map_err(user_error)?;
     let mut config_env = ConfigEnv::from_environment()?;
     let maybe_cwd_workspace_loader = DefaultWorkspaceLoaderFactory.create(find_workspace_dir(&cwd));

cli/src/config.rs

@@ -258,10 +258,9 @@ pub struct ConfigEnv {
 impl ConfigEnv {
     /// Initializes configuration loader based on environment variables.
     pub fn from_environment() -> Result<Self, ConfigEnvError> {
-        // Canonicalize home as we do canonicalize cwd in CliRunner.
-        // TODO: Maybe better to switch to dunce::canonicalize() and remove
-        // canonicalization of cwd, home, etc.?
-        let home_dir = dirs::home_dir().map(|path| path.canonicalize().unwrap_or(path));
+        // Canonicalize home as we do canonicalize cwd in CliRunner. $HOME might
+        // point to symlink.
+        let home_dir = dirs::home_dir().map(|path| dunce::canonicalize(&path).unwrap_or(path));
         let env = UnresolvedConfigEnv {
             config_dir: dirs::config_dir(),
             home_dir: home_dir.clone(),

cli/src/git_util.rs

@@ -85,10 +85,10 @@ pub fn is_colocated_git_workspace(workspace: &Workspace, repo: &ReadonlyRepo) ->
     }
     // Colocated workspace should have ".git" directory, file, or symlink. Compare
     // its parent as the git_workdir might be resolved from the real ".git" path.
-    let Ok(dot_git_path) = workspace.workspace_root().join(".git").canonicalize() else {
+    let Ok(dot_git_path) = dunce::canonicalize(workspace.workspace_root().join(".git")) else {
         return false;
     };
-    git_workdir.canonicalize().ok().as_deref() == dot_git_path.parent()
+    dunce::canonicalize(git_workdir).ok().as_deref() == dot_git_path.parent()
 }
 
 fn terminal_get_username(ui: &Ui, url: &str) -> Option<String> {

cli/tests/common/mod.rs

@@ -56,7 +56,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");

cli/tests/test_config_command.rs

@@ -1149,7 +1149,8 @@ fn test_config_conditional_without_home_dir() {
                 --when.repositories = [{repo_path}]
                 foo = 'repo'
             "},
-            repo_path = to_toml_value(repo_path.to_str().unwrap())
+            // "\\?\" paths shouldn't be required on Windows
+            repo_path = to_toml_value(dunce::simplified(&repo_path).to_str().unwrap())
         ),
     )
     .unwrap();

lib/Cargo.toml

@@ -40,6 +40,7 @@ chrono = { workspace = true }
 chrono-english = { workspace = true }
 clru = { workspace = true }
 digest = { workspace = true }
+dunce = { workspace = true }
 either = { workspace = true }
 futures = { workspace = true }
 git2 = { workspace = true, optional = true }

lib/src/git_backend.rs

@@ -206,7 +206,7 @@ impl GitBackend {
     ) -> Result<Self, Box<GitBackendInitError>> {
         let canonical_workspace_root = {
             let path = store_path.join(workspace_root);
-            path.canonicalize()
+            dunce::canonicalize(&path)
                 .context(&path)
                 .map_err(GitBackendInitError::Path)?
         };
@@ -472,9 +472,9 @@ impl GitBackend {
 pub fn canonicalize_git_repo_path(path: &Path) -> io::Result<PathBuf> {
     if path.ends_with(".git") {
         let workdir = path.parent().unwrap();
-        workdir.canonicalize().map(|dir| dir.join(".git"))
+        dunce::canonicalize(workdir).map(|dir| dir.join(".git"))
     } else {
-        path.canonicalize()
+        dunce::canonicalize(path)
     }
 }
 
@@ -806,8 +806,8 @@ fn run_git_gc(git_dir: &Path) -> Result<(), GitGcError> {
     let mut git = Command::new("git");
     git.arg("--git-dir=."); // turn off discovery
     git.arg("gc");
-    // Don't specify it by GIT_DIR/--git-dir. On Windows, the "\\?\" path might
-    // not be supported by git.
+    // Don't specify it by GIT_DIR/--git-dir. On Windows, the path could be
+    // canonicalized as UNC path, which wouldn't be supported by git.
     git.current_dir(git_dir);
     // TODO: pass output to UI layer instead of printing directly here
     let status = git.status().map_err(GitGcError::GcCommand)?;

lib/src/repo.rs

@@ -186,7 +186,7 @@ impl ReadonlyRepo {
         index_store_initializer: &IndexStoreInitializer,
         submodule_store_initializer: &SubmoduleStoreInitializer,
     ) -> Result<Arc<ReadonlyRepo>, RepoInitError> {
-        let repo_path = repo_path.canonicalize().context(repo_path)?;
+        let repo_path = dunce::canonicalize(repo_path).context(repo_path)?;
 
         let store_path = repo_path.join("store");
         fs::create_dir(&store_path).context(&store_path)?;

lib/src/workspace.rs

@@ -161,7 +161,7 @@ impl Workspace {
         working_copy: Box<dyn WorkingCopy>,
         repo_loader: RepoLoader,
     ) -> Result<Workspace, PathError> {
-        let workspace_root = workspace_root.canonicalize().context(workspace_root)?;
+        let workspace_root = dunce::canonicalize(workspace_root).context(workspace_root)?;
         Ok(Self::new_no_canonicalize(
             workspace_root,
             repo_path,
@@ -224,7 +224,7 @@ impl Workspace {
             // ReadonlyRepo::init(). workspace_root will be canonicalized by
             // Workspace::new(), but it's not yet here.
             let store_relative_workspace_root =
-                if let Ok(workspace_root) = workspace_root.canonicalize() {
+                if let Ok(workspace_root) = dunce::canonicalize(workspace_root) {
                     crate::file_util::relative_path(store_path, &workspace_root)
                 } else {
                     workspace_root.to_owned()
@@ -259,7 +259,7 @@ impl Workspace {
             // ReadonlyRepo::init(). workspace_root will be canonicalized by
             // Workspace::new(), but it's not yet here.
             let store_relative_git_repo_path = match (
-                workspace_root.canonicalize(),
+                dunce::canonicalize(workspace_root),
                 crate::git_backend::canonicalize_git_repo_path(git_repo_path),
             ) {
                 (Ok(workspace_root), Ok(git_repo_path))
@@ -359,7 +359,7 @@ impl Workspace {
     ) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
         let jj_dir = create_jj_dir(workspace_root)?;
 
-        let repo_dir = repo_path.canonicalize().context(repo_path)?;
+        let repo_dir = dunce::canonicalize(repo_path).context(repo_path)?;
         let repo_file_path = jj_dir.join("repo");
         let mut repo_file = File::create(&repo_file_path).context(&repo_file_path)?;
         repo_file
@@ -566,10 +566,7 @@ impl DefaultWorkspaceLoader {
             let buf = fs::read(&repo_dir).context(&repo_dir)?;
             let repo_path_str =
                 String::from_utf8(buf).map_err(|_| WorkspaceLoadError::NonUnicodePath)?;
-            repo_dir = jj_dir
-                .join(&repo_path_str)
-                .canonicalize()
-                .context(&repo_path_str)?;
+            repo_dir = dunce::canonicalize(jj_dir.join(&repo_path_str)).context(&repo_path_str)?;
             if !repo_dir.is_dir() {
                 return Err(WorkspaceLoadError::RepoDoesNotExist(repo_dir));
             }

lib/tests/test_init.rs

@@ -28,7 +28,7 @@ use testutils::TestWorkspace;
 
 fn canonicalize(input: &Path) -> (PathBuf, PathBuf) {
     let uncanonical = input.join("..").join(input.file_name().unwrap());
-    let canonical = uncanonical.canonicalize().unwrap();
+    let canonical = dunce::canonicalize(&uncanonical).unwrap();
     (canonical, uncanonical)
 }
 

lib/tests/test_workspace.rs

@@ -71,9 +71,12 @@ fn test_init_additional_workspace() {
     assert_eq!(ws2.workspace_id(), &ws2_id);
     assert_eq!(
         *ws2.repo_path(),
-        workspace.repo_path().canonicalize().unwrap()
+        dunce::canonicalize(workspace.repo_path()).unwrap()
+    );
+    assert_eq!(
+        *ws2.workspace_root(),
+        dunce::canonicalize(&ws2_root).unwrap()
     );
-    assert_eq!(*ws2.workspace_root(), ws2_root.canonicalize().unwrap());
     let same_workspace = Workspace::load(
         &settings,
         &ws2_root,
@@ -85,7 +88,7 @@ fn test_init_additional_workspace() {
     assert_eq!(same_workspace.workspace_id(), &ws2_id);
     assert_eq!(
         *same_workspace.repo_path(),
-        workspace.repo_path().canonicalize().unwrap()
+        dunce::canonicalize(workspace.repo_path()).unwrap()
     );
     assert_eq!(same_workspace.workspace_root(), ws2.workspace_root());
 }