From 3555a26c1e385e6484f2d263781673feff8c77f3 Mon Sep 17 00:00:00 2001 From: maciektr Date: Fri, 20 Dec 2024 11:02:52 +0100 Subject: [PATCH] Add allowed prebuilts list (#1846) commit-id:66715aa1 --- scarb-metadata/CHANGELOG.md | 1 + scarb-metadata/src/lib.rs | 3 + scarb/src/compiler/compilation_unit.rs | 1 + scarb/src/core/manifest/toml_manifest.rs | 6 ++ scarb/src/core/package/mod.rs | 11 ++- scarb/src/core/resolver.rs | 71 +++++++++++++-- scarb/src/ops/metadata.rs | 1 + scarb/src/ops/resolve.rs | 45 ++++++++++ scarb/tests/metadata.rs | 109 +++++++++++++++++++++++ 9 files changed, 242 insertions(+), 6 deletions(-) diff --git a/scarb-metadata/CHANGELOG.md b/scarb-metadata/CHANGELOG.md index 1bd8d6eb1..728fe458a 100644 --- a/scarb-metadata/CHANGELOG.md +++ b/scarb-metadata/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file. ## Unreleased +- Add `prebuilt_allowed` field to `CompilationUnitCairoPluginMetadata`. ## 1.13.0 (2024-10-28) - Add `CompilationUnitComponentId`. diff --git a/scarb-metadata/src/lib.rs b/scarb-metadata/src/lib.rs index 46053e90b..50f61c95e 100644 --- a/scarb-metadata/src/lib.rs +++ b/scarb-metadata/src/lib.rs @@ -420,6 +420,9 @@ pub struct CompilationUnitCairoPluginMetadata { /// Package ID. pub package: PackageId, + /// Whether Scarb will attempt to load prebuilt binaries associated with this plugin. + pub prebuilt_allowed: Option, + /// Additional data not captured by deserializer. #[cfg_attr(feature = "builder", builder(default))] #[serde(flatten)] diff --git a/scarb/src/compiler/compilation_unit.rs b/scarb/src/compiler/compilation_unit.rs index 14af04350..cae3328d0 100644 --- a/scarb/src/compiler/compilation_unit.rs +++ b/scarb/src/compiler/compilation_unit.rs @@ -97,6 +97,7 @@ pub struct CompilationUnitCairoPlugin { /// The Scarb plugin [`Package`] to load. pub package: Package, pub builtin: bool, + pub prebuilt_allowed: bool, } /// Unique identifier of the compilation unit component. diff --git a/scarb/src/core/manifest/toml_manifest.rs b/scarb/src/core/manifest/toml_manifest.rs index 486df81d8..db65e3c53 100644 --- a/scarb/src/core/manifest/toml_manifest.rs +++ b/scarb/src/core/manifest/toml_manifest.rs @@ -357,6 +357,12 @@ impl DefaultForProfile for TomlProfile { } } +#[derive(Debug, Default, Deserialize, Serialize, Clone)] +#[serde(rename_all = "kebab-case")] +pub struct TomlToolScarbMetadata { + pub allow_prebuilt_plugins: Option>, +} + impl TomlManifest { pub fn read_from_path(path: &Utf8Path) -> Result { let contents = fs::read_to_string(path) diff --git a/scarb/src/core/package/mod.rs b/scarb/src/core/package/mod.rs index 919c250c9..2a50516fd 100644 --- a/scarb/src/core/package/mod.rs +++ b/scarb/src/core/package/mod.rs @@ -11,7 +11,7 @@ pub use name::*; use scarb_ui::args::WithManifestPath; use crate::core::manifest::Manifest; -use crate::core::{Target, TargetKind}; +use crate::core::{Target, TargetKind, TomlToolScarbMetadata}; use crate::internal::fsx; mod id; @@ -137,6 +137,15 @@ impl Package { Ok(structured) } + pub fn scarb_tool_metadata(&self) -> Result { + Ok(self + .tool_metadata("scarb") + .cloned() + .map(toml::Value::try_into) + .transpose()? + .unwrap_or_default()) + } + pub fn manifest_mut(&mut self) -> &mut Manifest { &mut Arc::make_mut(&mut self.0).manifest } diff --git a/scarb/src/core/resolver.rs b/scarb/src/core/resolver.rs index 82ddd7cdd..fc418e85d 100644 --- a/scarb/src/core/resolver.rs +++ b/scarb/src/core/resolver.rs @@ -1,14 +1,14 @@ -use std::collections::HashMap; - +use crate::core::lockfile::Lockfile; +use crate::core::{PackageId, Summary, TargetKind}; use anyhow::{bail, Result}; use indoc::formatdoc; use itertools::Itertools; +use petgraph::algo::kosaraju_scc; use petgraph::graphmap::DiGraphMap; use petgraph::visit::{Dfs, EdgeFiltered, IntoNeighborsDirected, Walker}; use smallvec::SmallVec; - -use crate::core::lockfile::Lockfile; -use crate::core::{PackageId, Summary, TargetKind}; +use std::collections::{HashMap, HashSet}; +use std::hash::Hash; /// Represents a fully-resolved package dependency graph. /// @@ -72,6 +72,67 @@ impl Resolve { .neighbors_directed(package_id, petgraph::Direction::Outgoing) .collect_vec() } + + /// Find all subtress of the graph, that are reachable from nodes, that can be identified by + /// keys from the `start` vector, where each key is a result of applying `key` function to a + /// package id. + pub fn filter_subtrees( + &self, + target_kind: &TargetKind, + start: Vec, + key: impl Fn(PackageId) -> T, + ) -> HashSet { + // We want to traverse the graph in topological order, so that for each node we can decide + // if the subtree should be included. + // However, we cannot actually topologically sort the graph, as it's not guaranteed to be + // a DAG (it may contain cycles of dependencies). + // Instead, we use Kosaraju's algorithm to find strongly connected components (scc) of the graph. + // Each of SCCs is a cycle in the original graph. + // The graph of SCCs is a DAG, thus we can traverse it in a topological order. + let scc = self.scc(); + let mut allowed_prebuilds = SubTreeFilter::new(start); + for comp in &scc { + if comp.iter().any(|x| allowed_prebuilds.check(&key(*x))) { + allowed_prebuilds.allow(comp.iter().map(|x| key(*x))); + for package in comp { + let deps = self.package_dependencies_for_target_kind(*package, target_kind); + allowed_prebuilds.allow(deps.iter().map(|x| key(*x))); + } + } + } + + allowed_prebuilds.0 + } + + /// Return a vector where each element is a strongly connected component (scc) of the graph. + /// The order of node ids within each scc is arbitrary, + /// but the order of the sccs is their topological order. + fn scc(&self) -> Vec> { + kosaraju_scc(&self.graph) + .iter() + .map(|scc| scc.iter().copied().collect_vec()) + // We need to reverse the iterator here, as kosaraju algorithm returns + // the sccs in a postorder (reversed topological order). + .rev() + .collect_vec() + } +} + +#[derive(Debug, Default)] +struct SubTreeFilter(HashSet); + +impl SubTreeFilter { + fn new(allowed: Vec) -> Self { + Self(allowed.into_iter().collect()) + } + + fn allow>(&mut self, iter: I) { + self.0.extend(iter) + } + + fn check(&self, key: &T) -> bool { + self.0.contains(key) + } } #[derive(Debug, Default, Clone, PartialEq, Eq)] diff --git a/scarb/src/ops/metadata.rs b/scarb/src/ops/metadata.rs index 52abdc57e..4429dec6d 100644 --- a/scarb/src/ops/metadata.rs +++ b/scarb/src/ops/metadata.rs @@ -239,6 +239,7 @@ fn collect_cairo_compilation_unit_metadata( .map(|c| { m::CompilationUnitCairoPluginMetadataBuilder::default() .package(wrap_package_id(c.package.id)) + .prebuilt_allowed(c.prebuilt_allowed) .build() .unwrap() }) diff --git a/scarb/src/ops/resolve.rs b/scarb/src/ops/resolve.rs index 959538502..ae2cd9cbb 100644 --- a/scarb/src/ops/resolve.rs +++ b/scarb/src/ops/resolve.rs @@ -63,6 +63,33 @@ impl WorkspaceResolve { .map(|id| self.packages[id].clone()) .collect_vec() } + + /// Get all dependencies with allowed prebuilt macros for a given package. + pub fn allowed_prebuilt( + &self, + package: Package, + target_kind: &TargetKind, + ) -> Result { + let metadata = package.scarb_tool_metadata()?; + let allowed = metadata.allow_prebuilt_plugins.unwrap_or_default(); + let allowed = allowed + .into_iter() + .filter_map(|name| PackageName::try_new(name).ok()) + .map(|name| name.to_smol_str()) + .collect(); + let allowed = + self.resolve + .filter_subtrees(target_kind, allowed, |package_id: PackageId| { + package_id.name.to_smol_str() + }); + let allowed_prebuilds = AllowedPrebuiltFilter::new( + allowed + .into_iter() + .map(PackageName::new) + .collect::>(), + ); + Ok(allowed_prebuilds) + } } #[derive(Debug, Default)] @@ -185,6 +212,19 @@ async fn collect_packages_from_resolve_graph( Ok(packages) } +#[derive(Debug, Default)] +pub struct AllowedPrebuiltFilter(HashSet); + +impl AllowedPrebuiltFilter { + pub fn new(allowed: HashSet) -> Self { + Self(allowed) + } + + pub fn check(&self, package: &Package) -> bool { + self.0.contains(&package.id.name) + } +} + #[tracing::instrument(skip_all, level = "debug")] pub fn generate_compilation_units( resolve: &WorkspaceResolve, @@ -548,6 +588,9 @@ impl<'a> PackageSolutionCollector<'a> { target_kind: &TargetKind, ignore_cairo_version: bool, ) -> Result<(Vec, Vec)> { + let allowed_prebuilds = self + .resolve + .allowed_prebuilt(self.member.clone(), target_kind)?; let mut classes = self .resolve .solution_of(self.member.id, target_kind) @@ -602,12 +645,14 @@ impl<'a> PackageSolutionCollector<'a> { let cairo_plugins = cairo_plugins .into_iter() .map(|package| { + let prebuilt_allowed = allowed_prebuilds.check(&package); // 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()?; Ok(CompilationUnitCairoPlugin::builder() .package(package) .builtin(props.builtin) + .prebuilt_allowed(prebuilt_allowed) .build()) }) .collect::>>()?; diff --git a/scarb/tests/metadata.rs b/scarb/tests/metadata.rs index e33f801d5..de024602e 100644 --- a/scarb/tests/metadata.rs +++ b/scarb/tests/metadata.rs @@ -6,6 +6,7 @@ use itertools::Itertools; use serde_json::json; use scarb_metadata::{Cfg, DepKind, ManifestMetadataBuilder, Metadata, PackageMetadata}; +use scarb_test_support::cairo_plugin_project_builder::CairoPluginProjectBuilder; use scarb_test_support::command::{CommandExt, Scarb}; use scarb_test_support::fsx; use scarb_test_support::project_builder::{Dep, DepBuilder, ProjectBuilder}; @@ -1443,3 +1444,111 @@ fn can_enable_add_redeposit_gas() { .unwrap(); assert!(add_redeposit_gas); } + +#[test] +fn prebuilt_plugins_disallowed_by_default() { + let t = assert_fs::TempDir::new().unwrap(); + + CairoPluginProjectBuilder::default() + .name("q") + .scarb_project(|builder| { + builder + .name("q") + .version("1.0.0") + .manifest_extra("[cairo-plugin]") + }) + .build(&t.child("q")); + ProjectBuilder::start() + .name("y") + .version("1.0.0") + .lib_cairo(r"fn f() -> felt252 { z::f() }") + .dep_cairo_test() + .dep("q", Dep.path("../q")) + .build(&t.child("y")); + ProjectBuilder::start() + .name("x") + .version("1.0.0") + .lib_cairo(r"fn f() -> felt252 { y::f() }") + .dep_cairo_test() + .dep("y", Dep.path("y")) + .dep("q", Dep.path("q")) + .build(&t); + + let meta = Scarb::quick_snapbox() + .arg("--json") + .arg("metadata") + .arg("--format-version") + .arg("1") + .current_dir(&t) + .stdout_json::(); + + let cu = meta + .compilation_units + .iter() + .find(|cu| cu.target.name == "x") + .unwrap(); + + assert_eq!(cu.cairo_plugins.len(), 1); + assert!(cu.cairo_plugins[0].package.repr.starts_with("q")); + assert!(!cu.cairo_plugins[0].prebuilt_allowed.unwrap()); +} + +#[test] +fn can_allow_prebuilt_plugins_for_subtree() { + let t = assert_fs::TempDir::new().unwrap(); + + CairoPluginProjectBuilder::default() + .name("q") + .scarb_project(|builder| { + builder + .name("q") + .version("1.0.0") + .manifest_extra("[cairo-plugin]") + }) + .build(&t.child("q")); + + ProjectBuilder::start() + .name("z") + .version("1.0.0") + .lib_cairo(r"fn f() -> felt252 { q::f() }") + .dep_cairo_test() + .dep("q", Dep.path("../q")) + .build(&t.child("z")); + + ProjectBuilder::start() + .name("y") + .version("1.0.0") + .lib_cairo(r"fn f() -> felt252 { z::f() }") + .dep_cairo_test() + .dep("z", Dep.path("../z")) + .build(&t.child("y")); + + ProjectBuilder::start() + .name("x") + .version("1.0.0") + .lib_cairo(r"fn f() -> felt252 { y::f() }") + .manifest_extra(indoc! {r#" + [tool.scarb] + allow-prebuilt-plugins = ["y"] + "#}) + .dep_cairo_test() + .dep("z", Dep.path("z")) + .dep("y", Dep.path("y")) + .build(&t); + + let meta = Scarb::quick_snapbox() + .arg("--json") + .arg("metadata") + .arg("--format-version") + .arg("1") + .current_dir(&t) + .stdout_json::(); + let cu = meta + .compilation_units + .iter() + .find(|cu| cu.target.name == "x") + .unwrap(); + assert_eq!(cu.cairo_plugins.len(), 1); + assert!(cu.cairo_plugins[0].package.repr.starts_with("q")); + assert!(cu.cairo_plugins[0].prebuilt_allowed.unwrap()); +}