From bb591f1e653a492c2d6c85353181e5151dbf8ac7 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Mon, 8 Dec 2025 21:49:26 +0100 Subject: [PATCH] extension_cli: Properly populate manifest with snippet location (#44425) This fixes an issue where the snippet file location would not be the proper one for compiled extensions because it would be populated with an absolute path instead of a relative one in relation to the extension output directory. This caused the copy operation downstream to not do anything, because it copied the file to the location it already was (which was not the output directory for that extension). Also adds some tests and pulls in the `Fs` so we do not have such issues with snippets a third time hopefully. Release Notes: - N/A --- Cargo.lock | 1 + crates/extension/Cargo.toml | 3 + crates/extension/src/extension_builder.rs | 169 ++++++++++++++---- crates/extension_cli/src/main.rs | 1 + .../extension_compilation_benchmark.rs | 11 +- crates/extension_host/src/extension_host.rs | 5 +- 6 files changed, 154 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5276a9eac963964531e7e03dceac2713d361443b..9de96dfe48ccd211c94539e541ac55da3de8ac63 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5799,6 +5799,7 @@ dependencies = [ "gpui", "heck 0.5.0", "http_client", + "indoc", "language", "log", "lsp", diff --git a/crates/extension/Cargo.toml b/crates/extension/Cargo.toml index a00406c43e796b1b2bafafc3d0481b09ee5fd43a..307a3a19bd5ec6502270ae2f579cbd6b6f378746 100644 --- a/crates/extension/Cargo.toml +++ b/crates/extension/Cargo.toml @@ -37,5 +37,8 @@ wasm-encoder.workspace = true wasmparser.workspace = true [dev-dependencies] +fs = { workspace = true, "features" = ["test-support"] } +gpui = { workspace = true, "features" = ["test-support"] } +indoc.workspace = true pretty_assertions.workspace = true tempfile.workspace = true diff --git a/crates/extension/src/extension_builder.rs b/crates/extension/src/extension_builder.rs index b6d4cc0c4da3d1f7d998512f099eba6c9c04e1a5..8b9bf994d17e0594c719bed29907630fedf11497 100644 --- a/crates/extension/src/extension_builder.rs +++ b/crates/extension/src/extension_builder.rs @@ -2,8 +2,9 @@ use crate::{ ExtensionLibraryKind, ExtensionManifest, GrammarManifestEntry, build_debug_adapter_schema_path, parse_wasm_extension_version, }; +use ::fs::Fs; use anyhow::{Context as _, Result, bail}; -use futures::AsyncReadExt; +use futures::{AsyncReadExt, StreamExt}; use heck::ToSnakeCase; use http_client::{self, AsyncBody, HttpClient}; use serde::Deserialize; @@ -77,8 +78,9 @@ impl ExtensionBuilder { extension_dir: &Path, extension_manifest: &mut ExtensionManifest, options: CompileExtensionOptions, + fs: Arc, ) -> Result<()> { - populate_defaults(extension_manifest, extension_dir)?; + populate_defaults(extension_manifest, extension_dir, fs).await?; if extension_dir.is_relative() { bail!( @@ -546,7 +548,11 @@ impl ExtensionBuilder { } } -fn populate_defaults(manifest: &mut ExtensionManifest, extension_path: &Path) -> Result<()> { +async fn populate_defaults( + manifest: &mut ExtensionManifest, + extension_path: &Path, + fs: Arc, +) -> Result<()> { // For legacy extensions on the v0 schema (aka, using `extension.json`), clear out any existing // contents of the computed fields, since we don't care what the existing values are. if manifest.schema_version.is_v0() { @@ -561,12 +567,16 @@ fn populate_defaults(manifest: &mut ExtensionManifest, extension_path: &Path) -> } let languages_dir = extension_path.join("languages"); - if languages_dir.exists() { - for entry in fs::read_dir(&languages_dir).context("failed to list languages dir")? { - let entry = entry?; - let language_dir = entry.path(); + if fs.is_dir(&languages_dir).await { + let mut language_dir_entries = fs + .read_dir(&languages_dir) + .await + .context("failed to list languages dir")?; + + while let Some(language_dir) = language_dir_entries.next().await { + let language_dir = language_dir?; let config_path = language_dir.join("config.toml"); - if config_path.exists() { + if fs.is_file(config_path.as_path()).await { let relative_language_dir = language_dir.strip_prefix(extension_path)?.to_path_buf(); if !manifest.languages.contains(&relative_language_dir) { @@ -577,10 +587,14 @@ fn populate_defaults(manifest: &mut ExtensionManifest, extension_path: &Path) -> } let themes_dir = extension_path.join("themes"); - if themes_dir.exists() { - for entry in fs::read_dir(&themes_dir).context("failed to list themes dir")? { - let entry = entry?; - let theme_path = entry.path(); + if fs.is_dir(&themes_dir).await { + let mut theme_dir_entries = fs + .read_dir(&themes_dir) + .await + .context("failed to list themes dir")?; + + while let Some(theme_path) = theme_dir_entries.next().await { + let theme_path = theme_path?; if theme_path.extension() == Some("json".as_ref()) { let relative_theme_path = theme_path.strip_prefix(extension_path)?.to_path_buf(); if !manifest.themes.contains(&relative_theme_path) { @@ -591,10 +605,14 @@ fn populate_defaults(manifest: &mut ExtensionManifest, extension_path: &Path) -> } let icon_themes_dir = extension_path.join("icon_themes"); - if icon_themes_dir.exists() { - for entry in fs::read_dir(&icon_themes_dir).context("failed to list icon themes dir")? { - let entry = entry?; - let icon_theme_path = entry.path(); + if fs.is_dir(&icon_themes_dir).await { + let mut icon_theme_dir_entries = fs + .read_dir(&icon_themes_dir) + .await + .context("failed to list icon themes dir")?; + + while let Some(icon_theme_path) = icon_theme_dir_entries.next().await { + let icon_theme_path = icon_theme_path?; if icon_theme_path.extension() == Some("json".as_ref()) { let relative_icon_theme_path = icon_theme_path.strip_prefix(extension_path)?.to_path_buf(); @@ -603,21 +621,26 @@ fn populate_defaults(manifest: &mut ExtensionManifest, extension_path: &Path) -> } } } - } - - let snippets_json_path = extension_path.join("snippets.json"); - if snippets_json_path.exists() { - manifest.snippets = Some(snippets_json_path); + }; + if manifest.snippets.is_none() + && let snippets_json_path = extension_path.join("snippets.json") + && fs.is_file(&snippets_json_path).await + { + manifest.snippets = Some("snippets.json".into()); } // For legacy extensions on the v0 schema (aka, using `extension.json`), we want to populate the grammars in // the manifest using the contents of the `grammars` directory. if manifest.schema_version.is_v0() { let grammars_dir = extension_path.join("grammars"); - if grammars_dir.exists() { - for entry in fs::read_dir(&grammars_dir).context("failed to list grammars dir")? { - let entry = entry?; - let grammar_path = entry.path(); + if fs.is_dir(&grammars_dir).await { + let mut grammar_dir_entries = fs + .read_dir(&grammars_dir) + .await + .context("failed to list grammars dir")?; + + while let Some(grammar_path) = grammar_dir_entries.next().await { + let grammar_path = grammar_path?; if grammar_path.extension() == Some("toml".as_ref()) { #[derive(Deserialize)] struct GrammarConfigToml { @@ -627,7 +650,7 @@ fn populate_defaults(manifest: &mut ExtensionManifest, extension_path: &Path) -> pub path: Option, } - let grammar_config = fs::read_to_string(&grammar_path)?; + let grammar_config = fs.load(&grammar_path).await?; let grammar_config: GrammarConfigToml = toml::from_str(&grammar_config)?; let grammar_name = grammar_path @@ -677,9 +700,20 @@ fn file_newer_than_deps(target: &Path, dependencies: &[&Path]) -> Result Result<()> { &extension_path, &mut manifest, CompileExtensionOptions { release: true }, + fs.clone(), ) .await .context("failed to compile extension")?; diff --git a/crates/extension_host/benches/extension_compilation_benchmark.rs b/crates/extension_host/benches/extension_compilation_benchmark.rs index c3459cf116b5510bc98fd167ea32c242bd48ad9a..a28f617dc36e5cba3ad36d7ab6477e7a665dd5c4 100644 --- a/crates/extension_host/benches/extension_compilation_benchmark.rs +++ b/crates/extension_host/benches/extension_compilation_benchmark.rs @@ -7,7 +7,7 @@ use extension::{ extension_builder::{CompileExtensionOptions, ExtensionBuilder}, }; use extension_host::wasm_host::WasmHost; -use fs::RealFs; +use fs::{Fs, RealFs}; use gpui::{TestAppContext, TestDispatcher}; use http_client::{FakeHttpClient, Response}; use node_runtime::NodeRuntime; @@ -24,7 +24,11 @@ fn extension_benchmarks(c: &mut Criterion) { let mut group = c.benchmark_group("load"); let mut manifest = manifest(); - let wasm_bytes = wasm_bytes(&cx, &mut manifest); + let wasm_bytes = wasm_bytes( + &cx, + &mut manifest, + Arc::new(RealFs::new(None, cx.executor())), + ); let manifest = Arc::new(manifest); let extensions_dir = TempTree::new(json!({ "installed": {}, @@ -60,7 +64,7 @@ fn init() -> TestAppContext { cx } -fn wasm_bytes(cx: &TestAppContext, manifest: &mut ExtensionManifest) -> Vec { +fn wasm_bytes(cx: &TestAppContext, manifest: &mut ExtensionManifest, fs: Arc) -> Vec { let extension_builder = extension_builder(); let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) .parent() @@ -73,6 +77,7 @@ fn wasm_bytes(cx: &TestAppContext, manifest: &mut ExtensionManifest) -> Vec &path, manifest, CompileExtensionOptions { release: true }, + fs, )) .unwrap(); std::fs::read(path.join("extension.wasm")).unwrap() diff --git a/crates/extension_host/src/extension_host.rs b/crates/extension_host/src/extension_host.rs index c1c598f1895f6897fd2989752f74fde077af97b3..09e8259771668346c237c1cc05e6074ca3b37797 100644 --- a/crates/extension_host/src/extension_host.rs +++ b/crates/extension_host/src/extension_host.rs @@ -980,12 +980,14 @@ impl ExtensionStore { cx.background_spawn({ let extension_source_path = extension_source_path.clone(); + let fs = fs.clone(); async move { builder .compile_extension( &extension_source_path, &mut extension_manifest, CompileExtensionOptions { release: false }, + fs, ) .await } @@ -1042,12 +1044,13 @@ impl ExtensionStore { cx.notify(); let compile = cx.background_spawn(async move { - let mut manifest = ExtensionManifest::load(fs, &path).await?; + let mut manifest = ExtensionManifest::load(fs.clone(), &path).await?; builder .compile_extension( &path, &mut manifest, CompileExtensionOptions { release: true }, + fs, ) .await });