mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
Stop using wrong paths for zed --diff (#56380)
Closes https://github.com/zed-industries/zed/issues/56219 Contains 3 commist: *a8d55273d6shows better errors when CLI or main binary fail early: no backtraces, better error context, diff file paths shown *d389f7ee23stops opening Zed if cli `--diff` path argument(s) does not exist, also switches over async fs API on the Zed side. This is a behavior change, as before Zed tried to open or connect to an instance — can be reverted if needed. With `Path::exists` check CLI will do now: <img width="669" height="55" alt="now" src="https://github.com/user-attachments/assets/bdfbef2f-1b28-443d-8a01-0ff73ec0bba1" /> If I remove that bit, Zed will now open in the same cwd where the CLI is invoked in: <img width="1724" height="639" alt="reverted" src="https://github.com/user-attachments/assets/69cd171b-aca3-445b-8647-5786f3360ce4" /> *49787b7366fixes an underlying bug leading to memory leak. If on current `main`, I apply ```diff diff --git a/crates/zed/src/zed/open_listener.rs b/crates/zed/src/zed/open_listener.rs index 18ea7c0869..5db22521f2 100644 --- a/crates/zed/src/zed/open_listener.rs +++ b/crates/zed/src/zed/open_listener.rs @@ -791,6 +791,7 @@ async fn open_local_workspace( // working directory so the workspace opens with the right context. if !user_provided_paths && !diff_paths.is_empty() { if let Ok(cwd) = std::env::current_dir() { + log::error!("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ {cwd:?}"); workspace_paths.push(cwd.to_string_lossy().into_owned()); } } ``` I see the following logs: ``` 2026-05-11T09:52:35+03:00 INFO [zed] ========== starting zed version 1.3.0+dev.7bdcb6172263dc05c0b59be76e09f3e89e23e4f1, sha7bdcb61========== 2026-05-11T09:52:36+03:00 INFO [zed] Using git binary path: "/Applications/Zed Dev.app/Contents/MacOS/git" 2026-05-11T09:52:36+03:00 INFO [util] set environment variables from shell:/bin/zsh, path:/opt/homebrew/opt/llvm/bin:/Applications/Postgres.app/Contents/Versions/16/bin/:/Users/someonetoignore/Developer/PlaydateSDK/bin/:/Users/someonetoignore/.docker/bin/:/opt/homebrew/opt/armv7-unknown-linux-gnueabihf/bin/:/opt/homebrew/opt/rustup/bin/:/opt/homebrew/opt/go@1.19/bin/:/usr/local/opt/llvm/bin/:/Users/someonetoignore/.jetbrains/bin/:/Users/someonetoignore/.cargo/bin/:/usr/local/git/bin/:/opt/homebrew/Cellar/openjdk@21/21.0.6//bin/:/Users/someonetoignore/.local/state/fnm_multishells/75815_1778482356501/bin:/opt/homebrew/opt/ruby/bin:/opt/homebrew/lib/ruby/gems/4.0.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pkg/env/active/bin:/opt/pmk/env/global/bin:/opt/X11/bin:/Library/Apple/usr/bin:/Applications/Wireshark.app/Contents/MacOS:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/Users/someonetoignore/Library/pnpm:/opt/homebrew/opt/llvm/bin:/Applications/Postgres.app/Contents/Versions/16/bin/:/Users/someonetoignore/Developer/PlaydateSDK/bin/:/Users/someonetoignore/.docker/bin/:/opt/homebrew/opt/armv7-unknown-linux-gnueabihf/bin/:/opt/homebrew/opt/rustup/bin/:/opt/homebrew/opt/go@1.19/bin/:/usr/local/opt/llvm/bin/:/Users/someonetoignore/.jetbrains/bin/:/Users/someonetoignore/.cargo/bin/:/usr/local/git/bin/:/opt/homebrew/Cellar/openjdk@21/21.0.6//bin/:/Users/someonetoignore/.local/state/fnm_multishells/23413_1778480085615/bin:/opt/homebrew/opt/ruby/bin:/opt/homebrew/lib/ruby/gems/4.0.0/bin:/Users/someonetoignore/.cargo/bin:/Applications/iTerm.app/Contents/Resources/utilities:/Users/someonetoignore/.orbstack/bin:/Users/someonetoignore/.orbstack/bin 2026-05-11T09:52:36+03:00 INFO [zed::reliability] Debug assertions enabled, skipping hang monitoring 2026-05-11T09:52:36+03:00 WARN [zed::reliability] Minidump endpoint not set 2026-05-11T09:52:36+03:00 INFO [extension_host] extensions updated. loading 21, reloading 0, unloading 0 2026-05-11T09:52:37+03:00 ERROR [crates/zed/src/main.rs:1936] canonicalizing "crates/grammars/src": No such file or directory (os error 2) 2026-05-11T09:52:37+03:00 INFO [client] set status on client 0: Authenticating 2026-05-11T09:52:37+03:00 ERROR [zed::zed::open_listener] @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ "/" 2026-05-11T09:52:37+03:00 INFO [project::trusted_worktrees] Worktree "/" is not trusted 2026-05-11T09:52:37+03:00 ERROR [worktree] error reading target of symlink "/.VolumeIcon.icns": canonicalizing "/.VolumeIcon.icns": No such file or directory (os error 2) 2026-05-11T09:52:37+03:00 ERROR [worktree] error reading target of symlink "/Users/someonetoignore/.gitconfig.zed": canonicalizing "/Users/someonetoignore/.gitconfig.zed": No such file or directory (os error 2) 2026-05-11T09:52:37+03:00 INFO [workspace] Rendered first frame 2026-05-11T09:52:37+03:00 ERROR [worktree] error reading target of symlink "/usr/lib/libnetwork.dylib": canonicalizing "/usr/lib/libnetwork.dylib": No such file or directory (os error 2) 2026-05-11T09:52:37+03:00 WARN [fs] Failed to read symlink target metadata for path "/usr/sbin/weakpass_edit": Permission denied (os error 13) 2026-05-11T09:52:37+03:00 ERROR [worktree] error reading target of symlink "/usr/sbin/weakpass_edit": canonicalizing "/usr/sbin/weakpass_edit": Permission denied (os error 13) 2026-05-11T09:52:37+03:00 ERROR [worktree] error reading target of symlink "/usr/lib/libz.1.2.12.dylib": canonicalizing "/usr/lib/libz.1.2.12.dylib": No such file or directory (os error 2) 2026-05-11T09:52:37+03:00 ERROR [worktree] error reading target of symlink "/usr/lib/libpcre2-8.dylib": canonicalizing "/usr/lib/libpcre2-8.dylib": No such file or directory (os error 2) 2026-05-11T09:52:38+03:00 ERROR [worktree] error reading target of symlink "/var/run/docker.sock": canonicalizing "/var/run/docker.sock": No such file or directory (os error 2) 2026-05-11T09:52:38+03:00 ERROR [worktree] error processing "/var/db/DifferentialPrivacy": Operation not permitted (os error 1) 2026-05-11T09:52:38+03:00 ERROR [worktree] error reading target of symlink "/usr/lib/libipconfig.dylib": canonicalizing "/usr/lib/libipconfig.dylib": No such file or directory (os error 2) 2026-05-11T09:52:38+03:00 ERROR [worktree] error reading target of symlink "/private/var/run/docker.sock": canonicalizing "/private/var/run/docker.sock": No such file or directory (os error 2) 2026-05-11T09:52:38+03:00 ERROR [worktree] error processing "/private/var/db/DifferentialPrivacy": Operation not permitted (os error 1) 2026-05-11T09:52:38+03:00 ERROR [worktree] error processing "/Library/Caches/com.apple.amsengagementd.classicdatavault": Operation not permitted (os error 1) 2026-05-11T09:52:38+03:00 ERROR [worktree] error reading target of symlink "/usr/lib/libpcre2-posix.dylib": canonicalizing "/usr/lib/libpcre2-posix.dylib": No such file or directory (os error 2) 2026-05-11T09:52:38+03:00 ERROR [worktree] error processing "/Library/Caches/com.apple.aneuserd": Operation not permitted (os error 1) 2026-05-11T09:52:38+03:00 ERROR [worktree] error processing "/Library/Caches/com.apple.aned": Operation not permitted (os error 1) ``` According to https://apple.stackexchange.com/questions/284754/what-is-the-default-working-directory-of-a-script-run-via-launchd , the current directory of a running macOS app could be `/` if started the way similar to how we do it via the CLI:7bdcb61722/crates/cli/src/main.rs (L1306-L1329)This means that every `std::env::current_dir()` is potentially dangerous currently, as e.g. diff code tries to open this `/` as a worktree and index it fully. It seems that we're "ok" for now: the dangerous code is mostly in extensions (there we set the cwd) and cli tools, and 2 places in "development" Zed's code are left after this one is fixed. There's one in `fs.rs` but that one is cfg-gated to Windows only hence should not be an issue, at least the related one. I'm not sure if this is the best way to fix the issue: setting `/` as an app's current directory seems also wrong and maybe that invocation CLI code could be altered somehow? Maybe, `open_local_workspace` could be reworked somehow? Seems that now we need a "shared directory" for both files we diff which seems inevitable though, hence I've went on with passing the CLI's current dir when opening items and that fixes the `/` issue along with the OOM for now. Release Notes: - Fixed a memory leak with diffing non-existing files with Zed cli
This commit is contained in:
parent
aa16a3bf9d
commit
d77425fe9c
5 changed files with 78 additions and 17 deletions
|
|
@ -1,3 +1,5 @@
|
|||
use std::path::PathBuf;
|
||||
|
||||
use anyhow::Result;
|
||||
use collections::HashMap;
|
||||
pub use ipc_channel::ipc;
|
||||
|
|
@ -65,6 +67,8 @@ pub enum CliRequest {
|
|||
env: Option<HashMap<String, String>>,
|
||||
user_data_dir: Option<String>,
|
||||
dev_container: bool,
|
||||
#[serde(default)]
|
||||
cwd: Option<PathBuf>,
|
||||
},
|
||||
SetOpenBehavior {
|
||||
behavior: CliBehaviorSetting,
|
||||
|
|
|
|||
|
|
@ -459,7 +459,14 @@ fn parse_path_in_wsl(source: &str, wsl: &str) -> Result<String> {
|
|||
Ok(source.to_string(&|path| path.to_string_lossy().into_owned()))
|
||||
}
|
||||
|
||||
fn main() -> Result<()> {
|
||||
fn main() {
|
||||
if let Err(error) = run() {
|
||||
eprintln!("error: {error:#}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
fn run() -> Result<()> {
|
||||
#[cfg(unix)]
|
||||
util::prevent_root_execution();
|
||||
|
||||
|
|
@ -601,10 +608,15 @@ fn main() -> Result<()> {
|
|||
.any(|pair| Path::new(&pair[0]).is_dir() || Path::new(&pair[1]).is_dir());
|
||||
|
||||
for path in args.diff.chunks(2) {
|
||||
diff_paths.push([
|
||||
parse_path_with_position(&path[0])?,
|
||||
parse_path_with_position(&path[1])?,
|
||||
]);
|
||||
let left = parse_path_with_position(&path[0])?;
|
||||
let right = parse_path_with_position(&path[1])?;
|
||||
for diff_path in [&left, &right] {
|
||||
anyhow::ensure!(
|
||||
Path::new(diff_path).exists(),
|
||||
"--diff path does not exist: {diff_path}"
|
||||
);
|
||||
}
|
||||
diff_paths.push([left, right]);
|
||||
}
|
||||
|
||||
let (expanded_diff_paths, temp_dirs) = expand_directory_diff_pairs(diff_paths)?;
|
||||
|
|
@ -679,6 +691,7 @@ fn main() -> Result<()> {
|
|||
env,
|
||||
user_data_dir: user_data_dir_for_thread,
|
||||
dev_container: args.dev_container,
|
||||
cwd: env::current_dir().ok(),
|
||||
};
|
||||
|
||||
tx.send(open_request)?;
|
||||
|
|
|
|||
|
|
@ -1299,7 +1299,7 @@ fn handle_open_request(request: OpenRequest, app_state: Arc<AppState>, cx: &mut
|
|||
.await?;
|
||||
for result in results.into_iter().flatten() {
|
||||
if let Err(err) = result {
|
||||
log::error!("Error opening path: {err}",);
|
||||
log::error!("Error opening path: {err:#}");
|
||||
}
|
||||
}
|
||||
anyhow::Ok(())
|
||||
|
|
|
|||
|
|
@ -399,7 +399,7 @@ pub async fn open_paths_with_positions(
|
|||
opened_items: mut items,
|
||||
..
|
||||
} = cx
|
||||
.update(|cx| workspace::open_paths(&paths, app_state, open_options, cx))
|
||||
.update(|cx| workspace::open_paths(&paths, app_state.clone(), open_options, cx))
|
||||
.await?;
|
||||
|
||||
if diff_all && !diff_paths.is_empty() {
|
||||
|
|
@ -416,9 +416,26 @@ pub async fn open_paths_with_positions(
|
|||
let workspace_weak = multi_workspace.read_with(cx, |multi_workspace, _cx| {
|
||||
multi_workspace.workspace().downgrade()
|
||||
})?;
|
||||
let canonicalize = async |raw: &str| {
|
||||
app_state
|
||||
.fs
|
||||
.canonicalize(Path::new(raw))
|
||||
.await
|
||||
.with_context(|| format!("opening --diff path {raw:?}"))
|
||||
};
|
||||
for diff_pair in diff_paths {
|
||||
let old_path = Path::new(&diff_pair[0]).canonicalize()?;
|
||||
let new_path = Path::new(&diff_pair[1]).canonicalize()?;
|
||||
let (old_path, new_path) =
|
||||
match futures::join!(canonicalize(&diff_pair[0]), canonicalize(&diff_pair[1])) {
|
||||
(Ok(old), Ok(new)) => (old, new),
|
||||
(old, new) => {
|
||||
for result in [old, new] {
|
||||
if let Err(err) = result {
|
||||
items.push(Some(Err(err)));
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if let Ok(diff_view) = multi_workspace.update(cx, |_multi_workspace, window, cx| {
|
||||
FileDiffView::open(old_path, new_path, workspace_weak.clone(), window, cx)
|
||||
}) {
|
||||
|
|
@ -431,7 +448,7 @@ pub async fn open_paths_with_positions(
|
|||
|
||||
for (item, path) in items.iter_mut().zip(&paths) {
|
||||
if let Some(Err(error)) = item {
|
||||
*error = anyhow!("error opening {path:?}: {error}");
|
||||
*error = anyhow!("error opening {path:?}: {error:#}");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -465,6 +482,7 @@ pub async fn handle_cli_connection(
|
|||
env,
|
||||
user_data_dir: _,
|
||||
dev_container,
|
||||
cwd,
|
||||
} => {
|
||||
if !urls.is_empty() {
|
||||
cx.update(|cx| {
|
||||
|
|
@ -528,6 +546,7 @@ pub async fn handle_cli_connection(
|
|||
dev_container,
|
||||
app_state.clone(),
|
||||
env,
|
||||
cwd,
|
||||
cx,
|
||||
)
|
||||
.await;
|
||||
|
|
@ -648,6 +667,7 @@ async fn open_workspaces(
|
|||
dev_container: bool,
|
||||
app_state: Arc<AppState>,
|
||||
env: Option<collections::HashMap<String, String>>,
|
||||
cwd: Option<PathBuf>,
|
||||
cx: &mut AsyncApp,
|
||||
) -> Result<()> {
|
||||
if paths.is_empty() && diff_paths.is_empty() && open_behavior != cli::OpenBehavior::AlwaysNew {
|
||||
|
|
@ -737,6 +757,7 @@ async fn open_workspaces(
|
|||
diff_paths.clone(),
|
||||
diff_all,
|
||||
open_options,
|
||||
cwd.clone(),
|
||||
responses,
|
||||
&app_state,
|
||||
cx,
|
||||
|
|
@ -781,18 +802,23 @@ async fn open_local_workspace(
|
|||
diff_paths: Vec<[String; 2]>,
|
||||
diff_all: bool,
|
||||
open_options: workspace::OpenOptions,
|
||||
cwd: Option<PathBuf>,
|
||||
responses: &dyn CliResponseSink,
|
||||
app_state: &Arc<AppState>,
|
||||
cx: &mut AsyncApp,
|
||||
) -> bool {
|
||||
let user_provided_paths = !workspace_paths.is_empty();
|
||||
|
||||
// When only diff paths are provided (no regular paths), add the current
|
||||
// When only diff paths are provided (no regular paths), add the CLI's
|
||||
// working directory so the workspace opens with the right context.
|
||||
if !user_provided_paths && !diff_paths.is_empty() {
|
||||
if let Ok(cwd) = std::env::current_dir() {
|
||||
workspace_paths.push(cwd.to_string_lossy().into_owned());
|
||||
}
|
||||
// Note: must use the CLI process's cwd (forwarded via `cli_cwd`), not
|
||||
// `std::env::current_dir()`, since the Zed app process's cwd is typically
|
||||
// `/` on macOS bundles or the launch dir of an already-running instance.
|
||||
if !user_provided_paths
|
||||
&& !diff_paths.is_empty()
|
||||
&& let Some(cwd) = cwd
|
||||
{
|
||||
workspace_paths.push(cwd.to_string_lossy().to_string());
|
||||
}
|
||||
|
||||
let paths_with_position =
|
||||
|
|
@ -810,9 +836,15 @@ async fn open_local_workspace(
|
|||
{
|
||||
Ok(result) => result,
|
||||
Err(error) => {
|
||||
let paths = paths_with_position
|
||||
.iter()
|
||||
.map(|p| p.path.display().to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
log::error!("failed to open workspace [{paths}]: {error:#}");
|
||||
responses
|
||||
.send(CliResponse::Stderr {
|
||||
message: format!("error opening {paths_with_position:?}: {error}"),
|
||||
message: format!("error opening [{paths}]: {error:#}"),
|
||||
})
|
||||
.log_err();
|
||||
return true;
|
||||
|
|
@ -863,9 +895,10 @@ async fn open_local_workspace(
|
|||
}
|
||||
}
|
||||
Some(Err(err)) => {
|
||||
log::error!("{err:#}");
|
||||
responses
|
||||
.send(CliResponse::Stderr {
|
||||
message: err.to_string(),
|
||||
message: format!("{err:#}"),
|
||||
})
|
||||
.log_err();
|
||||
errored = true;
|
||||
|
|
@ -1304,6 +1337,7 @@ mod tests {
|
|||
wait: true,
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1423,6 +1457,7 @@ mod tests {
|
|||
vec![],
|
||||
false,
|
||||
open_options,
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1493,6 +1528,7 @@ mod tests {
|
|||
vec![],
|
||||
false,
|
||||
workspace::OpenOptions::default(),
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1529,6 +1565,7 @@ mod tests {
|
|||
requesting_window: Some(window_to_replace),
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1675,6 +1712,7 @@ mod tests {
|
|||
Vec::new(),
|
||||
false,
|
||||
workspace::OpenOptions::default(),
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1702,6 +1740,7 @@ mod tests {
|
|||
workspace_matching: workspace::WorkspaceMatching::None, // Force new window
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1748,6 +1787,7 @@ mod tests {
|
|||
workspace_matching: workspace::WorkspaceMatching::MatchSubdirectory, // --add flag
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1812,6 +1852,7 @@ mod tests {
|
|||
open_in_dev_container: true,
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1866,6 +1907,7 @@ mod tests {
|
|||
open_in_dev_container: true,
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
&response_sink,
|
||||
&app_state,
|
||||
&mut cx,
|
||||
|
|
@ -1909,6 +1951,7 @@ mod tests {
|
|||
env: None,
|
||||
user_data_dir: None,
|
||||
dev_container: false,
|
||||
cwd: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -162,6 +162,7 @@ fn send_args_to_instance(args: &Args) -> anyhow::Result<()> {
|
|||
env: None,
|
||||
user_data_dir: args.user_data_dir.clone(),
|
||||
dev_container: args.dev_container,
|
||||
cwd: std::env::current_dir().ok(),
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue