diff --git a/forc-pkg/src/manifest.rs b/forc-pkg/src/manifest.rs index 3ef4bd0715c..9224602aecf 100644 --- a/forc-pkg/src/manifest.rs +++ b/forc-pkg/src/manifest.rs @@ -333,6 +333,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 0d46b0fb61c..a385e1f41a0 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -6,7 +6,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 fuel_tx::StorageSlot; use fuels_types::JsonABI; @@ -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}, @@ -243,8 +243,13 @@ impl BuildPlan { sway_git_tag: &str, ) -> Result { let lock_path = forc_util::lock_path(manifest.dir()); - let plan_result = BuildPlan::from_lock_file(&lock_path, sway_git_tag); + let from_lock = 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 .as_ref() @@ -268,10 +273,10 @@ impl BuildPlan { // 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)?; - if !diff.added.is_empty() || !diff.removed.is_empty() { + let diff = plan.generate_diff(manifest)?; + if !diff.added.is_empty() || !diff.removed.is_empty() || !removed_deps.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)?; } } @@ -294,12 +299,13 @@ 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, + 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(); @@ -308,7 +314,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() @@ -320,12 +326,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, @@ -334,26 +341,55 @@ impl BuildPlan { } /// 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()?; + /// 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. + 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, &graph, &tmp_compilation_order, sway_git_tag)?; + let removed_deps_vec: 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)?; + + // Since apply_diff_after_lock 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, - }) + 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 { + /// 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. + 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 { + /// 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. + pub fn generate_diff(&self, manifest: &Manifest) -> Result { let mut added = vec![]; let mut removed = vec![]; // Retrieve project's graph node. @@ -384,7 +420,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 )); } @@ -412,6 +448,11 @@ impl BuildPlan { .collect(); } + 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]; @@ -426,7 +467,7 @@ impl BuildPlan { ); } } - Ok(PkgDiff { added, removed }) + Ok(()) } /// View the build plan's compilation graph. @@ -810,20 +851,58 @@ 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 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 +/// (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) -> Result<()> { + for dep in deps_to_remove { + let dep_name = &graph[dep].name.clone(); + println_red(&format!(" Removing {}", dep_name)); + // Remove the deleted node + graph.remove_node(dep); + } + 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. +/// +/// 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, compilation_order: &[NodeIx], sway_git_tag: &str, -) -> Result { +) -> Result<(PathMap, HashSet)> { let mut path_map = PathMap::new(); + let mut removed_deps = HashSet::new(); // 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() @@ -870,6 +949,13 @@ 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 check_manifest_for_pinned(&parent_manifest, dep) { + removed_deps.insert(dep_node); + continue; + } let detailed = parent_manifest .dependencies .as_ref() @@ -917,7 +1003,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 diff --git a/sway-lsp/src/core/document.rs b/sway-lsp/src/core/document.rs index 3eea41621a5..9ae9bc8490b 100644 --- a/sway-lsp/src/core/document.rs +++ b/sway-lsp/src/core/document.rs @@ -160,8 +160,9 @@ 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 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 {