From a391d67366c32c6c82534bcbb9a9dab47edbea26 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Tue, 17 Jun 2025 16:39:45 +0200 Subject: [PATCH] supermaven_api: Ensure downloaded Supermaven binary has executable permissions set (#32576) Closes #32068 Closes #15653 Not entirely sure that it fixes the latter issue, but I am fairly certain given the comments in #32068 and the available logs in the issue. This PR fixes an issue where the Supermaven provider would not leave the "Initializing" stage. This happened due to the downloaded binary missing executable permissions. The change here ensures that freshly downloaded binaries as well as existing binaries downloaded by Zed have executable permissions set. I decided on also adding this for the latter since existing downloads would continue to be broken and Supermaven does not seem to change versions often given the logs provided by users. While I was at it, I also added a `make_file_executable` to the util crate mirroring the method of the `zed_extensions_api` and refactored existing usages where possible to use that method instead. This makes the code slightly more readable in my opinion, yet adds a method to non-unix systems that practically does nothing. I can revert this should that be preferred. Release Notes: - Fixed an issue where the Supermaven completion provider would not leave the "Initializing" stage. --------- Co-authored-by: Bennet Bo Fenner --- Cargo.lock | 1 + crates/askpass/src/askpass.rs | 8 +++----- .../src/wasm_host/wit/since_v0_1_0.rs | 20 +++++-------------- .../src/wasm_host/wit/since_v0_6_0.rs | 19 +++++------------- .../src/extension_lsp_adapter.rs | 13 ++++-------- crates/languages/src/rust.rs | 15 ++++++-------- crates/supermaven_api/Cargo.toml | 1 + crates/supermaven_api/src/supermaven_api.rs | 18 +++++++++-------- crates/util/src/fs.rs | 19 ++++++++++++++++++ 9 files changed, 54 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 75cb0930b8..990b052cdf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15479,6 +15479,7 @@ dependencies = [ "serde", "serde_json", "smol", + "util", "workspace-hack", ] diff --git a/crates/askpass/src/askpass.rs b/crates/askpass/src/askpass.rs index 58308b72ab..519c08aa26 100644 --- a/crates/askpass/src/askpass.rs +++ b/crates/askpass/src/askpass.rs @@ -13,11 +13,9 @@ use gpui::{AsyncApp, BackgroundExecutor, Task}; #[cfg(unix)] use smol::fs; #[cfg(unix)] -use smol::{fs::unix::PermissionsExt as _, net::unix::UnixListener}; +use smol::net::unix::UnixListener; #[cfg(unix)] -use util::ResultExt as _; -#[cfg(unix)] -use util::get_shell_safe_zed_path; +use util::{ResultExt as _, fs::make_file_executable, get_shell_safe_zed_path}; #[derive(PartialEq, Eq)] pub enum AskPassResult { @@ -122,7 +120,7 @@ impl AskPassSession { shebang = "#!/bin/sh", ); fs::write(&askpass_script_path, askpass_script).await?; - fs::set_permissions(&askpass_script_path, std::fs::Permissions::from_mode(0o755)).await?; + make_file_executable(&askpass_script_path).await?; Ok(Self { script_path: askpass_script_path, diff --git a/crates/extension_host/src/wasm_host/wit/since_v0_1_0.rs b/crates/extension_host/src/wasm_host/wit/since_v0_1_0.rs index 53ea6cab76..9c726ebd1c 100644 --- a/crates/extension_host/src/wasm_host/wit/since_v0_1_0.rs +++ b/crates/extension_host/src/wasm_host/wit/since_v0_1_0.rs @@ -16,8 +16,7 @@ use std::{ path::{Path, PathBuf}, sync::{Arc, OnceLock}, }; -use util::archive::extract_zip; -use util::maybe; +use util::{archive::extract_zip, fs::make_file_executable, maybe}; use wasmtime::component::{Linker, Resource}; use super::latest; @@ -558,22 +557,13 @@ impl ExtensionImports for WasmState { } async fn make_file_executable(&mut self, path: String) -> wasmtime::Result> { - #[allow(unused)] let path = self .host .writeable_path_from_extension(&self.manifest.id, Path::new(&path))?; - #[cfg(unix)] - { - use std::fs::{self, Permissions}; - use std::os::unix::fs::PermissionsExt; - - return fs::set_permissions(&path, Permissions::from_mode(0o755)) - .with_context(|| format!("setting permissions for path {path:?}")) - .to_wasmtime_result(); - } - - #[cfg(not(unix))] - Ok(Ok(())) + make_file_executable(&path) + .await + .with_context(|| format!("setting permissions for path {path:?}")) + .to_wasmtime_result() } } diff --git a/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs b/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs index 214c06ed12..158dee7989 100644 --- a/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs +++ b/crates/extension_host/src/wasm_host/wit/since_v0_6_0.rs @@ -30,7 +30,7 @@ use std::{ sync::{Arc, OnceLock}, }; use task::{SpawnInTerminal, ZedDebugConfig}; -use util::{archive::extract_zip, maybe}; +use util::{archive::extract_zip, fs::make_file_executable, maybe}; use wasmtime::component::{Linker, Resource}; pub const MIN_VERSION: SemanticVersion = SemanticVersion::new(0, 6, 0); @@ -1065,22 +1065,13 @@ impl ExtensionImports for WasmState { } async fn make_file_executable(&mut self, path: String) -> wasmtime::Result> { - #[allow(unused)] let path = self .host .writeable_path_from_extension(&self.manifest.id, Path::new(&path))?; - #[cfg(unix)] - { - use std::fs::{self, Permissions}; - use std::os::unix::fs::PermissionsExt; - - return fs::set_permissions(&path, Permissions::from_mode(0o755)) - .with_context(|| format!("setting permissions for path {path:?}")) - .to_wasmtime_result(); - } - - #[cfg(not(unix))] - Ok(Ok(())) + make_file_executable(&path) + .await + .with_context(|| format!("setting permissions for path {path:?}")) + .to_wasmtime_result() } } diff --git a/crates/language_extension/src/extension_lsp_adapter.rs b/crates/language_extension/src/extension_lsp_adapter.rs index e6c0b2b796..a32292daa3 100644 --- a/crates/language_extension/src/extension_lsp_adapter.rs +++ b/crates/language_extension/src/extension_lsp_adapter.rs @@ -18,7 +18,7 @@ use language::{ use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerBinaryOptions, LanguageServerName}; use serde::Serialize; use serde_json::Value; -use util::{ResultExt, maybe}; +use util::{ResultExt, fs::make_file_executable, maybe}; use crate::LanguageServerRegistryProxy; @@ -146,14 +146,9 @@ impl LspAdapter for ExtensionLspAdapter { if ["toml", "zig"].contains(&self.extension.manifest().id.as_ref()) && path.starts_with(&self.extension.work_dir()) { - #[cfg(not(windows))] - { - use std::fs::{self, Permissions}; - use std::os::unix::fs::PermissionsExt; - - fs::set_permissions(&path, Permissions::from_mode(0o755)) - .context("failed to set file permissions")?; - } + make_file_executable(&path) + .await + .context("failed to set file permissions")?; } Ok(LanguageServerBinary { diff --git a/crates/languages/src/rust.rs b/crates/languages/src/rust.rs index 31318a1dba..1397d5dbf1 100644 --- a/crates/languages/src/rust.rs +++ b/crates/languages/src/rust.rs @@ -25,7 +25,11 @@ use std::{ use task::{TaskTemplate, TaskTemplates, TaskVariables, VariableName}; use util::archive::extract_zip; use util::merge_json_value_into; -use util::{ResultExt, fs::remove_matching, maybe}; +use util::{ + ResultExt, + fs::{make_file_executable, remove_matching}, + maybe, +}; use crate::language_settings::language_settings; @@ -226,14 +230,7 @@ impl LspAdapter for RustLspAdapter { }; // todo("windows") - #[cfg(not(windows))] - { - fs::set_permissions( - &server_path, - ::from_mode(0o755), - ) - .await?; - } + make_file_executable(&server_path).await?; } Ok(LanguageServerBinary { diff --git a/crates/supermaven_api/Cargo.toml b/crates/supermaven_api/Cargo.toml index 25a1c0477a..6b6823095d 100644 --- a/crates/supermaven_api/Cargo.toml +++ b/crates/supermaven_api/Cargo.toml @@ -20,4 +20,5 @@ paths.workspace = true serde.workspace = true serde_json.workspace = true smol.workspace = true +util.workspace = true workspace-hack.workspace = true diff --git a/crates/supermaven_api/src/supermaven_api.rs b/crates/supermaven_api/src/supermaven_api.rs index 7e99050b90..61d14d5dc7 100644 --- a/crates/supermaven_api/src/supermaven_api.rs +++ b/crates/supermaven_api/src/supermaven_api.rs @@ -5,10 +5,11 @@ use http_client::{AsyncBody, HttpClient, Request as HttpRequest}; use paths::supermaven_dir; use serde::{Deserialize, Serialize}; use smol::fs::{self, File}; -use smol::stream::StreamExt; use std::path::{Path, PathBuf}; use std::sync::Arc; +use util::fs::{make_file_executable, remove_matching}; + #[derive(Serialize)] pub struct GetExternalUserRequest { pub id: String, @@ -253,6 +254,11 @@ pub async fn get_supermaven_agent_path(client: Arc) -> Result) -> Result>( Ok(()) } + +#[cfg(unix)] +/// Set the permissions for the given path so that the file becomes executable. +/// This is a noop for non-unix platforms. +pub async fn make_file_executable(path: &PathBuf) -> std::io::Result<()> { + fs::set_permissions( + &path, + ::from_mode(0o755), + ) + .await +} + +#[cfg(not(unix))] +#[allow(clippy::unused_async)] +/// Set the permissions for the given path so that the file becomes executable. +/// This is a noop for non-unix platforms. +pub async fn make_file_executable(_path: &PathBuf) -> std::io::Result<()> { + Ok(()) +}