Revert "linux: Fix saving file with root ownership (#22045)" (#23162)

Conrad Irwin created

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

Change summary

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

Detailed changes

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",
 ]
 

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"]
-

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<PathBuf> {
@@ -1979,84 +1963,6 @@ fn chunks(rope: &Rope, line_ending: LineEnding) -> impl Iterator<Item = &str> {
     })
 }
 
-fn create_temp_file(path: &Path) -> Result<NamedTempFile> {
-    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() {

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`