[10/x][move-package/lock] Integrate DependencyGraph into ResolvedGraph

language/tools/move-cli/src/base/test.rs

@@ -193,7 +193,7 @@ pub fn run_move_unit_tests<W: Write + Send>(
                 .collect::<HashMap<_, _>>()
         })
         .collect();
-    let root_package = resolution_graph.root_package.package.name;
+    let root_package = resolution_graph.root_package();
     let build_plan = BuildPlan::create(resolution_graph)?;
     // Compile the package. We need to intercede in the compilation, process being performed by the
     // Move package system, to first grab the compilation env, construct the test plan from it, and

language/tools/move-cli/tests/build_tests/dependency_chain/args.exp

@@ -1,4 +1,4 @@
 Command `build -v`:
-INCLUDING DEPENDENCY Bar
 INCLUDING DEPENDENCY Foo
+INCLUDING DEPENDENCY Bar
 BUILDING A

language/tools/move-cli/tests/build_tests/unbound_address/args.evm.exp

@@ -1,8 +1,12 @@
 Command `build -v --arch ethereum`:
-Error: Unresolved addresses found: [
-Named address 'A' in package 'A'
-]
-To fix this, add an entry for each unresolved address to the [addresses] section of ./Move.toml: e.g.,
+Error: Unresolved addresses found. To fix this, add an entry for each unresolved address to the [addresses] section of ./Move.toml: e.g.,
+
 [addresses]
-Std = "0x1"
+std = "0x1"
+
 Alternatively, you can also define [dev-addresses] and call with the -d flag
+
+Caused by:
+    Unresolved addresses: [
+      Named address 'A' in package 'A'
+    ]

language/tools/move-cli/tests/build_tests/unbound_address/args.exp

@@ -1,8 +1,12 @@
 Command `build -v`:
-Error: Unresolved addresses found: [
-Named address 'A' in package 'A'
-]
-To fix this, add an entry for each unresolved address to the [addresses] section of ./Move.toml: e.g.,
+Error: Unresolved addresses found. To fix this, add an entry for each unresolved address to the [addresses] section of ./Move.toml: e.g.,
+
 [addresses]
-Std = "0x1"
+std = "0x1"
+
 Alternatively, you can also define [dev-addresses] and call with the -d flag
+
+Caused by:
+    Unresolved addresses: [
+      Named address 'A' in package 'A'
+    ]

language/tools/move-cli/tests/build_tests/unbound_dependency/args.evm.exp

@@ -3,5 +3,5 @@ Error: Failed to resolve dependencies for package 'A'
 
 Caused by:
     0: Parsing manifest for 'Foo'
-    1: Unable to find package manifest for 'Foo' at "./foo/Move.toml"
+    1: Unable to find package manifest at "./foo"
     2: No such file or directory (os error 2)

language/tools/move-cli/tests/build_tests/unbound_dependency/args.exp

@@ -3,5 +3,5 @@ Error: Failed to resolve dependencies for package 'A'
 
 Caused by:
     0: Parsing manifest for 'Foo'
-    1: Unable to find package manifest for 'Foo' at "./foo/Move.toml"
+    1: Unable to find package manifest at "./foo"
     2: No such file or directory (os error 2)

language/tools/move-package/src/compilation/build_plan.rs

@@ -12,7 +12,6 @@ use move_compiler::{
     diagnostics::{report_diagnostics_to_color_buffer, report_warnings, FilesSourceText},
     Compiler,
 };
-use petgraph::algo::toposort;
 use std::{collections::BTreeSet, io::Write, path::Path};
 
 use super::package_layout::CompiledPackageLayout;
