diff --git a/assets/settings/default.json b/assets/settings/default.json index fd82db12473d23e0df0df6c2b1328d3ec9af7cc8..58a3f97cd9b033a67c2102130ca6890fabbd9300 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1487,6 +1487,25 @@ // Should the name or path be displayed first in the git view. // "path_style": "file_name_first" or "file_path_first" "path_style": "file_name_first", + // Directory where git worktrees are created, relative to the repository + // working directory. + // + // When the resolved directory is outside the project root, the + // project's directory name is automatically appended so that + // sibling repos don't collide. For example, with the default + // "../worktrees" and a project at ~/code/zed, worktrees are + // created under ~/code/worktrees/zed/. + // + // When the resolved directory is inside the project root, no + // extra component is added (it's already project-scoped). + // + // Examples: + // "../worktrees" — ~/code/worktrees// (default) + // ".git/zed-worktrees" — /.git/zed-worktrees/ + // "my-worktrees" — /my-worktrees/ + // + // Trailing slashes are ignored. + "worktree_directory": "../worktrees", }, // The list of custom Git hosting providers. "git_hosting_providers": [ diff --git a/crates/agent/src/db.rs b/crates/agent/src/db.rs index fa4b37dba3e789b499bfe5db4f0b76ccf12e5a09..14ec9bb9af92c2f9720af5714c7344b986f5f7b5 100644 --- a/crates/agent/src/db.rs +++ b/crates/agent/src/db.rs @@ -23,6 +23,17 @@ pub type DbMessage = crate::Message; pub type DbSummary = crate::legacy_thread::DetailedSummaryState; pub type DbLanguageModel = crate::legacy_thread::SerializedLanguageModel; +/// Metadata about the git worktree associated with an agent thread. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct AgentGitWorktreeInfo { + /// The branch name in the git worktree. + pub branch: String, + /// Absolute path to the git worktree on disk. + pub worktree_path: std::path::PathBuf, + /// The base branch/commit the worktree was created from. + pub base_ref: String, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct DbThreadMetadata { pub id: acp::SessionId, @@ -30,6 +41,10 @@ pub struct DbThreadMetadata { #[serde(alias = "summary")] pub title: SharedString, pub updated_at: DateTime, + /// Denormalized from `DbThread::git_worktree_info.branch` for efficient + /// listing without decompressing thread data. The blob is the source of + /// truth; this column is populated on save for query convenience. + pub worktree_branch: Option, } #[derive(Debug, Serialize, Deserialize)] @@ -53,6 +68,8 @@ pub struct DbThread { pub imported: bool, #[serde(default)] pub subagent_context: Option, + #[serde(default)] + pub git_worktree_info: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -91,6 +108,7 @@ impl SharedThread { profile: None, imported: true, subagent_context: None, + git_worktree_info: None, } } @@ -265,6 +283,7 @@ impl DbThread { profile: thread.profile, imported: false, subagent_context: None, + git_worktree_info: None, }) } } @@ -369,6 +388,13 @@ impl ThreadsDatabase { s().ok(); } + if let Ok(mut s) = connection.exec(indoc! {" + ALTER TABLE threads ADD COLUMN worktree_branch TEXT + "}) + { + s().ok(); + } + let db = Self { executor, connection: Arc::new(Mutex::new(connection)), @@ -397,6 +423,10 @@ impl ThreadsDatabase { .subagent_context .as_ref() .map(|ctx| ctx.parent_thread_id.0.clone()); + let worktree_branch = thread + .git_worktree_info + .as_ref() + .map(|info| info.branch.clone()); let json_data = serde_json::to_string(&SerializedThread { thread, version: DbThread::VERSION, @@ -408,11 +438,19 @@ impl ThreadsDatabase { let data_type = DataType::Zstd; let data = compressed; - let mut insert = connection.exec_bound::<(Arc, Option>, String, String, DataType, Vec)>(indoc! {" - INSERT OR REPLACE INTO threads (id, parent_id, summary, updated_at, data_type, data) VALUES (?, ?, ?, ?, ?, ?) + let mut insert = connection.exec_bound::<(Arc, Option>, Option, String, String, DataType, Vec)>(indoc! {" + INSERT OR REPLACE INTO threads (id, parent_id, worktree_branch, summary, updated_at, data_type, data) VALUES (?, ?, ?, ?, ?, ?, ?) "})?; - insert((id.0, parent_id, title, updated_at, data_type, data))?; + insert(( + id.0, + parent_id, + worktree_branch, + title, + updated_at, + data_type, + data, + ))?; Ok(()) } @@ -424,19 +462,20 @@ impl ThreadsDatabase { let connection = connection.lock(); let mut select = connection - .select_bound::<(), (Arc, Option>, String, String)>(indoc! {" - SELECT id, parent_id, summary, updated_at FROM threads ORDER BY updated_at DESC + .select_bound::<(), (Arc, Option>, Option, String, String)>(indoc! {" + SELECT id, parent_id, worktree_branch, summary, updated_at FROM threads ORDER BY updated_at DESC "})?; let rows = select(())?; let mut threads = Vec::new(); - for (id, parent_id, summary, updated_at) in rows { + for (id, parent_id, worktree_branch, summary, updated_at) in rows { threads.push(DbThreadMetadata { id: acp::SessionId::new(id), parent_session_id: parent_id.map(acp::SessionId::new), title: summary.into(), updated_at: DateTime::parse_from_rfc3339(&updated_at)?.with_timezone(&Utc), + worktree_branch, }); } @@ -570,6 +609,7 @@ mod tests { profile: None, imported: false, subagent_context: None, + git_worktree_info: None, } } @@ -713,4 +753,94 @@ mod tests { "Regular threads should have no subagent_context" ); } + + #[gpui::test] + async fn test_git_worktree_info_roundtrip(cx: &mut TestAppContext) { + let database = ThreadsDatabase::new(cx.executor()).unwrap(); + + let thread_id = session_id("worktree-thread"); + let mut thread = make_thread( + "Worktree Thread", + Utc.with_ymd_and_hms(2024, 6, 15, 12, 0, 0).unwrap(), + ); + thread.git_worktree_info = Some(AgentGitWorktreeInfo { + branch: "zed/agent/a4Xiu".to_string(), + worktree_path: std::path::PathBuf::from("/repo/worktrees/zed/agent/a4Xiu"), + base_ref: "main".to_string(), + }); + + database + .save_thread(thread_id.clone(), thread) + .await + .unwrap(); + + let loaded = database + .load_thread(thread_id) + .await + .unwrap() + .expect("thread should exist"); + + let info = loaded + .git_worktree_info + .expect("git_worktree_info should be restored"); + assert_eq!(info.branch, "zed/agent/a4Xiu"); + assert_eq!( + info.worktree_path, + std::path::PathBuf::from("/repo/worktrees/zed/agent/a4Xiu") + ); + assert_eq!(info.base_ref, "main"); + } + + #[gpui::test] + async fn test_session_list_includes_worktree_meta(cx: &mut TestAppContext) { + let database = ThreadsDatabase::new(cx.executor()).unwrap(); + + // Save a thread with worktree info + let worktree_id = session_id("wt-thread"); + let mut worktree_thread = make_thread( + "With Worktree", + Utc.with_ymd_and_hms(2024, 6, 15, 12, 0, 0).unwrap(), + ); + worktree_thread.git_worktree_info = Some(AgentGitWorktreeInfo { + branch: "zed/agent/bR9kz".to_string(), + worktree_path: std::path::PathBuf::from("/repo/worktrees/zed/agent/bR9kz"), + base_ref: "develop".to_string(), + }); + + database + .save_thread(worktree_id.clone(), worktree_thread) + .await + .unwrap(); + + // Save a thread without worktree info + let plain_id = session_id("plain-thread"); + let plain_thread = make_thread( + "Without Worktree", + Utc.with_ymd_and_hms(2024, 6, 15, 11, 0, 0).unwrap(), + ); + + database + .save_thread(plain_id.clone(), plain_thread) + .await + .unwrap(); + + // List threads and verify worktree_branch is populated correctly + let threads = database.list_threads().await.unwrap(); + assert_eq!(threads.len(), 2); + + let wt_entry = threads + .iter() + .find(|t| t.id == worktree_id) + .expect("should find worktree thread"); + assert_eq!(wt_entry.worktree_branch.as_deref(), Some("zed/agent/bR9kz")); + + let plain_entry = threads + .iter() + .find(|t| t.id == plain_id) + .expect("should find plain thread"); + assert!( + plain_entry.worktree_branch.is_none(), + "plain thread should have no worktree_branch" + ); + } } diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 3d68d39d5fb6f61ab188f632c203f9da8a43d1e5..2abfa7b4b182a74d32adbb4acf181855e0a6659b 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -1,8 +1,8 @@ use crate::{ - ContextServerRegistry, CopyPathTool, CreateDirectoryTool, DbLanguageModel, DbThread, - DeletePathTool, DiagnosticsTool, EditFileTool, FetchTool, FindPathTool, GrepTool, - ListDirectoryTool, MovePathTool, NowTool, OpenTool, ProjectSnapshot, ReadFileTool, - RestoreFileFromDiskTool, SaveFileTool, StreamingEditFileTool, SubagentTool, + AgentGitWorktreeInfo, ContextServerRegistry, CopyPathTool, CreateDirectoryTool, + DbLanguageModel, DbThread, DeletePathTool, DiagnosticsTool, EditFileTool, FetchTool, + FindPathTool, GrepTool, ListDirectoryTool, MovePathTool, NowTool, OpenTool, ProjectSnapshot, + ReadFileTool, RestoreFileFromDiskTool, SaveFileTool, StreamingEditFileTool, SubagentTool, SystemPromptTemplate, Template, Templates, TerminalTool, ToolPermissionDecision, WebSearchTool, decide_permission_from_settings, }; @@ -891,6 +891,8 @@ pub struct Thread { subagent_context: Option, /// Weak references to running subagent threads for cancellation propagation running_subagents: Vec>, + /// Git worktree info if this thread is running in an agent worktree. + git_worktree_info: Option, } impl Thread { @@ -981,6 +983,7 @@ impl Thread { imported: false, subagent_context: None, running_subagents: Vec::new(), + git_worktree_info: None, } } @@ -1205,6 +1208,7 @@ impl Thread { imported: db_thread.imported, subagent_context: db_thread.subagent_context, running_subagents: Vec::new(), + git_worktree_info: db_thread.git_worktree_info, } } @@ -1225,6 +1229,7 @@ impl Thread { profile: Some(self.profile_id.clone()), imported: self.imported, subagent_context: self.subagent_context.clone(), + git_worktree_info: self.git_worktree_info.clone(), }; cx.background_spawn(async move { diff --git a/crates/agent/src/thread_store.rs b/crates/agent/src/thread_store.rs index 83548b69d126462fac1766df5d0ec5bb931be493..6add31fdb39302d3d02c250829dc14b0c10850af 100644 --- a/crates/agent/src/thread_store.rs +++ b/crates/agent/src/thread_store.rs @@ -162,6 +162,7 @@ mod tests { profile: None, imported: false, subagent_context: None, + git_worktree_info: None, } } diff --git a/crates/editor/src/editor_settings.rs b/crates/editor/src/editor_settings.rs index ffd5d323c0f303986ba5653410f22ae0e81fffcc..336d2492c9a393e8747325cb4ab0161098fb5be0 100644 --- a/crates/editor/src/editor_settings.rs +++ b/crates/editor/src/editor_settings.rs @@ -221,7 +221,13 @@ impl Settings for EditorSettings { scrollbar: Scrollbar { show: scrollbar.show.map(Into::into).unwrap(), git_diff: scrollbar.git_diff.unwrap() - && content.git.unwrap().enabled.unwrap().is_git_diff_enabled(), + && content + .git + .as_ref() + .unwrap() + .enabled + .unwrap() + .is_git_diff_enabled(), selected_text: scrollbar.selected_text.unwrap(), selected_symbol: scrollbar.selected_symbol.unwrap(), search_results: scrollbar.search_results.unwrap(), diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index a96a8d78e349efe7e166df4eef0a308fcba69ffe..6513d5a33b6eb96f7a69c5f96530f1d44a71c3ec 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -893,125 +893,134 @@ mod tests { #[gpui::test] async fn test_fake_worktree_lifecycle(cx: &mut TestAppContext) { - let fs = FakeFs::new(cx.executor()); - fs.insert_tree("/project", json!({".git": {}, "file.txt": "content"})) - .await; - let repo = fs - .open_repo(Path::new("/project/.git"), None) - .expect("should open fake repo"); - - // Initially no worktrees - let worktrees = repo.worktrees().await.unwrap(); - assert!(worktrees.is_empty()); - - // Create a worktree - repo.create_worktree( - "feature-branch".to_string(), - PathBuf::from("/worktrees"), - Some("abc123".to_string()), - ) - .await - .unwrap(); - - // List worktrees — should have one - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 1); - assert_eq!(worktrees[0].path, Path::new("/worktrees/feature-branch")); - assert_eq!(worktrees[0].ref_name.as_ref(), "refs/heads/feature-branch"); - assert_eq!(worktrees[0].sha.as_ref(), "abc123"); - - // Directory should exist in FakeFs after create - assert!( - fs.is_dir(Path::new("/worktrees/feature-branch")).await, - "worktree directory should be created in FakeFs" - ); - - // Create a second worktree (without explicit commit) - repo.create_worktree( - "bugfix-branch".to_string(), - PathBuf::from("/worktrees"), - None, - ) - .await - .unwrap(); - - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 2); - assert!( - fs.is_dir(Path::new("/worktrees/bugfix-branch")).await, - "second worktree directory should be created in FakeFs" - ); - - // Rename the first worktree - repo.rename_worktree( - PathBuf::from("/worktrees/feature-branch"), - PathBuf::from("/worktrees/renamed-branch"), - ) - .await - .unwrap(); - - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 2); - assert!( - worktrees - .iter() - .any(|w| w.path == Path::new("/worktrees/renamed-branch")), - "renamed worktree should exist at new path" - ); - assert!( - worktrees - .iter() - .all(|w| w.path != Path::new("/worktrees/feature-branch")), - "old path should no longer exist" - ); - - // Directory should be moved in FakeFs after rename - assert!( - !fs.is_dir(Path::new("/worktrees/feature-branch")).await, - "old worktree directory should not exist after rename" - ); - assert!( - fs.is_dir(Path::new("/worktrees/renamed-branch")).await, - "new worktree directory should exist after rename" - ); - - // Rename a nonexistent worktree should fail - let result = repo - .rename_worktree(PathBuf::from("/nonexistent"), PathBuf::from("/somewhere")) - .await; - assert!(result.is_err()); - - // Remove a worktree - repo.remove_worktree(PathBuf::from("/worktrees/renamed-branch"), false) + let worktree_dir_settings = &["../worktrees", ".git/zed-worktrees", "my-worktrees/"]; + + for worktree_dir_setting in worktree_dir_settings { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/project", json!({".git": {}, "file.txt": "content"})) + .await; + let repo = fs + .open_repo(Path::new("/project/.git"), None) + .expect("should open fake repo"); + + // Initially no worktrees + let worktrees = repo.worktrees().await.unwrap(); + assert!(worktrees.is_empty()); + + let expected_dir = git::repository::resolve_worktree_directory( + Path::new("/project"), + worktree_dir_setting, + ); + + // Create a worktree + repo.create_worktree( + "feature-branch".to_string(), + expected_dir.clone(), + Some("abc123".to_string()), + ) .await .unwrap(); - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 1); - assert_eq!(worktrees[0].path, Path::new("/worktrees/bugfix-branch")); - - // Directory should be removed from FakeFs after remove - assert!( - !fs.is_dir(Path::new("/worktrees/renamed-branch")).await, - "worktree directory should be removed from FakeFs" - ); - - // Remove a nonexistent worktree should fail - let result = repo - .remove_worktree(PathBuf::from("/nonexistent"), false) - .await; - assert!(result.is_err()); - - // Remove the last worktree - repo.remove_worktree(PathBuf::from("/worktrees/bugfix-branch"), false) + // List worktrees — should have one + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 1); + assert_eq!( + worktrees[0].path, + expected_dir.join("feature-branch"), + "failed for worktree_directory setting: {worktree_dir_setting:?}" + ); + assert_eq!(worktrees[0].ref_name.as_ref(), "refs/heads/feature-branch"); + assert_eq!(worktrees[0].sha.as_ref(), "abc123"); + + // Directory should exist in FakeFs after create + assert!( + fs.is_dir(&expected_dir.join("feature-branch")).await, + "worktree directory should be created in FakeFs for setting {worktree_dir_setting:?}" + ); + + // Create a second worktree (without explicit commit) + repo.create_worktree("bugfix-branch".to_string(), expected_dir.clone(), None) + .await + .unwrap(); + + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 2); + assert!( + fs.is_dir(&expected_dir.join("bugfix-branch")).await, + "second worktree directory should be created in FakeFs for setting {worktree_dir_setting:?}" + ); + + // Rename the first worktree + repo.rename_worktree( + expected_dir.join("feature-branch"), + expected_dir.join("renamed-branch"), + ) .await .unwrap(); - let worktrees = repo.worktrees().await.unwrap(); - assert!(worktrees.is_empty()); - assert!( - !fs.is_dir(Path::new("/worktrees/bugfix-branch")).await, - "last worktree directory should be removed from FakeFs" - ); + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 2); + assert!( + worktrees + .iter() + .any(|w| w.path == expected_dir.join("renamed-branch")), + "renamed worktree should exist at new path for setting {worktree_dir_setting:?}" + ); + assert!( + worktrees + .iter() + .all(|w| w.path != expected_dir.join("feature-branch")), + "old path should no longer exist for setting {worktree_dir_setting:?}" + ); + + // Directory should be moved in FakeFs after rename + assert!( + !fs.is_dir(&expected_dir.join("feature-branch")).await, + "old worktree directory should not exist after rename for setting {worktree_dir_setting:?}" + ); + assert!( + fs.is_dir(&expected_dir.join("renamed-branch")).await, + "new worktree directory should exist after rename for setting {worktree_dir_setting:?}" + ); + + // Rename a nonexistent worktree should fail + let result = repo + .rename_worktree(PathBuf::from("/nonexistent"), PathBuf::from("/somewhere")) + .await; + assert!(result.is_err()); + + // Remove a worktree + repo.remove_worktree(expected_dir.join("renamed-branch"), false) + .await + .unwrap(); + + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 1); + assert_eq!(worktrees[0].path, expected_dir.join("bugfix-branch")); + + // Directory should be removed from FakeFs after remove + assert!( + !fs.is_dir(&expected_dir.join("renamed-branch")).await, + "worktree directory should be removed from FakeFs for setting {worktree_dir_setting:?}" + ); + + // Remove a nonexistent worktree should fail + let result = repo + .remove_worktree(PathBuf::from("/nonexistent"), false) + .await; + assert!(result.is_err()); + + // Remove the last worktree + repo.remove_worktree(expected_dir.join("bugfix-branch"), false) + .await + .unwrap(); + + let worktrees = repo.worktrees().await.unwrap(); + assert!(worktrees.is_empty()); + assert!( + !fs.is_dir(&expected_dir.join("bugfix-branch")).await, + "last worktree directory should be removed from FakeFs for setting {worktree_dir_setting:?}" + ); + } } } diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index d7e631dabe1e1a3dabe18e31d74c555e8d5087a2..2db9e48a2e77bdb3e49fce0b16ea9b67ffaacbc0 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -33,9 +33,11 @@ use is_executable::IsExecutable; use rope::Rope; use serde::{Deserialize, Serialize}; use smol::io::AsyncWriteExt; +#[cfg(any(target_os = "windows", feature = "test-support"))] +use std::path::Component; use std::{ io::{self, Write}, - path::{Component, Path, PathBuf}, + path::{Path, PathBuf}, pin::Pin, sync::Arc, time::{Duration, SystemTime, UNIX_EPOCH}, @@ -2813,30 +2815,7 @@ impl Fs for FakeFs { } pub fn normalize_path(path: &Path) -> PathBuf { - let mut components = path.components().peekable(); - let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { - components.next(); - PathBuf::from(c.as_os_str()) - } else { - PathBuf::new() - }; - - for component in components { - match component { - Component::Prefix(..) => unreachable!(), - Component::RootDir => { - ret.push(component.as_os_str()); - } - Component::CurDir => {} - Component::ParentDir => { - ret.pop(); - } - Component::Normal(c) => { - ret.push(c); - } - } - } - ret + util::normalize_path(path) } pub async fn copy_recursive<'a>( diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 7d50e25780b4541a703c4524e24f73569a29926a..baa0ed09239fcd62b7e58a49397c33a0eb3813b6 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -21,6 +21,7 @@ use text::LineEnding; use std::collections::HashSet; use std::ffi::{OsStr, OsString}; + use std::process::ExitStatus; use std::str::FromStr; use std::{ @@ -34,7 +35,7 @@ use thiserror::Error; use util::command::{Stdio, new_command}; use util::paths::PathStyle; use util::rel_path::RelPath; -use util::{ResultExt, paths}; +use util::{ResultExt, normalize_path, paths}; use uuid::Uuid; pub use askpass::{AskPassDelegate, AskPassResult, AskPassSession}; @@ -51,6 +52,100 @@ static GRAPH_COMMIT_FORMAT: &str = "--format=%H%x00%P%x00%D"; /// Number of commits to load per chunk for the git graph. pub const GRAPH_CHUNK_SIZE: usize = 1000; +/// Default value for the `git.worktree_directory` setting. +pub const DEFAULT_WORKTREE_DIRECTORY: &str = "../worktrees"; + +/// Resolves the configured worktree directory to an absolute path. +/// +/// `worktree_directory_setting` is the raw string from the user setting +/// (e.g. `"../worktrees"`, `".git/zed-worktrees"`, `"my-worktrees/"`). +/// Trailing slashes are stripped. The path is resolved relative to +/// `working_directory` (the repository's working directory root). +/// +/// When the resolved directory falls outside the working directory +/// (e.g. `"../worktrees"`), the repository's directory name is +/// automatically appended so that sibling repos don't collide. +/// For example, with working directory `~/code/zed` and setting +/// `"../worktrees"`, this returns `~/code/worktrees/zed`. +/// +/// When the resolved directory is inside the working directory +/// (e.g. `".git/zed-worktrees"`), no extra component is added +/// because the path is already project-scoped. +pub fn resolve_worktree_directory( + working_directory: &Path, + worktree_directory_setting: &str, +) -> PathBuf { + let trimmed = worktree_directory_setting.trim_end_matches(['/', '\\']); + let joined = working_directory.join(trimmed); + let resolved = normalize_path(&joined); + + if resolved.starts_with(working_directory) { + resolved + } else if let Some(repo_dir_name) = working_directory.file_name() { + resolved.join(repo_dir_name) + } else { + resolved + } +} + +/// Validates that the resolved worktree directory is acceptable: +/// - The setting must not be an absolute path. +/// - The resolved path must be either a subdirectory of the working +/// directory or a subdirectory of its parent (i.e., a sibling). +/// +/// Returns `Ok(resolved_path)` or an error with a user-facing message. +pub fn validate_worktree_directory( + working_directory: &Path, + worktree_directory_setting: &str, +) -> Result { + // Check the original setting before trimming, since a path like "///" + // is absolute but becomes "" after stripping trailing separators. + // Also check for leading `/` or `\` explicitly, because on Windows + // `Path::is_absolute()` requires a drive letter — so `/tmp/worktrees` + // would slip through even though it's clearly not a relative path. + if Path::new(worktree_directory_setting).is_absolute() + || worktree_directory_setting.starts_with('/') + || worktree_directory_setting.starts_with('\\') + { + anyhow::bail!( + "git.worktree_directory must be a relative path, got: {worktree_directory_setting:?}" + ); + } + + if worktree_directory_setting.is_empty() { + anyhow::bail!("git.worktree_directory must not be empty"); + } + + let trimmed = worktree_directory_setting.trim_end_matches(['/', '\\']); + if trimmed == ".." { + anyhow::bail!("git.worktree_directory must not be \"..\" (use \"../some-name\" instead)"); + } + + let resolved = resolve_worktree_directory(working_directory, worktree_directory_setting); + + let parent = working_directory.parent().unwrap_or(working_directory); + + if !resolved.starts_with(parent) { + anyhow::bail!( + "git.worktree_directory resolved to {resolved:?}, which is outside \ + the project root and its parent directory. It must resolve to a \ + subdirectory of {working_directory:?} or a sibling of it." + ); + } + + Ok(resolved) +} + +/// Returns the full absolute path for a specific branch's worktree +/// given the resolved worktree directory. +pub fn worktree_path_for_branch( + working_directory: &Path, + worktree_directory_setting: &str, + branch: &str, +) -> PathBuf { + resolve_worktree_directory(working_directory, worktree_directory_setting).join(branch) +} + /// Commit data needed for the git graph visualization. #[derive(Debug, Clone)] pub struct GraphCommitData { @@ -1597,8 +1692,10 @@ impl GitRepository for RealGitRepository { } else { args.push(OsString::from("HEAD")); } + self.executor .spawn(async move { + std::fs::create_dir_all(final_path.parent().unwrap_or(&final_path))?; let output = new_command(&git_binary_path) .current_dir(working_directory?) .args(args) @@ -3721,73 +3818,86 @@ mod tests { assert_eq!(result[0].ref_name.as_ref(), "refs/heads/main"); } + const TEST_WORKTREE_DIRECTORIES: &[&str] = + &["../worktrees", ".git/zed-worktrees", "my-worktrees/"]; + #[gpui::test] async fn test_create_and_list_worktrees(cx: &mut TestAppContext) { disable_git_global_config(); cx.executor().allow_parking(); - let repo_dir = tempfile::tempdir().unwrap(); - git2::Repository::init(repo_dir.path()).unwrap(); + for worktree_dir_setting in TEST_WORKTREE_DIRECTORIES { + let repo_dir = tempfile::tempdir().unwrap(); + git2::Repository::init(repo_dir.path()).unwrap(); - let repo = RealGitRepository::new( - &repo_dir.path().join(".git"), - None, - Some("git".into()), - cx.executor(), - ) - .unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); - // Create an initial commit (required for worktrees) - smol::fs::write(repo_dir.path().join("file.txt"), "content") + // Create an initial commit (required for worktrees) + smol::fs::write(repo_dir.path().join("file.txt"), "content") + .await + .unwrap(); + repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + .await + .unwrap(); + repo.commit( + "Initial commit".into(), + None, + CommitOptions::default(), + AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), + Arc::new(checkpoint_author_envs()), + ) .await .unwrap(); - repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + + // List worktrees — should have just the main one + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 1); + assert_eq!( + worktrees[0].path.canonicalize().unwrap(), + repo_dir.path().canonicalize().unwrap() + ); + + // Create a new worktree + repo.create_worktree( + "test-branch".to_string(), + resolve_worktree_directory(repo_dir.path(), worktree_dir_setting), + Some("HEAD".to_string()), + ) .await .unwrap(); - repo.commit( - "Initial commit".into(), - None, - CommitOptions::default(), - AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), - Arc::new(checkpoint_author_envs()), - ) - .await - .unwrap(); - // List worktrees — should have just the main one - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 1); - assert_eq!( - worktrees[0].path.canonicalize().unwrap(), - repo_dir.path().canonicalize().unwrap() - ); - - // Create a new worktree - let worktree_dir = tempfile::tempdir().unwrap(); - repo.create_worktree( - "test-branch".to_string(), - worktree_dir.path().to_path_buf(), - Some("HEAD".to_string()), - ) - .await - .unwrap(); + // List worktrees — should have two + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 2); + + let expected_path = + worktree_path_for_branch(repo_dir.path(), worktree_dir_setting, "test-branch"); + let new_worktree = worktrees + .iter() + .find(|w| w.branch() == "test-branch") + .expect("should find worktree with test-branch"); + assert_eq!( + new_worktree.path.canonicalize().unwrap(), + expected_path.canonicalize().unwrap(), + "failed for worktree_directory setting: {worktree_dir_setting:?}" + ); - // List worktrees — should have two - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 2); + // Clean up so the next iteration starts fresh + repo.remove_worktree(expected_path, true).await.unwrap(); - let new_worktree = worktrees - .iter() - .find(|w| w.branch() == "test-branch") - .expect("should find worktree with test-branch"); - assert_eq!( - new_worktree.path.canonicalize().unwrap(), - worktree_dir - .path() - .join("test-branch") - .canonicalize() - .unwrap() - ); + // Clean up the worktree base directory if it was created outside repo_dir + // (e.g. for the "../worktrees" setting, it won't be inside the TempDir) + let resolved_dir = resolve_worktree_directory(repo_dir.path(), worktree_dir_setting); + if !resolved_dir.starts_with(repo_dir.path()) { + let _ = std::fs::remove_dir_all(&resolved_dir); + } + } } #[gpui::test] @@ -3795,62 +3905,71 @@ mod tests { disable_git_global_config(); cx.executor().allow_parking(); - let repo_dir = tempfile::tempdir().unwrap(); - git2::Repository::init(repo_dir.path()).unwrap(); + for worktree_dir_setting in TEST_WORKTREE_DIRECTORIES { + let repo_dir = tempfile::tempdir().unwrap(); + git2::Repository::init(repo_dir.path()).unwrap(); - let repo = RealGitRepository::new( - &repo_dir.path().join(".git"), - None, - Some("git".into()), - cx.executor(), - ) - .unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); - // Create an initial commit - smol::fs::write(repo_dir.path().join("file.txt"), "content") + // Create an initial commit + smol::fs::write(repo_dir.path().join("file.txt"), "content") + .await + .unwrap(); + repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + .await + .unwrap(); + repo.commit( + "Initial commit".into(), + None, + CommitOptions::default(), + AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), + Arc::new(checkpoint_author_envs()), + ) .await .unwrap(); - repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + + // Create a worktree + repo.create_worktree( + "to-remove".to_string(), + resolve_worktree_directory(repo_dir.path(), worktree_dir_setting), + Some("HEAD".to_string()), + ) .await .unwrap(); - repo.commit( - "Initial commit".into(), - None, - CommitOptions::default(), - AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), - Arc::new(checkpoint_author_envs()), - ) - .await - .unwrap(); - // Create a worktree - let worktree_dir = tempfile::tempdir().unwrap(); - repo.create_worktree( - "to-remove".to_string(), - worktree_dir.path().to_path_buf(), - Some("HEAD".to_string()), - ) - .await - .unwrap(); - - let worktree_path = worktree_dir.path().join("to-remove"); - assert!(worktree_path.exists()); + let worktree_path = + worktree_path_for_branch(repo_dir.path(), worktree_dir_setting, "to-remove"); + assert!(worktree_path.exists()); - // Remove the worktree - repo.remove_worktree(worktree_path.clone(), false) - .await - .unwrap(); + // Remove the worktree + repo.remove_worktree(worktree_path.clone(), false) + .await + .unwrap(); + + // Verify it's gone from the list + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 1); + assert!( + worktrees.iter().all(|w| w.branch() != "to-remove"), + "removed worktree should not appear in list" + ); - // Verify it's gone from the list - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 1); - assert!( - worktrees.iter().all(|w| w.branch() != "to-remove"), - "removed worktree should not appear in list" - ); + // Verify the directory is removed + assert!(!worktree_path.exists()); - // Verify the directory is removed - assert!(!worktree_path.exists()); + // Clean up the worktree base directory if it was created outside repo_dir + // (e.g. for the "../worktrees" setting, it won't be inside the TempDir) + let resolved_dir = resolve_worktree_directory(repo_dir.path(), worktree_dir_setting); + if !resolved_dir.starts_with(repo_dir.path()) { + let _ = std::fs::remove_dir_all(&resolved_dir); + } + } } #[gpui::test] @@ -3858,66 +3977,75 @@ mod tests { disable_git_global_config(); cx.executor().allow_parking(); - let repo_dir = tempfile::tempdir().unwrap(); - git2::Repository::init(repo_dir.path()).unwrap(); + for worktree_dir_setting in TEST_WORKTREE_DIRECTORIES { + let repo_dir = tempfile::tempdir().unwrap(); + git2::Repository::init(repo_dir.path()).unwrap(); - let repo = RealGitRepository::new( - &repo_dir.path().join(".git"), - None, - Some("git".into()), - cx.executor(), - ) - .unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); - // Create an initial commit - smol::fs::write(repo_dir.path().join("file.txt"), "content") + // Create an initial commit + smol::fs::write(repo_dir.path().join("file.txt"), "content") + .await + .unwrap(); + repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + .await + .unwrap(); + repo.commit( + "Initial commit".into(), + None, + CommitOptions::default(), + AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), + Arc::new(checkpoint_author_envs()), + ) .await .unwrap(); - repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + + // Create a worktree + repo.create_worktree( + "dirty-wt".to_string(), + resolve_worktree_directory(repo_dir.path(), worktree_dir_setting), + Some("HEAD".to_string()), + ) .await .unwrap(); - repo.commit( - "Initial commit".into(), - None, - CommitOptions::default(), - AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), - Arc::new(checkpoint_author_envs()), - ) - .await - .unwrap(); - // Create a worktree - let worktree_dir = tempfile::tempdir().unwrap(); - repo.create_worktree( - "dirty-wt".to_string(), - worktree_dir.path().to_path_buf(), - Some("HEAD".to_string()), - ) - .await - .unwrap(); + let worktree_path = + worktree_path_for_branch(repo_dir.path(), worktree_dir_setting, "dirty-wt"); - let worktree_path = worktree_dir.path().join("dirty-wt"); + // Add uncommitted changes in the worktree + smol::fs::write(worktree_path.join("dirty-file.txt"), "uncommitted") + .await + .unwrap(); - // Add uncommitted changes in the worktree - smol::fs::write(worktree_path.join("dirty-file.txt"), "uncommitted") - .await - .unwrap(); + // Non-force removal should fail with dirty worktree + let result = repo.remove_worktree(worktree_path.clone(), false).await; + assert!( + result.is_err(), + "non-force removal of dirty worktree should fail" + ); - // Non-force removal should fail with dirty worktree - let result = repo.remove_worktree(worktree_path.clone(), false).await; - assert!( - result.is_err(), - "non-force removal of dirty worktree should fail" - ); + // Force removal should succeed + repo.remove_worktree(worktree_path.clone(), true) + .await + .unwrap(); - // Force removal should succeed - repo.remove_worktree(worktree_path.clone(), true) - .await - .unwrap(); + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 1); + assert!(!worktree_path.exists()); - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 1); - assert!(!worktree_path.exists()); + // Clean up the worktree base directory if it was created outside repo_dir + // (e.g. for the "../worktrees" setting, it won't be inside the TempDir) + let resolved_dir = resolve_worktree_directory(repo_dir.path(), worktree_dir_setting); + if !resolved_dir.starts_with(repo_dir.path()) { + let _ = std::fs::remove_dir_all(&resolved_dir); + } + } } #[gpui::test] @@ -3925,67 +4053,206 @@ mod tests { disable_git_global_config(); cx.executor().allow_parking(); - let repo_dir = tempfile::tempdir().unwrap(); - git2::Repository::init(repo_dir.path()).unwrap(); + for worktree_dir_setting in TEST_WORKTREE_DIRECTORIES { + let repo_dir = tempfile::tempdir().unwrap(); + git2::Repository::init(repo_dir.path()).unwrap(); - let repo = RealGitRepository::new( - &repo_dir.path().join(".git"), - None, - Some("git".into()), - cx.executor(), - ) - .unwrap(); + let repo = RealGitRepository::new( + &repo_dir.path().join(".git"), + None, + Some("git".into()), + cx.executor(), + ) + .unwrap(); - // Create an initial commit - smol::fs::write(repo_dir.path().join("file.txt"), "content") + // Create an initial commit + smol::fs::write(repo_dir.path().join("file.txt"), "content") + .await + .unwrap(); + repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + .await + .unwrap(); + repo.commit( + "Initial commit".into(), + None, + CommitOptions::default(), + AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), + Arc::new(checkpoint_author_envs()), + ) .await .unwrap(); - repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default())) + + // Create a worktree + repo.create_worktree( + "old-name".to_string(), + resolve_worktree_directory(repo_dir.path(), worktree_dir_setting), + Some("HEAD".to_string()), + ) .await .unwrap(); - repo.commit( - "Initial commit".into(), - None, - CommitOptions::default(), - AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}), - Arc::new(checkpoint_author_envs()), - ) - .await - .unwrap(); - // Create a worktree - let worktree_dir = tempfile::tempdir().unwrap(); - repo.create_worktree( - "old-name".to_string(), - worktree_dir.path().to_path_buf(), - Some("HEAD".to_string()), - ) - .await - .unwrap(); + let old_path = + worktree_path_for_branch(repo_dir.path(), worktree_dir_setting, "old-name"); + assert!(old_path.exists()); - let old_path = worktree_dir.path().join("old-name"); - assert!(old_path.exists()); + // Move the worktree to a new path + let new_path = + resolve_worktree_directory(repo_dir.path(), worktree_dir_setting).join("new-name"); + repo.rename_worktree(old_path.clone(), new_path.clone()) + .await + .unwrap(); + + // Verify the old path is gone and new path exists + assert!(!old_path.exists()); + assert!(new_path.exists()); + + // Verify it shows up in worktree list at the new path + let worktrees = repo.worktrees().await.unwrap(); + assert_eq!(worktrees.len(), 2); + let moved_worktree = worktrees + .iter() + .find(|w| w.branch() == "old-name") + .expect("should find worktree by branch name"); + assert_eq!( + moved_worktree.path.canonicalize().unwrap(), + new_path.canonicalize().unwrap() + ); - // Move the worktree to a new path - let new_path = worktree_dir.path().join("new-name"); - repo.rename_worktree(old_path.clone(), new_path.clone()) - .await - .unwrap(); + // Clean up so the next iteration starts fresh + repo.remove_worktree(new_path, true).await.unwrap(); - // Verify the old path is gone and new path exists - assert!(!old_path.exists()); - assert!(new_path.exists()); + // Clean up the worktree base directory if it was created outside repo_dir + // (e.g. for the "../worktrees" setting, it won't be inside the TempDir) + let resolved_dir = resolve_worktree_directory(repo_dir.path(), worktree_dir_setting); + if !resolved_dir.starts_with(repo_dir.path()) { + let _ = std::fs::remove_dir_all(&resolved_dir); + } + } + } - // Verify it shows up in worktree list at the new path - let worktrees = repo.worktrees().await.unwrap(); - assert_eq!(worktrees.len(), 2); - let moved_worktree = worktrees - .iter() - .find(|w| w.branch() == "old-name") - .expect("should find worktree by branch name"); + #[test] + fn test_resolve_worktree_directory() { + let work_dir = Path::new("/code/my-project"); + + // Sibling directory — outside project, so repo dir name is appended + assert_eq!( + resolve_worktree_directory(work_dir, "../worktrees"), + PathBuf::from("/code/worktrees/my-project") + ); + + // Git subdir — inside project, no repo name appended + assert_eq!( + resolve_worktree_directory(work_dir, ".git/zed-worktrees"), + PathBuf::from("/code/my-project/.git/zed-worktrees") + ); + + // Simple subdir — inside project, no repo name appended + assert_eq!( + resolve_worktree_directory(work_dir, "my-worktrees"), + PathBuf::from("/code/my-project/my-worktrees") + ); + + // Trailing slash is stripped + assert_eq!( + resolve_worktree_directory(work_dir, "../worktrees/"), + PathBuf::from("/code/worktrees/my-project") + ); + assert_eq!( + resolve_worktree_directory(work_dir, "my-worktrees/"), + PathBuf::from("/code/my-project/my-worktrees") + ); + + // Multiple trailing slashes + assert_eq!( + resolve_worktree_directory(work_dir, "foo///"), + PathBuf::from("/code/my-project/foo") + ); + + // Trailing backslashes (Windows-style) + assert_eq!( + resolve_worktree_directory(work_dir, "my-worktrees\\"), + PathBuf::from("/code/my-project/my-worktrees") + ); + assert_eq!( + resolve_worktree_directory(work_dir, "foo\\/\\"), + PathBuf::from("/code/my-project/foo") + ); + + // Empty string resolves to the working directory itself (inside) + assert_eq!( + resolve_worktree_directory(work_dir, ""), + PathBuf::from("/code/my-project") + ); + + // Just ".." — outside project, repo dir name appended + assert_eq!( + resolve_worktree_directory(work_dir, ".."), + PathBuf::from("/code/my-project") + ); + } + + #[test] + fn test_validate_worktree_directory() { + let work_dir = Path::new("/code/my-project"); + + // Valid: sibling + assert!(validate_worktree_directory(work_dir, "../worktrees").is_ok()); + + // Valid: subdirectory + assert!(validate_worktree_directory(work_dir, ".git/zed-worktrees").is_ok()); + assert!(validate_worktree_directory(work_dir, "my-worktrees").is_ok()); + + // Invalid: just ".." would resolve back to the working directory itself + let err = validate_worktree_directory(work_dir, "..").unwrap_err(); + assert!(err.to_string().contains("must not be \"..\"")); + + // Invalid: ".." with trailing separators + let err = validate_worktree_directory(work_dir, "..\\").unwrap_err(); + assert!(err.to_string().contains("must not be \"..\"")); + let err = validate_worktree_directory(work_dir, "../").unwrap_err(); + assert!(err.to_string().contains("must not be \"..\"")); + + // Invalid: empty string would resolve to the working directory itself + let err = validate_worktree_directory(work_dir, "").unwrap_err(); + assert!(err.to_string().contains("must not be empty")); + + // Invalid: absolute path + let err = validate_worktree_directory(work_dir, "/tmp/worktrees").unwrap_err(); + assert!(err.to_string().contains("relative path")); + + // Invalid: "/" is absolute on Unix + let err = validate_worktree_directory(work_dir, "/").unwrap_err(); + assert!(err.to_string().contains("relative path")); + + // Invalid: "///" is absolute + let err = validate_worktree_directory(work_dir, "///").unwrap_err(); + assert!(err.to_string().contains("relative path")); + + // Invalid: escapes too far up + let err = validate_worktree_directory(work_dir, "../../other-project/wt").unwrap_err(); + assert!(err.to_string().contains("outside")); + } + + #[test] + fn test_worktree_path_for_branch() { + let work_dir = Path::new("/code/my-project"); + + // Outside project — repo dir name is part of the resolved directory + assert_eq!( + worktree_path_for_branch(work_dir, "../worktrees", "feature/foo"), + PathBuf::from("/code/worktrees/my-project/feature/foo") + ); + + // Inside project — no repo dir name inserted + assert_eq!( + worktree_path_for_branch(work_dir, ".git/zed-worktrees", "my-branch"), + PathBuf::from("/code/my-project/.git/zed-worktrees/my-branch") + ); + + // Trailing slash on setting (inside project) assert_eq!( - moved_worktree.path.canonicalize().unwrap(), - new_path.canonicalize().unwrap() + worktree_path_for_branch(work_dir, "my-worktrees/", "branch"), + PathBuf::from("/code/my-project/my-worktrees/branch") ); } diff --git a/crates/git_ui/src/worktree_picker.rs b/crates/git_ui/src/worktree_picker.rs index a871df88bc30c42248eb5e3f5cfdb27e880703e9..f2826a2b543a73c5341653c42bbb5f1540213b2a 100644 --- a/crates/git_ui/src/worktree_picker.rs +++ b/crates/git_ui/src/worktree_picker.rs @@ -2,21 +2,21 @@ use anyhow::Context as _; use collections::HashSet; use fuzzy::StringMatchCandidate; -use git::repository::Worktree as GitWorktree; +use git::repository::{Worktree as GitWorktree, validate_worktree_directory}; use gpui::{ Action, App, AsyncWindowContext, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, Modifiers, ModifiersChangedEvent, ParentElement, - PathPromptOptions, Render, SharedString, Styled, Subscription, Task, WeakEntity, Window, - actions, rems, + Render, SharedString, Styled, Subscription, Task, WeakEntity, Window, actions, rems, }; use picker::{Picker, PickerDelegate, PickerEditorPosition}; +use project::project_settings::ProjectSettings; use project::{ - DirectoryLister, git_store::Repository, trusted_worktrees::{PathTrust, TrustedWorktrees}, }; use remote::{RemoteConnectionOptions, remote_client::ConnectionIdentifier}; use remote_connection::{RemoteConnectionModal, connect}; +use settings::Settings; use std::{path::PathBuf, sync::Arc}; use ui::{HighlightedLabel, KeyBinding, ListItem, ListItemSpacing, prelude::*}; use util::ResultExt; @@ -267,40 +267,22 @@ impl WorktreeListDelegate { return; }; - let worktree_path = self - .workspace - .clone() - .update(cx, |this, cx| { - this.prompt_for_open_path( - PathPromptOptions { - files: false, - directories: true, - multiple: false, - prompt: Some("Select directory for new worktree".into()), - }, - DirectoryLister::Project(this.project().clone()), - window, - cx, - ) - }) - .log_err(); - let Some(worktree_path) = worktree_path else { - return; - }; - let branch = worktree_branch.to_string(); let workspace = self.workspace.clone(); cx.spawn_in(window, async move |_, cx| { - let Some(paths) = worktree_path.await? else { - return anyhow::Ok(()); - }; - let path = paths.get(0).cloned().context("No path selected")?; - - repo.update(cx, |repo, _| { - repo.create_worktree(branch.clone(), path.clone(), commit) - }) - .await??; - let new_worktree_path = path.join(branch); + let (receiver, new_worktree_path) = repo.update(cx, |repo, cx| { + let worktree_directory_setting = ProjectSettings::get_global(cx) + .git + .worktree_directory + .clone(); + let work_dir = repo.work_directory_abs_path.clone(); + let directory = + validate_worktree_directory(&work_dir, &worktree_directory_setting)?; + let new_worktree_path = directory.join(&branch); + let receiver = repo.create_worktree(branch.clone(), directory, commit); + anyhow::Ok((receiver, new_worktree_path)) + })?; + receiver.await??; workspace.update(cx, |workspace, cx| { if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) { @@ -364,7 +346,12 @@ impl WorktreeListDelegate { anyhow::Ok(()) }) .detach_and_prompt_err("Failed to create worktree", window, cx, |e, _, _| { - Some(e.to_string()) + let msg = e.to_string(); + if msg.contains("git.worktree_directory") { + Some(format!("Invalid git.worktree_directory setting: {}", e)) + } else { + Some(msg) + } }); } diff --git a/crates/outline_panel/src/outline_panel_settings.rs b/crates/outline_panel/src/outline_panel_settings.rs index bf73aebecc194baca0156c9cdb850ed89627e001..b744ca6399dd16ad216d1cb4c6dda5e1d93baa4b 100644 --- a/crates/outline_panel/src/outline_panel_settings.rs +++ b/crates/outline_panel/src/outline_panel_settings.rs @@ -53,6 +53,7 @@ impl Settings for OutlinePanelSettings { git_status: panel.git_status.unwrap() && content .git + .as_ref() .unwrap() .enabled .unwrap() diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 92b9876f9919a1e016ebe578f8e43c2a7e861b20..f5b91c85d93ca01917ce3679cfbf40384c98206b 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -5541,7 +5541,7 @@ impl Repository { pub fn create_worktree( &mut self, name: String, - path: PathBuf, + directory: PathBuf, commit: Option, ) -> oneshot::Receiver> { let id = self.id; @@ -5550,7 +5550,7 @@ impl Repository { move |repo, _cx| async move { match repo { RepositoryState::Local(LocalRepositoryState { backend, .. }) => { - backend.create_worktree(name, path, commit).await + backend.create_worktree(name, directory, commit).await } RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => { client @@ -5558,7 +5558,7 @@ impl Repository { project_id: project_id.0, repository_id: id.to_proto(), name, - directory: path.to_string_lossy().to_string(), + directory: directory.to_string_lossy().to_string(), commit, }) .await?; diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index c295938ef56f69101a7210db63f4f7ecdc8517e4..75a3faf4f82d9e98e3c85a96222486cac217afd4 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -4,6 +4,7 @@ use context_server::ContextServerCommand; use dap::adapters::DebugAdapterName; use fs::Fs; use futures::StreamExt as _; +use git::repository::DEFAULT_WORKTREE_DIRECTORY; use gpui::{AsyncApp, BorrowAppContext, Context, Entity, EventEmitter, Subscription, Task}; use lsp::{DEFAULT_LSP_REQUEST_TIMEOUT_SECS, LanguageServerName}; use paths::{ @@ -421,7 +422,7 @@ impl GoToDiagnosticSeverityFilter { } } -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug)] pub struct GitSettings { /// Whether or not git integration is enabled. /// @@ -454,6 +455,13 @@ pub struct GitSettings { /// /// Default: file_name_first pub path_style: GitPathStyle, + /// Directory where git worktrees are created, relative to the repository + /// working directory. When the resolved directory is outside the project + /// root, the project's directory name is automatically appended so that + /// sibling repos don't collide. + /// + /// Default: ../worktrees + pub worktree_directory: String, } #[derive(Clone, Copy, Debug)] @@ -643,6 +651,10 @@ impl Settings for ProjectSettings { }, hunk_style: git.hunk_style.unwrap(), path_style: git.path_style.unwrap().into(), + worktree_directory: git + .worktree_directory + .clone() + .unwrap_or_else(|| DEFAULT_WORKTREE_DIRECTORY.to_string()), }; Self { context_servers: project diff --git a/crates/project_panel/src/project_panel_settings.rs b/crates/project_panel/src/project_panel_settings.rs index ffa126a01addd62cc30b4a3f0205f3f45b9f707e..6b6b7a377276a9fb8b812e495a07a6c4c7aac15e 100644 --- a/crates/project_panel/src/project_panel_settings.rs +++ b/crates/project_panel/src/project_panel_settings.rs @@ -96,6 +96,7 @@ impl Settings for ProjectPanelSettings { git_status: project_panel.git_status.unwrap() && content .git + .as_ref() .unwrap() .enabled .unwrap() diff --git a/crates/settings_content/src/project.rs b/crates/settings_content/src/project.rs index 59576651de0463f4460f12c7ac7417152dc30bb9..70544646b1878c163bf5c17d2364eeebd98f6908 100644 --- a/crates/settings_content/src/project.rs +++ b/crates/settings_content/src/project.rs @@ -439,7 +439,7 @@ impl std::fmt::Debug for ContextServerCommand { } #[with_fallible_options] -#[derive(Copy, Clone, Debug, PartialEq, Default, Serialize, Deserialize, JsonSchema, MergeFrom)] +#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize, JsonSchema, MergeFrom)] pub struct GitSettings { /// Whether or not to enable git integration. /// @@ -473,6 +473,27 @@ pub struct GitSettings { /// /// Default: file_name_first pub path_style: Option, + /// Directory where git worktrees are created, relative to the repository + /// working directory. + /// + /// When the resolved directory is outside the project root, the + /// project's directory name is automatically appended so that + /// sibling repos don't collide. For example, with the default + /// `"../worktrees"` and a project at `~/code/zed`, worktrees are + /// created under `~/code/worktrees/zed/`. + /// + /// When the resolved directory is inside the project root, no + /// extra component is added (it's already project-scoped). + /// + /// Examples: + /// - `"../worktrees"` — `~/code/worktrees//` (default) + /// - `".git/zed-worktrees"` — `/.git/zed-worktrees/` + /// - `"my-worktrees"` — `/my-worktrees/` + /// + /// Trailing slashes are ignored. + /// + /// Default: ../worktrees + pub worktree_directory: Option, } #[with_fallible_options] diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 47247b67084f485f24a7411842a0cff877e6ddf4..bbcc323fd584c9d1db51e915f555f96300f2c751 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -22,7 +22,7 @@ use futures::Future; use itertools::Either; use paths::PathExt; use regex::Regex; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::{LazyLock, OnceLock}; use std::{ borrow::Cow, @@ -1055,6 +1055,36 @@ pub fn some_or_debug_panic(option: Option) -> Option { option } +/// Normalizes a path by resolving `.` and `..` components without +/// requiring the path to exist on disk (unlike `canonicalize`). +pub fn normalize_path(path: &Path) -> PathBuf { + use std::path::Component; + let mut components = path.components().peekable(); + let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { + components.next(); + PathBuf::from(c.as_os_str()) + } else { + PathBuf::new() + }; + + for component in components { + match component { + Component::Prefix(..) => unreachable!(), + Component::RootDir => { + ret.push(component.as_os_str()); + } + Component::CurDir => {} + Component::ParentDir => { + ret.pop(); + } + Component::Normal(c) => { + ret.push(c); + } + } + } + ret +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index 8771bc0f2e490b6d87e5cbe60d8063805f10e2b8..4153373fdb0e107aa08c1fe643600635f63edafe 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -79,6 +79,7 @@ impl Settings for ItemSettings { git_status: tabs.git_status.unwrap() && content .git + .as_ref() .unwrap() .enabled .unwrap()