From f985bf07b91008b86d7de5b8af9129e16ab9085a Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Tue, 14 Jun 2022 20:26:09 +0300 Subject: [PATCH 01/20] BuildPlan generation from lock file does not break when there is a dependency removal from manifest file. --- forc-pkg/src/pkg.rs | 66 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index f237d92e423..338b3cec263 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -163,6 +163,13 @@ pub struct BuildPlan { graph: Graph, path_map: PathMap, compilation_order: Vec, + /// The differences between graph described by lock_file and the graph that actually constructed. + /// + /// If the BuildPlan is constructed from a manifest file, this will be empty. + /// + /// If the BuildPlan is constructed from a lock file there might be some differences between the graph described by the lock file and the one that actually created. + /// The differences between them are the path dependencies that are removed from the manifest file after the lock file is created. + removed_deps: Vec, } /// Parameters to pass through to the `sway_core::BuildConfig` during compilation. @@ -218,7 +225,7 @@ pub type DependencyName = String; pub struct PkgDiff { pub added: Vec<(DependencyName, Pkg)>, - pub removed: Vec<(DependencyName, Pkg)>, + pub removed: Vec, } impl BuildPlan { @@ -231,6 +238,7 @@ impl BuildPlan { graph, path_map, compilation_order, + removed_deps: vec![], }) } @@ -271,18 +279,23 @@ impl BuildPlan { graph, path_map, compilation_order, + removed_deps: vec![], }) } /// Attempt to load the build plan from the `Lock`. pub fn from_lock(proj_path: &Path, lock: &Lock, sway_git_tag: &str) -> Result { - let graph = lock.to_graph()?; + let mut graph = lock.to_graph()?; + let tmp_compilation_order = compilation_order(&graph)?; + let (path_map, removed_deps) = + graph_to_path_map(proj_path, &mut graph, &tmp_compilation_order, sway_git_tag)?; + // Since path_map removes removed dependencies from the graph compilation order may change let compilation_order = compilation_order(&graph)?; - let path_map = graph_to_path_map(proj_path, &graph, &compilation_order, sway_git_tag)?; Ok(Self { graph, path_map, compilation_order, + removed_deps, }) } @@ -296,7 +309,7 @@ impl BuildPlan { /// Ensure that the build plan is valid for the given manifest. pub fn validate(&self, manifest: &Manifest, sway_git_tag: &str) -> Result { let mut added = vec![]; - let mut removed = vec![]; + let mut removed = self.removed_deps.clone(); // Retrieve project's graph node. let proj_node = *self .compilation_order @@ -345,11 +358,12 @@ impl BuildPlan { .into_iter() .map(|pkg| (pkg.0.clone(), pkg.1.clone())) .collect(); - removed = plan_dep_pkgs + let mut temp_removed = plan_dep_pkgs .difference(&manifest_dep_pkgs) .into_iter() - .map(|pkg| (pkg.0.clone(), pkg.1.clone())) + .map(|pkg| (pkg.0.clone())) .collect(); + removed.append(&mut temp_removed); } // Ensure the pkg names of all nodes match their associated manifests. @@ -366,7 +380,13 @@ impl BuildPlan { ); } } - Ok(PkgDiff { added, removed }) + println!("{:?}", added); + println!("{:?}", removed); + + Ok(PkgDiff { + added, + removed, + }) } /// View the build plan's compilation graph. @@ -392,7 +412,7 @@ fn remove_deps( graph: &mut Graph, path_map: &PathMap, proj_node: NodeIx, - to_remove: &[(DependencyName, Pkg)], + to_remove: &[DependencyName], ) { use petgraph::visit::Bfs; @@ -406,7 +426,7 @@ fn remove_deps( .is_none() || to_remove .iter() - .any(|removed_dep| removed_dep.1 == graph[node].unpinned(path_map)) + .any(|removed_dep| *removed_dep == graph[node].unpinned(path_map).name) { graph.remove_node(node); } @@ -754,11 +774,12 @@ pub fn compilation_order(graph: &Graph) -> Result> { /// containing the path to the local source for every node in the graph. pub fn graph_to_path_map( proj_manifest_dir: &Path, - graph: &Graph, + graph: &mut Graph, compilation_order: &[NodeIx], sway_git_tag: &str, -) -> Result { +) -> Result<(PathMap, Vec)> { let mut path_map = PathMap::new(); + let mut removed_deps = vec![]; // We resolve all paths in reverse compilation order. // That is, we follow paths starting from the project root. @@ -778,6 +799,27 @@ pub fn graph_to_path_map( // Resolve all following dependencies, knowing their parents' paths will already be resolved. for dep_node in path_resolve_order { let dep = &graph[dep_node]; + // If the dep is a path dependency check if it is removed. + if let SourcePinned::Path(_) = &dep.source { + // Retrieve the parent node to construct the relative path. + let (parent_node, _) = graph + .edges_directed(dep_node, Direction::Incoming) + .next() + .map(|edge| (edge.source(), edge.weight().clone())) + .ok_or_else(|| anyhow!("more than one root package detected in graph"))?; + let parent = &graph[parent_node]; + + // Construct the path relative to the parent's path. + let parent_path = &path_map[&parent.id()]; + let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; + + // Check if parent's manifest file includes this dependency. If not this is a removed dependency and we should continue with the following dep and remove the removed dependency from the graph. + if parent_manifest.dep(&dep.name).is_none() { + removed_deps.push(dep.name.clone()); + graph.remove_node(dep_node); + continue; + } + } let dep_path = match &dep.source { SourcePinned::Root => bail!("more than one root package detected in graph"), SourcePinned::Git(git) => { @@ -843,7 +885,7 @@ pub fn graph_to_path_map( path_map.insert(dep.id(), dep_path.canonicalize()?); } - Ok(path_map) + Ok((path_map, removed_deps)) } /// Given a `graph`, the node index of a path dependency within that `graph`, and the supposed From b4219a7c2c6185d0ba94e155a5392480c4ed27c3 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Thu, 16 Jun 2022 21:10:01 +0300 Subject: [PATCH 02/20] While removing the deleted nodes handle orphaned childs --- forc-pkg/src/pkg.rs | 36 +++++++++++++++++++++++------------- forc/src/ops/forc_build.rs | 1 + 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 338b3cec263..e6a75fbcd75 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -163,10 +163,10 @@ pub struct BuildPlan { graph: Graph, path_map: PathMap, compilation_order: Vec, - /// The differences between graph described by lock_file and the graph that actually constructed. - /// + /// The differences between graph described by lock_file and the graph that actually constructed. + /// /// If the BuildPlan is constructed from a manifest file, this will be empty. - /// + /// /// If the BuildPlan is constructed from a lock file there might be some differences between the graph described by the lock file and the one that actually created. /// The differences between them are the path dependencies that are removed from the manifest file after the lock file is created. removed_deps: Vec, @@ -380,13 +380,7 @@ impl BuildPlan { ); } } - println!("{:?}", added); - println!("{:?}", removed); - - Ok(PkgDiff { - added, - removed, - }) + Ok(PkgDiff { added, removed }) } /// View the build plan's compilation graph. @@ -784,7 +778,6 @@ pub fn graph_to_path_map( // We resolve all paths in reverse compilation order. // That is, we follow paths starting from the project root. let mut path_resolve_order = compilation_order.iter().cloned().rev(); - // Add the project's package to the map. let proj_node = path_resolve_order .next() @@ -807,15 +800,32 @@ pub fn graph_to_path_map( .next() .map(|edge| (edge.source(), edge.weight().clone())) .ok_or_else(|| anyhow!("more than one root package detected in graph"))?; - let parent = &graph[parent_node]; + + let parent = &graph[parent_node].clone(); // Construct the path relative to the parent's path. let parent_path = &path_map[&parent.id()]; let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; - // Check if parent's manifest file includes this dependency. If not this is a removed dependency and we should continue with the following dep and remove the removed dependency from the graph. + // Check if parent's manifest file includes this dependency. + // If not this is a removed dependency and we should continue with the following dep and remove the removed dependency from the graph. if parent_manifest.dep(&dep.name).is_none() { removed_deps.push(dep.name.clone()); + // We should be connecting the parent of the removed dependency to each child of removed node. + // Consider the following scenerio: a -> b -> c + // If you remove b, the edges between b -> c will be unvalidated so while processing c, you cannot find the parent. + // Instead you need to fetch all children of b and add an edge between those children and a before removing. + + // Get children of the node to delete + let child_nodes: Vec = graph + .edges_directed(dep_node, Direction::Outgoing) + .map(|edges| (edges.target())) + .collect(); + // Add an edge between the node that is going to be removed's child nodes and parent of the same node. + for child in child_nodes { + graph.add_edge(parent_node, child, parent.name.clone()); + } + // Remove the deleted node graph.remove_node(dep_node); continue; } diff --git a/forc/src/ops/forc_build.rs b/forc/src/ops/forc_build.rs index 06e4ff18cc4..92360bfcda9 100644 --- a/forc/src/ops/forc_build.rs +++ b/forc/src/ops/forc_build.rs @@ -91,6 +91,7 @@ pub fn build(command: BuildCommand) -> Result { let mut new_lock_cause = None; let mut plan = plan_result.or_else(|e| -> Result { if locked { + println!("{}", e); bail!( "The lock file {} needs to be updated but --locked was passed to prevent this.", lock_path.to_string_lossy() From 9495057008985ada19c15b75bfec529b7810ed51 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Thu, 16 Jun 2022 21:15:43 +0300 Subject: [PATCH 03/20] removed debug println --- forc/src/ops/forc_build.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/forc/src/ops/forc_build.rs b/forc/src/ops/forc_build.rs index 92360bfcda9..06e4ff18cc4 100644 --- a/forc/src/ops/forc_build.rs +++ b/forc/src/ops/forc_build.rs @@ -91,7 +91,6 @@ pub fn build(command: BuildCommand) -> Result { let mut new_lock_cause = None; let mut plan = plan_result.or_else(|e| -> Result { if locked { - println!("{}", e); bail!( "The lock file {} needs to be updated but --locked was passed to prevent this.", lock_path.to_string_lossy() From e97c684ee75e42778e9980dd1230b8b2f48c717e Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Fri, 17 Jun 2022 19:10:45 +0300 Subject: [PATCH 04/20] removed_deps removed from BuildPlan --- forc-pkg/src/pkg.rs | 136 +++++++++++++++++++++++++------------ forc/src/ops/forc_build.rs | 11 ++- 2 files changed, 102 insertions(+), 45 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index e6a75fbcd75..3d24fc653b5 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -5,7 +5,7 @@ use crate::{ use anyhow::{anyhow, bail, Context, Error, Result}; use forc_util::{ find_file_name, git_checkouts_directory, kebab_to_snake_case, print_on_failure, - print_on_success, print_on_success_library, println_yellow_err, + print_on_success, print_on_success_library, println_red, println_yellow_err, }; use fuels_types::JsonABI; use petgraph::{ @@ -163,13 +163,6 @@ pub struct BuildPlan { graph: Graph, path_map: PathMap, compilation_order: Vec, - /// The differences between graph described by lock_file and the graph that actually constructed. - /// - /// If the BuildPlan is constructed from a manifest file, this will be empty. - /// - /// If the BuildPlan is constructed from a lock file there might be some differences between the graph described by the lock file and the one that actually created. - /// The differences between them are the path dependencies that are removed from the manifest file after the lock file is created. - removed_deps: Vec, } /// Parameters to pass through to the `sway_core::BuildConfig` during compilation. @@ -238,7 +231,6 @@ impl BuildPlan { graph, path_map, compilation_order, - removed_deps: vec![], }) } @@ -279,37 +271,54 @@ impl BuildPlan { graph, path_map, compilation_order, - removed_deps: vec![], }) } /// Attempt to load the build plan from the `Lock`. - pub fn from_lock(proj_path: &Path, lock: &Lock, sway_git_tag: &str) -> Result { + pub fn from_lock( + proj_path: &Path, + lock: &Lock, + sway_git_tag: &str, + ) -> Result<(Self, Vec)> { let mut graph = lock.to_graph()?; let tmp_compilation_order = compilation_order(&graph)?; let (path_map, removed_deps) = - graph_to_path_map(proj_path, &mut graph, &tmp_compilation_order, sway_git_tag)?; + graph_to_path_map(proj_path, &graph, &tmp_compilation_order, sway_git_tag)?; + let removed_deps_vec = removed_deps + .clone() + .into_iter() + .map(|dep| graph[dep].name.clone()) + .collect(); + // Remove the unresolveable path dependencies from the graph + apply_diff_after_lock(removed_deps, &mut graph, &path_map, sway_git_tag)?; // Since path_map removes removed dependencies from the graph compilation order may change let compilation_order = compilation_order(&graph)?; - Ok(Self { - graph, - path_map, - compilation_order, - removed_deps, - }) + Ok(( + Self { + graph, + path_map, + compilation_order, + }, + removed_deps_vec, + )) } /// Attempt to load the build plan from the `Forc.lock` file. - pub fn from_lock_file(lock_path: &Path, sway_git_tag: &str) -> Result { + pub fn from_lock_file(lock_path: &Path, sway_git_tag: &str) -> Result<(Self, Vec)> { let proj_path = lock_path.parent().unwrap(); let lock = Lock::from_path(lock_path)?; Self::from_lock(proj_path, &lock, sway_git_tag) } /// Ensure that the build plan is valid for the given manifest. - pub fn validate(&self, manifest: &Manifest, sway_git_tag: &str) -> Result { + pub fn validate( + &self, + removed: Vec, + manifest: &Manifest, + sway_git_tag: &str, + ) -> Result { let mut added = vec![]; - let mut removed = self.removed_deps.clone(); + let mut removed = removed; // Retrieve project's graph node. let proj_node = *self .compilation_order @@ -764,16 +773,75 @@ pub fn compilation_order(graph: &Graph) -> Result> { }) } +/// Applies the differences between the graph that is described by the lock file and the graph that +/// is actually possible to build. +/// +/// +/// There might be some differences between the graph that is described by the lock file and +/// the graph that can actually possible to build. This is happening in the case of a dependency +/// (described by path) removal after the lock file is generated. Since dependency is removed +/// graph_to_path_map cannot find the required path for the dependency entry in the lock file. +/// This requires us to remove those removed nodes from the graph so that we can have a BuildPlan +/// generated from lock file. After the BuildPlan is generated it can be used to apply diffs by +/// apply_pkg_diff(). +fn apply_diff_after_lock( + deps_to_remove: HashSet, + graph: &mut Graph, + path_map: &PathMap, + sway_git_tag: &str, +) -> Result<()> { + for dep in deps_to_remove { + let dep_name = &graph[dep].name.clone(); + + let (parent_node, _) = graph + .edges_directed(dep, Direction::Incoming) + .next() + .map(|edge| (edge.source(), edge.weight().clone())) + .ok_or_else(|| anyhow!("more than one root package detected in graph"))?; + + let parent = &graph[parent_node].clone(); + // Construct the path relative to the parent's path. + let parent_path = &path_map[&parent.id()]; + let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; + + // Check if parent's manifest file includes this dependency. + // If not this is a removed dependency and we should remove the removed dependency from the graph. + if parent_manifest.dep(dep_name).is_none() { + // We should be connecting the parent of the removed dependency to each child of removed node. + // Consider the following scenerio: a -> b -> c + // If you remove b, the edges between b -> c will be unvalidated so while processing c, you cannot find the parent. + // Instead you need to fetch all children of b and add an edge between those children and a before removing. + + // Get children of the node to delete + let child_nodes: Vec = graph + .edges_directed(dep, Direction::Outgoing) + .map(|edges| (edges.target())) + .collect(); + // Add an edge between the node that is going to be removed's child nodes and parent of the same node. + for child in child_nodes { + // Check if there is already an edge if there is not add it + if !graph.edges(parent_node).any(|edge| edge.target() == child) { + graph.add_edge(parent_node, child, graph[child].name.clone()); + } + } + println_red(&format!(" Removing {}", dep_name)); + // Remove the deleted node + graph.remove_node(dep); + } + } + Ok(()) +} + /// Given graph of pinned dependencies and the directory for the root node, produce a path map /// containing the path to the local source for every node in the graph. pub fn graph_to_path_map( proj_manifest_dir: &Path, - graph: &mut Graph, + graph: &Graph, compilation_order: &[NodeIx], sway_git_tag: &str, -) -> Result<(PathMap, Vec)> { +) -> Result<(PathMap, HashSet)> { let mut path_map = PathMap::new(); - let mut removed_deps = vec![]; + let mut removed_deps = HashSet::new(); // We resolve all paths in reverse compilation order. // That is, we follow paths starting from the project root. @@ -808,25 +876,9 @@ pub fn graph_to_path_map( let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; // Check if parent's manifest file includes this dependency. - // If not this is a removed dependency and we should continue with the following dep and remove the removed dependency from the graph. + // If not this is a removed dependency and we should continue with the following dep if parent_manifest.dep(&dep.name).is_none() { - removed_deps.push(dep.name.clone()); - // We should be connecting the parent of the removed dependency to each child of removed node. - // Consider the following scenerio: a -> b -> c - // If you remove b, the edges between b -> c will be unvalidated so while processing c, you cannot find the parent. - // Instead you need to fetch all children of b and add an edge between those children and a before removing. - - // Get children of the node to delete - let child_nodes: Vec = graph - .edges_directed(dep_node, Direction::Outgoing) - .map(|edges| (edges.target())) - .collect(); - // Add an edge between the node that is going to be removed's child nodes and parent of the same node. - for child in child_nodes { - graph.add_edge(parent_node, child, parent.name.clone()); - } - // Remove the deleted node - graph.remove_node(dep_node); + removed_deps.insert(dep_node); continue; } } diff --git a/forc/src/ops/forc_build.rs b/forc/src/ops/forc_build.rs index 06e4ff18cc4..039d5f4e4c8 100644 --- a/forc/src/ops/forc_build.rs +++ b/forc/src/ops/forc_build.rs @@ -77,7 +77,13 @@ pub fn build(command: BuildCommand) -> Result { let lock_path = lock_path(manifest.dir()); - let plan_result = pkg::BuildPlan::from_lock_file(&lock_path, SWAY_GIT_TAG); + let from_lock = pkg::BuildPlan::from_lock_file(&lock_path, SWAY_GIT_TAG); + let removed_deps = from_lock + .as_ref() + .map(|from_lock| from_lock.1.clone()) + .unwrap_or_default(); + + let plan_result = from_lock.map(|from_lock| from_lock.0); // Retrieve the old lock file state so we can produce a diff. let old_lock = plan_result @@ -85,7 +91,6 @@ pub fn build(command: BuildCommand) -> Result { .ok() .map(|plan| Lock::from_graph(plan.graph())) .unwrap_or_default(); - // Check if there are any errors coming from the BuildPlan generation from the lock file // If there are errors we will need to create the BuildPlan from scratch, i.e fetch & pin everything let mut new_lock_cause = None; @@ -108,7 +113,7 @@ pub fn build(command: BuildCommand) -> Result { // If there are no issues with the BuildPlan generated from the lock file // Check and apply the diff. if new_lock_cause.is_none() { - let diff = plan.validate(&manifest, SWAY_GIT_TAG)?; + let diff = plan.validate(removed_deps, &manifest, SWAY_GIT_TAG)?; if !diff.added.is_empty() || !diff.removed.is_empty() { new_lock_cause = Some(anyhow!("lock file did not match manifest `diff`")); plan = plan.apply_pkg_diff(diff, SWAY_GIT_TAG, offline)?; From 38af42ef5e64ab6ba5757a989d7152f07d186f60 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Fri, 17 Jun 2022 21:23:35 +0300 Subject: [PATCH 05/20] While checking from parent's manifest check for package name as well --- forc-pkg/src/pkg.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 3d24fc653b5..2fca89d5a54 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -870,14 +870,19 @@ pub fn graph_to_path_map( .ok_or_else(|| anyhow!("more than one root package detected in graph"))?; let parent = &graph[parent_node].clone(); - // Construct the path relative to the parent's path. let parent_path = &path_map[&parent.id()]; let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; // Check if parent's manifest file includes this dependency. // If not this is a removed dependency and we should continue with the following dep - if parent_manifest.dep(&dep.name).is_none() { + // Note that we need to check from package name as well since dependency name and package name is not the same thing + if parent_manifest.dep(&dep.name).is_none() + && !parent_manifest + .deps() + .filter_map(|dep| dep.1.package()) + .any(|package| package == &dep.name) + { removed_deps.insert(dep_node); continue; } From e469399f7ed052173006b928294022ff28620d42 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Fri, 17 Jun 2022 21:25:06 +0300 Subject: [PATCH 06/20] removed unnecessary reference --- forc-pkg/src/pkg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 2fca89d5a54..6d5e951c069 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -881,7 +881,7 @@ pub fn graph_to_path_map( && !parent_manifest .deps() .filter_map(|dep| dep.1.package()) - .any(|package| package == &dep.name) + .any(|package| package == dep.name) { removed_deps.insert(dep_node); continue; From 4be588085d67c79e2d16252ad38640e6d5392fd9 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 20 Jun 2022 12:21:09 +0300 Subject: [PATCH 07/20] split validate into generate_diff and validate --- forc-pkg/src/pkg.rs | 30 +++++++++++++++++++----------- forc/src/ops/forc_build.rs | 4 ++-- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 6d5e951c069..cba0212f385 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -237,10 +237,11 @@ impl BuildPlan { /// Create a new build plan from an existing one. Needs the difference with the existing plan with the lock. pub fn apply_pkg_diff( &self, - pkg_diff: PkgDiff, + pkg_diff: &PkgDiff, sway_git_tag: &str, offline_mode: bool, ) -> Result { + self.validate(sway_git_tag)?; let mut graph = self.graph.clone(); let mut path_map = self.path_map.clone(); @@ -249,7 +250,7 @@ impl BuildPlan { .last() .ok_or_else(|| anyhow!("Invalid Graph"))?; let PkgDiff { added, removed } = pkg_diff; - remove_deps(&mut graph, &path_map, proj_node, &removed); + remove_deps(&mut graph, &path_map, proj_node, removed); let mut visited_map: HashMap = graph .node_references() @@ -261,12 +262,13 @@ impl BuildPlan { &mut graph, &mut path_map, &self.compilation_order, - &added, + added, sway_git_tag, offline_mode, &mut visited_map, )?; let compilation_order = compilation_order(&graph)?; + self.validate(sway_git_tag)?; Ok(Self { graph, path_map, @@ -279,7 +281,7 @@ impl BuildPlan { proj_path: &Path, lock: &Lock, sway_git_tag: &str, - ) -> Result<(Self, Vec)> { + ) -> Result<(Self, Vec)> { let mut graph = lock.to_graph()?; let tmp_compilation_order = compilation_order(&graph)?; let (path_map, removed_deps) = @@ -304,18 +306,19 @@ impl BuildPlan { } /// Attempt to load the build plan from the `Forc.lock` file. - pub fn from_lock_file(lock_path: &Path, sway_git_tag: &str) -> Result<(Self, Vec)> { + pub fn from_lock_file( + lock_path: &Path, + sway_git_tag: &str, + ) -> Result<(Self, Vec)> { let proj_path = lock_path.parent().unwrap(); let lock = Lock::from_path(lock_path)?; Self::from_lock(proj_path, &lock, sway_git_tag) } - /// Ensure that the build plan is valid for the given manifest. - pub fn validate( + pub fn generate_diff( &self, - removed: Vec, + removed: Vec, manifest: &Manifest, - sway_git_tag: &str, ) -> Result { let mut added = vec![]; let mut removed = removed; @@ -347,7 +350,7 @@ impl BuildPlan { if det.version.is_some() { println_yellow_err(&format!( " WARNING! Dependency \"{}\" specifies the unused `version` field: \ - consider using `branch` or `tag` instead", + consider using `branch` or `tag` instead", dep_name )); } @@ -375,6 +378,11 @@ impl BuildPlan { removed.append(&mut temp_removed); } + Ok(PkgDiff { added, removed }) + } + + /// Ensure that the build plan is valid for the given manifest. + fn validate(&self, sway_git_tag: &str) -> Result<()> { // Ensure the pkg names of all nodes match their associated manifests. for node in self.graph.node_indices() { let pkg = &self.graph[node]; @@ -389,7 +397,7 @@ impl BuildPlan { ); } } - Ok(PkgDiff { added, removed }) + Ok(()) } /// View the build plan's compilation graph. diff --git a/forc/src/ops/forc_build.rs b/forc/src/ops/forc_build.rs index 039d5f4e4c8..b46212ca933 100644 --- a/forc/src/ops/forc_build.rs +++ b/forc/src/ops/forc_build.rs @@ -113,10 +113,10 @@ pub fn build(command: BuildCommand) -> Result { // If there are no issues with the BuildPlan generated from the lock file // Check and apply the diff. if new_lock_cause.is_none() { - let diff = plan.validate(removed_deps, &manifest, SWAY_GIT_TAG)?; + let diff = plan.generate_diff(removed_deps, &manifest)?; if !diff.added.is_empty() || !diff.removed.is_empty() { new_lock_cause = Some(anyhow!("lock file did not match manifest `diff`")); - plan = plan.apply_pkg_diff(diff, SWAY_GIT_TAG, offline)?; + plan = plan.apply_pkg_diff(&diff, SWAY_GIT_TAG, offline)?; } } From aa90853fa005084f9b21eb0829e75123c1317eaf Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 20 Jun 2022 14:28:43 +0300 Subject: [PATCH 08/20] apply_pkg_diff visits nodes to delete in reverse BFS order --- forc-pkg/src/pkg.rs | 54 +++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index cba0212f385..0a784878e69 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -286,14 +286,26 @@ impl BuildPlan { let tmp_compilation_order = compilation_order(&graph)?; let (path_map, removed_deps) = graph_to_path_map(proj_path, &graph, &tmp_compilation_order, sway_git_tag)?; - let removed_deps_vec = removed_deps + let removed_deps_vec: Vec = removed_deps .clone() .into_iter() .map(|dep| graph[dep].name.clone()) .collect(); + // Get the project node from compilation order. + let project_node = *tmp_compilation_order + .last() + .ok_or_else(|| anyhow!("can't get the project node"))?; + // Remove the unresolveable path dependencies from the graph - apply_diff_after_lock(removed_deps, &mut graph, &path_map, sway_git_tag)?; - // Since path_map removes removed dependencies from the graph compilation order may change + apply_diff_after_lock( + removed_deps, + &mut graph, + project_node, + &path_map, + sway_git_tag, + )?; + + // Since apply_diff_after_lock removes removed dependencies from the graph compilation order may change let compilation_order = compilation_order(&graph)?; Ok(( Self { @@ -306,6 +318,8 @@ impl BuildPlan { } /// Attempt to load the build plan from the `Forc.lock` file. + /// Return the best effor BuildPlan and the packages removed + /// from project's manifest file after the lock file is generated. pub fn from_lock_file( lock_path: &Path, sway_git_tag: &str, @@ -795,14 +809,21 @@ pub fn compilation_order(graph: &Graph) -> Result> { fn apply_diff_after_lock( deps_to_remove: HashSet, graph: &mut Graph, + project_node: NodeIx, path_map: &PathMap, sway_git_tag: &str, ) -> Result<()> { - for dep in deps_to_remove { - let dep_name = &graph[dep].name.clone(); + use petgraph::visit::{Bfs, Walker}; + // Visit all the nodes after their parent is visited. + let deps: Vec = Bfs::new(&*graph, project_node).iter(&*graph).collect(); + + // Reverse the order so that we are sure we will get to a child node before it's parent. + // This is needed because if we remove parent of a node before itself we cannot find and check parent's manifest. + for dep in deps.iter().rev().filter(|dep| deps_to_remove.contains(dep)) { + let dep_name = &graph[*dep].name.clone(); let (parent_node, _) = graph - .edges_directed(dep, Direction::Incoming) + .edges_directed(*dep, Direction::Incoming) .next() .map(|edge| (edge.source(), edge.weight().clone())) .ok_or_else(|| anyhow!("more than one root package detected in graph"))?; @@ -814,27 +835,12 @@ fn apply_diff_after_lock( // Check if parent's manifest file includes this dependency. // If not this is a removed dependency and we should remove the removed dependency from the graph. + + // After #1836 is merged, we will also need to check if it is inside patches section of the manifest. if parent_manifest.dep(dep_name).is_none() { - // We should be connecting the parent of the removed dependency to each child of removed node. - // Consider the following scenerio: a -> b -> c - // If you remove b, the edges between b -> c will be unvalidated so while processing c, you cannot find the parent. - // Instead you need to fetch all children of b and add an edge between those children and a before removing. - - // Get children of the node to delete - let child_nodes: Vec = graph - .edges_directed(dep, Direction::Outgoing) - .map(|edges| (edges.target())) - .collect(); - // Add an edge between the node that is going to be removed's child nodes and parent of the same node. - for child in child_nodes { - // Check if there is already an edge if there is not add it - if !graph.edges(parent_node).any(|edge| edge.target() == child) { - graph.add_edge(parent_node, child, graph[child].name.clone()); - } - } println_red(&format!(" Removing {}", dep_name)); // Remove the deleted node - graph.remove_node(dep); + graph.remove_node(*dep); } } Ok(()) From 316a871134cbaf81022055f15810207f578e017c Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 20 Jun 2022 16:36:29 +0300 Subject: [PATCH 09/20] docs added for generate_diff --- forc-pkg/src/pkg.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 0a784878e69..13030f67213 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -277,6 +277,10 @@ impl BuildPlan { } /// Attempt to load the build plan from the `Lock`. + /// Returns the best effor BuildPlan and the packages removed + /// from project's manifest file after the lock file is generated. + /// + /// The returned removed dependencies are already removed from the dependency graph. pub fn from_lock( proj_path: &Path, lock: &Lock, @@ -318,8 +322,10 @@ impl BuildPlan { } /// Attempt to load the build plan from the `Forc.lock` file. - /// Return the best effor BuildPlan and the packages removed + /// Returns the best effor BuildPlan and the packages removed /// from project's manifest file after the lock file is generated. + /// + /// The returned removed dependencies are already removed from the dependency graph. pub fn from_lock_file( lock_path: &Path, sway_git_tag: &str, @@ -329,6 +335,17 @@ impl BuildPlan { Self::from_lock(proj_path, &lock, sway_git_tag) } + /// Attempt to find the difference between the graph constructed from the lock file + /// and the graph that is described by the manifest file. The difference is used to + /// update BuildPlan. + /// + /// The `removed` is the dependencies that needed to be removed by the BuildPlan::from_lock + /// in order to construct the BuildPlan from the lock file. The removal is needed once there + /// is a dependency (provided as a path dependency) removed from the manifest file after the + /// lock file is generated. + /// + /// generate_diff combines those removed packages with other types of dependencies (ex: git dependencies) + /// and returnes the complete set of differences between the lock file and manifest file. pub fn generate_diff( &self, removed: Vec, From 38cc164d07a4e8a03db31d0a19d61e28cb7af775 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Thu, 23 Jun 2022 12:26:15 +0300 Subject: [PATCH 10/20] removed unnecessary logic in apply_diff_after_lock --- forc-pkg/src/pkg.rs | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index daadce19a63..943aff79cf0 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -886,35 +886,11 @@ fn apply_diff_after_lock( path_map: &PathMap, sway_git_tag: &str, ) -> Result<()> { - use petgraph::visit::{Bfs, Walker}; - // Visit all the nodes after their parent is visited. - let deps: Vec = Bfs::new(&*graph, project_node).iter(&*graph).collect(); - - // Reverse the order so that we are sure we will get to a child node before it's parent. - // This is needed because if we remove parent of a node before itself we cannot find and check parent's manifest. - for dep in deps.iter().rev().filter(|dep| deps_to_remove.contains(dep)) { + for dep in deps_to_remove { let dep_name = &graph[*dep].name.clone(); - - let (parent_node, _) = graph - .edges_directed(*dep, Direction::Incoming) - .next() - .map(|edge| (edge.source(), edge.weight().clone())) - .ok_or_else(|| anyhow!("more than one root package detected in graph"))?; - - let parent = &graph[parent_node].clone(); - // Construct the path relative to the parent's path. - let parent_path = &path_map[&parent.id()]; - let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; - - // Check if parent's manifest file includes this dependency. - // If not this is a removed dependency and we should remove the removed dependency from the graph. - - // After #1836 is merged, we will also need to check if it is inside patches section of the manifest. - if parent_manifest.dep(dep_name).is_none() { - println_red(&format!(" Removing {}", dep_name)); - // Remove the deleted node - graph.remove_node(*dep); - } + println_red(&format!(" Removing {}", dep_name)); + // Remove the deleted node + graph.remove_node(*dep); } Ok(()) } From 344ceaa4a605ba4a4103431b32e375b58a6082c4 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Thu, 23 Jun 2022 12:45:17 +0300 Subject: [PATCH 11/20] doc updates --- forc-pkg/src/pkg.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 943aff79cf0..3ccb5d74193 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -383,7 +383,7 @@ impl BuildPlan { } /// Attempt to load the build plan from the `Forc.lock` file. - /// Returns the best effor BuildPlan and the packages removed + /// Returns the best effort BuildPlan and the packages removed /// from project's manifest file after the lock file is generated. /// /// The returned removed dependencies are already removed from the dependency graph. @@ -399,14 +399,6 @@ impl BuildPlan { /// Attempt to find the difference between the graph constructed from the lock file /// and the graph that is described by the manifest file. The difference is used to /// update BuildPlan. - /// - /// The `removed` is the dependencies that needed to be removed by the BuildPlan::from_lock - /// in order to construct the BuildPlan from the lock file. The removal is needed once there - /// is a dependency (provided as a path dependency) removed from the manifest file after the - /// lock file is generated. - /// - /// generate_diff combines those removed packages with other types of dependencies (ex: git dependencies) - /// and returnes the complete set of differences between the lock file and manifest file. pub fn generate_diff(&self, manifest: &Manifest) -> Result { let mut added = vec![]; let mut removed = vec![]; @@ -871,6 +863,7 @@ pub fn compilation_order(graph: &Graph) -> Result> { /// Applies the differences between the graph that is described by the lock file and the graph that /// is actually possible to build. /// +/// `deps_to_remove` : Indices of path dependencies that are removed from manifest file after the lock file is generated. /// /// There might be some differences between the graph that is described by the lock file and /// the graph that can actually possible to build. This is happening in the case of a dependency From cb255e281c0704ff97c37fce05d98377f6c34fef Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Thu, 23 Jun 2022 12:47:40 +0300 Subject: [PATCH 12/20] clippy && extra dereferencing removed --- forc-pkg/src/pkg.rs | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 3ccb5d74193..c5f23bfd519 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -356,19 +356,9 @@ impl BuildPlan { .into_iter() .map(|dep| graph[dep].name.clone()) .collect(); - // Get the project node from compilation order. - let project_node = *tmp_compilation_order - .last() - .ok_or_else(|| anyhow!("can't get the project node"))?; // Remove the unresolveable path dependencies from the graph - apply_diff_after_lock( - removed_deps, - &mut graph, - project_node, - &path_map, - sway_git_tag, - )?; + apply_diff_after_lock(removed_deps, &mut graph)?; // Since apply_diff_after_lock removes removed dependencies from the graph compilation order may change let compilation_order = compilation_order(&graph)?; @@ -872,18 +862,12 @@ pub fn compilation_order(graph: &Graph) -> Result> { /// This requires us to remove those removed nodes from the graph so that we can have a BuildPlan /// generated from lock file. After the BuildPlan is generated it can be used to apply diffs by /// apply_pkg_diff(). -fn apply_diff_after_lock( - deps_to_remove: HashSet, - graph: &mut Graph, - project_node: NodeIx, - path_map: &PathMap, - sway_git_tag: &str, -) -> Result<()> { +fn apply_diff_after_lock(deps_to_remove: HashSet, graph: &mut Graph) -> Result<()> { for dep in deps_to_remove { - let dep_name = &graph[*dep].name.clone(); + let dep_name = &graph[dep].name.clone(); println_red(&format!(" Removing {}", dep_name)); // Remove the deleted node - graph.remove_node(*dep); + graph.remove_node(dep); } Ok(()) } From cb6a2e00fb995d121fee36119f0e437bf229e0d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kaya=20G=C3=B6kalp?= Date: Thu, 23 Jun 2022 12:49:21 +0300 Subject: [PATCH 13/20] Apply suggestions from code review effor -> effort Co-authored-by: mitchmindtree --- forc-pkg/src/pkg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index c5f23bfd519..7e6eb10d793 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -338,7 +338,7 @@ impl BuildPlan { } /// Attempt to load the build plan from the `Lock`. - /// Returns the best effor BuildPlan and the packages removed + /// Returns the best effort BuildPlan and the packages removed /// from project's manifest file after the lock file is generated. /// /// The returned removed dependencies are already removed from the dependency graph. From bb40017d86a73ed762d12cd2513dfd9d4e4b392c Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 27 Jun 2022 10:38:15 +0300 Subject: [PATCH 14/20] duplicate code removed --- forc-pkg/src/pkg.rs | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 7e6eb10d793..b3c62952938 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -900,33 +900,6 @@ pub fn graph_to_path_map( // Resolve all following dependencies, knowing their parents' paths will already be resolved. for dep_node in path_resolve_order { let dep = &graph[dep_node]; - // If the dep is a path dependency check if it is removed. - if let SourcePinned::Path(_) = &dep.source { - // Retrieve the parent node to construct the relative path. - let (parent_node, _) = graph - .edges_directed(dep_node, Direction::Incoming) - .next() - .map(|edge| (edge.source(), edge.weight().clone())) - .ok_or_else(|| anyhow!("more than one root package detected in graph"))?; - - let parent = &graph[parent_node].clone(); - // Construct the path relative to the parent's path. - let parent_path = &path_map[&parent.id()]; - let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; - - // Check if parent's manifest file includes this dependency. - // If not this is a removed dependency and we should continue with the following dep - // Note that we need to check from package name as well since dependency name and package name is not the same thing - if parent_manifest.dep(&dep.name).is_none() - && !parent_manifest - .deps() - .filter_map(|dep| dep.1.package()) - .any(|package| package == dep.name) - { - removed_deps.insert(dep_node); - continue; - } - } let dep_path = match &dep.source { SourcePinned::Root => bail!("more than one root package detected in graph"), SourcePinned::Git(git) => { @@ -959,6 +932,18 @@ pub fn graph_to_path_map( // Construct the path relative to the parent's path. let parent_path = &path_map[&parent.id()]; let parent_manifest = ManifestFile::from_dir(parent_path, sway_git_tag)?; + // Check if parent's manifest file includes this dependency. + // If not this is a removed dependency and we should continue with the following dep + // Note that we need to check from package name as well since dependency name and package name is not the same thing + if parent_manifest.dep(&dep.name).is_none() + && !parent_manifest + .deps() + .filter_map(|dep| dep.1.package()) + .any(|package| package == dep.name) + { + removed_deps.insert(dep_node); + continue; + } let detailed = parent_manifest .dependencies .as_ref() From 19050468415e3642844c95b05e73ae90eb3aaf7e Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 27 Jun 2022 11:30:58 +0300 Subject: [PATCH 15/20] change visibility of apply_pkg_diff and from_lock to private --- forc-pkg/src/pkg.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index b3c62952938..4df04a70649 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -296,7 +296,7 @@ impl BuildPlan { } /// Create a new build plan from an existing one. Needs the difference with the existing plan with the lock. - pub fn apply_pkg_diff( + fn apply_pkg_diff( &self, pkg_diff: &PkgDiff, sway_git_tag: &str, @@ -338,11 +338,11 @@ impl BuildPlan { } /// Attempt to load the build plan from the `Lock`. - /// Returns the best effort BuildPlan and the packages removed + /// Returns the best effort BuildPlan and the packages removed /// from project's manifest file after the lock file is generated. /// /// The returned removed dependencies are already removed from the dependency graph. - pub fn from_lock( + fn from_lock( proj_path: &Path, lock: &Lock, sway_git_tag: &str, From 5d466ec8dc7c479e72266231051bb49948532b36 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 27 Jun 2022 11:39:18 +0300 Subject: [PATCH 16/20] doc updated --- forc-pkg/src/pkg.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 4df04a70649..6940f50f174 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -853,7 +853,7 @@ pub fn compilation_order(graph: &Graph) -> Result> { /// Applies the differences between the graph that is described by the lock file and the graph that /// is actually possible to build. /// -/// `deps_to_remove` : Indices of path dependencies that are removed from manifest file after the lock file is generated. +/// `deps_to_remove` : Indices of path dependencies that are removed from their parent's manifest file after the lock file is generated. /// /// There might be some differences between the graph that is described by the lock file and /// the graph that can actually possible to build. This is happening in the case of a dependency @@ -874,6 +874,9 @@ fn apply_diff_after_lock(deps_to_remove: HashSet, graph: &mut Graph) -> /// Given graph of pinned dependencies and the directory for the root node, produce a path map /// containing the path to the local source for every node in the graph. +/// +/// Returns the generated path map and the nodes that should be removed from the graph (the nodes +/// that are removed from their parent's manifest file). pub fn graph_to_path_map( proj_manifest_dir: &Path, graph: &Graph, From 4ac21d430ad657b9b531b7f110c9806bb5d84c37 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 27 Jun 2022 12:00:04 +0300 Subject: [PATCH 17/20] patch checks are added --- forc-pkg/src/manifest.rs | 7 +++++++ forc-pkg/src/pkg.rs | 20 ++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/forc-pkg/src/manifest.rs b/forc-pkg/src/manifest.rs index cbb6b9c7b14..dc406c193f4 100644 --- a/forc-pkg/src/manifest.rs +++ b/forc-pkg/src/manifest.rs @@ -334,6 +334,13 @@ impl Manifest { .and_then(|deps| deps.get(dep_name)) } + /// Retrieve the listed patches for the given name. + pub fn patch(&self, patch_name: &str) -> Option<&BTreeMap> { + self.patch + .as_ref() + .and_then(|patches| patches.get(patch_name)) + } + /// Finds and returns the name of the dependency associated with a package of the specified /// name if there is one. /// diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 8a42691be61..536b6c69c8b 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -874,6 +874,19 @@ fn apply_diff_after_lock(deps_to_remove: HashSet, graph: &mut Graph) -> } Ok(()) } +/// Search dependencies and patches for the details for a pinned dependency +fn check_manifest_for_pinned(manifest: &ManifestFile, pinned_dep: &Pinned) -> bool { + manifest.dep(&pinned_dep.name).is_none() + && !manifest + .deps() + .filter_map(|dep| dep.1.package()) + .any(|package| package == pinned_dep.name) + && !manifest.patch(&pinned_dep.name).is_none() + && !manifest + .patches() + .find_map(|patches| patches.1.get(&pinned_dep.name)) + .is_none() +} /// Given graph of pinned dependencies and the directory for the root node, produce a path map /// containing the path to the local source for every node in the graph. @@ -941,12 +954,7 @@ pub fn graph_to_path_map( // Check if parent's manifest file includes this dependency. // If not this is a removed dependency and we should continue with the following dep // Note that we need to check from package name as well since dependency name and package name is not the same thing - if parent_manifest.dep(&dep.name).is_none() - && !parent_manifest - .deps() - .filter_map(|dep| dep.1.package()) - .any(|package| package == dep.name) - { + if check_manifest_for_pinned(&parent_manifest, dep) { removed_deps.insert(dep_node); continue; } From 057afeba14f47d3b282f07f636c8526a4b7afe85 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Mon, 27 Jun 2022 12:26:53 +0300 Subject: [PATCH 18/20] check for patches corrected --- forc-pkg/src/pkg.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 536b6c69c8b..6aaae992663 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -881,8 +881,8 @@ fn check_manifest_for_pinned(manifest: &ManifestFile, pinned_dep: &Pinned) -> bo .deps() .filter_map(|dep| dep.1.package()) .any(|package| package == pinned_dep.name) - && !manifest.patch(&pinned_dep.name).is_none() - && !manifest + && manifest.patch(&pinned_dep.name).is_none() + && manifest .patches() .find_map(|patches| patches.1.get(&pinned_dep.name)) .is_none() From f11442b25f9cc66bd5b24cbda1d6fda5362a2fea Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Wed, 29 Jun 2022 10:52:25 +0300 Subject: [PATCH 19/20] lsp uses load_from_manifest, from_lock_file is private --- forc-pkg/src/pkg.rs | 5 +---- sway-lsp/src/core/document.rs | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 6aaae992663..70e64ba4a27 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -379,10 +379,7 @@ impl BuildPlan { /// from project's manifest file after the lock file is generated. /// /// The returned removed dependencies are already removed from the dependency graph. - pub fn from_lock_file( - lock_path: &Path, - sway_git_tag: &str, - ) -> Result<(Self, Vec)> { + fn from_lock_file(lock_path: &Path, sway_git_tag: &str) -> Result<(Self, Vec)> { let proj_path = lock_path.parent().unwrap(); let lock = Lock::from_path(lock_path)?; Self::from_lock(proj_path, &lock, sway_git_tag) diff --git a/sway-lsp/src/core/document.rs b/sway-lsp/src/core/document.rs index cf5f74ed3b9..9ae9bc8490b 100644 --- a/sway-lsp/src/core/document.rs +++ b/sway-lsp/src/core/document.rs @@ -160,9 +160,10 @@ impl TextDocument { let silent_mode = true; let manifest = pkg::ManifestFile::from_dir(&manifest_dir, forc::utils::SWAY_GIT_TAG).unwrap(); - let lock_path = forc_util::lock_path(manifest.dir()); - let plan = pkg::BuildPlan::from_lock_file(&lock_path, forc::utils::SWAY_GIT_TAG).unwrap(); - let res = pkg::check(&plan.0, silent_mode, forc::utils::SWAY_GIT_TAG).unwrap(); + let plan = + pkg::BuildPlan::load_from_manifest(&manifest, false, false, forc::utils::SWAY_GIT_TAG) + .unwrap(); + let res = pkg::check(&plan, silent_mode, forc::utils::SWAY_GIT_TAG).unwrap(); match res { CompileAstResult::Failure { .. } => None, From 2bc0d94f48623573a1dc0e51cf5760ebac8ef383 Mon Sep 17 00:00:00 2001 From: kayagokalp Date: Wed, 29 Jun 2022 11:08:52 +0300 Subject: [PATCH 20/20] add use::collections::HashSet --- forc-pkg/src/pkg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 4770e665bcb..a385e1f41a0 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -17,7 +17,7 @@ use petgraph::{ }; use serde::{Deserialize, Serialize}; use std::{ - collections::{hash_map, BTreeSet, HashMap}, + collections::{hash_map, BTreeSet, HashMap, HashSet}, fmt, fs, hash::{Hash, Hasher}, path::{Path, PathBuf},