From b35f947e1769c6e2ca8de4e9b6436c75b8522a5b Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 17:57:43 +0100 Subject: [PATCH 01/33] Update `SharedLibraryProvider` to resolve prebuilt path --- Cargo.lock | 8 +++++ Cargo.toml | 1 + scarb/Cargo.toml | 1 + .../compiler/plugin/proc_macro/compilation.rs | 31 +++++++++++++++++++ 4 files changed, 41 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 144805935..ebcc34365 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3736,6 +3736,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2d16453e800a8cf6dd2fc3eb4bc99b786a9b90c663b8559a5b1a041bf89e472" dependencies = [ "cc", + "libc", "pkg-config", "vcpkg", ] @@ -4994,6 +4995,7 @@ dependencies = [ "smol_str", "snapbox", "tar", + "target-triple", "test-case", "test-for-each-example", "thiserror 2.0.3", @@ -5791,6 +5793,12 @@ dependencies = [ "xattr", ] +[[package]] +name = "target-triple" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42a4d50cdb458045afc8131fd91b64904da29548bcb63c7236e0844936c13078" + [[package]] name = "tempfile" version = "3.12.0" diff --git a/Cargo.toml b/Cargo.toml index 9f3eb970e..271daa1ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -130,6 +130,7 @@ xshell = "0.2" xxhash-rust = { version = "0.8", features = ["xxh3"] } zip = { version = "0.6", default-features = false, features = ["deflate"] } zstd = "0.13" +target-triple = "0.1" [profile.release] lto = true diff --git a/scarb/Cargo.toml b/scarb/Cargo.toml index d1d093ab7..02e149bc7 100644 --- a/scarb/Cargo.toml +++ b/scarb/Cargo.toml @@ -89,6 +89,7 @@ windows-sys.workspace = true zstd.workspace = true cargo_metadata.workspace = true flate2.workspace = true +target-triple.workspace = true [target.'cfg(not(target_os = "linux"))'.dependencies] reqwest = { workspace = true, default-features = true } diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index a51a48b7e..4541a7937 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -15,12 +15,14 @@ use ra_ap_toolchain::Tool; use scarb_ui::{Message, OutputFormat}; use serde::{Serialize, Serializer}; use serde_json::value::RawValue; +use std::env::consts::DLL_SUFFIX; use std::fmt::Display; use std::fs; use std::io::{Seek, SeekFrom}; use std::ops::Deref; use std::process::Command; use tar::Archive; +use target_triple::target; use tracing::trace_span; pub const PROC_MACRO_BUILD_PROFILE: &str = "release"; @@ -31,6 +33,10 @@ pub trait SharedLibraryProvider { fn target_path(&self, config: &Config) -> Filesystem; /// Location of the shared library for the package. fn shared_lib_path(&self, config: &Config) -> Result; + /// Location of the prebuilt binary for the package. + fn prebuilt_lib_path(&self) -> Option; + /// Returns true if the package contains a prebuilt binary. + fn is_prebuilt(&self) -> bool; } impl SharedLibraryProvider for Package { @@ -61,6 +67,31 @@ impl SharedLibraryProvider for Package { .path_unchecked() .join(lib_name)) } + + fn prebuilt_lib_path(&self) -> Option { + let target_triple = target!(); + + let prebuilt_name = format!( + "{name}_v{version}_{target}{suffix}", + name = self.id.name, + version = self.id.version, + target = target_triple, + suffix = DLL_SUFFIX + ); + + let prebuilt_path = self + .root() + .join("target") + .join("scarb") + .join("cairo-plugin") + .join(prebuilt_name); + + prebuilt_path.exists().then_some(prebuilt_path) + } + + fn is_prebuilt(&self) -> bool { + self.prebuilt_lib_path().is_some() + } } pub fn compile_unit(unit: ProcMacroCompilationUnit, ws: &Workspace<'_>) -> Result<()> { From 908cc47c2a4ed32d387bfb70c2589effefd33daa Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:07:15 +0100 Subject: [PATCH 02/33] Add helper to determine whether the package is prebuilt --- scarb/src/compiler/compilation_unit.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/scarb/src/compiler/compilation_unit.rs b/scarb/src/compiler/compilation_unit.rs index 14af04350..1eebc381c 100644 --- a/scarb/src/compiler/compilation_unit.rs +++ b/scarb/src/compiler/compilation_unit.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; use smol_str::SmolStr; use typed_builder::TypedBuilder; +use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::compiler::Profile; use crate::core::{ ManifestCompilerConfig, Package, PackageId, PackageName, Target, TargetKind, Workspace, @@ -130,6 +131,8 @@ pub trait CompilationUnitAttributes { fn components(&self) -> &[CompilationUnitComponent]; fn digest(&self) -> String; + fn is_prebuilt(&self) -> bool; + fn main_component(&self) -> &CompilationUnitComponent { // NOTE: This uses the order invariant of `component` field. let component = &self.components()[0]; @@ -195,6 +198,12 @@ impl CompilationUnitAttributes for CompilationUnit { Self::ProcMacro(unit) => unit.digest(), } } + fn is_prebuilt(&self) -> bool { + match self { + Self::Cairo(unit) => unit.is_prebuilt(), + Self::ProcMacro(unit) => unit.is_prebuilt(), + } + } } impl CompilationUnitAttributes for CairoCompilationUnit { @@ -215,6 +224,10 @@ impl CompilationUnitAttributes for CairoCompilationUnit { self.compiler_config.hash(&mut hasher); hasher.finish_as_short_hash() } + + fn is_prebuilt(&self) -> bool { + false + } } impl CompilationUnitAttributes for ProcMacroCompilationUnit { @@ -233,6 +246,13 @@ impl CompilationUnitAttributes for ProcMacroCompilationUnit { } hasher.finish_as_short_hash() } + + fn is_prebuilt(&self) -> bool { + self.components + .first() + .map(|c| c.package.is_prebuilt()) + .unwrap_or(false) + } } impl CairoCompilationUnit { From 7d399d3b19f365246e5249eb1d55d83ac37228ee Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:07:38 +0100 Subject: [PATCH 03/33] Update `ProcMacroInstance` and `ProcMacroInstance` to allow loading prebuilts --- scarb/src/compiler/plugin/proc_macro/ffi.rs | 14 ++++++++++++++ scarb/src/compiler/plugin/proc_macro/host.rs | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/scarb/src/compiler/plugin/proc_macro/ffi.rs b/scarb/src/compiler/plugin/proc_macro/ffi.rs index 4d42bab6c..fc95fe7db 100644 --- a/scarb/src/compiler/plugin/proc_macro/ffi.rs +++ b/scarb/src/compiler/plugin/proc_macro/ffi.rs @@ -73,6 +73,20 @@ impl ProcMacroInstance { }) } + /// Loads a procedural macro from a prebuilt shared library. + pub fn try_load_prebuilt(package: Package, _config: &Config) -> Result { + let prebuilt_path = package + .prebuilt_lib_path() + .context("could not resolve prebuilt library path")?; + let plugin = unsafe { Plugin::try_new(prebuilt_path)? }; + let expansions = unsafe { Self::load_expansions(&plugin, package.id)? }; + Ok(Self { + package_id: package.id, + plugin, + expansions, + }) + } + unsafe fn load_expansions(plugin: &Plugin, package_id: PackageId) -> Result> { // Make a call to the FFI interface to list declared expansions. let stable_expansions = (plugin.vtable.list_expansions)(); diff --git a/scarb/src/compiler/plugin/proc_macro/host.rs b/scarb/src/compiler/plugin/proc_macro/host.rs index 0886abb81..58a40c88f 100644 --- a/scarb/src/compiler/plugin/proc_macro/host.rs +++ b/scarb/src/compiler/plugin/proc_macro/host.rs @@ -1156,6 +1156,13 @@ impl ProcMacroHost { Ok(()) } + /// Registers a prebuilt procedural macro by loading the shared library from the specified path. + pub fn register_prebuilt(&mut self, package: Package, config: &Config) -> Result<()> { + let instance = ProcMacroInstance::try_load_prebuilt(package, config)?; + self.macros.push(Arc::new(instance)); + Ok(()) + } + pub fn into_plugin(self) -> Result { ProcMacroHostPlugin::try_new(self.macros) } From a95a25d9b88ddf40dc850a8750e11476683b6aea Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:29:13 +0100 Subject: [PATCH 04/33] Update `load_plugins` to load prebuilts --- scarb/src/compiler/db.rs | 13 +++++++++++++ scarb/src/ops/resolve.rs | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index 429dd5813..97cbb9b8f 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -14,9 +14,11 @@ use std::path::PathBuf; use std::sync::Arc; use tracing::trace; +use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin}; use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent}; use crate::core::Workspace; +use crate::ops::{compile_unit, generate_cairo_plugin_compilation_units}; use crate::DEFAULT_MODULE_MAIN_FILE; pub struct ScarbDatabase { @@ -59,6 +61,17 @@ fn load_plugins( let plugin = ws.config().cairo_plugins().fetch(package_id)?; let instance = plugin.instantiate()?; builder.with_plugin_suite(instance.plugin_suite()); + } else if plugin_info.package.is_prebuilt() + && proc_macros + .register_prebuilt(plugin_info.package.clone(), ws.config()) + .is_err() + { + let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? + .first() + .unwrap() + .clone(); + compile_unit(plugin_unit, ws, false)?; + proc_macros.register(plugin_info.package.clone(), ws.config())?; } else { proc_macros.register(plugin_info.package.clone(), ws.config())?; } diff --git a/scarb/src/ops/resolve.rs b/scarb/src/ops/resolve.rs index da0b99483..43cebac95 100644 --- a/scarb/src/ops/resolve.rs +++ b/scarb/src/ops/resolve.rs @@ -684,7 +684,7 @@ fn check_cairo_version_compatibility( Ok(()) } -fn generate_cairo_plugin_compilation_units(member: &Package) -> Result> { +pub fn generate_cairo_plugin_compilation_units(member: &Package) -> Result> { Ok(vec![CompilationUnit::ProcMacro(ProcMacroCompilationUnit { main_package_id: member.id, compiler_config: serde_json::Value::Null, From 2aa6ee85d03bcc6fa4090fc94de27539848ef422 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:49:13 +0100 Subject: [PATCH 05/33] Add `skip_prebuilt_proc_macros` flag and relevant logic --- scarb/src/ops/compile.rs | 59 +++++++++++++++++++++++++++++----------- scarb/src/ops/expand.rs | 2 +- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/scarb/src/ops/compile.rs b/scarb/src/ops/compile.rs index 3e63617e8..2e732732a 100644 --- a/scarb/src/ops/compile.rs +++ b/scarb/src/ops/compile.rs @@ -98,12 +98,12 @@ impl CompileOpts { #[tracing::instrument(skip_all, level = "debug")] pub fn compile(packages: Vec, opts: CompileOpts, ws: &Workspace<'_>) -> Result<()> { - process(packages, opts, ws, compile_units, None) + process(packages, opts, ws, true, compile_units, None) } #[tracing::instrument(skip_all, level = "debug")] pub fn check(packages: Vec, opts: CompileOpts, ws: &Workspace<'_>) -> Result<()> { - process(packages, opts, ws, check_units, Some("checking")) + process(packages, opts, ws, true, check_units, Some("checking")) } #[tracing::instrument(skip_all, level = "debug")] @@ -111,11 +111,12 @@ fn process( packages: Vec, opts: CompileOpts, ws: &Workspace<'_>, + skip_prebuilt_proc_macros: bool, mut operation: F, operation_type: Option<&str>, ) -> Result<()> where - F: FnMut(Vec, &Workspace<'_>) -> Result<()>, + F: FnMut(Vec, &Workspace<'_>, bool) -> Result<()>, { let resolve = ops::resolve_workspace(ws)?; let packages_to_process = ws @@ -156,7 +157,7 @@ where }) .collect::>(); - operation(compilation_units, ws)?; + operation(compilation_units, ws, skip_prebuilt_proc_macros)?; let elapsed_time = HumanDuration(ws.config().elapsed_time()); let profile = ws.current_profile()?; @@ -173,34 +174,56 @@ where /// Run compiler in a new thread. /// The stack size of created threads can be altered with `RUST_MIN_STACK` env variable. -pub fn compile_units(units: Vec, ws: &Workspace<'_>) -> Result<()> { +pub fn compile_units( + units: Vec, + ws: &Workspace<'_>, + skip_prebuilt_proc_macros: bool, +) -> Result<()> { for unit in units { - compile_unit(unit, ws)?; + compile_unit(unit, ws, skip_prebuilt_proc_macros)?; } Ok(()) } -pub fn compile_unit(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> { +pub fn compile_unit( + unit: CompilationUnit, + ws: &Workspace<'_>, + skip_prebuilt_proc_macros: bool, +) -> Result<()> { thread::scope(|s| { thread::Builder::new() .name(format!("scarb compile {}", unit.id())) - .spawn_scoped(s, || compile_unit_inner(unit, ws)) + .spawn_scoped(s, || { + compile_unit_inner(unit, ws, skip_prebuilt_proc_macros) + }) .expect("Failed to spawn compiler thread.") .join() .expect("Compiler thread has panicked.") }) } -fn compile_unit_inner(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> { +fn compile_unit_inner( + unit: CompilationUnit, + ws: &Workspace<'_>, + skip_prebuilt_proc_macros: bool, +) -> Result<()> { let package_name = unit.main_package_id().name.clone(); - ws.config() - .ui() - .print(Status::new("Compiling", &unit.name())); - let result = match unit { - CompilationUnit::ProcMacro(unit) => proc_macro::compile_unit(unit, ws), + CompilationUnit::ProcMacro(unit) => { + if skip_prebuilt_proc_macros && unit.is_prebuilt() { + Ok(()) + } else { + ws.config() + .ui() + .print(Status::new("Compiling", &unit.name())); + proc_macro::compile_unit(unit, ws) + } + } CompilationUnit::Cairo(unit) => { + ws.config() + .ui() + .print(Status::new("Compiling", &unit.name())); let ScarbDatabase { mut db, proc_macro_host, @@ -223,7 +246,11 @@ fn compile_unit_inner(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> { }) } -fn check_units(units: Vec, ws: &Workspace<'_>) -> Result<()> { +fn check_units( + units: Vec, + ws: &Workspace<'_>, + skip_prebuilt_proc_macros: bool, +) -> Result<()> { // Select proc macro units that need to be compiled for Cairo compilation units. let required_plugins = units .iter() @@ -251,7 +278,7 @@ fn check_units(units: Vec, ws: &Workspace<'_>) -> Result<()> { if matches!(unit, CompilationUnit::ProcMacro(_)) && required_plugins.contains(&unit.main_package_id()) { - compile_unit(unit, ws)?; + compile_unit(unit, ws, skip_prebuilt_proc_macros)?; } else { check_unit(unit, ws)?; } diff --git a/scarb/src/ops/expand.rs b/scarb/src/ops/expand.rs index 417fa4b1b..f6720bf4e 100644 --- a/scarb/src/ops/expand.rs +++ b/scarb/src/ops/expand.rs @@ -50,7 +50,7 @@ pub fn expand(package: Package, opts: ExpandOpts, ws: &Workspace<'_>) -> Result< compilation_units .iter() .filter(|unit| matches!(unit, CompilationUnit::ProcMacro(_))) - .map(|unit| ops::compile::compile_unit(unit.clone(), ws)) + .map(|unit| ops::compile::compile_unit(unit.clone(), ws, true)) .collect::>>()?; let compilation_units = compilation_units From bfeb6843286329099c0ea7d74efee02b21f89165 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:49:46 +0100 Subject: [PATCH 06/33] Update `load_plugins` to print compilation message to ui --- scarb/src/compiler/db.rs | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index 97cbb9b8f..9b256f31a 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -1,3 +1,9 @@ +use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; +use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin}; +use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent}; +use crate::core::Workspace; +use crate::ops::{compile_unit, generate_cairo_plugin_compilation_units}; +use crate::DEFAULT_MODULE_MAIN_FILE; use anyhow::{anyhow, Result}; use cairo_lang_compiler::db::{RootDatabase, RootDatabaseBuilder}; use cairo_lang_compiler::project::{AllCratesConfig, ProjectConfig, ProjectConfigContent}; @@ -9,18 +15,12 @@ use cairo_lang_filesystem::db::{ }; use cairo_lang_filesystem::ids::CrateLongId; use cairo_lang_utils::ordered_hash_map::OrderedHashMap; +use scarb_ui::components::Status; use smol_str::SmolStr; use std::path::PathBuf; use std::sync::Arc; use tracing::trace; -use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; -use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin}; -use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent}; -use crate::core::Workspace; -use crate::ops::{compile_unit, generate_cairo_plugin_compilation_units}; -use crate::DEFAULT_MODULE_MAIN_FILE; - pub struct ScarbDatabase { pub db: RootDatabase, pub proc_macro_host: Arc, @@ -61,17 +61,21 @@ fn load_plugins( let plugin = ws.config().cairo_plugins().fetch(package_id)?; let instance = plugin.instantiate()?; builder.with_plugin_suite(instance.plugin_suite()); - } else if plugin_info.package.is_prebuilt() - && proc_macros + } else if plugin_info.package.is_prebuilt() { + if proc_macros .register_prebuilt(plugin_info.package.clone(), ws.config()) .is_err() - { - let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? - .first() - .unwrap() - .clone(); - compile_unit(plugin_unit, ws, false)?; - proc_macros.register(plugin_info.package.clone(), ws.config())?; + { + let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? + .first() + .unwrap() + .clone(); + ws.config() + .ui() + .print(Status::new("Compiling", &plugin_unit.name())); + compile_unit(plugin_unit, ws, false)?; + proc_macros.register(plugin_info.package.clone(), ws.config())?; + } } else { proc_macros.register(plugin_info.package.clone(), ws.config())?; } From e570a9935a153b52fc89f035ec283d4b0c986b22 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:52:46 +0100 Subject: [PATCH 07/33] Update `proc_macro_server` --- .../bin/scarb/commands/proc_macro_server.rs | 60 ++++++++++++++----- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/scarb/src/bin/scarb/commands/proc_macro_server.rs b/scarb/src/bin/scarb/commands/proc_macro_server.rs index 212a9faa1..ddfb7ffc3 100644 --- a/scarb/src/bin/scarb/commands/proc_macro_server.rs +++ b/scarb/src/bin/scarb/commands/proc_macro_server.rs @@ -1,9 +1,13 @@ use anyhow::Result; use scarb::{ - compiler::{plugin::proc_macro::ProcMacroHost, CairoCompilationUnit, CompilationUnit}, - core::{Config, Workspace}, + compiler::{ + plugin::proc_macro::ProcMacroHost, CairoCompilationUnit, CompilationUnit, + CompilationUnitAttributes, + }, + core::{Config, PackageId, Workspace}, ops::{self, FeaturesOpts, FeaturesSelector}, }; +use std::collections::HashSet; #[tracing::instrument(skip_all, level = "info")] pub fn run(config: &mut Config) -> Result<()> { @@ -19,36 +23,64 @@ pub fn run(config: &mut Config) -> Result<()> { &ws, )?; - // Compile procedural macros only. + let mut proc_macros = ProcMacroHost::default(); + let mut loaded_plugins = HashSet::new(); + + // Try loading prebuilt plugins for unit in &compilation_units { - if let CompilationUnit::ProcMacro(_) = unit { - ops::compile_unit(unit.clone(), &ws)?; + if let CompilationUnit::Cairo(unit) = unit { + loaded_plugins.extend(load_prebuilt_plugins(unit.clone(), &ws, &mut proc_macros)?); } } - let mut proc_macros = ProcMacroHost::default(); + // Then compile remaining procedural macros. + for unit in &compilation_units { + if let CompilationUnit::ProcMacro(_) = unit { + if !loaded_plugins.contains(&unit.main_package_id()) { + ops::compile_unit(unit.clone(), &ws, false)?; + } + } + } - // Load previously compiled procedural macros. + // Load all compiled procedural macros. for unit in compilation_units { if let CompilationUnit::Cairo(unit) = unit { - load_plugins(unit, &ws, &mut proc_macros)?; + load_plugins(unit, &ws, &mut proc_macros, &loaded_plugins)?; } } ops::start_proc_macro_server(proc_macros) } +fn load_prebuilt_plugins( + unit: CairoCompilationUnit, + ws: &Workspace<'_>, + proc_macros: &mut ProcMacroHost, +) -> Result> { + let mut loaded = HashSet::new(); + + for plugin_info in unit.cairo_plugins.into_iter().filter(|p| !p.builtin) { + if proc_macros + .register_prebuilt(plugin_info.package.clone(), ws.config()) + .is_ok() + { + loaded.insert(plugin_info.package.id); + } + } + + Ok(loaded) +} + fn load_plugins( unit: CairoCompilationUnit, ws: &Workspace<'_>, proc_macros: &mut ProcMacroHost, + loaded_plugins: &HashSet, ) -> Result<()> { - for plugin_info in unit - .cairo_plugins - .into_iter() - .filter(|plugin_info| !plugin_info.builtin) - { - proc_macros.register(plugin_info.package, ws.config())?; + for plugin_info in unit.cairo_plugins.into_iter().filter(|p| !p.builtin) { + if !loaded_plugins.contains(&plugin_info.package.id) { + proc_macros.register(plugin_info.package, ws.config())?; + } } Ok(()) From c73c0f6a275ceabe41dbf0006ee7357cfd6f6e44 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:56:34 +0100 Subject: [PATCH 08/33] Add simple test --- scarb/tests/build_cairo_plugin.rs | 36 +++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index c93fb3b38..872aab383 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -5,13 +5,13 @@ use indoc::indoc; use scarb_test_support::cairo_plugin_project_builder::CairoPluginProjectBuilder; use scarb_test_support::command::Scarb; use scarb_test_support::fsx::ChildPathEx; -use scarb_test_support::project_builder::ProjectBuilder; +use scarb_test_support::project_builder::{Dep, DepBuilder, ProjectBuilder}; use scarb_test_support::workspace_builder::WorkspaceBuilder; use snapbox::assert_matches; -#[test] fn compile_cairo_plugin() { let t = TempDir::new().unwrap(); +#[test] CairoPluginProjectBuilder::default().build(&t); let output = Scarb::quick_snapbox() .arg("build") @@ -207,6 +207,36 @@ fn compile_cairo_plugin_with_other_target() { "#}); } +#[test] +fn compile_with_prebuilt_plugins() { + let t = TempDir::new().unwrap(); + + ProjectBuilder::start() + .name("hello") + .lib_cairo(indoc! {r#" + fn main() -> u32 { + 1 + } + "#}) + .dep( + "snforge_scarb_plugin", + Dep.version("0.34").registry("https://scarbs.dev/"), + ) + .manifest_extra(indoc! {r#" + "#}) + .build(&t); + Scarb::quick_snapbox() + .arg("build") + .current_dir(&t) + .assert() + .success() + .stdout_matches(indoc! {r#" + [..]Downloading snforge_scarb_plugin v0.34.0 ([..]) + [..]Compiling hello v1.0.0 ([..]Scarb.toml) + [..] Finished `dev` profile target(s) in [..] + "#}); +} + #[test] fn can_emit_plugin_warning() { let temp = TempDir::new().unwrap(); @@ -1530,3 +1560,5 @@ fn can_expand_impl_inner_func_attrr() { "#}); } + + From 87f87c52e365581873c0f8290d4a704151c99076 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Thu, 5 Dec 2024 18:58:02 +0100 Subject: [PATCH 09/33] misc --- scarb/tests/build_cairo_plugin.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index 872aab383..a80055d5e 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -1560,5 +1560,3 @@ fn can_expand_impl_inner_func_attrr() { "#}); } - - From a4a8d14242d7a858a15febfaad3e4258f4c13dfb Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau <43750648+DelevoXDG@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:36:00 +0100 Subject: [PATCH 10/33] fix test Signed-off-by: Maksim Zdobnikau <43750648+DelevoXDG@users.noreply.github.com> --- scarb/tests/build_cairo_plugin.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index a80055d5e..7a7fdeaba 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -11,7 +11,6 @@ use snapbox::assert_matches; fn compile_cairo_plugin() { let t = TempDir::new().unwrap(); -#[test] CairoPluginProjectBuilder::default().build(&t); let output = Scarb::quick_snapbox() .arg("build") From c81d9cee3cc8c4cdf9c57f9fccdc528e7236b51a Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Fri, 6 Dec 2024 13:19:29 +0100 Subject: [PATCH 11/33] Merge `try_new` and `try_load_prebuilt` --- scarb/src/compiler/plugin/proc_macro/ffi.rs | 28 ++++---------------- scarb/src/compiler/plugin/proc_macro/host.rs | 14 +++++++--- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/ffi.rs b/scarb/src/compiler/plugin/proc_macro/ffi.rs index fc95fe7db..c9804ffbe 100644 --- a/scarb/src/compiler/plugin/proc_macro/ffi.rs +++ b/scarb/src/compiler/plugin/proc_macro/ffi.rs @@ -1,4 +1,4 @@ -use crate::core::{Config, Package, PackageId}; +use crate::core::PackageId; use anyhow::{ensure, Context, Result}; use cairo_lang_defs::patcher::PatchBuilder; use cairo_lang_macro::{ @@ -18,7 +18,6 @@ use std::ffi::{c_char, CStr, CString}; use std::fmt::Debug; use std::slice; -use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::compiler::plugin::proc_macro::ProcMacroAuxData; #[cfg(not(windows))] @@ -61,32 +60,15 @@ impl Debug for ProcMacroInstance { impl ProcMacroInstance { /// Load shared library - pub fn try_new(package: Package, config: &Config) -> Result { - let lib_path = package - .shared_lib_path(config) - .context("could not resolve shared library path")?; - let plugin = unsafe { Plugin::try_new(lib_path.to_path_buf())? }; + pub fn try_new(package_id: PackageId, lib_path: Utf8PathBuf) -> Result { + let plugin = unsafe { Plugin::try_new(lib_path)? }; Ok(Self { - expansions: unsafe { Self::load_expansions(&plugin, package.id)? }, - package_id: package.id, + expansions: unsafe { Self::load_expansions(&plugin, package_id)? }, + package_id, plugin, }) } - /// Loads a procedural macro from a prebuilt shared library. - pub fn try_load_prebuilt(package: Package, _config: &Config) -> Result { - let prebuilt_path = package - .prebuilt_lib_path() - .context("could not resolve prebuilt library path")?; - let plugin = unsafe { Plugin::try_new(prebuilt_path)? }; - let expansions = unsafe { Self::load_expansions(&plugin, package.id)? }; - Ok(Self { - package_id: package.id, - plugin, - expansions, - }) - } - unsafe fn load_expansions(plugin: &Plugin, package_id: PackageId) -> Result> { // Make a call to the FFI interface to list declared expansions. let stable_expansions = (plugin.vtable.list_expansions)(); diff --git a/scarb/src/compiler/plugin/proc_macro/host.rs b/scarb/src/compiler/plugin/proc_macro/host.rs index 58a40c88f..5710fd296 100644 --- a/scarb/src/compiler/plugin/proc_macro/host.rs +++ b/scarb/src/compiler/plugin/proc_macro/host.rs @@ -1,8 +1,9 @@ +use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::compiler::plugin::proc_macro::{ Expansion, ExpansionKind, FromSyntaxNode, ProcMacroInstance, }; use crate::core::{Config, Package, PackageId}; -use anyhow::{ensure, Result}; +use anyhow::{ensure, Context, Result}; use cairo_lang_defs::ids::{ModuleItemId, TopLevelLanguageElementId}; use cairo_lang_defs::patcher::{PatchBuilder, RewriteNode}; use cairo_lang_defs::plugin::{ @@ -1151,14 +1152,19 @@ pub struct ProcMacroHost { impl ProcMacroHost { pub fn register(&mut self, package: Package, config: &Config) -> Result<()> { - let instance = ProcMacroInstance::try_new(package, config)?; + let lib_path = package + .shared_lib_path(config) + .context("could not resolve shared library path")?; + let instance = ProcMacroInstance::try_new(package.id, lib_path)?; self.macros.push(Arc::new(instance)); Ok(()) } - /// Registers a prebuilt procedural macro by loading the shared library from the specified path. pub fn register_prebuilt(&mut self, package: Package, config: &Config) -> Result<()> { - let instance = ProcMacroInstance::try_load_prebuilt(package, config)?; + let prebuilt_path = package + .prebuilt_lib_path() + .context("could not resolve prebuilt library path")?; + let instance = ProcMacroInstance::try_new(package.id, prebuilt_path)?; self.macros.push(Arc::new(instance)); Ok(()) } From 9cac62255757256981c9054ed37ccae5984e407e Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Fri, 6 Dec 2024 13:19:39 +0100 Subject: [PATCH 12/33] Fix test --- scarb/tests/build_cairo_plugin.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index 7a7fdeaba..e125c073f 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -9,6 +9,7 @@ use scarb_test_support::project_builder::{Dep, DepBuilder, ProjectBuilder}; use scarb_test_support::workspace_builder::WorkspaceBuilder; use snapbox::assert_matches; +#[test] fn compile_cairo_plugin() { let t = TempDir::new().unwrap(); CairoPluginProjectBuilder::default().build(&t); From eba6fe466000bdb5de464100ee327f21e98b0fb5 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Fri, 6 Dec 2024 18:42:37 +0100 Subject: [PATCH 13/33] Partially fix name resolution --- .../src/compiler/plugin/proc_macro/compilation.rs | 14 ++++++++------ scarb/src/compiler/plugin/proc_macro/host.rs | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index 4541a7937..c14f2d981 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -34,7 +34,7 @@ pub trait SharedLibraryProvider { /// Location of the shared library for the package. fn shared_lib_path(&self, config: &Config) -> Result; /// Location of the prebuilt binary for the package. - fn prebuilt_lib_path(&self) -> Option; + fn prebuilt_lib_path(&self) -> Result>; /// Returns true if the package contains a prebuilt binary. fn is_prebuilt(&self) -> bool; } @@ -68,13 +68,15 @@ impl SharedLibraryProvider for Package { .join(lib_name)) } - fn prebuilt_lib_path(&self) -> Option { + fn prebuilt_lib_path(&self) -> Result> { + let name = get_cargo_package_name(self)?; + let version = get_cargo_package_version(self)?; let target_triple = target!(); let prebuilt_name = format!( "{name}_v{version}_{target}{suffix}", - name = self.id.name, - version = self.id.version, + name = name, + version = version, target = target_triple, suffix = DLL_SUFFIX ); @@ -86,11 +88,11 @@ impl SharedLibraryProvider for Package { .join("cairo-plugin") .join(prebuilt_name); - prebuilt_path.exists().then_some(prebuilt_path) + Ok(prebuilt_path.exists().then_some(prebuilt_path)) } fn is_prebuilt(&self) -> bool { - self.prebuilt_lib_path().is_some() + self.prebuilt_lib_path().ok().is_some() } } diff --git a/scarb/src/compiler/plugin/proc_macro/host.rs b/scarb/src/compiler/plugin/proc_macro/host.rs index 5710fd296..b95ba7430 100644 --- a/scarb/src/compiler/plugin/proc_macro/host.rs +++ b/scarb/src/compiler/plugin/proc_macro/host.rs @@ -1162,7 +1162,7 @@ impl ProcMacroHost { pub fn register_prebuilt(&mut self, package: Package, config: &Config) -> Result<()> { let prebuilt_path = package - .prebuilt_lib_path() + .prebuilt_lib_path()? .context("could not resolve prebuilt library path")?; let instance = ProcMacroInstance::try_new(package.id, prebuilt_path)?; self.macros.push(Arc::new(instance)); From 57cc3d1781201e75e39d55ea92767c58adfd3b5d Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Fri, 6 Dec 2024 18:42:57 +0100 Subject: [PATCH 14/33] Add system-universal test --- scarb/tests/build_cairo_plugin.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index e125c073f..f71d36247 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -215,12 +215,13 @@ fn compile_with_prebuilt_plugins() { .name("hello") .lib_cairo(indoc! {r#" fn main() -> u32 { - 1 + let x = some!(42); + x } "#}) .dep( - "snforge_scarb_plugin", - Dep.version("0.34").registry("https://scarbs.dev/"), + "proc_macro_example", + Dep.version("0.1.1").registry("https://scarbs.dev/"), ) .manifest_extra(indoc! {r#" "#}) @@ -231,7 +232,7 @@ fn compile_with_prebuilt_plugins() { .assert() .success() .stdout_matches(indoc! {r#" - [..]Downloading snforge_scarb_plugin v0.34.0 ([..]) + [..]Downloading proc_macro_example v0.1.1 ([..]) [..]Compiling hello v1.0.0 ([..]Scarb.toml) [..] Finished `dev` profile target(s) in [..] "#}); From 6b342b1f2d3adbdedf9b45ee518f031235d62830 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 15:54:11 +0100 Subject: [PATCH 15/33] Update `load_plugins` to try registering before compiling --- scarb/src/compiler/db.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index 9b256f31a..9c2a39894 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -66,15 +66,17 @@ fn load_plugins( .register_prebuilt(plugin_info.package.clone(), ws.config()) .is_err() { - let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? - .first() - .unwrap() - .clone(); - ws.config() - .ui() - .print(Status::new("Compiling", &plugin_unit.name())); - compile_unit(plugin_unit, ws, false)?; - proc_macros.register(plugin_info.package.clone(), ws.config())?; + if proc_macros.register(plugin_info.package.clone(), ws.config()).is_err() { + let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? + .first() + .unwrap() + .clone(); + ws.config() + .ui() + .print(Status::new("Compiling", &plugin_unit.name())); + compile_unit(plugin_unit, ws, false)?; + proc_macros.register(plugin_info.package.clone(), ws.config())?; + } } } else { proc_macros.register(plugin_info.package.clone(), ws.config())?; From e76fcf2941c1b17c68ba7d0b71eb895319f78066 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 15:59:59 +0100 Subject: [PATCH 16/33] Add `load_plugins` comments --- scarb/src/compiler/db.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index 9c2a39894..c99a0e4c3 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -57,16 +57,20 @@ fn load_plugins( let mut proc_macros = ProcMacroHost::default(); for plugin_info in &unit.cairo_plugins { if plugin_info.builtin { + // For builtin plugins, fetch and instantiate directly let package_id = plugin_info.package.id; let plugin = ws.config().cairo_plugins().fetch(package_id)?; let instance = plugin.instantiate()?; builder.with_plugin_suite(instance.plugin_suite()); } else if plugin_info.package.is_prebuilt() { + // For prebuilt procedural macros, try loading from prebuilt binary first if proc_macros .register_prebuilt(plugin_info.package.clone(), ws.config()) .is_err() { + // If failed to load from prebuilt binary, try loading from shared library if proc_macros.register(plugin_info.package.clone(), ws.config()).is_err() { + // If failed to load from shared library, compile the plugin and try again let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? .first() .unwrap() @@ -79,6 +83,7 @@ fn load_plugins( } } } else { + // For non-prebuilt procedural macros, load from shared library proc_macros.register(plugin_info.package.clone(), ws.config())?; } } From 8424cb61a2ca863b08bc873184de841fa66ea526 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 17:11:14 +0100 Subject: [PATCH 17/33] Fix false positive `is_prebuilt` --- scarb/src/compiler/plugin/proc_macro/compilation.rs | 11 +++++++---- scarb/src/compiler/plugin/proc_macro/host.rs | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index c14f2d981..a88f1e505 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -34,7 +34,7 @@ pub trait SharedLibraryProvider { /// Location of the shared library for the package. fn shared_lib_path(&self, config: &Config) -> Result; /// Location of the prebuilt binary for the package. - fn prebuilt_lib_path(&self) -> Result>; + fn prebuilt_lib_path(&self) -> Result; /// Returns true if the package contains a prebuilt binary. fn is_prebuilt(&self) -> bool; } @@ -68,7 +68,7 @@ impl SharedLibraryProvider for Package { .join(lib_name)) } - fn prebuilt_lib_path(&self) -> Result> { + fn prebuilt_lib_path(&self) -> Result { let name = get_cargo_package_name(self)?; let version = get_cargo_package_version(self)?; let target_triple = target!(); @@ -88,11 +88,14 @@ impl SharedLibraryProvider for Package { .join("cairo-plugin") .join(prebuilt_name); - Ok(prebuilt_path.exists().then_some(prebuilt_path)) + prebuilt_path + .exists() + .then_some(prebuilt_path) + .ok_or_else(|| anyhow!("prebuilt library not found")) } fn is_prebuilt(&self) -> bool { - self.prebuilt_lib_path().ok().is_some() + self.prebuilt_lib_path().is_ok() } } diff --git a/scarb/src/compiler/plugin/proc_macro/host.rs b/scarb/src/compiler/plugin/proc_macro/host.rs index b95ba7430..5710fd296 100644 --- a/scarb/src/compiler/plugin/proc_macro/host.rs +++ b/scarb/src/compiler/plugin/proc_macro/host.rs @@ -1162,7 +1162,7 @@ impl ProcMacroHost { pub fn register_prebuilt(&mut self, package: Package, config: &Config) -> Result<()> { let prebuilt_path = package - .prebuilt_lib_path()? + .prebuilt_lib_path() .context("could not resolve prebuilt library path")?; let instance = ProcMacroInstance::try_new(package.id, prebuilt_path)?; self.macros.push(Arc::new(instance)); From be7b5c20455a01816331979ac77a15626e8b1654 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 17:11:51 +0100 Subject: [PATCH 18/33] Fix false positive `is_prebuilt` --- scarb/src/compiler/db.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index c99a0e4c3..b8f4515a4 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -69,12 +69,16 @@ fn load_plugins( .is_err() { // If failed to load from prebuilt binary, try loading from shared library - if proc_macros.register(plugin_info.package.clone(), ws.config()).is_err() { + if proc_macros + .register(plugin_info.package.clone(), ws.config()) + .is_err() + { // If failed to load from shared library, compile the plugin and try again - let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? - .first() - .unwrap() - .clone(); + let plugin_unit = + generate_cairo_plugin_compilation_units(&plugin_info.package)? + .first() + .unwrap() + .clone(); ws.config() .ui() .print(Status::new("Compiling", &plugin_unit.name())); From 1e2845af913ea2831bdddf59fa1ee7c900dcc61d Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 17:12:05 +0100 Subject: [PATCH 19/33] misc --- scarb/src/ops/compile.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scarb/src/ops/compile.rs b/scarb/src/ops/compile.rs index 2e732732a..ca6f78bee 100644 --- a/scarb/src/ops/compile.rs +++ b/scarb/src/ops/compile.rs @@ -211,7 +211,7 @@ fn compile_unit_inner( let result = match unit { CompilationUnit::ProcMacro(unit) => { - if skip_prebuilt_proc_macros && unit.is_prebuilt() { + if unit.is_prebuilt() && skip_prebuilt_proc_macros { Ok(()) } else { ws.config() From 8a468d34786b0de92119725252158b695c15f339 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 19:59:09 +0100 Subject: [PATCH 20/33] misc --- scarb/src/bin/scarb/commands/proc_macro_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scarb/src/bin/scarb/commands/proc_macro_server.rs b/scarb/src/bin/scarb/commands/proc_macro_server.rs index ddfb7ffc3..745d38caf 100644 --- a/scarb/src/bin/scarb/commands/proc_macro_server.rs +++ b/scarb/src/bin/scarb/commands/proc_macro_server.rs @@ -42,7 +42,7 @@ pub fn run(config: &mut Config) -> Result<()> { } } - // Load all compiled procedural macros. + // Load previously compiled procedural macros. for unit in compilation_units { if let CompilationUnit::Cairo(unit) = unit { load_plugins(unit, &ws, &mut proc_macros, &loaded_plugins)?; From 394bf557803b94793083a3a1f7417d7f674ec816 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 22:34:20 +0100 Subject: [PATCH 21/33] Use package name and version from Scarb manifest instead of Cargo manifest --- scarb/src/compiler/plugin/proc_macro/compilation.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index a88f1e505..305ac18fe 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -69,14 +69,12 @@ impl SharedLibraryProvider for Package { } fn prebuilt_lib_path(&self) -> Result { - let name = get_cargo_package_name(self)?; - let version = get_cargo_package_version(self)?; let target_triple = target!(); let prebuilt_name = format!( "{name}_v{version}_{target}{suffix}", - name = name, - version = version, + name = self.id.name, + version = self.id.version, target = target_triple, suffix = DLL_SUFFIX ); From 7b0162c69ed33cf92b72f0d19289a75671fb6952 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 22:47:22 +0100 Subject: [PATCH 22/33] misc: unused arg --- scarb/src/bin/scarb/commands/proc_macro_server.rs | 5 ++--- scarb/src/compiler/db.rs | 2 +- scarb/src/compiler/plugin/proc_macro/host.rs | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/scarb/src/bin/scarb/commands/proc_macro_server.rs b/scarb/src/bin/scarb/commands/proc_macro_server.rs index 745d38caf..df7e99b70 100644 --- a/scarb/src/bin/scarb/commands/proc_macro_server.rs +++ b/scarb/src/bin/scarb/commands/proc_macro_server.rs @@ -29,7 +29,7 @@ pub fn run(config: &mut Config) -> Result<()> { // Try loading prebuilt plugins for unit in &compilation_units { if let CompilationUnit::Cairo(unit) = unit { - loaded_plugins.extend(load_prebuilt_plugins(unit.clone(), &ws, &mut proc_macros)?); + loaded_plugins.extend(load_prebuilt_plugins(unit.clone(), &mut proc_macros)?); } } @@ -54,14 +54,13 @@ pub fn run(config: &mut Config) -> Result<()> { fn load_prebuilt_plugins( unit: CairoCompilationUnit, - ws: &Workspace<'_>, proc_macros: &mut ProcMacroHost, ) -> Result> { let mut loaded = HashSet::new(); for plugin_info in unit.cairo_plugins.into_iter().filter(|p| !p.builtin) { if proc_macros - .register_prebuilt(plugin_info.package.clone(), ws.config()) + .register_prebuilt(plugin_info.package.clone()) .is_ok() { loaded.insert(plugin_info.package.id); diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index b8f4515a4..8fad32800 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -65,7 +65,7 @@ fn load_plugins( } else if plugin_info.package.is_prebuilt() { // For prebuilt procedural macros, try loading from prebuilt binary first if proc_macros - .register_prebuilt(plugin_info.package.clone(), ws.config()) + .register_prebuilt(plugin_info.package.clone()) .is_err() { // If failed to load from prebuilt binary, try loading from shared library diff --git a/scarb/src/compiler/plugin/proc_macro/host.rs b/scarb/src/compiler/plugin/proc_macro/host.rs index 5710fd296..2c4b7b6a8 100644 --- a/scarb/src/compiler/plugin/proc_macro/host.rs +++ b/scarb/src/compiler/plugin/proc_macro/host.rs @@ -1160,7 +1160,7 @@ impl ProcMacroHost { Ok(()) } - pub fn register_prebuilt(&mut self, package: Package, config: &Config) -> Result<()> { + pub fn register_prebuilt(&mut self, package: Package) -> Result<()> { let prebuilt_path = package .prebuilt_lib_path() .context("could not resolve prebuilt library path")?; From 76118b0564c986160f7183316ec2fe1e4c4889de Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 22:48:46 +0100 Subject: [PATCH 23/33] Ensure `cargo fetch` is not called for prebuilt plugins --- scarb/src/compiler/db.rs | 4 +++- scarb/src/compiler/plugin/mod.rs | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index 8fad32800..ed3689e41 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -20,6 +20,7 @@ use smol_str::SmolStr; use std::path::PathBuf; use std::sync::Arc; use tracing::trace; +use crate::compiler::plugin::fetch_cairo_plugin; pub struct ScarbDatabase { pub db: RootDatabase, @@ -73,7 +74,8 @@ fn load_plugins( .register(plugin_info.package.clone(), ws.config()) .is_err() { - // If failed to load from shared library, compile the plugin and try again + // If failed to load from shared library, fetch the plugin, compile it and try again + fetch_cairo_plugin(&plugin_info.package, ws)?; let plugin_unit = generate_cairo_plugin_compilation_units(&plugin_info.package)? .first() diff --git a/scarb/src/compiler/plugin/mod.rs b/scarb/src/compiler/plugin/mod.rs index 30838566f..5bdc0b08d 100644 --- a/scarb/src/compiler/plugin/mod.rs +++ b/scarb/src/compiler/plugin/mod.rs @@ -9,6 +9,7 @@ use itertools::Itertools; use serde::{Deserialize, Serialize}; use crate::compiler::plugin::builtin::BuiltinCairoRunPlugin; +use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::core::{Package, PackageId, TargetKind, Workspace}; use self::builtin::{BuiltinStarkNetPlugin, BuiltinTestPlugin}; @@ -30,7 +31,7 @@ pub fn fetch_cairo_plugin(package: &Package, ws: &Workspace<'_>) -> Result<()> { let target = package.fetch_target(&TargetKind::CAIRO_PLUGIN)?; let props: CairoPluginProps = target.props()?; // No need to fetch for buildin plugins. - if !props.builtin { + if !props.builtin && !package.is_prebuilt() { proc_macro::fetch_crate(package, ws)?; } Ok(()) From 05cf39dd9267ac44aefdb0c8ca67e770abbd21ae Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 22:49:09 +0100 Subject: [PATCH 24/33] Update test --- scarb/tests/build_cairo_plugin.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index f71d36247..7e8f51e39 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -221,18 +221,21 @@ fn compile_with_prebuilt_plugins() { "#}) .dep( "proc_macro_example", - Dep.version("0.1.1").registry("https://scarbs.dev/"), + Dep.version("0.1.2").registry("https://scarbs.dev/"), ) .manifest_extra(indoc! {r#" "#}) .build(&t); Scarb::quick_snapbox() .arg("build") + // Disable Cargo and Rust compiler. + .env("CARGO", "/bin/false") + .env("RUSTC", "/bin/false") .current_dir(&t) .assert() .success() .stdout_matches(indoc! {r#" - [..]Downloading proc_macro_example v0.1.1 ([..]) + [..]Downloading proc_macro_example v0.1.2 ([..]) [..]Compiling hello v1.0.0 ([..]Scarb.toml) [..] Finished `dev` profile target(s) in [..] "#}); From 5a85ee26103346bad1ffc3b387eacc02ed053059 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 23:34:12 +0100 Subject: [PATCH 25/33] Add test with invalid prebuilt plugins --- scarb/tests/build_cairo_plugin.rs | 34 +++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/scarb/tests/build_cairo_plugin.rs b/scarb/tests/build_cairo_plugin.rs index 7e8f51e39..48c4095ef 100644 --- a/scarb/tests/build_cairo_plugin.rs +++ b/scarb/tests/build_cairo_plugin.rs @@ -241,6 +241,40 @@ fn compile_with_prebuilt_plugins() { "#}); } +#[test] +fn compile_with_invalid_prebuilt_plugins() { + let t = TempDir::new().unwrap(); + + ProjectBuilder::start() + .name("hello") + .lib_cairo(indoc! {r#" + fn main() -> u32 { + let x = some!(42); + x + } + "#}) + .dep( + "invalid_prebuilt_example", + Dep.version("0.1.0").registry("https://scarbs.dev/"), + ) + .manifest_extra(indoc! {r#" + "#}) + .build(&t); + Scarb::quick_snapbox() + .arg("build") + // Disable output from Cargo. + .env("CARGO_TERM_QUIET", "true") + .current_dir(&t) + .assert() + .success() + .stdout_matches(indoc! {r#" + [..]Downloading invalid_prebuilt_example v0.1.0 ([..]) + [..]Compiling invalid_prebuilt_example v0.1.0 ([..]) + [..]Compiling hello v1.0.0 ([..]Scarb.toml) + [..] Finished `dev` profile target(s) in [..] + "#}); +} + #[test] fn can_emit_plugin_warning() { let temp = TempDir::new().unwrap(); From cce42b78bc1d3a587103d3cb55e09ebb21f70336 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 8 Dec 2024 23:50:53 +0100 Subject: [PATCH 26/33] Fix comment --- scarb/src/compiler/plugin/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scarb/src/compiler/plugin/mod.rs b/scarb/src/compiler/plugin/mod.rs index 5bdc0b08d..91edc9362 100644 --- a/scarb/src/compiler/plugin/mod.rs +++ b/scarb/src/compiler/plugin/mod.rs @@ -30,7 +30,7 @@ pub fn fetch_cairo_plugin(package: &Package, ws: &Workspace<'_>) -> Result<()> { assert!(package.is_cairo_plugin()); let target = package.fetch_target(&TargetKind::CAIRO_PLUGIN)?; let props: CairoPluginProps = target.props()?; - // No need to fetch for buildin plugins. + // No need to fetch for builtin or prebuilt plugins. if !props.builtin && !package.is_prebuilt() { proc_macro::fetch_crate(package, ws)?; } From 06aab97e677b58d11ff8af3324d7c2acebd054e6 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Mon, 9 Dec 2024 00:17:13 +0100 Subject: [PATCH 27/33] Format --- scarb/src/compiler/db.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index ed3689e41..34ab02a12 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -1,3 +1,4 @@ +use crate::compiler::plugin::fetch_cairo_plugin; use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin}; use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent}; @@ -20,7 +21,6 @@ use smol_str::SmolStr; use std::path::PathBuf; use std::sync::Arc; use tracing::trace; -use crate::compiler::plugin::fetch_cairo_plugin; pub struct ScarbDatabase { pub db: RootDatabase, From 64f362ee8e0d92629e6a30bbb7a67aa2b216a339 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Tue, 10 Dec 2024 13:21:00 +0100 Subject: [PATCH 28/33] Add `test-prebuilt-plugins` for all available targets --- .github/workflows/ci.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 144278208..b52e7e78f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,3 +107,28 @@ jobs: cache-dependency-path: website/package-lock.json - run: npm ci - run: npm run fmt:check + + test-prebuilt-plugins: + name: test prebuilt plugins ${{ matrix.platform.name }} + runs-on: ${{ matrix.platform.os }} + strategy: + fail-fast: false + matrix: + platform: + - name: linux x86-64 + os: ubuntu-latest + - name: windows x86-64 + os: windows-latest + - name: macos arm64 + os: macos-latest + - name: macos x86-64 + os: macos-13 + + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - name: Run prebuilt plugin tests + run: | + cargo test -p scarb --test build_cairo_plugin compile_with_prebuilt_plugins -- --exact + cargo test -p scarb --test build_cairo_plugin compile_with_invalid_prebuilt_plugins -- --exact From e1873fd45bb25df2cfae9602384710ce1bd21d76 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Tue, 10 Dec 2024 14:12:47 +0100 Subject: [PATCH 29/33] Add `proc-macro-server` test --- .github/workflows/ci.yml | 1 + scarb/tests/proc_macro_server.rs | 31 ++++++++++++++++++- .../src/proc_macro_server.rs | 25 +++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b52e7e78f..dd1781180 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,3 +132,4 @@ jobs: run: | cargo test -p scarb --test build_cairo_plugin compile_with_prebuilt_plugins -- --exact cargo test -p scarb --test build_cairo_plugin compile_with_invalid_prebuilt_plugins -- --exact + cargo test -p scarb --test proc_macro_server load_prebuilt_proc_macros -- --exact diff --git a/scarb/tests/proc_macro_server.rs b/scarb/tests/proc_macro_server.rs index b01f865dc..b17790be8 100644 --- a/scarb/tests/proc_macro_server.rs +++ b/scarb/tests/proc_macro_server.rs @@ -12,7 +12,7 @@ use scarb_proc_macro_server_types::methods::expand::ExpandInlineMacroParams; use scarb_test_support::cairo_plugin_project_builder::CairoPluginProjectBuilder; use scarb_test_support::proc_macro_server::ProcMacroClient; use scarb_test_support::proc_macro_server::SIMPLE_MACROS; -use scarb_test_support::project_builder::ProjectBuilder; +use scarb_test_support::project_builder::{Dep, DepBuilder, ProjectBuilder}; #[test] fn defined_macros() { @@ -171,3 +171,32 @@ fn expand_inline() { TokenStream::new("struct A { field: 25 , other_field: macro_call!(12)}".to_string()) ); } + +#[test] +fn load_prebuilt_proc_macros() { + let t = TempDir::new().unwrap(); + + let project = t.child("test_package"); + + ProjectBuilder::start() + .name("test_package") + .version("1.0.0") + .lib_cairo("") + .dep( + "proc_macro_example", + Dep.version("0.1.2").registry("https://scarbs.dev/"), + ) + .build(&project); + + let mut proc_macro_server = ProcMacroClient::new_without_cargo(&project); + + let response = proc_macro_server + .request_and_wait::(ExpandInlineMacroParams { + name: "some".to_string(), + args: TokenStream::new("42".to_string()), + }) + .unwrap(); + + assert_eq!(response.diagnostics, vec![]); + assert_eq!(response.token_stream, TokenStream::new("42".to_string())); +} diff --git a/utils/scarb-test-support/src/proc_macro_server.rs b/utils/scarb-test-support/src/proc_macro_server.rs index 12e93176f..cf1fefab0 100644 --- a/utils/scarb-test-support/src/proc_macro_server.rs +++ b/utils/scarb-test-support/src/proc_macro_server.rs @@ -90,6 +90,31 @@ impl ProcMacroClient { responses: Default::default(), } } + pub fn new_without_cargo>(path: P) -> Self { + let mut server_process = Scarb::new() + .std() + .arg("--quiet") + .arg("proc-macro-server") + .env("CARGO", "/bin/false") + .env("RUSTC", "/bin/false") + .stdout(Stdio::piped()) + .stdin(Stdio::piped()) + .stderr(Stdio::inherit()) + .current_dir(path) + .spawn() + .unwrap(); + + let requester = server_process.stdin.take().unwrap(); + let responder = BufReader::new(server_process.stdout.take().unwrap()).lines(); + + Self { + requester, + responder, + server_process, + id_counter: Default::default(), + responses: Default::default(), + } + } pub fn request(&mut self, params: M::Params) -> PendingRequest { let id = self.id_counter; From 034af9a8419c2912a5537e0c1f8a776e549339e5 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Mon, 16 Dec 2024 20:50:07 +0100 Subject: [PATCH 30/33] Preload prebuilts during compilation --- .../bin/scarb/commands/proc_macro_server.rs | 5 +- scarb/src/compiler/compilation_unit.rs | 13 ++- scarb/src/compiler/db.rs | 35 +----- scarb/src/compiler/plugin/proc_macro/ffi.rs | 15 ++- scarb/src/compiler/plugin/proc_macro/host.rs | 15 +-- scarb/src/ops/compile.rs | 108 ++++++++---------- scarb/src/ops/expand.rs | 11 +- scarb/src/ops/metadata.rs | 1 + scarb/src/ops/resolve.rs | 26 ++++- 9 files changed, 118 insertions(+), 111 deletions(-) diff --git a/scarb/src/bin/scarb/commands/proc_macro_server.rs b/scarb/src/bin/scarb/commands/proc_macro_server.rs index df7e99b70..1e6a230c4 100644 --- a/scarb/src/bin/scarb/commands/proc_macro_server.rs +++ b/scarb/src/bin/scarb/commands/proc_macro_server.rs @@ -21,6 +21,7 @@ pub fn run(config: &mut Config) -> Result<()> { }, true, &ws, + true, )?; let mut proc_macros = ProcMacroHost::default(); @@ -37,7 +38,7 @@ pub fn run(config: &mut Config) -> Result<()> { for unit in &compilation_units { if let CompilationUnit::ProcMacro(_) = unit { if !loaded_plugins.contains(&unit.main_package_id()) { - ops::compile_unit(unit.clone(), &ws, false)?; + ops::compile_unit(unit.clone(), &ws)?; } } } @@ -78,7 +79,7 @@ fn load_plugins( ) -> Result<()> { for plugin_info in unit.cairo_plugins.into_iter().filter(|p| !p.builtin) { if !loaded_plugins.contains(&plugin_info.package.id) { - proc_macros.register(plugin_info.package, ws.config())?; + proc_macros.register_new(plugin_info.package, ws.config())?; } } diff --git a/scarb/src/compiler/compilation_unit.rs b/scarb/src/compiler/compilation_unit.rs index 1eebc381c..02813e2c2 100644 --- a/scarb/src/compiler/compilation_unit.rs +++ b/scarb/src/compiler/compilation_unit.rs @@ -1,15 +1,16 @@ -use std::fmt::Write; -use std::hash::{Hash, Hasher}; - use anyhow::{ensure, Result}; use cairo_lang_filesystem::cfg::CfgSet; use cairo_lang_filesystem::db::CrateIdentifier; use itertools::Itertools; use serde::{Deserialize, Serialize}; use smol_str::SmolStr; +use std::fmt::Write; +use std::hash::{Hash, Hasher}; +use std::sync::Arc; use typed_builder::TypedBuilder; use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; +use crate::compiler::plugin::proc_macro::ProcMacroInstance; use crate::compiler::Profile; use crate::core::{ ManifestCompilerConfig, Package, PackageId, PackageName, Target, TargetKind, Workspace, @@ -73,6 +74,9 @@ pub struct ProcMacroCompilationUnit { /// Rust compiler configuration parameters to use in this unit. pub compiler_config: serde_json::Value, + + /// Instance of the proc macro loaded from prebuilt library, if available. + pub(crate) prebuilt: Option>, } /// Information about a single package that is part of a [`CompilationUnit`]. @@ -98,6 +102,9 @@ pub struct CompilationUnitCairoPlugin { /// The Scarb plugin [`Package`] to load. pub package: Package, pub builtin: bool, + + /// Instance of the proc macro loaded from prebuilt library, if available. + pub(crate) prebuilt: Option>, } /// Unique identifier of the compilation unit component. diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index 34ab02a12..145f4a359 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -1,9 +1,6 @@ -use crate::compiler::plugin::fetch_cairo_plugin; -use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin}; use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent}; use crate::core::Workspace; -use crate::ops::{compile_unit, generate_cairo_plugin_compilation_units}; use crate::DEFAULT_MODULE_MAIN_FILE; use anyhow::{anyhow, Result}; use cairo_lang_compiler::db::{RootDatabase, RootDatabaseBuilder}; @@ -16,7 +13,6 @@ use cairo_lang_filesystem::db::{ }; use cairo_lang_filesystem::ids::CrateLongId; use cairo_lang_utils::ordered_hash_map::OrderedHashMap; -use scarb_ui::components::Status; use smol_str::SmolStr; use std::path::PathBuf; use std::sync::Arc; @@ -58,39 +54,14 @@ fn load_plugins( let mut proc_macros = ProcMacroHost::default(); for plugin_info in &unit.cairo_plugins { if plugin_info.builtin { - // For builtin plugins, fetch and instantiate directly let package_id = plugin_info.package.id; let plugin = ws.config().cairo_plugins().fetch(package_id)?; let instance = plugin.instantiate()?; builder.with_plugin_suite(instance.plugin_suite()); - } else if plugin_info.package.is_prebuilt() { - // For prebuilt procedural macros, try loading from prebuilt binary first - if proc_macros - .register_prebuilt(plugin_info.package.clone()) - .is_err() - { - // If failed to load from prebuilt binary, try loading from shared library - if proc_macros - .register(plugin_info.package.clone(), ws.config()) - .is_err() - { - // If failed to load from shared library, fetch the plugin, compile it and try again - fetch_cairo_plugin(&plugin_info.package, ws)?; - let plugin_unit = - generate_cairo_plugin_compilation_units(&plugin_info.package)? - .first() - .unwrap() - .clone(); - ws.config() - .ui() - .print(Status::new("Compiling", &plugin_unit.name())); - compile_unit(plugin_unit, ws, false)?; - proc_macros.register(plugin_info.package.clone(), ws.config())?; - } - } + } else if let Some(prebuilt) = &plugin_info.prebuilt { + proc_macros.register(prebuilt.clone()); } else { - // For non-prebuilt procedural macros, load from shared library - proc_macros.register(plugin_info.package.clone(), ws.config())?; + proc_macros.register_new(plugin_info.package.clone(), ws.config())?; } } let macro_host = Arc::new(proc_macros.into_plugin()?); diff --git a/scarb/src/compiler/plugin/proc_macro/ffi.rs b/scarb/src/compiler/plugin/proc_macro/ffi.rs index c9804ffbe..9c1a376ef 100644 --- a/scarb/src/compiler/plugin/proc_macro/ffi.rs +++ b/scarb/src/compiler/plugin/proc_macro/ffi.rs @@ -1,4 +1,4 @@ -use crate::core::PackageId; +use crate::core::{Package, PackageId}; use anyhow::{ensure, Context, Result}; use cairo_lang_defs::patcher::PatchBuilder; use cairo_lang_macro::{ @@ -18,6 +18,7 @@ use std::ffi::{c_char, CStr, CString}; use std::fmt::Debug; use std::slice; +use crate::compiler::plugin::proc_macro::compilation::SharedLibraryProvider; use crate::compiler::plugin::proc_macro::ProcMacroAuxData; #[cfg(not(windows))] @@ -69,6 +70,18 @@ impl ProcMacroInstance { }) } + pub fn try_load_prebuilt(package: Package) -> Result { + let prebuilt_path = package + .prebuilt_lib_path() + .context("could not resolve prebuilt library path")?; + let plugin = unsafe { Plugin::try_new(prebuilt_path)? }; + Ok(Self { + expansions: unsafe { Self::load_expansions(&plugin, package.id)? }, + package_id: package.id, + plugin, + }) + } + unsafe fn load_expansions(plugin: &Plugin, package_id: PackageId) -> Result> { // Make a call to the FFI interface to list declared expansions. let stable_expansions = (plugin.vtable.list_expansions)(); diff --git a/scarb/src/compiler/plugin/proc_macro/host.rs b/scarb/src/compiler/plugin/proc_macro/host.rs index 2c4b7b6a8..7f64d61fc 100644 --- a/scarb/src/compiler/plugin/proc_macro/host.rs +++ b/scarb/src/compiler/plugin/proc_macro/host.rs @@ -1151,21 +1151,22 @@ pub struct ProcMacroHost { } impl ProcMacroHost { - pub fn register(&mut self, package: Package, config: &Config) -> Result<()> { + pub fn register(&mut self, instance: Arc) { + self.macros.push(instance); + } + + pub fn register_new(&mut self, package: Package, config: &Config) -> Result<()> { let lib_path = package .shared_lib_path(config) .context("could not resolve shared library path")?; let instance = ProcMacroInstance::try_new(package.id, lib_path)?; - self.macros.push(Arc::new(instance)); + self.register(Arc::new(instance)); Ok(()) } pub fn register_prebuilt(&mut self, package: Package) -> Result<()> { - let prebuilt_path = package - .prebuilt_lib_path() - .context("could not resolve prebuilt library path")?; - let instance = ProcMacroInstance::try_new(package.id, prebuilt_path)?; - self.macros.push(Arc::new(instance)); + let instance = ProcMacroInstance::try_load_prebuilt(package)?; + self.register(Arc::new(instance)); Ok(()) } diff --git a/scarb/src/ops/compile.rs b/scarb/src/ops/compile.rs index ca6f78bee..33678520b 100644 --- a/scarb/src/ops/compile.rs +++ b/scarb/src/ops/compile.rs @@ -98,12 +98,12 @@ impl CompileOpts { #[tracing::instrument(skip_all, level = "debug")] pub fn compile(packages: Vec, opts: CompileOpts, ws: &Workspace<'_>) -> Result<()> { - process(packages, opts, ws, true, compile_units, None) + process(packages, opts, ws, compile_units, None) } #[tracing::instrument(skip_all, level = "debug")] pub fn check(packages: Vec, opts: CompileOpts, ws: &Workspace<'_>) -> Result<()> { - process(packages, opts, ws, true, check_units, Some("checking")) + process(packages, opts, ws, check_units, Some("checking")) } #[tracing::instrument(skip_all, level = "debug")] @@ -111,12 +111,11 @@ fn process( packages: Vec, opts: CompileOpts, ws: &Workspace<'_>, - skip_prebuilt_proc_macros: bool, mut operation: F, operation_type: Option<&str>, ) -> Result<()> where - F: FnMut(Vec, &Workspace<'_>, bool) -> Result<()>, + F: FnMut(Vec, &Workspace<'_>) -> Result<()>, { let resolve = ops::resolve_workspace(ws)?; let packages_to_process = ws @@ -126,38 +125,43 @@ where validate_features(&packages_to_process, &opts.features)?; // Add test compilation units to build let packages = get_test_package_ids(packages, ws); - let compilation_units = - ops::generate_compilation_units(&resolve, &opts.features, opts.ignore_cairo_version, ws)? - .into_iter() - .filter(|cu| { - let is_excluded = opts - .exclude_target_kinds - .contains(&cu.main_component().target_kind()); - let is_included = opts.include_target_kinds.is_empty() - || opts - .include_target_kinds - .contains(&cu.main_component().target_kind()); - let is_included = is_included - && (opts.include_target_names.is_empty() - || cu - .main_component() - .targets - .iter() - .any(|t| opts.include_target_names.contains(&t.name))); - let is_selected = packages.contains(&cu.main_package_id()); - let is_cairo_plugin = matches!(cu, CompilationUnit::ProcMacro(_)); - is_cairo_plugin || (is_selected && is_included && !is_excluded) - }) - .sorted_by_key(|cu| { - if matches!(cu, CompilationUnit::ProcMacro(_)) { - 0 - } else { - 1 - } - }) - .collect::>(); + let compilation_units = ops::generate_compilation_units( + &resolve, + &opts.features, + opts.ignore_cairo_version, + ws, + true, + )? + .into_iter() + .filter(|cu| { + let is_excluded = opts + .exclude_target_kinds + .contains(&cu.main_component().target_kind()); + let is_included = opts.include_target_kinds.is_empty() + || opts + .include_target_kinds + .contains(&cu.main_component().target_kind()); + let is_included = is_included + && (opts.include_target_names.is_empty() + || cu + .main_component() + .targets + .iter() + .any(|t| opts.include_target_names.contains(&t.name))); + let is_selected = packages.contains(&cu.main_package_id()); + let is_cairo_plugin = matches!(cu, CompilationUnit::ProcMacro(_)); + is_cairo_plugin || (is_selected && is_included && !is_excluded) + }) + .sorted_by_key(|cu| { + if matches!(cu, CompilationUnit::ProcMacro(_)) { + 0 + } else { + 1 + } + }) + .collect::>(); - operation(compilation_units, ws, skip_prebuilt_proc_macros)?; + operation(compilation_units, ws)?; let elapsed_time = HumanDuration(ws.config().elapsed_time()); let profile = ws.current_profile()?; @@ -174,44 +178,30 @@ where /// Run compiler in a new thread. /// The stack size of created threads can be altered with `RUST_MIN_STACK` env variable. -pub fn compile_units( - units: Vec, - ws: &Workspace<'_>, - skip_prebuilt_proc_macros: bool, -) -> Result<()> { +pub fn compile_units(units: Vec, ws: &Workspace<'_>) -> Result<()> { for unit in units { - compile_unit(unit, ws, skip_prebuilt_proc_macros)?; + compile_unit(unit, ws)?; } Ok(()) } -pub fn compile_unit( - unit: CompilationUnit, - ws: &Workspace<'_>, - skip_prebuilt_proc_macros: bool, -) -> Result<()> { +pub fn compile_unit(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> { thread::scope(|s| { thread::Builder::new() .name(format!("scarb compile {}", unit.id())) - .spawn_scoped(s, || { - compile_unit_inner(unit, ws, skip_prebuilt_proc_macros) - }) + .spawn_scoped(s, || compile_unit_inner(unit, ws)) .expect("Failed to spawn compiler thread.") .join() .expect("Compiler thread has panicked.") }) } -fn compile_unit_inner( - unit: CompilationUnit, - ws: &Workspace<'_>, - skip_prebuilt_proc_macros: bool, -) -> Result<()> { +fn compile_unit_inner(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> { let package_name = unit.main_package_id().name.clone(); let result = match unit { CompilationUnit::ProcMacro(unit) => { - if unit.is_prebuilt() && skip_prebuilt_proc_macros { + if unit.prebuilt.is_some() { Ok(()) } else { ws.config() @@ -246,11 +236,7 @@ fn compile_unit_inner( }) } -fn check_units( - units: Vec, - ws: &Workspace<'_>, - skip_prebuilt_proc_macros: bool, -) -> Result<()> { +fn check_units(units: Vec, ws: &Workspace<'_>) -> Result<()> { // Select proc macro units that need to be compiled for Cairo compilation units. let required_plugins = units .iter() @@ -278,7 +264,7 @@ fn check_units( if matches!(unit, CompilationUnit::ProcMacro(_)) && required_plugins.contains(&unit.main_package_id()) { - compile_unit(unit, ws, skip_prebuilt_proc_macros)?; + compile_unit(unit, ws)?; } else { check_unit(unit, ws)?; } diff --git a/scarb/src/ops/expand.rs b/scarb/src/ops/expand.rs index f6720bf4e..4a6b5d941 100644 --- a/scarb/src/ops/expand.rs +++ b/scarb/src/ops/expand.rs @@ -43,14 +43,19 @@ pub fn expand(package: Package, opts: ExpandOpts, ws: &Workspace<'_>) -> Result< let package_name = package.id.name.to_string(); let resolve = ops::resolve_workspace(ws)?; - let compilation_units = - ops::generate_compilation_units(&resolve, &opts.features, opts.ignore_cairo_version, ws)?; + let compilation_units = ops::generate_compilation_units( + &resolve, + &opts.features, + opts.ignore_cairo_version, + ws, + true, + )?; // Compile procedural macros. compilation_units .iter() .filter(|unit| matches!(unit, CompilationUnit::ProcMacro(_))) - .map(|unit| ops::compile::compile_unit(unit.clone(), ws, true)) + .map(|unit| ops::compile::compile_unit(unit.clone(), ws)) .collect::>>()?; let compilation_units = compilation_units diff --git a/scarb/src/ops/metadata.rs b/scarb/src/ops/metadata.rs index 52abdc57e..abe84111f 100644 --- a/scarb/src/ops/metadata.rs +++ b/scarb/src/ops/metadata.rs @@ -49,6 +49,7 @@ pub fn collect_metadata(opts: &MetadataOptions, ws: &Workspace<'_>) -> Result, + load_prebuilts: bool, ) -> Result> { let mut units = Vec::with_capacity(ws.members().size_hint().0); let members = ws @@ -204,7 +208,10 @@ pub fn generate_compilation_units( .collect_vec(); for plugin in cairo_plugins { - units.extend(generate_cairo_plugin_compilation_units(&plugin)?); + units.extend(generate_cairo_plugin_compilation_units( + &plugin, + load_prebuilts, + )?); } assert!( @@ -619,9 +626,13 @@ impl<'a> PackageSolutionCollector<'a> { // We can safely unwrap as all packages with `PackageClass::CairoPlugin` must define plugin target. let target = package.target(&TargetKind::CAIRO_PLUGIN).unwrap(); let props: CairoPluginProps = target.props()?; + let prebuilt = ProcMacroInstance::try_load_prebuilt(package.clone()) + .ok() + .map(Arc::new); Ok(CompilationUnitCairoPlugin::builder() .package(package) .builtin(props.builtin) + .prebuilt(prebuilt) .build()) }) .collect::>>()?; @@ -684,7 +695,17 @@ fn check_cairo_version_compatibility( Ok(()) } -pub fn generate_cairo_plugin_compilation_units(member: &Package) -> Result> { +pub fn generate_cairo_plugin_compilation_units( + member: &Package, + load_prebuilts: bool, +) -> Result> { + let prebuilt = if load_prebuilts && member.is_prebuilt() { + ProcMacroInstance::try_load_prebuilt(member.clone()) + .ok() + .map(Arc::new) + } else { + None + }; Ok(vec![CompilationUnit::ProcMacro(ProcMacroCompilationUnit { main_package_id: member.id, compiler_config: serde_json::Value::Null, @@ -697,6 +718,7 @@ pub fn generate_cairo_plugin_compilation_units(member: &Package) -> Result Date: Wed, 18 Dec 2024 14:29:47 +0100 Subject: [PATCH 31/33] Adjust proc macro server to loading prebuilts on compilation --- .../bin/scarb/commands/proc_macro_server.rs | 47 ++++--------------- scarb/src/compiler/compilation_unit.rs | 4 +- scarb/src/compiler/db.rs | 2 +- scarb/src/compiler/plugin/proc_macro/host.rs | 10 +--- 4 files changed, 14 insertions(+), 49 deletions(-) diff --git a/scarb/src/bin/scarb/commands/proc_macro_server.rs b/scarb/src/bin/scarb/commands/proc_macro_server.rs index 1e6a230c4..a12b731e7 100644 --- a/scarb/src/bin/scarb/commands/proc_macro_server.rs +++ b/scarb/src/bin/scarb/commands/proc_macro_server.rs @@ -2,12 +2,10 @@ use anyhow::Result; use scarb::{ compiler::{ plugin::proc_macro::ProcMacroHost, CairoCompilationUnit, CompilationUnit, - CompilationUnitAttributes, }, - core::{Config, PackageId, Workspace}, + core::{Config, Workspace}, ops::{self, FeaturesOpts, FeaturesSelector}, }; -use std::collections::HashSet; #[tracing::instrument(skip_all, level = "info")] pub fn run(config: &mut Config) -> Result<()> { @@ -24,61 +22,34 @@ pub fn run(config: &mut Config) -> Result<()> { true, )?; - let mut proc_macros = ProcMacroHost::default(); - let mut loaded_plugins = HashSet::new(); - - // Try loading prebuilt plugins - for unit in &compilation_units { - if let CompilationUnit::Cairo(unit) = unit { - loaded_plugins.extend(load_prebuilt_plugins(unit.clone(), &mut proc_macros)?); - } - } - - // Then compile remaining procedural macros. + // Compile procedural macros only. for unit in &compilation_units { if let CompilationUnit::ProcMacro(_) = unit { - if !loaded_plugins.contains(&unit.main_package_id()) { - ops::compile_unit(unit.clone(), &ws)?; - } + ops::compile_unit(unit.clone(), &ws)?; } } + let mut proc_macros = ProcMacroHost::default(); + // Load previously compiled procedural macros. for unit in compilation_units { if let CompilationUnit::Cairo(unit) = unit { - load_plugins(unit, &ws, &mut proc_macros, &loaded_plugins)?; + load_plugins(unit, &ws, &mut proc_macros)?; } } ops::start_proc_macro_server(proc_macros) } -fn load_prebuilt_plugins( - unit: CairoCompilationUnit, - proc_macros: &mut ProcMacroHost, -) -> Result> { - let mut loaded = HashSet::new(); - - for plugin_info in unit.cairo_plugins.into_iter().filter(|p| !p.builtin) { - if proc_macros - .register_prebuilt(plugin_info.package.clone()) - .is_ok() - { - loaded.insert(plugin_info.package.id); - } - } - - Ok(loaded) -} - fn load_plugins( unit: CairoCompilationUnit, ws: &Workspace<'_>, proc_macros: &mut ProcMacroHost, - loaded_plugins: &HashSet, ) -> Result<()> { for plugin_info in unit.cairo_plugins.into_iter().filter(|p| !p.builtin) { - if !loaded_plugins.contains(&plugin_info.package.id) { + if let Some(prebuilt) = plugin_info.prebuilt { + proc_macros.register_instance(prebuilt); + } else { proc_macros.register_new(plugin_info.package, ws.config())?; } } diff --git a/scarb/src/compiler/compilation_unit.rs b/scarb/src/compiler/compilation_unit.rs index 02813e2c2..789aff27c 100644 --- a/scarb/src/compiler/compilation_unit.rs +++ b/scarb/src/compiler/compilation_unit.rs @@ -76,7 +76,7 @@ pub struct ProcMacroCompilationUnit { pub compiler_config: serde_json::Value, /// Instance of the proc macro loaded from prebuilt library, if available. - pub(crate) prebuilt: Option>, + pub prebuilt: Option>, } /// Information about a single package that is part of a [`CompilationUnit`]. @@ -104,7 +104,7 @@ pub struct CompilationUnitCairoPlugin { pub builtin: bool, /// Instance of the proc macro loaded from prebuilt library, if available. - pub(crate) prebuilt: Option>, + pub prebuilt: Option>, } /// Unique identifier of the compilation unit component. diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs index 145f4a359..66fad856d 100644 --- a/scarb/src/compiler/db.rs +++ b/scarb/src/compiler/db.rs @@ -59,7 +59,7 @@ fn load_plugins( let instance = plugin.instantiate()?; builder.with_plugin_suite(instance.plugin_suite()); } else if let Some(prebuilt) = &plugin_info.prebuilt { - proc_macros.register(prebuilt.clone()); + proc_macros.register_instance(prebuilt.clone()); } else { proc_macros.register_new(plugin_info.package.clone(), ws.config())?; } diff --git a/scarb/src/compiler/plugin/proc_macro/host.rs b/scarb/src/compiler/plugin/proc_macro/host.rs index 7f64d61fc..7e8765ef9 100644 --- a/scarb/src/compiler/plugin/proc_macro/host.rs +++ b/scarb/src/compiler/plugin/proc_macro/host.rs @@ -1151,7 +1151,7 @@ pub struct ProcMacroHost { } impl ProcMacroHost { - pub fn register(&mut self, instance: Arc) { + pub fn register_instance(&mut self, instance: Arc) { self.macros.push(instance); } @@ -1160,13 +1160,7 @@ impl ProcMacroHost { .shared_lib_path(config) .context("could not resolve shared library path")?; let instance = ProcMacroInstance::try_new(package.id, lib_path)?; - self.register(Arc::new(instance)); - Ok(()) - } - - pub fn register_prebuilt(&mut self, package: Package) -> Result<()> { - let instance = ProcMacroInstance::try_load_prebuilt(package)?; - self.register(Arc::new(instance)); + self.register_instance(Arc::new(instance)); Ok(()) } From 3af99af94a1c73c7cea8fca49b25959a193614c5 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Wed, 18 Dec 2024 15:52:30 +0100 Subject: [PATCH 32/33] Format --- scarb/src/bin/scarb/commands/proc_macro_server.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scarb/src/bin/scarb/commands/proc_macro_server.rs b/scarb/src/bin/scarb/commands/proc_macro_server.rs index a12b731e7..56900eea8 100644 --- a/scarb/src/bin/scarb/commands/proc_macro_server.rs +++ b/scarb/src/bin/scarb/commands/proc_macro_server.rs @@ -1,8 +1,6 @@ use anyhow::Result; use scarb::{ - compiler::{ - plugin::proc_macro::ProcMacroHost, CairoCompilationUnit, CompilationUnit, - }, + compiler::{plugin::proc_macro::ProcMacroHost, CairoCompilationUnit, CompilationUnit}, core::{Config, Workspace}, ops::{self, FeaturesOpts, FeaturesSelector}, }; From 9eb6fc4056e6a0719a5a1cd18bb1d87b0e29a276 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Wed, 18 Dec 2024 16:13:31 +0100 Subject: [PATCH 33/33] Reorder `load_prebuilts` in `generate_compilation_units` --- scarb/src/bin/scarb/commands/proc_macro_server.rs | 2 +- scarb/src/ops/compile.rs | 2 +- scarb/src/ops/expand.rs | 2 +- scarb/src/ops/metadata.rs | 2 +- scarb/src/ops/resolve.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scarb/src/bin/scarb/commands/proc_macro_server.rs b/scarb/src/bin/scarb/commands/proc_macro_server.rs index 56900eea8..b2165899b 100644 --- a/scarb/src/bin/scarb/commands/proc_macro_server.rs +++ b/scarb/src/bin/scarb/commands/proc_macro_server.rs @@ -16,8 +16,8 @@ pub fn run(config: &mut Config) -> Result<()> { no_default_features: false, }, true, - &ws, true, + &ws, )?; // Compile procedural macros only. diff --git a/scarb/src/ops/compile.rs b/scarb/src/ops/compile.rs index 33678520b..64cbd9c75 100644 --- a/scarb/src/ops/compile.rs +++ b/scarb/src/ops/compile.rs @@ -129,8 +129,8 @@ where &resolve, &opts.features, opts.ignore_cairo_version, - ws, true, + ws, )? .into_iter() .filter(|cu| { diff --git a/scarb/src/ops/expand.rs b/scarb/src/ops/expand.rs index 4a6b5d941..0edb61da3 100644 --- a/scarb/src/ops/expand.rs +++ b/scarb/src/ops/expand.rs @@ -47,8 +47,8 @@ pub fn expand(package: Package, opts: ExpandOpts, ws: &Workspace<'_>) -> Result< &resolve, &opts.features, opts.ignore_cairo_version, - ws, true, + ws, )?; // Compile procedural macros. diff --git a/scarb/src/ops/metadata.rs b/scarb/src/ops/metadata.rs index abe84111f..ad69030ed 100644 --- a/scarb/src/ops/metadata.rs +++ b/scarb/src/ops/metadata.rs @@ -48,8 +48,8 @@ pub fn collect_metadata(opts: &MetadataOptions, ws: &Workspace<'_>) -> Result, load_prebuilts: bool, + ws: &Workspace<'_>, ) -> Result> { let mut units = Vec::with_capacity(ws.members().size_hint().0); let members = ws