(For CI -- test that the parent PR works)

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`

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.19.0"
+git2 = { version = "0.19.0", 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",
@@ -67,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/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)?;

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;
 
@@ -53,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");
@@ -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()
     }

cli/tests/test_git_push.rs

@@ -1235,8 +1235,6 @@ fn test_git_push_moved_sideways_untracked() {
 }
 
 #[test]
-// TODO: This test fails with libgit2 v1.8.1 on Windows.
-#[cfg(not(target_os = "windows"))]
 fn test_git_push_to_remote_named_git() {
     let (test_env, workspace_root) = set_up();
     let git_repo = {

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
     "###);
 }

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]

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(())
 }