From 4d8eee34168cf1b588f3252a8055c62fb39f2cc6 Mon Sep 17 00:00:00 2001
From: Scott Taylor <scott.g.taylor@jpmchase.com>
Date: Wed, 17 Jul 2024 17:14:50 -0500
Subject: [PATCH 1/8] test: update workspace path in test case

This test case was creating "workspace1" as a sub-directory of the
default workspace, which seems like a mistake.
---
 cli/tests/test_immutable_commits.rs | 31 +++++++++++++++--------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/cli/tests/test_immutable_commits.rs b/cli/tests/test_immutable_commits.rs
index ed806a13fa..f100f851f0 100644
--- a/cli/tests/test_immutable_commits.rs
+++ b/cli/tests/test_immutable_commits.rs
@@ -136,30 +136,31 @@ fn test_immutable_heads_set_to_working_copy() {
 #[test]
 fn test_new_wc_commit_when_wc_immutable_multi_workspace() {
     let test_env = TestEnvironment::default();
-    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init"]);
-    test_env.jj_cmd_ok(test_env.env_root(), &["branch", "create", "main"]);
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+    test_env.jj_cmd_ok(&repo_path, &["branch", "create", "main"]);
     test_env.add_config(r#"revset-aliases."immutable_heads()" = "main""#);
-    test_env.jj_cmd_ok(test_env.env_root(), &["new", "-m=a"]);
-    test_env.jj_cmd_ok(test_env.env_root(), &["workspace", "add", "workspace1"]);
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m=a"]);
+    test_env.jj_cmd_ok(&repo_path, &["workspace", "add", "../workspace1"]);
     let workspace1_envroot = test_env.env_root().join("workspace1");
-    test_env.jj_cmd_ok(workspace1_envroot.as_path(), &["edit", "default@"]);
-    let (_, stderr) = test_env.jj_cmd_ok(test_env.env_root(), &["branch", "set", "main"]);
+    test_env.jj_cmd_ok(&workspace1_envroot, &["edit", "default@"]);
+    let (_, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "set", "main"]);
     insta::assert_snapshot!(stderr, @r###"
-    Moved 1 branches to kkmpptxz 40cbbd52 main | a
+    Moved 1 branches to kkmpptxz 7796c4df main | (empty) a
     Warning: The working-copy commit in workspace 'default' became immutable, so a new commit has been created on top of it.
     Warning: The working-copy commit in workspace 'workspace1' became immutable, so a new commit has been created on top of it.
-    Working copy now at: royxmykx 5bcb7da6 (empty) (no description set)
-    Parent commit      : kkmpptxz 40cbbd52 main | a
+    Working copy now at: royxmykx 896465c4 (empty) (no description set)
+    Parent commit      : kkmpptxz 7796c4df main | (empty) a
     "###);
-    test_env.jj_cmd_ok(workspace1_envroot.as_path(), &["workspace", "update-stale"]);
-    let (stdout, _) = test_env.jj_cmd_ok(workspace1_envroot.as_path(), &["log", "--no-graph"]);
+    test_env.jj_cmd_ok(&workspace1_envroot, &["workspace", "update-stale"]);
+    let (stdout, _) = test_env.jj_cmd_ok(&workspace1_envroot, &["log", "--no-graph"]);
     insta::assert_snapshot!(stdout, @r###"
-    nppvrztz test.user@example.com 2001-02-03 08:05:11 workspace1@ 44082ceb
+    nppvrztz test.user@example.com 2001-02-03 08:05:11 workspace1@ ee0671fd
     (empty) (no description set)
-    royxmykx test.user@example.com 2001-02-03 08:05:12 default@ 5bcb7da6
+    royxmykx test.user@example.com 2001-02-03 08:05:12 default@ 896465c4
     (empty) (no description set)
-    kkmpptxz test.user@example.com 2001-02-03 08:05:12 main 40cbbd52
-    a
+    kkmpptxz test.user@example.com 2001-02-03 08:05:09 main 7796c4df
+    (empty) a
     zzzzzzzz root() 00000000
     "###);
 }

From 304f6dfc3f3f441e4dfe193f851aa5e70d979232 Mon Sep 17 00:00:00 2001
From: Scott Taylor <scott.g.taylor@jpmchase.com>
Date: Sat, 20 Jul 2024 16:36:30 -0500
Subject: [PATCH 2/8] workspace: warn if destination doesn't contain path
 separator

Users may try to run `jj workspace add <name>` without specifying a
path, which results in the workspace being created in the current
directory. This can be confusing, since the workspace contents will also
be snapshotted in the original workspace if it is not sparse. Adding a
warning should reduce confusion in this case.
---
 cli/src/commands/workspace.rs |  9 ++++++
 cli/tests/test_workspaces.rs  | 55 +++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/cli/src/commands/workspace.rs b/cli/src/commands/workspace.rs
index fde6de92eb..56e4661eb8 100644
--- a/cli/src/commands/workspace.rs
+++ b/cli/src/commands/workspace.rs
@@ -171,6 +171,15 @@ fn cmd_workspace_add(
         "Created workspace in \"{}\"",
         file_util::relative_path(command.cwd(), &destination_path).display()
     )?;
+    // Show a warning if the user passed a path without a separator, since they
+    // may have intended the argument to only be the name for the workspace.
+    if !args.destination.contains(std::path::is_separator) {
+        writeln!(
+            ui.warning_default(),
+            r#"Workspace created inside current directory. If this was unintentional, delete the "{}" directory and run `jj workspace forget {name}` to remove it."#,
+            args.destination
+        )?;
+    }
 
     // Copy sparse patterns from workspace where the command was run
     let mut new_workspace_command = command.for_workable_repo(ui, new_workspace, repo)?;
diff --git a/cli/tests/test_workspaces.rs b/cli/tests/test_workspaces.rs
index 4bd653ac16..cd045ad485 100644
--- a/cli/tests/test_workspaces.rs
+++ b/cli/tests/test_workspaces.rs
@@ -374,6 +374,61 @@ fn test_workspaces_add_workspace_from_subdir() {
     "###);
 }
 
+#[test]
+fn test_workspaces_add_workspace_in_current_workspace() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "main"]);
+    let main_path = test_env.env_root().join("main");
+
+    std::fs::write(main_path.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&main_path, &["commit", "-m", "initial"]);
+
+    // Try to create workspace using name instead of path
+    let (stdout, stderr) = test_env.jj_cmd_ok(&main_path, &["workspace", "add", "secondary"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @"");
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Created workspace in "secondary"
+    Warning: Workspace created inside current directory. If this was unintentional, delete the "secondary" directory and run `jj workspace forget secondary` to remove it.
+    Working copy now at: pmmvwywv 0a77a39d (empty) (no description set)
+    Parent commit      : qpvuntsm 751b12b7 initial
+    Added 1 files, modified 0 files, removed 0 files
+    "###);
+
+    // Workspace created despite warning
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz 46d9ba8b (no description set)
+    secondary: pmmvwywv 0a77a39d (empty) (no description set)
+    "###);
+
+    // Use explicit path instead (no warning)
+    let (stdout, stderr) = test_env.jj_cmd_ok(&main_path, &["workspace", "add", "./third"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @"");
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Created workspace in "third"
+    Working copy now at: zxsnswpr 64746d4b (empty) (no description set)
+    Parent commit      : qpvuntsm 751b12b7 initial
+    Added 1 files, modified 0 files, removed 0 files
+    "###);
+
+    // Both workspaces created
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz 477c647f (no description set)
+    secondary: pmmvwywv 0a77a39d (empty) (no description set)
+    third: zxsnswpr 64746d4b (empty) (no description set)
+    "###);
+
+    // Can see files from the other workspaces in main workspace, since they are
+    // child directories and will therefore be snapshotted
+    let stdout = test_env.jj_cmd_success(&main_path, &["file", "list"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    file
+    secondary/file
+    third/file
+    "###);
+}
+
 /// Test making changes to the working copy in a workspace as it gets rewritten
 /// from another workspace
 #[test]

From ce2982492ae0876102ce2dcbc70bdb801541d09c Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Thu, 25 Jul 2024 17:42:28 -0700
Subject: [PATCH 3/8] cargo: enable vendored-libgit2, document how to properly
 dynamically link libgit2

This changes less than it seems. Our CI builds already mostly linked a vendored
copy of libgit2. This is because before this commit, it turns out that `git2`
could link `libgit2` *either* statically or dynamically based on whether it could
find a version of libgit2 it liked to link dynamically. Our CI builds usually did
not provide such a version AFAIK.

This made the kind of binary `cargo install` would produce unpredictable and may
have contributed to #2896.  I was once very surprised when I did `brew upgrade libgit2` and then
`cargo build --release` suddenly switched from building dynamically linked `jj` to the vendored version.

Instead, if a packager wants to link `libgit2` dynamically, they should set an
environment variable, as described inside the diff of this commit. I also think
we should recommend static linking as `git2` is quite picky about the versions of
`libgit2` it supports. See also https://github.com/rust-lang/git2-rs/pull/1073

This might be related to #4115.
---
 CHANGELOG.md |  6 ++++++
 Cargo.toml   | 11 ++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 299894fa70..e1444c566b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,12 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Note to packagers
+
+* `jj` now links `libgit2` statically by default. To use dynamic linking, you
+  need to set the environment variable `LIBGIT2_NO_VENDOR=1` while compiling.
+  ([#4163](https://github.com/martinvonz/jj/pull/4163))
+
 ### Breaking changes
 
 * `jj rebase --skip-empty` has been renamed to `jj rebase --skip-emptied`
diff --git a/Cargo.toml b/Cargo.toml
index 8e6e514b28..4545317d89 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -48,7 +48,16 @@ dunce = "1.0.4"
 either = "1.13.0"
 esl01-renderdag = "0.3.0"
 futures = "0.3.30"
-git2 = "0.18.3"
+git2 = { version = "0.18.3", features = [
+    # Do *not* disable this feature even if you'd like dynamic linking. Instead,
+    # set the environment variable `LIBGIT2_NO_VENDOR=1` if dynamic linking must
+    # be used (this will override the Cargo feature), and allow static linking
+    # in other cases. Rationale: If neither the feature nor the environment
+    # variable are set, `git2` may still decide to vendor `libgit2` if it
+    # doesn't find a version of `libgit2` to link to dynamically. See also
+    # https://github.com/rust-lang/git2-rs/commit/3cef4119f
+    "vendored-libgit2"
+] }
 gix = { version = "0.63.0", default-features = false, features = [
     "index",
     "max-performance-safe",

From 8c38b76dffe0913e860a2035ce482423bb67eca2 Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Tue, 30 Jul 2024 20:36:51 -0700
Subject: [PATCH 4/8] path_slash

---
 Cargo.lock                    |  7 +++++++
 Cargo.toml                    |  1 +
 cli/Cargo.toml                |  1 +
 cli/src/commands/git/clone.rs | 16 +++++++++++-----
 cli/tests/common/mod.rs       |  8 +++++---
 5 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 1dbaf1d956..cf10983eb6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1879,6 +1879,7 @@ dependencies = [
  "maplit",
  "minus",
  "once_cell",
+ "path-slash",
  "pest",
  "pest_derive",
  "pollster",
@@ -2341,6 +2342,12 @@ version = "1.0.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a"
 
+[[package]]
+name = "path-slash"
+version = "0.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1e91099d4268b0e11973f036e885d652fb0b21fedcf69738c627f94db6a44f42"
+
 [[package]]
 name = "pathdiff"
 version = "0.2.1"
diff --git a/Cargo.toml b/Cargo.toml
index 4545317d89..812e27bc48 100644
--- a/Cargo.toml
+++ b/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"
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index ffb41e2997..e8c6656c9f 100644
--- a/cli/Cargo.toml
+++ b/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 }
diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs
index b7cfdf8101..018b5d3391 100644
--- a/cli/src/commands/git/clone.rs
+++ b/cli/src/commands/git/clone.rs
@@ -20,6 +20,7 @@ use jj_lib::git::{self, GitFetchError, GitFetchStats};
 use jj_lib::repo::Repo;
 use jj_lib::str_util::StringPattern;
 use jj_lib::workspace::Workspace;
+use path_slash::PathBufExt;
 
 use crate::cli_util::{CommandHelper, WorkspaceCommandHelper};
 use crate::command_error::{cli_error, user_error, user_error_with_message, CommandError};
@@ -53,12 +54,17 @@ fn absolute_git_source(cwd: &Path, source: &str) -> String {
     // 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.
+    // TODO: Match "This syntax is only recognized if there are no slashes before the first colon" from https://git-scm.com/docs/git-clone#_git_urls
+    // TODO: Pass paths like `c:\qq` to path_slash
     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())
+        // 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
+        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()
     }
diff --git a/cli/tests/common/mod.rs b/cli/tests/common/mod.rs
index 72f4eb5a51..32ce520b7f 100644
--- a/cli/tests/common/mod.rs
+++ b/cli/tests/common/mod.rs
@@ -17,6 +17,7 @@ use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
 use itertools::Itertools as _;
+use path_slash::PathBufExt;
 use regex::{Captures, Regex};
 use tempfile::TempDir;
 
@@ -320,13 +321,14 @@ 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())
         ))
         .unwrap();
         regex
             .replace_all(&text, |caps: &Captures| {
-                format!("$TEST_ENV{}", caps[1].replace('\\', "/"))
+                format!("$TEST_ENV{}", caps[2].replace('\\', "/"))
             })
             .to_string()
     }

From 48366f5b423bc3bea7d884a196f1095f6f46e7f9 Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Tue, 30 Jul 2024 21:05:36 -0700
Subject: [PATCH 5/8] looks_like_a_path_to_git

---
 cli/src/commands/git/clone.rs | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs
index 018b5d3391..ffb5fbc3ca 100644
--- a/cli/src/commands/git/clone.rs
+++ b/cli/src/commands/git/clone.rs
@@ -48,15 +48,24 @@ pub struct GitCloneArgs {
     colocate: bool,
 }
 
+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(":")
+}
+
 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.
-    // TODO: Match "This syntax is only recognized if there are no slashes before the first colon" from https://git-scm.com/docs/git-clone#_git_urls
-    // TODO: Pass paths like `c:\qq` to path_slash
-    if !source.contains(':') && Path::new(source).exists() {
+    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

From d6f8ff60050534c2c10b79db379a5edf399e1bc3 Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Tue, 30 Jul 2024 21:31:58 -0700
Subject: [PATCH 6/8] git clone: move `absolute_git_source` from cli to lib and
 rename to `sanitize_git_url_if_path`

---
 Cargo.lock                    |  1 +
 cli/src/commands/git/clone.rs | 36 ++---------------------------------
 lib/Cargo.toml                |  1 +
 lib/src/git.rs                | 34 ++++++++++++++++++++++++++++++++-
 4 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index cf10983eb6..2dfdd0e696 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1932,6 +1932,7 @@ dependencies = [
  "maplit",
  "num_cpus",
  "once_cell",
+ "path-slash",
  "pest",
  "pest_derive",
  "pollster",
diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs
index ffb5fbc3ca..9c5a4b9ed9 100644
--- a/cli/src/commands/git/clone.rs
+++ b/cli/src/commands/git/clone.rs
@@ -16,11 +16,10 @@ 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;
-use path_slash::PathBufExt;
 
 use crate::cli_util::{CommandHelper, WorkspaceCommandHelper};
 use crate::command_error::{cli_error, user_error, user_error_with_message, CommandError};
@@ -48,37 +47,6 @@ pub struct GitCloneArgs {
     colocate: bool,
 }
 
-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(":")
-}
-
-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 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
-        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()
-    }
-}
-
 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);