@@ -85,18 +84,11 @@ fn should_recompile(
 
 impl BuildPlan {
     pub fn create(resolution_graph: ResolvedGraph) -> Result<Self> {
-        let mut sorted_deps = match toposort(&resolution_graph.graph, None) {
-            Ok(nodes) => nodes,
-            Err(err) => {
-                // Is a DAG after resolution otherwise an error should be raised from that.
-                anyhow::bail!("IPE: Cyclic dependency found after resolution {:?}", err)
-            }
-        };
-
+        let mut sorted_deps = resolution_graph.topological_order();
         sorted_deps.reverse();
 
         Ok(Self {
-            root: resolution_graph.root_package.package.name,
+            root: resolution_graph.root_package(),
             sorted_deps,
             resolution_graph,
         })
@@ -139,13 +131,15 @@ impl BuildPlan {
         let root_package = &self.resolution_graph.package_table[&self.root];
         let project_root = match &self.resolution_graph.build_options.install_dir {
             Some(under_path) => under_path.clone(),
-            None => self.resolution_graph.root_package_path.clone(),
+            None => self.resolution_graph.graph.root_path.clone(),
         };
         let immediate_dependencies_names =
             root_package.immediate_dependencies(&self.resolution_graph);
-        let transitive_dependencies = root_package
-            .transitive_dependencies(&self.resolution_graph)
+        let transitive_dependencies = self
+            .resolution_graph
+            .topological_order()
             .into_iter()
+            .filter(|package_name| *package_name != self.root)
             .map(|package_name| {
                 let dep_package = self
                     .resolution_graph
@@ -159,7 +153,7 @@ impl BuildPlan {
                     package_name,
                     immediate_dependencies_names.contains(&package_name),
                     dep_source_paths,
-                    &dep_package.resolution_table,
+                    &dep_package.resolved_table,
                 )
             })
             .collect();
@@ -185,7 +179,7 @@ impl BuildPlan {
         let root_package = &self.resolution_graph.package_table[&self.root];
         let project_root = match &self.resolution_graph.build_options.install_dir {
             Some(under_path) => under_path.clone(),
-            None => self.resolution_graph.root_package_path.clone(),
+            None => self.resolution_graph.graph.root_path.clone(),
         };
         let build_root_path = project_root
             .join(CompiledPackageLayout::Root.path())

language/tools/move-package/src/compilation/compiled_package.rs

@@ -4,7 +4,7 @@
 
 use crate::{
     compilation::package_layout::CompiledPackageLayout,
-    resolution::resolution_graph::{Renaming, ResolvedGraph, ResolvedPackage, ResolvedTable},
+    resolution::resolution_graph::{Package, Renaming, ResolvedGraph, ResolvedTable},
     source_package::{
         layout::{SourcePackageLayout, REFERENCE_TEMPLATE_FILENAME},
         parsed_manifest::{FileName, PackageDigest, PackageName},
@@ -281,10 +281,7 @@ impl OnDiskCompiledPackage {
     }
 
     #[allow(unused)]
-    pub(crate) fn has_source_changed_since_last_compile(
-        &self,
-        resolved_package: &ResolvedPackage,
-    ) -> bool {
+    pub(crate) fn has_source_changed_since_last_compile(&self, resolved_package: &Package) -> bool {
         match &self.package.compiled_package_info.source_digest {
             // Don't have source available to us
             None => false,
@@ -509,7 +506,7 @@ impl CompiledPackage {
     fn can_load_cached(
         package: &OnDiskCompiledPackage,
         resolution_graph: &ResolvedGraph,
-        resolved_package: &ResolvedPackage,
+        resolved_package: &Package,
         is_root_package: bool,
     ) -> bool {
         // TODO: add more tests for the different caching cases
@@ -523,13 +520,13 @@ impl CompiledPackage {
                 // Dive deeper to make sure that instantiations haven't changed since that
                 // can be changed by other packages above us in the dependency graph possibly
                 package.package.compiled_package_info.address_alias_instantiation
-                    == resolved_package.resolution_table
+                    == resolved_package.resolved_table
     }
 
     pub(crate) fn build_all<W: Write>(
         w: &mut W,
         project_root: &Path,
-        resolved_package: ResolvedPackage,
+        resolved_package: Package,
         transitive_dependencies: Vec<(
             /* name */ Symbol,
             /* is immediate */ bool,
@@ -633,7 +630,7 @@ impl CompiledPackage {
         let compiled_package = CompiledPackage {
             compiled_package_info: CompiledPackageInfo {
                 package_name: resolved_package.source_package.package.name,
-                address_alias_instantiation: resolved_package.resolution_table,
+                address_alias_instantiation: resolved_package.resolved_table,
                 source_digest: Some(resolved_package.source_digest),
                 build_flags: resolution_graph.build_options.clone(),
             },
@@ -890,7 +887,7 @@ pub(crate) fn apply_named_address_renaming(
 
 pub(crate) fn make_source_and_deps_for_compiler(
     resolution_graph: &ResolvedGraph,
-    root: &ResolvedPackage,
+    root: &Package,
     deps: Vec<(
         /* name */ Symbol,
         /* source paths */ Vec<Symbol>,
@@ -918,7 +915,7 @@ pub(crate) fn make_source_and_deps_for_compiler(
         .collect::<Result<Vec<_>>>()?;
     let root_named_addrs = apply_named_address_renaming(
         root.source_package.package.name,
-        named_address_mapping_for_compiler(&root.resolution_table),
+        named_address_mapping_for_compiler(&root.resolved_table),
         &root.renaming,
     );
     let sources = root.get_sources(&resolution_graph.build_options)?;

language/tools/move-package/src/compilation/model_builder.rs

@@ -42,20 +42,20 @@ impl ModelBuilder {
         }
 
         // Targets are all files in the root package
-        let root_name = &self.resolution_graph.root_package.package.name;
+        let root_name = self.resolution_graph.root_package();
         let root_package = self.resolution_graph.get_package(root_name).clone();
         let deps_source_info = self
             .resolution_graph
             .package_table
             .iter()
             .filter_map(|(nm, pkg)| {
-                if nm == root_name {
+                if *nm == root_name {
                     return None;
                 }
                 let dep_source_paths = pkg
                     .get_sources(&self.resolution_graph.build_options)
                     .unwrap();
-                Some(Ok((*nm, dep_source_paths, &pkg.resolution_table)))
+                Some(Ok((*nm, dep_source_paths, &pkg.resolved_table)))
             })
             .collect::<Result<Vec<_>>>()?;
 

language/tools/move-package/src/lib.rs

@@ -13,7 +13,7 @@ use anyhow::{bail, Result};
 use clap::*;
 use move_core_types::account_address::AccountAddress;
 use move_model::model::GlobalEnv;
-use resolution::{dependency_graph::DependencyGraph, lock_file::LockFile};
+use resolution::{dependency_graph::DependencyGraph, resolution_graph::ResolvedGraph};
 use serde::{Deserialize, Serialize};
 use source_package::layout::SourcePackageLayout;
 use std::{
@@ -28,7 +28,6 @@ use crate::{
         build_plan::BuildPlan, compiled_package::CompiledPackage, model_builder::ModelBuilder,
     },
     package_lock::PackageLock,
-    resolution::resolution_graph::{ResolutionGraph, ResolvedGraph},
     source_package::manifest_parser,
 };
 
@@ -233,23 +232,17 @@ impl BuildConfig {
 
         // This should be locked as it inspects the environment for `MOVE_HOME` which could
         // possibly be set by a different process in parallel.
-        let mut lock = LockFile::new(&path)?;
         let manifest = manifest_parser::parse_source_manifest(toml_manifest)?;
 
-        let dependency_graph = DependencyGraph::new(
-            &manifest,
-            path.clone(),
-            self.skip_fetch_latest_git_deps,
-            writer,
-        )?;
+        let dependency_graph =
+            DependencyGraph::new(&manifest, path, self.skip_fetch_latest_git_deps, writer)?;
 
-        dependency_graph.write_to_lock(&mut lock)?;
+        let lock = dependency_graph.write_to_lock()?;
         if let Some(lock_path) = &self.lock_file {
             lock.commit(lock_path)?;
         }
 
-        let resolution_graph = ResolutionGraph::new(manifest, path, self, writer)?;
-        let ret = resolution_graph.resolve()?;
+        let ret = ResolvedGraph::resolve(dependency_graph, self, writer)?;
 
         mutx.unlock();
         Ok(ret)