From 37c2ebed7eceee63fd48bede4969f6d9a06f4382 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 14 Jan 2025 22:17:08 -0700 Subject: [PATCH] Revert "linux: Fix saving file with root ownership (#22045)" (#23162) Release Notes: - (temporarily) Removes the linux "save file as root" feature while we figure out bugs. Updates https://github.com/zed-industries/zed/pull/22045 --- Cargo.lock | 2 - crates/fs/Cargo.toml | 3 - crates/fs/src/fs.rs | 142 ++++++++----------------------------------- docs/src/linux.md | 10 --- 4 files changed, 24 insertions(+), 133 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 60ff786626..8cc326f987 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4733,13 +4733,11 @@ dependencies = [ "rope", "serde", "serde_json", - "shlex", "smol", "tempfile", "text", "time", "util", - "which 6.0.3", "windows 0.58.0", ] diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index f31f378ea2..fe145a6be5 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -47,12 +47,9 @@ windows.workspace = true [target.'cfg(any(target_os = "linux", target_os = "freebsd"))'.dependencies] ashpd.workspace = true -which.workspace = true -shlex.workspace = true [dev-dependencies] gpui = { workspace = true, features = ["test-support"] } [features] test-support = ["gpui/test-support", "git/test-support"] - diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index eedb243fce..e719f37760 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -9,9 +9,6 @@ use git::GitHostingProviderRegistry; #[cfg(any(target_os = "linux", target_os = "freebsd"))] use ashpd::desktop::trash; -#[cfg(any(target_os = "linux", target_os = "freebsd"))] -use smol::process::Command; - #[cfg(unix)] use std::os::fd::AsFd; #[cfg(unix)] @@ -521,7 +518,24 @@ impl Fs for RealFs { async fn atomic_write(&self, path: PathBuf, data: String) -> Result<()> { smol::unblock(move || { - let mut tmp_file = create_temp_file(&path)?; + let mut tmp_file = if cfg!(any(target_os = "linux", target_os = "freebsd")) { + // Use the directory of the destination as temp dir to avoid + // invalid cross-device link error, and XDG_CACHE_DIR for fallback. + // See https://github.com/zed-industries/zed/pull/8437 for more details. + NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir())) + } else if cfg!(target_os = "windows") { + // If temp dir is set to a different drive than the destination, + // we receive error: + // + // failed to persist temporary file: + // The system cannot move the file to a different disk drive. (os error 17) + // + // So we use the directory of the destination as a temp dir to avoid it. + // https://github.com/zed-industries/zed/issues/16571 + NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir())) + } else { + NamedTempFile::new() + }?; tmp_file.write_all(data.as_bytes())?; tmp_file.persist(path)?; Ok::<(), anyhow::Error>(()) @@ -536,43 +550,13 @@ impl Fs for RealFs { if let Some(path) = path.parent() { self.create_dir(path).await?; } - match smol::fs::File::create(path).await { - Ok(file) => { - let mut writer = smol::io::BufWriter::with_capacity(buffer_size, file); - for chunk in chunks(text, line_ending) { - writer.write_all(chunk.as_bytes()).await?; - } - writer.flush().await?; - Ok(()) - } - Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { - if cfg!(any(target_os = "linux", target_os = "freebsd")) { - let target_path = path.to_path_buf(); - let temp_file = smol::unblock(move || create_temp_file(&target_path)).await?; - - let temp_path = temp_file.into_temp_path(); - let temp_path_for_write = temp_path.to_path_buf(); - - let async_file = smol::fs::OpenOptions::new() - .write(true) - .open(&temp_path) - .await?; - - let mut writer = smol::io::BufWriter::with_capacity(buffer_size, async_file); - - for chunk in chunks(text, line_ending) { - writer.write_all(chunk.as_bytes()).await?; - } - writer.flush().await?; - - write_to_file_as_root(temp_path_for_write, path.to_path_buf()).await - } else { - // Todo: Implement for Mac and Windows - Err(e.into()) - } - } - Err(e) => Err(e.into()), + let file = smol::fs::File::create(path).await?; + let mut writer = smol::io::BufWriter::with_capacity(buffer_size, file); + for chunk in chunks(text, line_ending) { + writer.write_all(chunk.as_bytes()).await?; } + writer.flush().await?; + Ok(()) } async fn canonicalize(&self, path: &Path) -> Result { @@ -1979,84 +1963,6 @@ fn chunks(rope: &Rope, line_ending: LineEnding) -> impl Iterator { }) } -fn create_temp_file(path: &Path) -> Result { - let temp_file = if cfg!(any(target_os = "linux", target_os = "freebsd")) { - // Use the directory of the destination as temp dir to avoid - // invalid cross-device link error, and XDG_CACHE_DIR for fallback. - // See https://github.com/zed-industries/zed/pull/8437 for more details. - NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir()))? - } else if cfg!(target_os = "windows") { - // If temp dir is set to a different drive than the destination, - // we receive error: - // - // failed to persist temporary file: - // The system cannot move the file to a different disk drive. (os error 17) - // - // So we use the directory of the destination as a temp dir to avoid it. - // https://github.com/zed-industries/zed/issues/16571 - NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir()))? - } else { - NamedTempFile::new()? - }; - - Ok(temp_file) -} - -#[cfg(target_os = "macos")] -async fn write_to_file_as_root(_temp_file_path: PathBuf, _target_file_path: PathBuf) -> Result<()> { - unimplemented!("write_to_file_as_root is not implemented") -} - -#[cfg(target_os = "windows")] -async fn write_to_file_as_root(_temp_file_path: PathBuf, _target_file_path: PathBuf) -> Result<()> { - unimplemented!("write_to_file_as_root is not implemented") -} - -#[cfg(any(target_os = "linux", target_os = "freebsd"))] -async fn write_to_file_as_root(temp_file_path: PathBuf, target_file_path: PathBuf) -> Result<()> { - use shlex::try_quote; - use std::os::unix::fs::PermissionsExt; - use which::which; - - let pkexec_path = smol::unblock(|| which("pkexec")) - .await - .map_err(|_| anyhow::anyhow!("pkexec not found in PATH"))?; - - let script_file = smol::unblock(move || { - let script_file = tempfile::Builder::new() - .prefix("write-to-file-as-root-") - .tempfile_in(paths::temp_dir())?; - - writeln!( - script_file.as_file(), - "#!/usr/bin/env sh\nset -eu\ncat \"{}\" > \"{}\"", - try_quote(&temp_file_path.to_string_lossy())?, - try_quote(&target_file_path.to_string_lossy())? - )?; - - let mut perms = script_file.as_file().metadata()?.permissions(); - perms.set_mode(0o700); // rwx------ - script_file.as_file().set_permissions(perms)?; - - Result::<_>::Ok(script_file) - }) - .await?; - - let script_path = script_file.into_temp_path(); - - let output = Command::new(&pkexec_path) - .arg("--disable-internal-agent") - .arg(&script_path) - .output() - .await?; - - if !output.status.success() { - return Err(anyhow::anyhow!("Failed to write to file as root")); - } - - Ok(()) -} - 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() { diff --git a/docs/src/linux.md b/docs/src/linux.md index f87de5b5bd..6e62300ec8 100644 --- a/docs/src/linux.md +++ b/docs/src/linux.md @@ -170,13 +170,3 @@ rm ~/.local/zed.app/lib/libcrypto.so.1.1 ``` This will force zed to fallback to the system `libssl` and `libcrypto` libraries. - -### Editing files requiring root access - -When you try to edit files that require root access, Zed requires `pkexec` (part of polkit) to handle authentication prompts. - -Polkit comes pre-installed with most desktop environments like GNOME and KDE. If you're using a minimal system and polkit is not installed, you can install it with: - -- Ubuntu/Debian: `sudo apt install policykit-1` -- Fedora: `sudo dnf install polkit` -- Arch Linux: `sudo pacman -S polkit`