@@ -104,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()
diff --git a/lib/Cargo.toml b/lib/Cargo.toml
index 5750331785..b0a7189de8 100644
--- a/lib/Cargo.toml
+++ b/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 }
diff --git a/lib/src/git.rs b/lib/src/git.rs
index d7066a22f7..f1e2d6a38d 100644
--- a/lib/src/git.rs
+++ b/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,6 +1144,37 @@ 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
+        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,

From d10a28de1ad3cf0d6337d838e26dbce25d79d4ca Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Tue, 30 Jul 2024 23:34:36 -0700
Subject: [PATCH 7/8] dunce

---
 cli/tests/common/mod.rs | 2 +-
 lib/src/git.rs          | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/cli/tests/common/mod.rs b/cli/tests/common/mod.rs
index 32ce520b7f..e7ddec69e8 100644
--- a/cli/tests/common/mod.rs
+++ b/cli/tests/common/mod.rs
@@ -54,7 +54,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");
diff --git a/lib/src/git.rs b/lib/src/git.rs
index f1e2d6a38d..01714c4de5 100644
--- a/lib/src/git.rs
+++ b/lib/src/git.rs
@@ -1165,6 +1165,7 @@ pub fn sanitize_git_url_if_path(cwd: &Path, source: &str) -> String {
         // 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(),

From 5727139ac305203e5cc12ad7824c8eb53158caad Mon Sep 17 00:00:00 2001
From: Ilya Grigoriev <ilyagr@users.noreply.github.com>
Date: Tue, 30 Jul 2024 21:39:17 -0700
Subject: [PATCH 8/8] cli `git remote add/set-url`: use
 `sanitize_git_url_if_path`

It seems like an oversight that the algorithm from `jj git clone`
wasn't used in them before
---
 cli/src/commands/git/remote/add.rs     | 8 ++++++--
 cli/src/commands/git/remote/set_url.rs | 2 +-
 lib/src/git.rs                         | 3 ++-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/cli/src/commands/git/remote/add.rs b/cli/src/commands/git/remote/add.rs
index a66a7faac8..986b35cad5 100644
--- a/cli/src/commands/git/remote/add.rs
+++ b/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(())
 }
diff --git a/cli/src/commands/git/remote/set_url.rs b/cli/src/commands/git/remote/set_url.rs
index cee2da0559..ca386dbb2b 100644
--- a/cli/src/commands/git/remote/set_url.rs
+++ b/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(())
 }
diff --git a/lib/src/git.rs b/lib/src/git.rs
index 01714c4de5..f9ac855eee 100644
--- a/lib/src/git.rs
+++ b/lib/src/git.rs
@@ -1180,6 +1180,7 @@ 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);
@@ -1197,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(())
 }