Skip to content

Commit

Permalink
Fix check command for cairo compilation units dependant on Scarb proc…
Browse files Browse the repository at this point in the history
… macros (#1780)

fix #1779

---------

Signed-off-by: maciektr <[email protected]>
Co-authored-by: Maksim Zdobnikau <[email protected]>
  • Loading branch information
maciektr and DelevoXDG authored Nov 27, 2024
1 parent 91fc0ad commit 121696d
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 6 deletions.
54 changes: 48 additions & 6 deletions scarb/src/ops/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use scarb_ui::args::FeaturesSpec;
use scarb_ui::components::Status;
use scarb_ui::HumanDuration;
use smol_str::{SmolStr, ToSmolStr};
use std::collections::HashSet;
use std::thread;

use crate::compiler::db::{build_scarb_root_database, has_starknet_plugin, ScarbDatabase};
Expand Down Expand Up @@ -97,12 +98,12 @@ impl CompileOpts {

#[tracing::instrument(skip_all, level = "debug")]
pub fn compile(packages: Vec<PackageId>, opts: CompileOpts, ws: &Workspace<'_>) -> Result<()> {
process(packages, opts, ws, compile_unit, None)
process(packages, opts, ws, compile_units, None)
}

#[tracing::instrument(skip_all, level = "debug")]
pub fn check(packages: Vec<PackageId>, opts: CompileOpts, ws: &Workspace<'_>) -> Result<()> {
process(packages, opts, ws, check_unit, Some("checking"))
process(packages, opts, ws, check_units, Some("checking"))
}

#[tracing::instrument(skip_all, level = "debug")]
Expand All @@ -114,7 +115,7 @@ fn process<F>(
operation_type: Option<&str>,
) -> Result<()>
where
F: FnMut(CompilationUnit, &Workspace<'_>) -> Result<()>,
F: FnMut(Vec<CompilationUnit>, &Workspace<'_>) -> Result<()>,
{
let resolve = ops::resolve_workspace(ws)?;
let packages_to_process = ws
Expand Down Expand Up @@ -155,9 +156,7 @@ where
})
.collect::<Vec<_>>();

for unit in compilation_units {
operation(unit, ws)?;
}
operation(compilation_units, ws)?;

let elapsed_time = HumanDuration(ws.config().elapsed_time());
let profile = ws.current_profile()?;
Expand All @@ -174,6 +173,13 @@ 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<CompilationUnit>, ws: &Workspace<'_>) -> Result<()> {
for unit in units {
compile_unit(unit, ws)?;
}
Ok(())
}

pub fn compile_unit(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> {
thread::scope(|s| {
thread::Builder::new()
Expand Down Expand Up @@ -217,6 +223,42 @@ fn compile_unit_inner(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> {
})
}

fn check_units(units: Vec<CompilationUnit>, ws: &Workspace<'_>) -> Result<()> {
// Select proc macro units that need to be compiled for Cairo compilation units.
let required_plugins = units
.iter()
.flat_map(|unit| match unit {
CompilationUnit::Cairo(unit) => unit
.cairo_plugins
.iter()
.map(|p| p.package.id)
.collect_vec(),
_ => Vec::new(),
})
.collect::<HashSet<PackageId>>();

// We guarantee that proc-macro units are always processed first,
// so that all required plugins are compiled before we start checking Cairo units.
let units = units.into_iter().sorted_by_key(|unit| {
if matches!(unit, CompilationUnit::ProcMacro(_)) {
0
} else {
1
}
});

for unit in units {
if matches!(unit, CompilationUnit::ProcMacro(_))
&& required_plugins.contains(&unit.main_package_id())
{
compile_unit(unit, ws)?;
} else {
check_unit(unit, ws)?;
}
}
Ok(())
}

fn check_unit(unit: CompilationUnit, ws: &Workspace<'_>) -> Result<()> {
let package_name = unit.main_package_id().name.clone();

Expand Down
33 changes: 33 additions & 0 deletions scarb/tests/build_cairo_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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::workspace_builder::WorkspaceBuilder;
use snapbox::assert_matches;

#[test]
Expand Down Expand Up @@ -70,6 +71,38 @@ fn check_cairo_plugin() {
);
}

#[test]
fn can_check_cairo_project_with_plugins() {
let temp = TempDir::new().unwrap();
let t = temp.child("some");
CairoPluginProjectBuilder::default().build(&t);
let project = temp.child("hello");
let y = project.child("other");
CairoPluginProjectBuilder::default().name("other").build(&y);
WorkspaceBuilder::start()
.add_member("other")
.package(
ProjectBuilder::start()
.name("hello")
.version("1.0.0")
.dep("some", &t),
)
.build(&project);
Scarb::quick_snapbox()
.arg("check")
// Disable output from Cargo.
.env("CARGO_TERM_QUIET", "true")
.current_dir(&project)
.assert()
.success()
.stdout_matches(indoc! {r#"
[..]Compiling some v1.0.0 ([..]Scarb.toml)
[..]Checking other v1.0.0 ([..]Scarb.toml)
[..]Checking hello v1.0.0 ([..]Scarb.toml)
[..]Finished checking `dev` profile target(s) in [..]
"#});
}

#[test]
fn resolve_fetched_plugins() {
let t = TempDir::new().unwrap();
Expand Down

0 comments on commit 121696d

Please sign in to comment.