A refactoring and test regeneration

MystenLabs · Sep 18, 2023 · b1b2559 · b1b2559
1 parent 245b121
commit b1b2559
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 51 deletions.
diff --git a/crates/sui-core/src/unit_tests/move_package_publish_tests.rs b/crates/sui-core/src/unit_tests/move_package_publish_tests.rs
@@ -215,6 +215,7 @@ async fn test_generate_lock_file() {
         [move]
         version = 0
         manifest_digest = "1401DE1C3C3FF6D20EB27741A0A7B5D61E34836CB6C90ECC2F2DE97C47B4D0F9"
+        deps_digest = "3C4103934B1E040BB6B23F1D610B4EF9F2F1166A50A104EADCF77467C004C600"
 
         dependencies = [
           { name = "Examples" },

diff --git a/external-crates/move/tools/move-package/src/resolution/dependency_graph.rs b/external-crates/move/tools/move-package/src/resolution/dependency_graph.rs
@@ -181,42 +181,22 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
         }
     }
 
-    /// Get a graph for a given manifest file and a given lock file. Return false if lock file was
-    /// up-to-date and used to create a new graph, return true otherwise.
+    /// Get a new graph by either reading it from Move.lock file (if this file is up-to-date, in
+    /// which case also return false) or by computing a new graph based on the content of the
+    /// Move.toml (manifest) file (in which case also return true).
     pub fn get_graph(
         &mut self,
         parent: &PM::DependencyKind,
         root_path: PathBuf,
         manifest_string: String,
-        lock_string: Option<String>,
+        lock_string_opt: Option<String>,
     ) -> Result<(DependencyGraph, bool)> {
         let toml_manifest = parse_move_manifest_string(manifest_string.clone())?;
-        let manifest = parse_source_manifest(toml_manifest)?;
+        let root_manifest = parse_source_manifest(toml_manifest)?;
 
         // compute digests eagerly as even if we can't reuse existing lock file, they need to become
         // part of the newly computed dependency graph
         let new_manifest_digest = digest_str(manifest_string.into_bytes().as_slice());
-
-        Ok(self.new_graph(
-            parent,
-            &manifest,
-            root_path.to_path_buf(),
-            new_manifest_digest,
-            lock_string,
-        )?)
-    }
-
-    /// Create a new graph by either reading it from Move.lock file (if this file is up-to-date, in
-    /// which case also return false) or by computing a new graph based on the content of the
-    /// Move.toml (manifest) file (in which case also return false true).
-    pub fn new_graph(
-        &mut self,
-        parent: &PM::DependencyKind,
-        root_manifest: &PM::SourceManifest,
-        root_path: PathBuf,
-        new_manifest_digest: String,
-        lock_string_opt: Option<String>,
-    ) -> Result<(DependencyGraph, bool)> {
         let (old_manifest_digest_opt, old_deps_digest_opt, lock_string) = match lock_string_opt {
             Some(lock_string) => match schema::read_header(&lock_string) {
                 Ok(header) => (

diff --git a/external-crates/move/tools/move-package/tests/test_additional_addresses.rs b/external-crates/move/tools/move-package/tests/test_additional_addresses.rs
@@ -5,7 +5,7 @@
 use move_core_types::account_address::AccountAddress;
 use move_package::{
     resolution::{dependency_graph as DG, resolution_graph as RG},
-    source_package::{manifest_parser as MP, parsed_manifest as PM},
+    source_package::{layout::SourcePackageLayout, parsed_manifest as PM},
     BuildConfig,
 };
 use std::{collections::BTreeMap, path::PathBuf};
@@ -21,19 +21,19 @@ fn test_additonal_addresses() {
     .into_iter()
     .collect();
 
-    let pm = MP::parse_move_manifest_from_file(&path).unwrap();
+    let manifest_string =
+        std::fs::read_to_string(&path.join(SourcePackageLayout::Manifest.path())).unwrap();
 
     let mut dep_graph_builder = DG::DependencyGraphBuilder::new(
         /* skip_fetch_latest_git_deps */ true,
         std::io::sink(),
         tempdir().unwrap().path().to_path_buf(),
     );
     let (dg, _) = dep_graph_builder
-        .new_graph(
+        .get_graph(
             &PM::DependencyKind::default(),
-            &pm,
             path,
-            /* new_manifest_digest */ "DUMMY".to_string(),
+            manifest_string,
             /* lock_string_opt */ None,
         )
         .unwrap();
@@ -77,19 +77,19 @@ fn test_additonal_addresses_already_assigned_same_value() {
         .into_iter()
         .collect();
 
-    let pm = MP::parse_move_manifest_from_file(&path).unwrap();
+    let manifest_string =
+        std::fs::read_to_string(&path.join(SourcePackageLayout::Manifest.path())).unwrap();
 
     let mut dep_graph_builder = DG::DependencyGraphBuilder::new(
         /* skip_fetch_latest_git_deps */ true,
         std::io::sink(),
         tempdir().unwrap().path().to_path_buf(),
     );
     let (dg, _) = dep_graph_builder
-        .new_graph(
+        .get_graph(
             &PM::DependencyKind::default(),
-            &pm,
             path,
-            /* new_manifest_digest */ "DUMMY".to_string(),
+            manifest_string,
             /* lock_string_opt */ None,
         )
         .unwrap();
@@ -122,19 +122,19 @@ fn test_additonal_addresses_already_assigned_different_value() {
         .into_iter()
         .collect();
 
-    let pm = MP::parse_move_manifest_from_file(&path).unwrap();
+    let manifest_string =
+        std::fs::read_to_string(&path.join(SourcePackageLayout::Manifest.path())).unwrap();
 
     let mut dep_graph_builder = DG::DependencyGraphBuilder::new(
         /* skip_fetch_latest_git_deps */ true,
         std::io::sink(),
         tempdir().unwrap().path().to_path_buf(),
     );
     let (dg, _) = dep_graph_builder
-        .new_graph(
+        .get_graph(
             &PM::DependencyKind::default(),
-            &pm,
             path,
-            /* new_manifest_digest */ "DUMMY".to_string(),
+            manifest_string,
             /* lock_string_opt */ None,
         )
         .unwrap();

diff --git a/external-crates/move/tools/move-package/tests/test_dependency_graph.rs b/external-crates/move/tools/move-package/tests/test_dependency_graph.rs
@@ -14,7 +14,7 @@ use move_package::{
         DependencyGraph, DependencyGraphBuilder, DependencyGraphInfo, DependencyMode,
     },
     source_package::{
-        manifest_parser::parse_move_manifest_from_file,
+        layout::SourcePackageLayout,
         parsed_manifest::{Dependency, DependencyKind, InternalDependency},
     },
 };
@@ -32,18 +32,18 @@ macro_rules! assert_error_contains {
 fn no_dep_graph() {
     let pkg = no_dep_test_package();
 
-    let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest");
+    let manifest_string = std::fs::read_to_string(&pkg.join(SourcePackageLayout::Manifest.path()))
+        .expect("Loading manifest");
     let mut dep_graph_builder = DependencyGraphBuilder::new(
         /* skip_fetch_latest_git_deps */ true,
         std::io::sink(),
         tempfile::tempdir().unwrap().path().to_path_buf(),
     );
     let (graph, _) = dep_graph_builder
-        .new_graph(
+        .get_graph(
             &DependencyKind::default(),
-            &manifest,
             pkg,
-            /* new_manifest_digest */ "DUMMY".to_string(),
+            manifest_string,
             /* lock_string_opt */ None,
         )
         .expect("Creating DependencyGraph");
@@ -145,18 +145,18 @@ fn lock_file_missing_dependency() {
 fn always_deps() {
     let pkg = dev_dep_test_package();
 
-    let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest");
+    let manifest_string = std::fs::read_to_string(&pkg.join(SourcePackageLayout::Manifest.path()))
+        .expect("Loading manifest");
     let mut dep_graph_builder = DependencyGraphBuilder::new(
         /* skip_fetch_latest_git_deps */ true,
         std::io::sink(),
         tempfile::tempdir().unwrap().path().to_path_buf(),
     );
     let (graph, _) = dep_graph_builder
-        .new_graph(
+        .get_graph(
             &DependencyKind::default(),
-            &manifest,
             pkg,
-            /* new_manifest_digest */ "DUMMY".to_string(),
+            manifest_string,
             /* lock_string_opt */ None,
         )
         .expect("Creating DependencyGraph");
@@ -509,18 +509,18 @@ fn merge_overlapping_different_deps() {
 fn immediate_dependencies() {
     let pkg = dev_dep_test_package();
 
-    let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest");
+    let manifest_string = std::fs::read_to_string(&pkg.join(SourcePackageLayout::Manifest.path()))
+        .expect("Loading manifest");
     let mut dep_graph_builder = DependencyGraphBuilder::new(
         /* skip_fetch_latest_git_deps */ true,
         std::io::sink(),
         tempfile::tempdir().unwrap().path().to_path_buf(),
     );
     let (graph, _) = dep_graph_builder
-        .new_graph(
+        .get_graph(
             &DependencyKind::default(),
-            &manifest,
             pkg,
-            /* new_manifest_digest */ "DUMMY".to_string(),
+            manifest_string,
             /* lock_string_opt */ None,
         )
         .expect("Creating DependencyGraph");