Skip to content

Commit

Permalink
fix: issues with external modules extra resolve
Browse files Browse the repository at this point in the history
  • Loading branch information
cwkang1998 committed Aug 30, 2024
1 parent 1c78f08 commit 3d70594
Show file tree
Hide file tree
Showing 17 changed files with 112 additions and 73 deletions.
28 changes: 23 additions & 5 deletions crates/voyager-resolver-cairo/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use cairo_lang_defs::ids::ModuleId;
use cairo_lang_filesystem::db::FilesGroup;
use cairo_lang_filesystem::ids::{CrateId, CrateLongId, FileLongId};
use camino::Utf8PathBuf;
use scarb_utils::get_external_nonlocal_packages;
use std::clone;
use std::collections::HashMap;
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -127,7 +129,6 @@ impl Compiler for VoyagerGenerator {

// Creates a graph where nodes are cairo modules, and edges are the dependencies between modules.
let graph = create_graph(&project_modules);
_display_graphviz(&graph);

// Read Scarb manifest file to get the list of contracts to verify from the [tool.voyager] section.
// This returns the relative file paths of the contracts to verify.
Expand Down Expand Up @@ -157,8 +158,21 @@ impl Compiler for VoyagerGenerator {
.filter(|m| contracts_to_verify.contains(&m.relative_filepath))
.collect::<Vec<_>>();

let external_packages = get_external_nonlocal_packages(metadata.clone());

let (required_modules_paths, attachment_modules_data) =
self.get_reduced_project(&graph, modules_to_verify)?;
self.get_reduced_project(&graph, modules_to_verify.clone())?;
println!("rmp {:?}", required_modules_paths.clone());

// Filter away packages that are external, imported from git repository & std.
let attachment_modules_data = attachment_modules_data
.iter()
.filter(|(k, _)| {
let base_package_name = &k.0.split("::").collect::<Vec<&str>>()[0];
!external_packages.contains(&base_package_name.to_string())
})
.map(|(k, v)| (k.clone(), v.clone()))
.collect::<HashMap<ModulePath, CairoAttachmentModule>>();

let target_dir = Utf8PathBuf::from(
manifest_path
Expand Down Expand Up @@ -188,15 +202,19 @@ impl Compiler for VoyagerGenerator {
// Get the Cairo Modules corresponding to the required modules paths.
let required_modules = project_modules
.iter()
.filter(|m| required_modules_paths.contains(&m.path))
.filter(|m| {
let base_package_name = &m.path.0.split("::").collect::<Vec<&str>>()[0].trim();
required_modules_paths.contains(&m.path)
&& !external_packages.contains(&base_package_name.to_string())
})
.collect::<Vec<_>>();
// Copy these modules in the target directory.
// Copy readme files and license files over too
copy_required_files(&required_modules, &target_dir, ws)?;

// Generate each of the scarb manifest files for the output directory.
// The dependencies are updated to include the required modules as local dependencies.
generate_scarb_updated_files(metadata, &target_dir, required_modules)?;
generate_scarb_updated_files(metadata, &target_dir, required_modules, external_packages)?;

let package_name = unit.main_component().package.id.name.to_string();
let generated_crate_dir = target_dir.path_existent().unwrap().join(package_name);
Expand Down Expand Up @@ -297,7 +315,6 @@ impl VoyagerGenerator {
&required_modules_paths,
imports_path_not_matching_resolved_path.clone(),
);
println!("{:?}\n {:?}\n {:?}", required_modules_paths, imports_path_not_matching_resolved_path.clone(), attachment_modules_data);

let unrequired_attachment_modules: HashMap<ModulePath, CairoAttachmentModule> =
attachment_modules_data
Expand All @@ -321,6 +338,7 @@ mod tests {
use cairo_lang_filesystem::ids::{CrateLongId, Directory};
use cairo_lang_semantic::plugin::PluginSuite;
use cairo_lang_starknet::plugin::StarkNetPlugin;
use scarb_metadata::Metadata;
use std::collections::HashSet;
use std::path::PathBuf;

Expand Down
3 changes: 2 additions & 1 deletion crates/voyager-resolver-cairo/src/compiler/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use anyhow::{anyhow, Context, Result};
use cairo_lang_compiler::db::RootDatabase;
use cairo_lang_defs::db::DefsGroup;
use cairo_lang_defs::ids::{
FileIndex, GenericTypeId, ModuleFileId, ModuleId, NamedLanguageElementId, TopLevelLanguageElementId, UseId, UseLongId
FileIndex, GenericTypeId, ModuleFileId, ModuleId, NamedLanguageElementId,
TopLevelLanguageElementId, UseId, UseLongId,
};
use cairo_lang_filesystem::db::FilesGroup;
use cairo_lang_filesystem::ids::{CrateId, Directory, FileId, FileLongId};
Expand Down
41 changes: 41 additions & 0 deletions crates/voyager-resolver-cairo/src/compiler/scarb_utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use anyhow::{anyhow, ensure, Context, Result};
use itertools::Itertools;
use scarb::flock::Filesystem;
use scarb_metadata::Metadata;
use serde::Deserialize;
use std::collections::HashSet;
use std::fs;
use std::path::{Path, PathBuf};
use std::str::FromStr;
Expand Down Expand Up @@ -60,6 +63,35 @@ pub fn read_scarb_metadata(manifest_path: &PathBuf) -> anyhow::Result<scarb_meta
.map_err(Into::into)
}

/// Extract non-local external packages, which comes from a source that is not
/// - local
/// - a std library
/// The extract list should only contain dependencies coming from
/// - package registry
/// - github registry
pub fn get_external_nonlocal_packages(metadata: Metadata) -> Vec<String> {
let mut package_set: HashSet<String> = HashSet::new();

for p in metadata.packages.iter() {
if p.source.repr != "std"
&& !p.source.repr.starts_with("path")
&& p.source.repr != "registry+https://there-is-no-default-registry-yet.com/"
{
package_set.insert(p.name.clone());
}
for dep in p.dependencies.clone() {
if dep.source.repr != "std"
&& !dep.source.repr.starts_with("path")
&& dep.source.repr != "registry+https://there-is-no-default-registry-yet.com/"
{
package_set.insert(dep.name.clone());
}
}
}

package_set.iter().map(|p| p.to_owned()).collect_vec()
}

/// Updates the crate roots in the compiler database using the metadata from a Scarb compilation.
/// The crate roots are set to the source roots of each compilation unit in the metadata.
/// The function does not return a value, but modifies the semantic group object referenced by `db`.
Expand Down Expand Up @@ -137,6 +169,7 @@ pub fn generate_scarb_updated_files(
scarb_metadata: scarb_metadata::Metadata,
target_dir: &Filesystem,
required_modules: Vec<&CairoModule>,
external_packages: Vec<String>,
) -> Result<()> {
let mut metadata = scarb_metadata.clone();
let required_packages = required_modules
Expand All @@ -158,6 +191,7 @@ pub fn generate_scarb_updated_files(
manifest_path.into_std_path_buf(),
target_path.as_std_path(),
&required_packages,
&external_packages,
)?;
}
Ok(())
Expand Down Expand Up @@ -186,6 +220,7 @@ pub fn generate_updated_scarb_toml(
manifest_path: PathBuf,
target_path: &Path,
required_packages: &[String],
external_packages: &[String],
) -> Result<()> {
let manifest_path = fs::canonicalize(manifest_path)?;
let original_raw_manifest = fs::read_to_string(&manifest_path)?;
Expand Down Expand Up @@ -213,6 +248,12 @@ pub fn generate_updated_scarb_toml(
if *k == "starknet" {
return;
}

// Ignore any external packages and keep as it is.
if external_packages.contains(&k.to_string()) {
return;
}

// remove unused dependencies
if !required_packages.contains(&k.to_string()) && *k != "starknet" {
tab.as_table_like_mut().unwrap().remove(k);
Expand Down
1 change: 0 additions & 1 deletion crates/voyager-resolver-cairo/src/model/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,3 @@ impl CairoAttachmentModule {
self.imports.insert(import);
}
}

35 changes: 34 additions & 1 deletion crates/voyager-resolver-cairo/tests/integration_test.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::any::Any;
use std::env;

use anyhow::{anyhow, Result};
Expand Down Expand Up @@ -209,3 +208,37 @@ fn test_project_with_simple_super_import() -> Result<()> {
run_scarb_build(reduced_project_path.to_str().unwrap()).unwrap();
Ok(())
}

#[test]
fn test_project_with_external_import_resolved() -> Result<()> {
let source_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("tests/test_data")
.join("import_external_deps_with_workspace");
let mut compilers: CompilerRepository = CompilerRepository::empty();
compilers.add(Box::new(VoyagerGenerator)).unwrap();

let manifest_path = source_dir.join("Scarb.toml");

let config = Config::builder(manifest_path.to_str().unwrap())
.ui_verbosity(Verbosity::Verbose)
.log_filter_directive(env::var_os("SCARB_LOG"))
.compilers(compilers)
.build()
.unwrap();

let ws = ops::read_workspace(config.manifest_path(), &config).unwrap_or_else(|err| {
eprintln!("error: {}", err);
std::process::exit(1);
});
let package_ids = ws.members().map(|p| p.id).collect();
let compile_opts = ops::CompileOpts {
include_targets: vec![TargetKind::STARKNET_CONTRACT],
exclude_targets: vec![],
};

ops::compile(package_ids, compile_opts, &ws).unwrap();

let reduced_project_path = source_dir.join("voyager-verify/import_external_deps_with_workspace");
run_scarb_build(reduced_project_path.to_str().unwrap()).unwrap();
Ok(())
}
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,18 @@
version = 1

[[package]]
name = "openzeppelin"
version = "0.13.0"
source = "git+https://github.com/OpenZeppelin/cairo-contracts.git?tag=v0.13.0#978b4e75209da355667d8954d2450e32bd71fe49"
name = "dependency"
version = "0.1.0"

[[package]]
name = "super_import_with_external_deps"
name = "import_external_deps_with_workspace"
version = "0.1.0"
dependencies = [
"dependency",
"openzeppelin",
]

[[package]]
name = "openzeppelin"
version = "0.13.0"
source = "git+https://github.com/OpenZeppelin/cairo-contracts.git?tag=v0.13.0#978b4e75209da355667d8954d2450e32bd71fe49"
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
[package]
name = "super_import_with_external_deps"
name = "import_external_deps_with_workspace"
version = "0.1.0"

# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest

[dependencies]
starknet = ">=2.3.0"
dependency = { path = "../dependency" }
openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", tag = "v0.13.0" }


Expand Down
2 changes: 1 addition & 1 deletion examples/cairo_ds/Scarb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ version = "0.1.0"
# See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest

[dependencies]
starknet = "=2.4.3"
starknet = "=2.6.3"
dependency = { path = "../dependency" }

[[target.starknet-contract]]
Expand Down
6 changes: 0 additions & 6 deletions examples/super_import/Scarb.lock

This file was deleted.

25 changes: 0 additions & 25 deletions examples/super_import/Scarb.toml

This file was deleted.

3 changes: 0 additions & 3 deletions examples/super_import/src/contracts.cairo

This file was deleted.

2 changes: 0 additions & 2 deletions examples/super_import/src/lib.cairo

This file was deleted.

21 changes: 0 additions & 21 deletions examples/super_import/src/simple.cairo

This file was deleted.

2 changes: 0 additions & 2 deletions examples/super_import_with_external_deps/.gitignore

This file was deleted.

0 comments on commit 3d70594

Please sign in to comment.