Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allowed prebuilts list #1846

Merged
merged 1 commit into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scarb-metadata/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
3 changes: 3 additions & 0 deletions scarb-metadata/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>,

/// Additional data not captured by deserializer.
#[cfg_attr(feature = "builder", builder(default))]
#[serde(flatten)]
Expand Down
1 change: 1 addition & 0 deletions scarb/src/compiler/compilation_unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions scarb/src/core/manifest/toml_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<String>>,
}

impl TomlManifest {
pub fn read_from_path(path: &Utf8Path) -> Result<Self> {
let contents = fs::read_to_string(path)
Expand Down
11 changes: 10 additions & 1 deletion scarb/src/core/package/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -137,6 +137,15 @@ impl Package {
Ok(structured)
}

pub fn scarb_tool_metadata(&self) -> Result<TomlToolScarbMetadata> {
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
}
Expand Down
71 changes: 66 additions & 5 deletions scarb/src/core/resolver.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -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<T: Sized + Eq + Hash>(
&self,
target_kind: &TargetKind,
start: Vec<T>,
key: impl Fn(PackageId) -> T,
) -> HashSet<T> {
// 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<Vec<PackageId>> {
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<T: Sized + Eq + Hash>(HashSet<T>);

impl<T: Sized + Eq + Hash> SubTreeFilter<T> {
fn new(allowed: Vec<T>) -> Self {
Self(allowed.into_iter().collect())
}

fn allow<I: IntoIterator<Item = T>>(&mut self, iter: I) {
self.0.extend(iter)
}

fn check(&self, key: &T) -> bool {
self.0.contains(key)
}
}

#[derive(Debug, Default, Clone, PartialEq, Eq)]
Expand Down
1 change: 1 addition & 0 deletions scarb/src/ops/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Expand Down
45 changes: 45 additions & 0 deletions scarb/src/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<AllowedPrebuiltFilter> {
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::<HashSet<_>>(),
);
Ok(allowed_prebuilds)
}
}

#[derive(Debug, Default)]
Expand Down Expand Up @@ -185,6 +212,19 @@ async fn collect_packages_from_resolve_graph(
Ok(packages)
}

#[derive(Debug, Default)]
pub struct AllowedPrebuiltFilter(HashSet<PackageName>);

impl AllowedPrebuiltFilter {
pub fn new(allowed: HashSet<PackageName>) -> 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,
Expand Down Expand Up @@ -548,6 +588,9 @@ impl<'a> PackageSolutionCollector<'a> {
target_kind: &TargetKind,
ignore_cairo_version: bool,
) -> Result<(Vec<Package>, Vec<CompilationUnitCairoPlugin>)> {
let allowed_prebuilds = self
.resolve
.allowed_prebuilt(self.member.clone(), target_kind)?;
let mut classes = self
.resolve
.solution_of(self.member.id, target_kind)
Expand Down Expand Up @@ -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::<Result<Vec<_>>>()?;
Expand Down
109 changes: 109 additions & 0 deletions scarb/tests/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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::<Metadata>();

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::<Metadata>();
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());
}
Loading