Skip to content

Commit

Permalink
Prevent I/O while sourcing each path dependency (FuelLabs#3368)
Browse files Browse the repository at this point in the history
  • Loading branch information
kayagokalp authored Nov 30, 2022
1 parent b68aa9e commit 7bd2c63
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 17 deletions.
41 changes: 25 additions & 16 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
BuildProfile, ConfigTimeConstant, Dependency, ManifestFile, MemberManifestFiles,
PackageManifest, PackageManifestFile,
},
WorkspaceManifestFile, CORE, PRELUDE, STD,
CORE, PRELUDE, STD,
};
use anyhow::{anyhow, bail, Context, Error, Result};
use forc_util::{
Expand Down Expand Up @@ -791,7 +791,7 @@ fn validate_dep(
let dep_entry = node_manifest
.dep(dep_name)
.ok_or_else(|| anyhow!("no entry in parent manifest"))?;
let dep_source = dep_to_source_patched(node_manifest, dep_name, dep_entry)?;
let dep_source = dep_to_source_patched(node_manifest, dep_name, dep_entry, manifests)?;
let dep_pkg = graph[dep_node].unpinned(&dep_path);
if dep_pkg.source != dep_source {
bail!("dependency node's source does not match manifest entry");
Expand Down Expand Up @@ -1378,6 +1378,7 @@ fn fetch_graph(
offline,
graph,
manifest_map,
member_manifests,
)?);
}
Ok(added_nodes)
Expand All @@ -1401,6 +1402,7 @@ fn fetch_pkg_graph(
offline: bool,
graph: &mut Graph,
manifest_map: &mut ManifestMap,
member_manifests: &MemberManifestFiles,
) -> Result<HashSet<NodeIx>> {
// Retrieve the project node, or create one if it does not exist.
let proj_node = match find_proj_node(graph, &proj_manifest.project.name) {
Expand Down Expand Up @@ -1438,6 +1440,7 @@ fn fetch_pkg_graph(
manifest_map,
&mut fetched,
&mut visited,
member_manifests,
)
}

Expand All @@ -1454,6 +1457,7 @@ fn fetch_deps(
manifest_map: &mut ManifestMap,
fetched: &mut HashMap<Pkg, NodeIx>,
visited: &mut HashSet<NodeIx>,
member_manifests: &MemberManifestFiles,
) -> Result<HashSet<NodeIx>> {
let mut added = HashSet::default();
let parent_id = graph[node].id();
Expand All @@ -1471,7 +1475,7 @@ fn fetch_deps(
for (dep_name, dep, dep_kind) in deps {
let name = dep.package().unwrap_or(&dep_name).to_string();
let parent_manifest = &manifest_map[&parent_id];
let source = dep_to_source_patched(parent_manifest, &name, &dep)
let source = dep_to_source_patched(parent_manifest, &name, &dep, member_manifests)
.context("Failed to source dependency")?;

// If we haven't yet fetched this dependency, fetch it, pin it and add it to the graph.
Expand Down Expand Up @@ -1522,6 +1526,7 @@ fn fetch_deps(
manifest_map,
fetched,
visited,
member_manifests,
)?);
}
Ok(added)
Expand Down Expand Up @@ -1968,7 +1973,11 @@ where

/// Given the path to a package and a `Dependency` parsed from one of its forc dependencies,
/// produce the `Source` for that dependendency.
fn dep_to_source(pkg_path: &Path, dep: &Dependency) -> Result<Source> {
fn dep_to_source(
pkg_path: &Path,
dep: &Dependency,
member_manifests: &MemberManifestFiles,
) -> Result<Source> {
let source = match dep {
Dependency::Simple(ref ver_str) => {
bail!(
Expand All @@ -1985,15 +1994,13 @@ fn dep_to_source(pkg_path: &Path, dep: &Dependency) -> Result<Source> {
anyhow!("Failed to canonicalize dependency path {:?}: {}", path, e)
})?;
// Check if path is a member of a workspace.
let workspace_manifest = canonical_path
.parent()
.and_then(|parent_dir| WorkspaceManifestFile::from_dir(parent_dir).ok());

match workspace_manifest {
Some(ws) if ws.is_member_path(&canonical_path)? => {
Source::Member(canonical_path)
}
_ => Source::Path(canonical_path),
if member_manifests
.values()
.any(|pkg_manifest| pkg_manifest.dir() == canonical_path)
{
Source::Member(canonical_path)
} else {
Source::Path(canonical_path)
}
}
(_, _, Some(repo)) => {
Expand Down Expand Up @@ -2044,9 +2051,10 @@ fn apply_patch(
manifest: &PackageManifestFile,
dep_name: &str,
dep_source: &Source,
member_manifests: &MemberManifestFiles,
) -> Result<Source> {
match dep_source_patch(manifest, dep_name, dep_source) {
Some(patch) => dep_to_source(manifest.dir(), patch),
Some(patch) => dep_to_source(manifest.dir(), patch, member_manifests),
None => Ok(dep_source.clone()),
}
}
Expand All @@ -2057,9 +2065,10 @@ fn dep_to_source_patched(
manifest: &PackageManifestFile,
dep_name: &str,
dep: &Dependency,
member_manifests: &MemberManifestFiles,
) -> Result<Source> {
let unpatched = dep_to_source(manifest.dir(), dep)?;
apply_patch(manifest, dep_name, &unpatched)
let unpatched = dep_to_source(manifest.dir(), dep, member_manifests)?;
apply_patch(manifest, dep_name, &unpatched, member_manifests)
}

/// Given a `forc_pkg::BuildProfile`, produce the necessary `sway_core::BuildConfig` required for
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
category = "fail"

# check: dependency cycle detected: dependency_a -> dependency_b -> dependency_a
# check: dependency cycle detected: dependency_b -> dependency_a -> dependency_b

0 comments on commit 7bd2c63

Please sign in to comment.