diff --git a/crates/dap/src/adapters.rs b/crates/dap/src/adapters.rs index 73e2881521..32862ad274 100644 --- a/crates/dap/src/adapters.rs +++ b/crates/dap/src/adapters.rs @@ -309,7 +309,7 @@ pub async fn download_adapter_from_github( let mut file = File::create(&zip_path).await?; futures::io::copy(response.body_mut(), &mut file).await?; let file = File::open(&zip_path).await?; - extract_zip(&version_path, BufReader::new(file)) + extract_zip(&version_path, file) .await // we cannot check the status as some adapter include files with names that trigger `Illegal byte sequence` .ok(); diff --git a/crates/dap_adapters/src/codelldb.rs b/crates/dap_adapters/src/codelldb.rs index a123f399da..fbe2e49147 100644 --- a/crates/dap_adapters/src/codelldb.rs +++ b/crates/dap_adapters/src/codelldb.rs @@ -399,34 +399,6 @@ impl DebugAdapter for CodeLldbDebugAdapter { }; let adapter_dir = version_path.join("extension").join("adapter"); let path = adapter_dir.join("codelldb").to_string_lossy().to_string(); - // todo("windows") - #[cfg(not(windows))] - { - use smol::fs; - - fs::set_permissions( - &path, - ::from_mode(0o755), - ) - .await - .with_context(|| format!("Settings executable permissions to {path:?}"))?; - - let lldb_binaries_dir = version_path.join("extension").join("lldb").join("bin"); - let mut lldb_binaries = - fs::read_dir(&lldb_binaries_dir).await.with_context(|| { - format!("reading lldb binaries dir contents {lldb_binaries_dir:?}") - })?; - while let Some(binary) = lldb_binaries.next().await { - let binary_entry = binary?; - let path = binary_entry.path(); - fs::set_permissions( - &path, - ::from_mode(0o755), - ) - .await - .with_context(|| format!("Settings executable permissions to {path:?}"))?; - } - } self.path_to_codelldb.set(path.clone()).ok(); command = Some(path); }; diff --git a/crates/languages/src/c.rs b/crates/languages/src/c.rs index 3128810124..ed5d5fefb3 100644 --- a/crates/languages/src/c.rs +++ b/crates/languages/src/c.rs @@ -7,7 +7,7 @@ pub use language::*; use lsp::{DiagnosticTag, InitializeParams, LanguageServerBinary, LanguageServerName}; use project::lsp_store::clangd_ext; use serde_json::json; -use smol::{fs, io::BufReader}; +use smol::fs; use std::{any::Any, env::consts, path::PathBuf, sync::Arc}; use util::{ResultExt, archive::extract_zip, fs::remove_matching, maybe, merge_json_value_into}; @@ -83,20 +83,10 @@ impl super::LspAdapter for CLspAdapter { "download failed with status {}", response.status().to_string() ); - extract_zip(&container_dir, BufReader::new(response.body_mut())) + extract_zip(&container_dir, response.body_mut()) .await .with_context(|| format!("unzipping clangd archive to {container_dir:?}"))?; remove_matching(&container_dir, |entry| entry != version_dir).await; - - // todo("windows") - #[cfg(not(windows))] - { - fs::set_permissions( - &binary_path, - ::from_mode(0o755), - ) - .await?; - } } Ok(LanguageServerBinary { diff --git a/crates/languages/src/json.rs b/crates/languages/src/json.rs index 3618b9956a..31fa5a471a 100644 --- a/crates/languages/src/json.rs +++ b/crates/languages/src/json.rs @@ -442,11 +442,7 @@ impl LspAdapter for NodeVersionAdapter { .await .context("downloading release")?; if version.url.ends_with(".zip") { - extract_zip( - &destination_container_path, - BufReader::new(response.body_mut()), - ) - .await?; + extract_zip(&destination_container_path, response.body_mut()).await?; } else if version.url.ends_with(".tar.gz") { let decompressed_bytes = GzipDecoder::new(BufReader::new(response.body_mut())); let archive = Archive::new(decompressed_bytes); @@ -462,15 +458,6 @@ impl LspAdapter for NodeVersionAdapter { &destination_path, ) .await?; - // todo("windows") - #[cfg(not(windows))] - { - fs::set_permissions( - &destination_path, - ::from_mode(0o755), - ) - .await?; - } remove_matching(&container_dir, |entry| entry != destination_path).await; } Ok(LanguageServerBinary { diff --git a/crates/languages/src/rust.rs b/crates/languages/src/rust.rs index 59a14fd932..fea4b1b8b5 100644 --- a/crates/languages/src/rust.rs +++ b/crates/languages/src/rust.rs @@ -216,7 +216,7 @@ impl LspAdapter for RustLspAdapter { })?; } AssetKind::Zip => { - extract_zip(&destination_path, BufReader::new(response.body_mut())) + extract_zip(&destination_path, response.body_mut()) .await .with_context(|| { format!("unzipping {} to {:?}", version.url, destination_path) diff --git a/crates/languages/src/typescript.rs b/crates/languages/src/typescript.rs index a728b97501..d114b50178 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -490,7 +490,7 @@ impl LspAdapter for EsLintLspAdapter { })?; } AssetKind::Zip => { - extract_zip(&destination_path, BufReader::new(response.body_mut())) + extract_zip(&destination_path, response.body_mut()) .await .with_context(|| { format!("unzipping {} to {:?}", version.url, destination_path) diff --git a/crates/project/src/image_store.rs b/crates/project/src/image_store.rs index 88d48a4f40..a290c521d5 100644 --- a/crates/project/src/image_store.rs +++ b/crates/project/src/image_store.rs @@ -376,12 +376,6 @@ impl ImageStore { entry.insert(rx.clone()); let project_path = project_path.clone(); - // TODO kb this is causing another error, and we also pass a worktree nearby — seems ok to pass "" here? - // let image_path = worktree - // .read(cx) - // .absolutize(&project_path.path) - // .map(Arc::from) - // .unwrap_or_else(|_| project_path.path.clone()); let load_image = self .state .open_image(project_path.path.clone(), worktree, cx); diff --git a/crates/supermaven_api/src/supermaven_api.rs b/crates/supermaven_api/src/supermaven_api.rs index f51822333c..7e99050b90 100644 --- a/crates/supermaven_api/src/supermaven_api.rs +++ b/crates/supermaven_api/src/supermaven_api.rs @@ -271,14 +271,6 @@ pub async fn get_supermaven_agent_path(client: Arc) -> Result::from_mode( - 0o755, - )) - .await?; - } - let mut old_binary_paths = fs::read_dir(supermaven_dir()).await?; while let Some(old_binary_path) = old_binary_paths.next().await { let old_binary_path = old_binary_path?; diff --git a/crates/util/Cargo.toml b/crates/util/Cargo.toml index 7371b577cb..f6fc4b5164 100644 --- a/crates/util/Cargo.toml +++ b/crates/util/Cargo.toml @@ -13,7 +13,7 @@ path = "src/util.rs" doctest = true [features] -test-support = ["tempfile", "git2", "rand", "util_macros"] +test-support = ["git2", "rand", "util_macros"] [dependencies] anyhow.workspace = true @@ -35,7 +35,7 @@ serde_json.workspace = true serde_json_lenient.workspace = true smol.workspace = true take-until.workspace = true -tempfile = { workspace = true, optional = true } +tempfile.workspace = true unicase.workspace = true util_macros = { workspace = true, optional = true } walkdir.workspace = true @@ -51,5 +51,4 @@ dunce = "1.0" [dev-dependencies] git2.workspace = true rand.workspace = true -tempfile.workspace = true util_macros.workspace = true diff --git a/crates/util/src/archive.rs b/crates/util/src/archive.rs index 76f754eca2..d10b996716 100644 --- a/crates/util/src/archive.rs +++ b/crates/util/src/archive.rs @@ -1,11 +1,12 @@ use std::path::Path; -use anyhow::Result; -use async_zip::base::read::stream::ZipFileReader; +use anyhow::{Context as _, Result}; +use async_zip::base::read; use futures::{AsyncRead, io::BufReader}; +#[cfg(windows)] pub async fn extract_zip(destination: &Path, reader: R) -> Result<()> { - let mut reader = ZipFileReader::new(BufReader::new(reader)); + let mut reader = read::stream::ZipFileReader::new(BufReader::new(reader)); let destination = &destination .canonicalize() @@ -14,18 +15,98 @@ pub async fn extract_zip(destination: &Path, reader: R) -> while let Some(mut item) = reader.next_with_entry().await? { let entry_reader = item.reader_mut(); let entry = entry_reader.entry(); - let path = destination.join(entry.filename().as_str().unwrap()); + let path = destination.join( + entry + .filename() + .as_str() + .context("reading zip entry file name")?, + ); - if entry.dir().unwrap() { - std::fs::create_dir_all(&path)?; + if entry + .dir() + .with_context(|| format!("reading zip entry metadata for path {path:?}"))? + { + std::fs::create_dir_all(&path) + .with_context(|| format!("creating directory {path:?}"))?; } else { - let parent_dir = path.parent().expect("failed to get parent directory"); - std::fs::create_dir_all(parent_dir)?; - let mut file = smol::fs::File::create(&path).await?; - futures::io::copy(entry_reader, &mut file).await?; + let parent_dir = path + .parent() + .with_context(|| format!("no parent directory for {path:?}"))?; + std::fs::create_dir_all(parent_dir) + .with_context(|| format!("creating parent directory {parent_dir:?}"))?; + let mut file = smol::fs::File::create(&path) + .await + .with_context(|| format!("creating file {path:?}"))?; + futures::io::copy(entry_reader, &mut file) + .await + .with_context(|| format!("extracting into file {path:?}"))?; } - reader = item.skip().await?; + reader = item.skip().await.context("reading next zip entry")?; + } + + Ok(()) +} + +#[cfg(not(windows))] +pub async fn extract_zip(destination: &Path, reader: R) -> Result<()> { + // Unix needs file permissions copied when extracting. + // This is only possible to do when a reader impls `AsyncSeek` and `seek::ZipFileReader` is used. + // `stream::ZipFileReader` also has the `unix_permissions` method, but it will always return `Some(0)`. + // + // A typical `reader` comes from a streaming network response, so cannot be sought right away, + // and reading the entire archive into the memory seems wasteful. + // + // So, save the stream into a temporary file first and then get it read with a seeking reader. + let mut file = async_fs::File::from(tempfile::tempfile().context("creating a temporary file")?); + futures::io::copy(&mut BufReader::new(reader), &mut file) + .await + .context("saving archive contents into the temporary file")?; + let mut reader = read::seek::ZipFileReader::new(BufReader::new(file)) + .await + .context("reading the zip archive")?; + let destination = &destination + .canonicalize() + .unwrap_or_else(|_| destination.to_path_buf()); + for (i, entry) in reader.file().entries().to_vec().into_iter().enumerate() { + let path = destination.join( + entry + .filename() + .as_str() + .context("reading zip entry file name")?, + ); + + if entry + .dir() + .with_context(|| format!("reading zip entry metadata for path {path:?}"))? + { + std::fs::create_dir_all(&path) + .with_context(|| format!("creating directory {path:?}"))?; + } else { + let parent_dir = path + .parent() + .with_context(|| format!("no parent directory for {path:?}"))?; + std::fs::create_dir_all(parent_dir) + .with_context(|| format!("creating parent directory {parent_dir:?}"))?; + let mut file = smol::fs::File::create(&path) + .await + .with_context(|| format!("creating file {path:?}"))?; + let mut entry_reader = reader + .reader_with_entry(i) + .await + .with_context(|| format!("reading entry for path {path:?}"))?; + futures::io::copy(&mut entry_reader, &mut file) + .await + .with_context(|| format!("extracting into file {path:?}"))?; + + if let Some(perms) = entry.unix_permissions() { + use std::os::unix::fs::PermissionsExt; + let permissions = std::fs::Permissions::from_mode(u32::from(perms)); + file.set_permissions(permissions) + .await + .with_context(|| format!("setting permissions for file {path:?}"))?; + } + } } Ok(()) @@ -33,11 +114,9 @@ pub async fn extract_zip(destination: &Path, reader: R) -> #[cfg(test)] mod tests { - use std::path::PathBuf; - use async_zip::ZipEntryBuilder; use async_zip::base::write::ZipFileWriter; - use futures::AsyncWriteExt; + use futures::{AsyncSeek, AsyncWriteExt}; use smol::io::Cursor; use tempfile::TempDir; @@ -59,9 +138,23 @@ mod tests { let data = smol::fs::read(&path).await?; let filename = relative_path.display().to_string(); - let builder = ZipEntryBuilder::new(filename.into(), async_zip::Compression::Deflate); - writer.write_entry_whole(builder, &data).await?; + #[cfg(unix)] + { + let mut builder = + ZipEntryBuilder::new(filename.into(), async_zip::Compression::Deflate); + use std::os::unix::fs::PermissionsExt; + let metadata = std::fs::metadata(&path)?; + let perms = metadata.permissions().mode() as u16; + builder = builder.unix_permissions(perms); + writer.write_entry_whole(builder, &data).await?; + } + #[cfg(not(unix))] + { + let builder = + ZipEntryBuilder::new(filename.into(), async_zip::Compression::Deflate); + writer.write_entry_whole(builder, &data).await?; + } } writer.close().await?; @@ -91,7 +184,7 @@ mod tests { dir } - async fn read_archive(path: &PathBuf) -> impl AsyncRead + Unpin { + async fn read_archive(path: &Path) -> impl AsyncRead + AsyncSeek + Unpin { let data = smol::fs::read(&path).await.unwrap(); Cursor::new(data) } @@ -115,4 +208,36 @@ mod tests { assert_file_content(&dst.join("foo/bar/dar你好.txt"), "你好世界"); }); } + + #[cfg(unix)] + #[test] + fn test_extract_zip_preserves_executable_permissions() { + use std::os::unix::fs::PermissionsExt; + + smol::block_on(async { + let test_dir = tempfile::tempdir().unwrap(); + let executable_path = test_dir.path().join("my_script"); + + // Create an executable file + std::fs::write(&executable_path, "#!/bin/bash\necho 'Hello'").unwrap(); + let mut perms = std::fs::metadata(&executable_path).unwrap().permissions(); + perms.set_mode(0o755); // rwxr-xr-x + std::fs::set_permissions(&executable_path, perms).unwrap(); + + // Create zip + let zip_file = test_dir.path().join("test.zip"); + compress_zip(test_dir.path(), &zip_file).await.unwrap(); + + // Extract to new location + let extract_dir = tempfile::tempdir().unwrap(); + let reader = read_archive(&zip_file).await; + extract_zip(extract_dir.path(), reader).await.unwrap(); + + // Check permissions are preserved + let extracted_path = extract_dir.path().join("my_script"); + assert!(extracted_path.exists()); + let extracted_perms = std::fs::metadata(&extracted_path).unwrap().permissions(); + assert_eq!(extracted_perms.mode() & 0o777, 0o755); + }); + } }