From 18ce6b241dfa9c6c59cfe537841291bc95a7e925 Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Thu, 22 Dec 2022 00:54:10 +0000 Subject: [PATCH 1/8] [7/x][move-package/lock] Move addr_subst and digest to dependency edge The initial design of `DependencyGraph` only allowed for one `addr_subst` and `digest` per package in the transitive dependency graph, but in actuality there can be as many of these as there are dependency edges to that package: - Different `addr_subst` for different packages `P`, `Q` depending on `R` allows the same address in `R` to appear as different addresses in `P` and `Q`. - If `R` is a dependency of `P` and a dev-dependency of `Q`, then its source digest may differ between the two. Fixed by moving the `subst` and `digest` to be edge labels in the graph, rather than node labels. Note that this also changes the lock file format in the following ways: - A package's dependencies are now no longer just the name of the dependency, but an inline table containing the name and optionally the digest and address substitution. - The key within the `move` table that contains all the transitive dependencies is now called `package` rather than `dependency` (this was mainly driven by the naming within the schema, which required a name for the new, richer format for a package's dependencies.) Test Plan: ``` move/language/tools/move-cli$ cargo nextest move/language/tools/move-package$ cargo nextest ``` --- .../build_tests/unbound_dependency/args.exp | 2 +- .../src/resolution/dependency_graph.rs | 340 ++++++++++++------ .../src/resolution/lock_file/schema.rs | 47 ++- .../tools/move-package/src/resolution/mod.rs | 6 +- .../src/resolution/resolution_graph.rs | 2 +- .../src/source_package/manifest_parser.rs | 9 +- .../dep_dev_dep_diamond/Move.locked | 14 +- .../test_sources/dep_good_digest/Move.locked | 4 +- .../diamond_problem_no_conflict/Move.locked | 16 +- .../multiple_deps_no_rename/Move.locked | 4 +- .../nested_deps_git_local/Move.locked | 6 +- .../nested_deps_local_local/Move.locked | 6 +- .../tests/test_sources/one_dep/Move.locked | 4 +- .../one_dep_bad_digest/Move.locked | 4 +- .../parsing_full_manifest/Move.resolved | 2 +- .../Move.resolved | 2 +- .../Move.resolved | 2 +- .../Move.resolved | 2 +- .../Move.resolved | 2 +- 19 files changed, 299 insertions(+), 175 deletions(-) diff --git a/language/tools/move-cli/tests/build_tests/unbound_dependency/args.exp b/language/tools/move-cli/tests/build_tests/unbound_dependency/args.exp index 383b5affe7..7869db6c90 100644 --- a/language/tools/move-cli/tests/build_tests/unbound_dependency/args.exp +++ b/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) diff --git a/language/tools/move-package/src/resolution/dependency_graph.rs b/language/tools/move-package/src/resolution/dependency_graph.rs index 555549e3cb..51b37d10dc 100644 --- a/language/tools/move-package/src/resolution/dependency_graph.rs +++ b/language/tools/move-package/src/resolution/dependency_graph.rs @@ -14,18 +14,14 @@ use std::{ use crate::{ package_hooks, source_package::{ - manifest_parser::parse_dependency, - parsed_manifest::{ - CustomDepInfo, Dependency, DependencyKind, GitInfo, NamedAddress, PackageName, - SourceManifest, SubstOrRename, Substitution, - }, + manifest_parser::{parse_dependency, parse_move_manifest_from_file, parse_substitution}, + parsed_manifest as PM, }, }; use super::{ - download_and_update_if_remote, + download_and_update_if_remote, local_path, lock_file::{schema, LockFile}, - parse_package_manifest, }; /// A representation of the transitive dependency graph of a Move package. If successfully created, @@ -46,34 +42,48 @@ use super::{ #[derive(Debug)] pub struct DependencyGraph { /// Path to the root package and its name (according to its manifest) - root_path: PathBuf, - root_package: PackageName, + pub root_path: PathBuf, + pub root_package: PM::PackageName, /// Transitive dependency graph, with dependency edges `P -> Q` labelled according to whether Q /// is always a dependency of P or only in dev-mode. - package_graph: DiGraphMap, + pub package_graph: DiGraphMap, /// The dependency that each package (keyed by name) originates from. The root package is the /// only node in `package_graph` that does not have an entry in `package_table`. - package_table: BTreeMap, + pub package_table: BTreeMap, /// Packages that are transitive dependencies regardless of mode (the transitive closure of /// `DependencyMode::Always` edges in `package_graph`). - pub always_deps: BTreeSet, + pub always_deps: BTreeSet, } -/// Edge label indicating whether one package always depends on another, or only in dev-mode. +#[derive(Debug, PartialEq, Eq)] +pub struct Package { + kind: PM::DependencyKind, + version: Option, +} + +#[derive(Debug)] +pub struct Dependency { + pub mode: DependencyMode, + pub subst: Option, + pub digest: Option, +} + +/// Indicates whether one package always depends on another, or only in dev-mode. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] -enum DependencyMode { +pub enum DependencyMode { Always, DevOnly, } -/// Wrapper struct to display a dependency as an inline table in the lock file (matching the +/// Wrapper struct to display a package as an inline table in the lock file (matching the /// convention in the source manifest). This is necessary becase the `toml` crate does not /// currently support serializing types as inline tables. -struct DependencyTOML<'a>(&'a Dependency); -struct SubstTOML<'a>(&'a Substitution); +struct PackageTOML<'a>(&'a Package); +struct DependencyTOML<'a>(PM::PackageName, &'a Dependency); +struct SubstTOML<'a>(&'a PM::Substitution); impl DependencyGraph { /// Build a graph from the transitive dependencies and dev-dependencies of `root_package`. @@ -84,7 +94,7 @@ impl DependencyGraph { /// `progress_output` is an output stream that is written to while generating the graph, to /// provide human-readable progress updates. pub fn new( - root_package: &SourceManifest, + root_package: &PM::SourceManifest, root_path: PathBuf, skip_fetch_latest_git_deps: bool, progress_output: &mut Progress, @@ -99,7 +109,7 @@ impl DependencyGraph { graph .extend_graph( - DependencyKind::default(), + PM::DependencyKind::default(), root_package, skip_fetch_latest_git_deps, progress_output, @@ -127,7 +137,7 @@ impl DependencyGraph { /// the `lock_file::schema` module). pub fn read_from_lock( root_path: PathBuf, - root_package: SourceManifest, + root_package: PM::SourceManifest, lock: &mut impl Read, ) -> Result { let mut package_graph = DiGraphMap::new(); @@ -136,49 +146,102 @@ impl DependencyGraph { // Seed graph with edges from the root package let root = root_package.package.name; - for dep in root_package.dependencies.keys() { - package_graph.add_edge(root, *dep, DependencyMode::Always); + for (pkg, mut dep) in root_package.dependencies { + package_graph.add_edge( + root, + pkg, + Dependency { + mode: DependencyMode::Always, + subst: dep.subst.take(), + digest: dep.digest.take(), + }, + ); } - for dep in root_package.dev_dependencies.keys() { - package_graph.add_edge(root, *dep, DependencyMode::DevOnly); + for (pkg, mut dep) in root_package.dev_dependencies { + package_graph.add_edge( + root, + pkg, + Dependency { + mode: DependencyMode::DevOnly, + subst: dep.subst.take(), + digest: dep.digest.take(), + }, + ); } // Fill in the remaining dependencies, and the package source information from the lock // file. - for schema::Dependency { - name, + for schema::Package { + name: pkg_name, source, dependencies, dev_dependencies, - } in schema::Dependencies::read(lock)? + } in schema::Packages::read(lock)? { - let package = PackageName::from(name.as_str()); - let source = parse_dependency(package.as_str(), source) - .with_context(|| format!("Deserializing dependency {}", package))?; + let pkg_name = PM::PackageName::from(pkg_name.as_str()); + let source = parse_dependency(pkg_name.as_str(), source) + .with_context(|| format!("Deserializing dependency '{pkg_name}'"))?; + + if source.subst.is_some() { + bail!("Unexpected 'addr_subst' in source for '{pkg_name}'") + } + + if source.digest.is_some() { + bail!("Unexpected 'digest' in source for '{pkg_name}'") + } + + let pkg = Package { + kind: source.kind, + version: source.version, + }; - match package_table.entry(package) { + match package_table.entry(pkg_name) { Entry::Vacant(entry) => { - entry.insert(source); + entry.insert(pkg); } Entry::Occupied(entry) => { bail!( "Duplicate dependency in lock file:\n{0} = {1}\n{0} = {2}\n", - package, - DependencyTOML(entry.get()), - DependencyTOML(&source), + pkg_name, + PackageTOML(entry.get()), + PackageTOML(&pkg), ); } }; - for dep in dependencies.iter().flatten() { - let dep = PackageName::from(dep.as_str()); - package_graph.add_edge(package, dep, DependencyMode::Always); + for schema::Dependency { + name: dep_name, + subst, + digest, + } in dependencies.into_iter().flatten() + { + package_graph.add_edge( + pkg_name, + PM::PackageName::from(dep_name.as_str()), + Dependency { + mode: DependencyMode::Always, + subst: subst.map(parse_substitution).transpose()?, + digest: digest.map(Symbol::from), + }, + ); } - for dep in dev_dependencies.iter().flatten() { - let dep = PackageName::from(dep.as_str()); - package_graph.add_edge(package, dep, DependencyMode::DevOnly); + for schema::Dependency { + name: dep_name, + subst, + digest, + } in dev_dependencies.into_iter().flatten() + { + package_graph.add_edge( + pkg_name, + PM::PackageName::from(dep_name.as_str()), + Dependency { + mode: DependencyMode::DevOnly, + subst: subst.map(parse_substitution).transpose()?, + digest: digest.map(Symbol::from), + }, + ); } } @@ -196,100 +259,130 @@ impl DependencyGraph { Ok(graph) } - /// Serialize this dependency graph into a lock file, consuming it in the process. + /// Serialize this dependency graph into a lock file. /// /// This operation fails, writing nothing, if the graph contains a cycle, and can fail with an /// undefined output if it cannot be represented in a TOML file. - pub fn write_to_lock(self, lock: &mut LockFile) -> Result<()> { + pub fn write_to_lock(&self, lock: &mut LockFile) -> Result<()> { let mut writer = BufWriter::new(&**lock); - for (pkg, dep) in self.package_table { - writeln!(writer, "\n[[move.dependency]]")?; + for (name, pkg) in &self.package_table { + writeln!(writer, "\n[[move.package]]")?; - writeln!(writer, "name = {}", str_escape(pkg.as_str())?)?; - writeln!(writer, "source = {}", DependencyTOML(&dep))?; + writeln!(writer, "name = {}", str_escape(name.as_str())?)?; + writeln!(writer, "source = {}", PackageTOML(pkg))?; let mut deps: Vec<_> = self .package_graph - .edges(pkg) - .map(|(_, dep, kind)| (*kind, dep)) + .edges(*name) + .map(|(_, pkg, dep)| (dep, pkg)) .collect(); // Sort by kind ("always" dependencies go first), and by name, to keep the output // stable. - deps.sort(); - + deps.sort_by_key(|(dep, pkg)| (dep.mode, *pkg)); let mut deps = deps.into_iter().peekable(); - if let Some((DependencyMode::Always, _)) = deps.peek() { - writeln!(writer, "dependencies = [")?; - while let Some((DependencyMode::Always, dep)) = deps.peek() { - writeln!(writer, " {},", str_escape(dep.as_str())?)?; - deps.next(); - } - writeln!(writer, "]")?; - } - if let Some((DependencyMode::DevOnly, _)) = deps.peek() { - writeln!(writer, "dev-dependencies = [")?; - while let Some((DependencyMode::DevOnly, dep)) = deps.peek() { - writeln!(writer, " {},", str_escape(dep.as_str())?)?; - deps.next(); - } - writeln!(writer, "]")?; + macro_rules! write_deps { + ($mode: pat, $label: literal) => { + if let Some((Dependency { mode: $mode, .. }, _)) = deps.peek() { + writeln!(writer, "{} = [", $label)?; + while let Some((dep @ Dependency { mode: $mode, .. }, pkg)) = deps.peek() { + writeln!(writer, " {},", DependencyTOML(*pkg, dep))?; + deps.next(); + } + writeln!(writer, "]")?; + } + }; } + + write_deps!(DependencyMode::Always, "dependencies"); + write_deps!(DependencyMode::DevOnly, "dev-dependencies"); } writer.flush()?; Ok(()) } + /// Return packages in the graph in topological order (a package is ordered before its + /// dependencies). + /// + /// The ordering is agnostic to dependency mode (dev-mode or not) and contains all packagesd + /// (including packages that are exclusively dev-mode-only). + /// + /// Guaranteed to succeed because `DependencyGraph` instances cannot contain cycles. + pub fn topological_order(&self) -> Vec { + algo::toposort(&self.package_graph, None) + .expect("Graph is determined to be acyclic when created") + } + /// Add the transitive dependencies and dev-dependencies from `package` to the dependency graph. fn extend_graph( &mut self, - parent: DependencyKind, - package: &SourceManifest, + parent: PM::DependencyKind, + package: &PM::SourceManifest, skip_fetch_latest_git_deps: bool, progress_output: &mut Progress, ) -> Result<()> { let from = package.package.name; for (to, dep) in &package.dependencies { - let mut dep = dep.clone(); - dep.kind.reroot(&parent)?; - - self.process_dependency(dep, *to, skip_fetch_latest_git_deps, progress_output)?; + let mut pkg = Package { + kind: dep.kind.clone(), + version: dep.version, + }; - self.package_graph - .add_edge(from, *to, DependencyMode::Always); + pkg.kind.reroot(&parent)?; + self.process_dependency(pkg, *to, skip_fetch_latest_git_deps, progress_output)?; + + self.package_graph.add_edge( + from, + *to, + Dependency { + mode: DependencyMode::Always, + subst: dep.subst.clone(), + digest: dep.digest, + }, + ); } for (to, dep) in &package.dev_dependencies { - let mut dep = dep.clone(); - dep.kind.reroot(&parent)?; - - self.process_dependency(dep, *to, skip_fetch_latest_git_deps, progress_output)?; + let mut pkg = Package { + kind: dep.kind.clone(), + version: dep.version, + }; - self.package_graph - .add_edge(from, *to, DependencyMode::DevOnly); + pkg.kind.reroot(&parent)?; + self.process_dependency(pkg, *to, skip_fetch_latest_git_deps, progress_output)?; + + self.package_graph.add_edge( + from, + *to, + Dependency { + mode: DependencyMode::DevOnly, + subst: dep.subst.clone(), + digest: dep.digest, + }, + ); } Ok(()) } - /// Ensures that package `dep_name` and all its transitive dependencies are present in the - /// graph, all sourced from their respective `dep`endencies. Fails if any of the packages in - /// the dependency sub-graph rooted at `dep_name` are already present in `self` but sourced from + /// Ensures that package `pkg_name` and all its transitive dependencies are present in the + /// graph, all sourced from their respective packages, `pkg`. Fails if any of the packages in + /// the dependency sub-graph rooted at `pkg_name` are already present in `self` but sourced from /// a different dependency. fn process_dependency( &mut self, - dep: Dependency, - dep_name: PackageName, + pkg: Package, + name: PM::PackageName, skip_fetch_latest_git_deps: bool, progress_output: &mut Progress, ) -> Result<()> { - let dep = match self.package_table.entry(dep_name) { - Entry::Vacant(entry) => entry.insert(dep), + let pkg = match self.package_table.entry(name) { + Entry::Vacant(entry) => entry.insert(pkg), // Seeing the same package again, pointing to the same dependency: OK, return early. - Entry::Occupied(entry) if entry.get().kind == dep.kind => { + Entry::Occupied(entry) if entry.get() == &pkg => { return Ok(()); } @@ -297,31 +390,32 @@ impl DependencyGraph { Entry::Occupied(entry) => { bail!( "Conflicting dependencies found:\n{0} = {1}\n{0} = {2}\n", - dep_name, - DependencyTOML(entry.get()), - DependencyTOML(&dep), + name, + PackageTOML(entry.get()), + PackageTOML(&pkg), ); } }; - download_and_update_if_remote(dep_name, dep, skip_fetch_latest_git_deps, progress_output) - .with_context(|| format!("Fetching '{}'", dep_name))?; + download_and_update_if_remote(name, &pkg.kind, skip_fetch_latest_git_deps, progress_output) + .with_context(|| format!("Fetching '{}'", name))?; - let (manifest, _) = parse_package_manifest(dep, &dep_name, self.root_path.clone()) - .with_context(|| format!("Parsing manifest for '{}'", dep_name))?; + let pkg_path = self.root_path.join(local_path(&pkg.kind)); + let manifest = parse_move_manifest_from_file(&pkg_path) + .with_context(|| format!("Parsing manifest for '{}'", name))?; - if dep_name != manifest.package.name { + if name != manifest.package.name { bail!( "Name of dependency declared in package '{}' \ does not match dependency's package name '{}'", - dep_name, + name, manifest.package.name, ) } - let kind = dep.kind.clone(); + let kind = pkg.kind.clone(); self.extend_graph(kind, &manifest, skip_fetch_latest_git_deps, progress_output) - .with_context(|| format!("Resolving dependencies for package '{}'", dep_name)) + .with_context(|| format!("Resolving dependencies for package '{}'", name)) } /// Check that every dependency in the graph, excluding the root package, is present in the @@ -385,31 +479,26 @@ impl DependencyGraph { frontier.extend( self.package_graph .edges(package) - .filter(|(_, _, mode)| **mode == DependencyMode::Always) - .map(|(_, dep, _)| dep), + .filter(|(_, _, dep)| dep.mode == DependencyMode::Always) + .map(|(_, pkg, _)| pkg), ); } } } -impl<'a> fmt::Display for DependencyTOML<'a> { +impl<'a> fmt::Display for PackageTOML<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let Dependency { - kind, - subst, - version, - digest, - } = self.0; + let Package { kind, version } = self.0; f.write_str("{ ")?; match kind { - DependencyKind::Local(local) => { + PM::DependencyKind::Local(local) => { write!(f, "local = ")?; f.write_str(&path_escape(local)?)?; } - DependencyKind::Git(GitInfo { + PM::DependencyKind::Git(PM::GitInfo { git_url, git_rev, subdir, @@ -424,7 +513,7 @@ impl<'a> fmt::Display for DependencyTOML<'a> { f.write_str(&path_escape(subdir)?)?; } - DependencyKind::Custom(CustomDepInfo { + PM::DependencyKind::Custom(PM::CustomDepInfo { node_url, package_address, subdir, @@ -448,6 +537,27 @@ impl<'a> fmt::Display for DependencyTOML<'a> { write!(f, ", version = \"{}.{}.{}\"", major, minor, bugfix)?; } + f.write_str(" }")?; + Ok(()) + } +} + +impl<'a> fmt::Display for DependencyTOML<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let DependencyTOML( + name, + Dependency { + mode: _, + subst, + digest, + }, + ) = self; + + f.write_str("{ ")?; + + write!(f, "name = ")?; + f.write_str(&str_escape(name.as_str())?)?; + if let Some(digest) = digest { write!(f, ", digest = ")?; f.write_str(&str_escape(digest.as_str())?)?; @@ -467,18 +577,18 @@ impl<'a> fmt::Display for SubstTOML<'a> { /// Write an individual key value pair in the substitution. fn write_subst( f: &mut fmt::Formatter<'_>, - addr: &NamedAddress, - subst: &SubstOrRename, + addr: &PM::NamedAddress, + subst: &PM::SubstOrRename, ) -> fmt::Result { f.write_str(&str_escape(addr.as_str())?)?; write!(f, " = ")?; match subst { - SubstOrRename::RenameFrom(named) => { + PM::SubstOrRename::RenameFrom(named) => { f.write_str(&str_escape(named.as_str())?)?; } - SubstOrRename::Assign(account) => { + PM::SubstOrRename::Assign(account) => { f.write_str(&str_escape(&account.to_canonical_string())?)?; } } diff --git a/language/tools/move-package/src/resolution/lock_file/schema.rs b/language/tools/move-package/src/resolution/lock_file/schema.rs index 810fb10c14..9b82a66b17 100644 --- a/language/tools/move-package/src/resolution/lock_file/schema.rs +++ b/language/tools/move-package/src/resolution/lock_file/schema.rs @@ -20,24 +20,38 @@ use toml::value::Value; pub const VERSION: u64 = 0; #[derive(Deserialize)] -pub struct Dependencies { - #[serde(rename = "dependency")] - dependencies: Option>, +pub struct Packages { + #[serde(rename = "package")] + packages: Option>, } #[derive(Deserialize)] -pub struct Dependency { - /// The name of the dependency (corresponds to the key for the dependency in the source - /// manifest). +pub struct Package { + /// The name of the package (corresponds to the name field from its source manifest). pub name: String, - /// The description of the dependency from its source manifest. Its schema is not described in - /// terms of serde-compatible structs, so it is deserialized into a generic data structure. + /// Where to find this dependency. Schema is not described in terms of serde-compatible + /// structs, so it is deserialized into a generic data structure. pub source: Value, - pub dependencies: Option>, + pub dependencies: Option>, #[serde(rename = "dev-dependencies")] - pub dev_dependencies: Option>, + pub dev_dependencies: Option>, +} + +#[derive(Deserialize)] +pub struct Dependency { + /// The name of the dependency (corresponds to the key for the dependency in the depending + /// package's source manifest). + pub name: String, + + /// Mappings for named addresses to apply to the package being depended on, when referred to by + /// the depending package. + #[serde(rename = "addr_subst")] + pub subst: Option, + + /// Expected hash for the source and manifest of the package being depended upon. + pub digest: Option, } #[derive(Serialize, Deserialize)] @@ -51,10 +65,10 @@ struct Header { version: u64, } -impl Dependencies { - /// Read dependencies from the lock file, assuming the file's format matches the schema expected +impl Packages { + /// Read packages from the lock file, assuming the file's format matches the schema expected /// by this lock file, and its version is not newer than the version supported by this library. - pub fn read(lock: &mut impl Read) -> Result> { + pub fn read(lock: &mut impl Read) -> Result> { let contents = { let mut buf = String::new(); lock.read_to_string(&mut buf).context("Reading lock file")?; @@ -74,11 +88,10 @@ impl Dependencies { } let Schema { - move_: Dependencies { dependencies }, - } = toml::de::from_str::>(&contents) - .context("Deserializing dependencies")?; + move_: Packages { packages }, + } = toml::de::from_str::>(&contents).context("Deserializing packages")?; - Ok(dependencies.unwrap_or_default()) + Ok(packages.unwrap_or_default()) } } diff --git a/language/tools/move-package/src/resolution/mod.rs b/language/tools/move-package/src/resolution/mod.rs index e0ec41b21e..7bffc9de43 100644 --- a/language/tools/move-package/src/resolution/mod.rs +++ b/language/tools/move-package/src/resolution/mod.rs @@ -49,7 +49,7 @@ pub fn download_dependency_repos( for (dep_name, dep) in manifest.dependencies.iter().chain(additional_deps.iter()) { download_and_update_if_remote( *dep_name, - dep, + &dep.kind, build_options.skip_fetch_latest_git_deps, progress_output, )?; @@ -85,11 +85,11 @@ fn parse_package_manifest( fn download_and_update_if_remote( dep_name: PackageName, - dep: &Dependency, + kind: &DependencyKind, skip_fetch_latest_git_deps: bool, progress_output: &mut Progress, ) -> Result<()> { - match &dep.kind { + match kind { DependencyKind::Local(_) => Ok(()), DependencyKind::Custom(node_info) => { diff --git a/language/tools/move-package/src/resolution/resolution_graph.rs b/language/tools/move-package/src/resolution/resolution_graph.rs index 72b49a7358..c3bc4e99a7 100644 --- a/language/tools/move-package/src/resolution/resolution_graph.rs +++ b/language/tools/move-package/src/resolution/resolution_graph.rs @@ -398,7 +398,7 @@ impl ResolvingGraph { ) -> Result<(Renaming, ResolvingTable)> { download_and_update_if_remote( dep_name_in_pkg, - &dep, + &dep.kind, self.build_options.skip_fetch_latest_git_deps, progress_output, )?; diff --git a/language/tools/move-package/src/source_package/manifest_parser.rs b/language/tools/move-package/src/source_package/manifest_parser.rs index 365c10c083..6b6a814337 100644 --- a/language/tools/move-package/src/source_package/manifest_parser.rs +++ b/language/tools/move-package/src/source_package/manifest_parser.rs @@ -36,10 +36,11 @@ const REQUIRED_FIELDS: &[&str] = &[PACKAGE_NAME]; pub fn parse_move_manifest_from_file(path: &Path) -> Result { let file_contents = if path.is_file() { - std::fs::read_to_string(path)? + std::fs::read_to_string(path) } else { - std::fs::read_to_string(path.join(SourcePackageLayout::Manifest.path()))? - }; + std::fs::read_to_string(path.join(SourcePackageLayout::Manifest.path())) + } + .with_context(|| format!("Unable to find package manifest at {:?}", path))?; parse_source_manifest(parse_move_manifest_string(file_contents)?) } @@ -433,7 +434,7 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result }) } -fn parse_substitution(tval: TV) -> Result { +pub fn parse_substitution(tval: TV) -> Result { match tval { TV::Table(table) => { let mut subst = BTreeMap::new(); diff --git a/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked b/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked index 71dba8a72f..57f7923d81 100644 --- a/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked +++ b/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked @@ -3,27 +3,27 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "A" source = { local = "deps_only/A" } dependencies = [ - "B", + { name = "B" }, ] dev-dependencies = [ - "D", + { name = "D" }, ] -[[move.dependency]] +[[move.package]] name = "B" source = { local = "deps_only/B" } dev-dependencies = [ - "C", + { name = "C" }, ] -[[move.dependency]] +[[move.package]] name = "C" source = { local = "deps_only/C" } -[[move.dependency]] +[[move.package]] name = "D" source = { local = "deps_only/D" } diff --git a/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked b/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked index 6ae4a9cbdf..f0b82a4c44 100644 --- a/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked +++ b/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked @@ -3,6 +3,6 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "OtherDep" -source = { local = "deps_only/other_dep", digest = "6A88B7888D6049EB0121900E22B6FA2C0E702F042C8C8D4FD62AD5C990B9F9A8", addr_subst = { "A" = "B" } } +source = { local = "deps_only/other_dep" } diff --git a/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked b/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked index 502e5fcff4..efc1cb5c5b 100644 --- a/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked +++ b/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked @@ -3,20 +3,20 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "A" -source = { local = "deps_only/A", addr_subst = { "AA" = "00000000000000000000000000000001" } } +source = { local = "deps_only/A" } dependencies = [ - "C", + { name = "C", addr_subst = { "AA" = "A" } }, ] -[[move.dependency]] +[[move.package]] name = "B" -source = { local = "deps_only/B", addr_subst = { "BA" = "00000000000000000000000000000001" } } +source = { local = "deps_only/B" } dependencies = [ - "C", + { name = "C", addr_subst = { "BA" = "A" } }, ] -[[move.dependency]] +[[move.package]] name = "C" -source = { local = "deps_only/C", addr_subst = { "AA" = "A" } } +source = { local = "deps_only/C" } diff --git a/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked b/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked index feec9fe410..c9c24a5e85 100644 --- a/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked +++ b/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked @@ -3,10 +3,10 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "C" source = { local = "deps_only/C" } -[[move.dependency]] +[[move.package]] name = "D" source = { local = "deps_only/D" } diff --git a/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked b/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked index 45f339d8cf..e852bbde79 100644 --- a/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked +++ b/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked @@ -3,13 +3,13 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "MoveNursery" source = { git = "https://github.com/move-language/move", rev = "781c844", subdir = "language/move-stdlib/nursery" } dependencies = [ - "MoveStdlib", + { name = "MoveStdlib" }, ] -[[move.dependency]] +[[move.package]] name = "MoveStdlib" source = { git = "https://github.com/move-language/move", rev = "781c844", subdir = "language/move-stdlib" } diff --git a/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked b/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked index 55e58f0cbd..a5fcfdf75a 100644 --- a/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked +++ b/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked @@ -3,13 +3,13 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "More" source = { local = "deps_only/nested/more" } -[[move.dependency]] +[[move.package]] name = "Nested" source = { local = "deps_only/nested" } dependencies = [ - "More", + { name = "More" }, ] diff --git a/language/tools/move-package/tests/test_sources/one_dep/Move.locked b/language/tools/move-package/tests/test_sources/one_dep/Move.locked index 8f849179fe..f0b82a4c44 100644 --- a/language/tools/move-package/tests/test_sources/one_dep/Move.locked +++ b/language/tools/move-package/tests/test_sources/one_dep/Move.locked @@ -3,6 +3,6 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "OtherDep" -source = { local = "deps_only/other_dep", addr_subst = { "A" = "B" } } +source = { local = "deps_only/other_dep" } diff --git a/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked b/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked index 97060fc306..f0b82a4c44 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked +++ b/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked @@ -3,6 +3,6 @@ [move] version = 0 -[[move.dependency]] +[[move.package]] name = "OtherDep" -source = { local = "deps_only/other_dep", digest = "BAD_DIGEST", addr_subst = { "A" = "B" } } +source = { local = "deps_only/other_dep" } diff --git a/language/tools/move-package/tests/test_sources/parsing_full_manifest/Move.resolved b/language/tools/move-package/tests/test_sources/parsing_full_manifest/Move.resolved index e8a8e57d5d..cb13e87cf8 100644 --- a/language/tools/move-package/tests/test_sources/parsing_full_manifest/Move.resolved +++ b/language/tools/move-package/tests/test_sources/parsing_full_manifest/Move.resolved @@ -1 +1 @@ -Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest for 'A' at "tests/test_sources/parsing_full_manifest/../a/Move.toml": No such file or directory (os error 2) +Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest at "tests/test_sources/parsing_full_manifest/../a": No such file or directory (os error 2) diff --git a/language/tools/move-package/tests/test_sources/parsing_full_manifest_with_extra_field/Move.resolved b/language/tools/move-package/tests/test_sources/parsing_full_manifest_with_extra_field/Move.resolved index 3095528361..d3c3d3e267 100644 --- a/language/tools/move-package/tests/test_sources/parsing_full_manifest_with_extra_field/Move.resolved +++ b/language/tools/move-package/tests/test_sources/parsing_full_manifest_with_extra_field/Move.resolved @@ -1 +1 @@ -Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest for 'A' at "tests/test_sources/parsing_full_manifest_with_extra_field/../a/Move.toml": No such file or directory (os error 2) +Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest at "tests/test_sources/parsing_full_manifest_with_extra_field/../a": No such file or directory (os error 2) diff --git a/language/tools/move-package/tests/test_sources/parsing_invalid_hex_address_in_subst/Move.resolved b/language/tools/move-package/tests/test_sources/parsing_invalid_hex_address_in_subst/Move.resolved index 0a875f9c96..e2c86e6f6d 100644 --- a/language/tools/move-package/tests/test_sources/parsing_invalid_hex_address_in_subst/Move.resolved +++ b/language/tools/move-package/tests/test_sources/parsing_invalid_hex_address_in_subst/Move.resolved @@ -1 +1 @@ -Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest for 'A' at "tests/test_sources/parsing_invalid_hex_address_in_subst/a/Move.toml": No such file or directory (os error 2) +Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest at "tests/test_sources/parsing_invalid_hex_address_in_subst/a": No such file or directory (os error 2) diff --git a/language/tools/move-package/tests/test_sources/parsing_non_identifier_address_name_in_subst/Move.resolved b/language/tools/move-package/tests/test_sources/parsing_non_identifier_address_name_in_subst/Move.resolved index 71f9626b9e..0c290e7fb2 100644 --- a/language/tools/move-package/tests/test_sources/parsing_non_identifier_address_name_in_subst/Move.resolved +++ b/language/tools/move-package/tests/test_sources/parsing_non_identifier_address_name_in_subst/Move.resolved @@ -1 +1 @@ -Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest for 'A' at "tests/test_sources/parsing_non_identifier_address_name_in_subst/a/Move.toml": No such file or directory (os error 2) +Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest at "tests/test_sources/parsing_non_identifier_address_name_in_subst/a": No such file or directory (os error 2) diff --git a/language/tools/move-package/tests/test_sources/parsing_unknown_toplevel_field/Move.resolved b/language/tools/move-package/tests/test_sources/parsing_unknown_toplevel_field/Move.resolved index c66954f998..2c186982ba 100644 --- a/language/tools/move-package/tests/test_sources/parsing_unknown_toplevel_field/Move.resolved +++ b/language/tools/move-package/tests/test_sources/parsing_unknown_toplevel_field/Move.resolved @@ -1 +1 @@ -Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest for 'A' at "tests/test_sources/parsing_unknown_toplevel_field/../a/Move.toml": No such file or directory (os error 2) +Failed to resolve dependencies for package 'name': Parsing manifest for 'A': Unable to find package manifest at "tests/test_sources/parsing_unknown_toplevel_field/../a": No such file or directory (os error 2) From 203030b4ff3dc0910ce1ec952a01b26e33e3c69b Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Fri, 23 Dec 2022 00:06:08 +0000 Subject: [PATCH 2/8] fixup: Write root's dependencies and dev-dependencies --- language/tools/move-package/src/lib.rs | 5 +- .../src/resolution/dependency_graph.rs | 113 +++++++++++------- .../src/resolution/lock_file/schema.rs | 17 ++- .../tests/test_dependency_graph.rs | 19 ++- .../dep_dev_dep_diamond/Move.locked | 12 ++ .../test_sources/dep_good_digest/Move.locked | 4 + .../diamond_problem_no_conflict/Move.locked | 7 ++ .../multiple_deps_no_rename/Move.locked | 5 + .../nested_deps_git_local/Move.locked | 5 + .../nested_deps_local_local/Move.locked | 5 + .../tests/test_sources/one_dep/Move.locked | 4 + .../one_dep_bad_digest/Move.locked | 4 + 12 files changed, 137 insertions(+), 63 deletions(-) diff --git a/language/tools/move-package/src/lib.rs b/language/tools/move-package/src/lib.rs index 1219dd75a5..757ba43289 100644 --- a/language/tools/move-package/src/lib.rs +++ b/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; use serde::{Deserialize, Serialize}; use source_package::layout::SourcePackageLayout; use std::{ @@ -233,7 +233,6 @@ 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( @@ -243,7 +242,7 @@ impl BuildConfig { 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)?; } diff --git a/language/tools/move-package/src/resolution/dependency_graph.rs b/language/tools/move-package/src/resolution/dependency_graph.rs index 51b37d10dc..2b1ceda888 100644 --- a/language/tools/move-package/src/resolution/dependency_graph.rs +++ b/language/tools/move-package/src/resolution/dependency_graph.rs @@ -137,35 +137,44 @@ impl DependencyGraph { /// the `lock_file::schema` module). pub fn read_from_lock( root_path: PathBuf, - root_package: PM::SourceManifest, + root_package: PM::PackageName, lock: &mut impl Read, ) -> Result { let mut package_graph = DiGraphMap::new(); let mut package_table = BTreeMap::new(); - // Seed graph with edges from the root package - let root = root_package.package.name; + let packages = schema::Packages::read(lock)?; - for (pkg, mut dep) in root_package.dependencies { + for schema::Dependency { + name, + subst, + digest, + } in packages.root_dependencies.into_iter().flatten() + { package_graph.add_edge( - root, - pkg, + root_package, + Symbol::from(name), Dependency { mode: DependencyMode::Always, - subst: dep.subst.take(), - digest: dep.digest.take(), + subst: subst.map(parse_substitution).transpose()?, + digest: digest.map(Symbol::from), }, ); } - for (pkg, mut dep) in root_package.dev_dependencies { + for schema::Dependency { + name, + subst, + digest, + } in packages.root_dev_dependencies.into_iter().flatten() + { package_graph.add_edge( - root, - pkg, + root_package, + Symbol::from(name), Dependency { mode: DependencyMode::DevOnly, - subst: dep.subst.take(), - digest: dep.digest.take(), + subst: subst.map(parse_substitution).transpose()?, + digest: digest.map(Symbol::from), }, ); } @@ -177,7 +186,7 @@ impl DependencyGraph { source, dependencies, dev_dependencies, - } in schema::Packages::read(lock)? + } in packages.packages.into_iter().flatten() { let pkg_name = PM::PackageName::from(pkg_name.as_str()); let source = parse_dependency(pkg_name.as_str(), source) @@ -247,7 +256,7 @@ impl DependencyGraph { let mut graph = DependencyGraph { root_path, - root_package: root, + root_package, package_graph, package_table, always_deps: BTreeSet::new(), @@ -259,47 +268,65 @@ impl DependencyGraph { Ok(graph) } - /// Serialize this dependency graph into a lock file. + /// Serialize this dependency graph into a lock file and return it. /// /// This operation fails, writing nothing, if the graph contains a cycle, and can fail with an /// undefined output if it cannot be represented in a TOML file. - pub fn write_to_lock(&self, lock: &mut LockFile) -> Result<()> { - let mut writer = BufWriter::new(&**lock); + pub fn write_to_lock(&self) -> Result { + let lock = LockFile::new(&self.root_path)?; + let mut writer = BufWriter::new(&*lock); + + self.write_dependencies_to_lock(self.root_package, &mut writer)?; + for (name, pkg) in &self.package_table { writeln!(writer, "\n[[move.package]]")?; writeln!(writer, "name = {}", str_escape(name.as_str())?)?; writeln!(writer, "source = {}", PackageTOML(pkg))?; - let mut deps: Vec<_> = self - .package_graph - .edges(*name) - .map(|(_, pkg, dep)| (dep, pkg)) - .collect(); + self.write_dependencies_to_lock(*name, &mut writer)?; + } - // Sort by kind ("always" dependencies go first), and by name, to keep the output - // stable. - deps.sort_by_key(|(dep, pkg)| (dep.mode, *pkg)); - let mut deps = deps.into_iter().peekable(); - - macro_rules! write_deps { - ($mode: pat, $label: literal) => { - if let Some((Dependency { mode: $mode, .. }, _)) = deps.peek() { - writeln!(writer, "{} = [", $label)?; - while let Some((dep @ Dependency { mode: $mode, .. }, pkg)) = deps.peek() { - writeln!(writer, " {},", DependencyTOML(*pkg, dep))?; - deps.next(); - } - writeln!(writer, "]")?; - } - }; - } + writer.flush()?; + std::mem::drop(writer); + + Ok(lock) + } - write_deps!(DependencyMode::Always, "dependencies"); - write_deps!(DependencyMode::DevOnly, "dev-dependencies"); + /// Helper function to output the dependencies and dev-dependencies of `name` from this + /// dependency graph, to the lock file under `writer`. + fn write_dependencies_to_lock( + &self, + name: PM::PackageName, + writer: &mut W, + ) -> Result<()> { + let mut deps: Vec<_> = self + .package_graph + .edges(name) + .map(|(_, pkg, dep)| (dep, pkg)) + .collect(); + + // Sort by kind ("always" dependencies go first), and by name, to keep the output + // stable. + deps.sort_by_key(|(dep, pkg)| (dep.mode, *pkg)); + let mut deps = deps.into_iter().peekable(); + + macro_rules! write_deps { + ($mode: pat, $label: literal) => { + if let Some((Dependency { mode: $mode, .. }, _)) = deps.peek() { + writeln!(writer, "\n{} = [", $label)?; + while let Some((dep @ Dependency { mode: $mode, .. }, pkg)) = deps.peek() { + writeln!(writer, " {},", DependencyTOML(*pkg, dep))?; + deps.next(); + } + writeln!(writer, "]")?; + } + }; } - writer.flush()?; + write_deps!(DependencyMode::Always, "dependencies"); + write_deps!(DependencyMode::DevOnly, "dev-dependencies"); + Ok(()) } diff --git a/language/tools/move-package/src/resolution/lock_file/schema.rs b/language/tools/move-package/src/resolution/lock_file/schema.rs index 9b82a66b17..cea15a1fbc 100644 --- a/language/tools/move-package/src/resolution/lock_file/schema.rs +++ b/language/tools/move-package/src/resolution/lock_file/schema.rs @@ -22,7 +22,13 @@ pub const VERSION: u64 = 0; #[derive(Deserialize)] pub struct Packages { #[serde(rename = "package")] - packages: Option>, + pub packages: Option>, + + #[serde(rename = "dependencies")] + pub root_dependencies: Option>, + + #[serde(rename = "dev-dependencies")] + pub root_dev_dependencies: Option>, } #[derive(Deserialize)] @@ -68,7 +74,7 @@ struct Header { impl Packages { /// Read packages from the lock file, assuming the file's format matches the schema expected /// by this lock file, and its version is not newer than the version supported by this library. - pub fn read(lock: &mut impl Read) -> Result> { + pub fn read(lock: &mut impl Read) -> Result { let contents = { let mut buf = String::new(); lock.read_to_string(&mut buf).context("Reading lock file")?; @@ -87,11 +93,10 @@ impl Packages { ); } - let Schema { - move_: Packages { packages }, - } = toml::de::from_str::>(&contents).context("Deserializing packages")?; + let Schema { move_: packages } = + toml::de::from_str::>(&contents).context("Deserializing packages")?; - Ok(packages.unwrap_or_default()) + Ok(packages) } } diff --git a/language/tools/move-package/tests/test_dependency_graph.rs b/language/tools/move-package/tests/test_dependency_graph.rs index ab2757fc42..52a4c6ebf1 100644 --- a/language/tools/move-package/tests/test_dependency_graph.rs +++ b/language/tools/move-package/tests/test_dependency_graph.rs @@ -4,6 +4,7 @@ use std::{ collections::BTreeSet, fs::{self, File}, + io::Write, path::PathBuf, }; @@ -20,19 +21,15 @@ fn lock_file_roundtrip() { let snapshot = pkg.join("Move.locked"); let commit = tmp.path().join("Move.lock"); - let mut lock = LockFile::new(&pkg).expect("Creating new lock file"); - let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest"); let graph = DependencyGraph::read_from_lock( pkg, - manifest, + Symbol::from("Root"), &mut File::open(&snapshot).expect("Opening snapshot"), ) .expect("Reading DependencyGraph"); - graph - .write_to_lock(&mut lock) - .expect("Writing DependencyGraph"); + let lock = graph.write_to_lock().expect("Writing DependencyGraph"); lock.commit(&commit).expect("Committing lock file"); @@ -52,13 +49,14 @@ fn lock_file_missing_dependency() { let commit = tmp.path().join("Move.lock"); let lock = LockFile::new(&pkg).expect("Creating new lock file"); - let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest"); - lock.commit(&commit).expect("Writing empty lock file"); + // Write a reference to a dependency that there isn't package information for. + writeln!(&*lock, r#"dependencies = [{{ name = "OtherDep" }}]"#).unwrap(); + lock.commit(&commit).expect("Writing partial lock file"); let Err(err) = DependencyGraph::read_from_lock( pkg, - manifest, + Symbol::from("Root"), &mut File::open(&commit).expect("Opening empty lock file"), ) else { panic!("Expected reading dependencies to fail."); @@ -100,10 +98,9 @@ fn always_deps_from_lock() { let pkg = dev_dep_test_package(); let snapshot = pkg.join("Move.locked"); - let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest"); let graph = DependencyGraph::read_from_lock( pkg, - manifest, + Symbol::from("Root"), &mut File::open(&snapshot).expect("Opening snapshot"), ) .expect("Creating DependencyGraph"); diff --git a/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked b/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked index 57f7923d81..47247f5f57 100644 --- a/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked +++ b/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.locked @@ -3,12 +3,23 @@ [move] version = 0 +dependencies = [ + { name = "A" }, + { name = "C" }, +] + +dev-dependencies = [ + { name = "B" }, +] + [[move.package]] name = "A" source = { local = "deps_only/A" } + dependencies = [ { name = "B" }, ] + dev-dependencies = [ { name = "D" }, ] @@ -16,6 +27,7 @@ dev-dependencies = [ [[move.package]] name = "B" source = { local = "deps_only/B" } + dev-dependencies = [ { name = "C" }, ] diff --git a/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked b/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked index f0b82a4c44..1a58ab4ade 100644 --- a/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked +++ b/language/tools/move-package/tests/test_sources/dep_good_digest/Move.locked @@ -3,6 +3,10 @@ [move] version = 0 +dependencies = [ + { name = "OtherDep", digest = "6A88B7888D6049EB0121900E22B6FA2C0E702F042C8C8D4FD62AD5C990B9F9A8", addr_subst = { "A" = "B" } }, +] + [[move.package]] name = "OtherDep" source = { local = "deps_only/other_dep" } diff --git a/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked b/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked index efc1cb5c5b..0fa39d610c 100644 --- a/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked +++ b/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.locked @@ -3,9 +3,15 @@ [move] version = 0 +dependencies = [ + { name = "A", addr_subst = { "AA" = "00000000000000000000000000000001" } }, + { name = "B", addr_subst = { "BA" = "00000000000000000000000000000001" } }, +] + [[move.package]] name = "A" source = { local = "deps_only/A" } + dependencies = [ { name = "C", addr_subst = { "AA" = "A" } }, ] @@ -13,6 +19,7 @@ dependencies = [ [[move.package]] name = "B" source = { local = "deps_only/B" } + dependencies = [ { name = "C", addr_subst = { "BA" = "A" } }, ] diff --git a/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked b/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked index c9c24a5e85..b559e64805 100644 --- a/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked +++ b/language/tools/move-package/tests/test_sources/multiple_deps_no_rename/Move.locked @@ -3,6 +3,11 @@ [move] version = 0 +dependencies = [ + { name = "C" }, + { name = "D" }, +] + [[move.package]] name = "C" source = { local = "deps_only/C" } diff --git a/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked b/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked index e852bbde79..c921aa3978 100644 --- a/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked +++ b/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.locked @@ -3,9 +3,14 @@ [move] version = 0 +dependencies = [ + { name = "MoveNursery" }, +] + [[move.package]] name = "MoveNursery" source = { git = "https://github.com/move-language/move", rev = "781c844", subdir = "language/move-stdlib/nursery" } + dependencies = [ { name = "MoveStdlib" }, ] diff --git a/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked b/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked index a5fcfdf75a..c8dbf13329 100644 --- a/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked +++ b/language/tools/move-package/tests/test_sources/nested_deps_local_local/Move.locked @@ -3,6 +3,10 @@ [move] version = 0 +dependencies = [ + { name = "Nested" }, +] + [[move.package]] name = "More" source = { local = "deps_only/nested/more" } @@ -10,6 +14,7 @@ source = { local = "deps_only/nested/more" } [[move.package]] name = "Nested" source = { local = "deps_only/nested" } + dependencies = [ { name = "More" }, ] diff --git a/language/tools/move-package/tests/test_sources/one_dep/Move.locked b/language/tools/move-package/tests/test_sources/one_dep/Move.locked index f0b82a4c44..fb1915365b 100644 --- a/language/tools/move-package/tests/test_sources/one_dep/Move.locked +++ b/language/tools/move-package/tests/test_sources/one_dep/Move.locked @@ -3,6 +3,10 @@ [move] version = 0 +dependencies = [ + { name = "OtherDep", addr_subst = { "A" = "B" } }, +] + [[move.package]] name = "OtherDep" source = { local = "deps_only/other_dep" } diff --git a/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked b/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked index f0b82a4c44..19d4a9fc45 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked +++ b/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.locked @@ -3,6 +3,10 @@ [move] version = 0 +dependencies = [ + { name = "OtherDep", digest = "BAD_DIGEST", addr_subst = { "A" = "B" } }, +] + [[move.package]] name = "OtherDep" source = { local = "deps_only/other_dep" } From 171fa88c5d03a9f875a6252b3a18aaca9828bc04 Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Fri, 23 Dec 2022 00:08:52 +0000 Subject: [PATCH 3/8] fixup: Fix EVM tests --- .../move-cli/tests/build_tests/unbound_dependency/args.evm.exp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/language/tools/move-cli/tests/build_tests/unbound_dependency/args.evm.exp b/language/tools/move-cli/tests/build_tests/unbound_dependency/args.evm.exp index e0349dc654..50f285bf42 100644 --- a/language/tools/move-cli/tests/build_tests/unbound_dependency/args.evm.exp +++ b/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) From 33a4d2cabd38533c659467bc00a19c0cec316dfd Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Sun, 1 Jan 2023 00:22:38 +0000 Subject: [PATCH 4/8] fixup: No dep graph contains root node --- .../src/resolution/dependency_graph.rs | 6 +++ .../tests/test_dependency_graph.rs | 47 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/language/tools/move-package/src/resolution/dependency_graph.rs b/language/tools/move-package/src/resolution/dependency_graph.rs index 2b1ceda888..f19a94a7f2 100644 --- a/language/tools/move-package/src/resolution/dependency_graph.rs +++ b/language/tools/move-package/src/resolution/dependency_graph.rs @@ -107,6 +107,9 @@ impl DependencyGraph { always_deps: BTreeSet::new(), }; + // Ensure there's always a root node, even if it has no edges. + graph.package_graph.add_node(graph.root_package); + graph .extend_graph( PM::DependencyKind::default(), @@ -145,6 +148,9 @@ impl DependencyGraph { let packages = schema::Packages::read(lock)?; + // Ensure there's always a root node, even if it has no edges. + package_graph.add_node(root_package); + for schema::Dependency { name, subst, diff --git a/language/tools/move-package/tests/test_dependency_graph.rs b/language/tools/move-package/tests/test_dependency_graph.rs index 52a4c6ebf1..103f205eda 100644 --- a/language/tools/move-package/tests/test_dependency_graph.rs +++ b/language/tools/move-package/tests/test_dependency_graph.rs @@ -14,6 +14,47 @@ use move_package::{ }; use move_symbol_pool::Symbol; +#[test] +fn no_dep_graph() { + let pkg = no_dep_test_package(); + + let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest"); + let graph = DependencyGraph::new( + &manifest, + pkg, + /* skip_fetch_latest_git_deps */ true, + &mut std::io::sink(), + ) + .expect("Creating DependencyGraph"); + + assert!( + graph.package_graph.contains_node(graph.root_package), + "A graph for a package with no dependencies should still contain the root package", + ); + + assert_eq!(graph.topological_order(), vec![graph.root_package]); +} + +#[test] +fn no_dep_graph_from_lock() { + let pkg = no_dep_test_package(); + + let snapshot = pkg.join("Move.locked"); + let graph = DependencyGraph::read_from_lock( + pkg, + Symbol::from("Root"), + &mut File::open(&snapshot).expect("Opening snapshot"), + ) + .expect("Reading DependencyGraph"); + + assert!( + graph.package_graph.contains_node(graph.root_package), + "A graph for a package with no dependencies should still contain the root package", + ); + + assert_eq!(graph.topological_order(), vec![graph.root_package]); +} + #[test] fn lock_file_roundtrip() { let tmp = tempfile::tempdir().unwrap(); @@ -116,6 +157,12 @@ fn always_deps_from_lock() { ); } +fn no_dep_test_package() -> PathBuf { + [".", "tests", "test_sources", "basic_no_deps"] + .into_iter() + .collect() +} + fn one_dep_test_package() -> PathBuf { [".", "tests", "test_sources", "one_dep"] .into_iter() From 3b2466bc99a9ee2499f8d3bdf7cf117dbbc2f34a Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Fri, 23 Dec 2022 11:50:45 +0000 Subject: [PATCH 5/8] [8/x][move-package/lock] DependencyGraph::immediate_dependencies Function to return a package's immediate dependencies in the transitive dependency graph, for use by `ResolutionGraph` when resolving renamings and in-scope addresses during resolution Test Plan: New tests: ``` move/language/tools/move-package$ cargo nextest ``` --- .../src/resolution/dependency_graph.rs | 14 ++++++ .../tests/test_dependency_graph.rs | 44 ++++++++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/language/tools/move-package/src/resolution/dependency_graph.rs b/language/tools/move-package/src/resolution/dependency_graph.rs index f19a94a7f2..cd3a94138f 100644 --- a/language/tools/move-package/src/resolution/dependency_graph.rs +++ b/language/tools/move-package/src/resolution/dependency_graph.rs @@ -348,6 +348,20 @@ impl DependencyGraph { .expect("Graph is determined to be acyclic when created") } + /// Return an iterator over `pkg`'s immediate dependencies in the graph. If `mode` is + /// `DependencyMode::Always`, only always dependencies are included, whereas if `mode` is + /// `DependencyMode::DevOnly`, both always and dev-only dependecies are included. + pub fn immediate_dependencies( + &'_ self, + pkg: PM::PackageName, + mode: DependencyMode, + ) -> impl Iterator { + self.package_graph + .edges(pkg) + .filter(move |(_, _, dep)| dep.mode <= mode) + .map(|(_, dep_name, dep)| (dep_name, dep, &self.package_table[&dep_name])) + } + /// Add the transitive dependencies and dev-dependencies from `package` to the dependency graph. fn extend_graph( &mut self, diff --git a/language/tools/move-package/tests/test_dependency_graph.rs b/language/tools/move-package/tests/test_dependency_graph.rs index 103f205eda..c698daec8a 100644 --- a/language/tools/move-package/tests/test_dependency_graph.rs +++ b/language/tools/move-package/tests/test_dependency_graph.rs @@ -9,7 +9,10 @@ use std::{ }; use move_package::{ - resolution::{dependency_graph::DependencyGraph, lock_file::LockFile}, + resolution::{ + dependency_graph::{DependencyGraph, DependencyMode}, + lock_file::LockFile, + }, source_package::manifest_parser::parse_move_manifest_from_file, }; use move_symbol_pool::Symbol; @@ -157,6 +160,45 @@ fn always_deps_from_lock() { ); } +#[test] +fn immediate_dependencies() { + let pkg = dev_dep_test_package(); + + let manifest = parse_move_manifest_from_file(&pkg).expect("Loading manifest"); + let graph = DependencyGraph::new( + &manifest, + pkg, + /* skip_fetch_latest_git_deps */ true, + &mut std::io::sink(), + ) + .expect("Creating DependencyGraph"); + + let r = Symbol::from("Root"); + let a = Symbol::from("A"); + let b = Symbol::from("B"); + let c = Symbol::from("C"); + let d = Symbol::from("D"); + + let deps = |pkg, mode| { + graph + .immediate_dependencies(pkg, mode) + .map(|(pkg, _, _)| pkg) + .collect::>() + }; + + assert_eq!(deps(r, DependencyMode::Always), BTreeSet::from([a, c]),); + assert_eq!(deps(a, DependencyMode::Always), BTreeSet::from([b]),); + assert_eq!(deps(b, DependencyMode::Always), BTreeSet::from([]),); + assert_eq!(deps(c, DependencyMode::Always), BTreeSet::from([]),); + assert_eq!(deps(d, DependencyMode::Always), BTreeSet::from([]),); + + assert_eq!(deps(r, DependencyMode::DevOnly), BTreeSet::from([a, b, c])); + assert_eq!(deps(a, DependencyMode::DevOnly), BTreeSet::from([b, d]),); + assert_eq!(deps(b, DependencyMode::DevOnly), BTreeSet::from([c]),); + assert_eq!(deps(c, DependencyMode::DevOnly), BTreeSet::from([]),); + assert_eq!(deps(d, DependencyMode::DevOnly), BTreeSet::from([]),); +} + fn no_dep_test_package() -> PathBuf { [".", "tests", "test_sources", "basic_no_deps"] .into_iter() From ef6207151639b6efbc00d22f58774e5b6abc30f9 Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Fri, 23 Dec 2022 21:40:53 +0000 Subject: [PATCH 6/8] [9/x][move-package/lock] ResolvingTable Extract the logic for unifying named addresses across address assignments, substitutions and renamings from `ResolvingGraph` into its own class -- `ResolvingTable`. This is to be used when integrating `DependencyGraph` into `ResolutionGraph`. Test Plan: New unit tests: ``` move/language/tools/move-package$ cargo nextest ``` --- .../tools/move-package/src/resolution/mod.rs | 1 + .../src/resolution/resolving_table.rs | 187 ++++++++++++++++++ .../tests/test_resolving_table.rs | 140 +++++++++++++ 3 files changed, 328 insertions(+) create mode 100644 language/tools/move-package/src/resolution/resolving_table.rs create mode 100644 language/tools/move-package/tests/test_resolving_table.rs diff --git a/language/tools/move-package/src/resolution/mod.rs b/language/tools/move-package/src/resolution/mod.rs index 7bffc9de43..33aa2e8c18 100644 --- a/language/tools/move-package/src/resolution/mod.rs +++ b/language/tools/move-package/src/resolution/mod.rs @@ -30,6 +30,7 @@ pub mod dependency_graph; mod digest; pub mod lock_file; pub mod resolution_graph; +pub mod resolving_table; pub fn download_dependency_repos( manifest: &SourceManifest, diff --git a/language/tools/move-package/src/resolution/resolving_table.rs b/language/tools/move-package/src/resolution/resolving_table.rs new file mode 100644 index 0000000000..5e7f2f3291 --- /dev/null +++ b/language/tools/move-package/src/resolution/resolving_table.rs @@ -0,0 +1,187 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +use std::{cmp::Ordering, collections::BTreeMap}; + +use anyhow::{anyhow, Result}; +use move_core_types::account_address::AccountAddress; + +use crate::source_package::parsed_manifest::{NamedAddress, PackageName}; + +/// A named address qualified with the package name whose scope it belongs to. +pub type QualifiedAddress = (PackageName, NamedAddress); + +/// A data structure for unifying named addresses across packages according to renamings and +/// assigning them numerical addresses. +#[derive(Debug)] +pub struct ResolvingTable { + /// Disjoint set data structure for assignments which can either hold no value, a fixed value or + /// a forwarding reference to another element in the table. Each entry in the `redirection` + /// table gets a slot in the `assignment` table. + assignments: Vec, + + /// Mapping named addresses to an entry in the `assignments` table. + redirection: BTreeMap, +} + +#[derive(Debug, PartialEq, Eq)] +enum Assignment { + Assign(Option), + Linked(usize), +} + +impl ResolvingTable { + /// A fresh `ResolvingTable` with no bindings. + pub fn new() -> ResolvingTable { + ResolvingTable { + assignments: Vec::new(), + redirection: BTreeMap::new(), + } + } + + /// Iterates over the bindings in this table that are within `pkg`'s scope. + pub fn bindings( + &self, + pkg: PackageName, + ) -> impl Iterator)> { + let start = (pkg, NamedAddress::from("")); + self.redirection + .range(start..) + .take_while(move |((scope, _), _)| *scope == pkg) + .map(|((_, name), ix)| (*name, self.parent(*ix))) + } + + /// Return a reference to the address that `name` is currently bound to, if one exists, or + /// `None` otherwise. + pub fn get(&self, name: QualifiedAddress) -> Option<&AccountAddress> { + self.parent(*self.redirection.get(&name)?).as_ref() + } + + /// Indicates whether there is a binding in this resolving table for `name`. A table contains a + /// binding if it has previously been passed into a call to `define` or `unify` (even if it has + /// not been assigned a concrete numerical address). + pub fn contains(&self, name: QualifiedAddress) -> bool { + self.redirection.contains_key(&name) + } + + /// Add the binding `name = addr` to the table and propagate it across all renamings that + /// transitively involve `name`. Fails if this introduces a contradiction (A path through + /// bindings between two account addresses that are unequal to each other), and succeeds + /// otherwise. + pub fn define(&mut self, name: QualifiedAddress, addr: Option) -> Result<()> { + let ix = self.get_or_create_assignment(name); + let Assignment::Assign(slot) = &mut self.assignments[ix] else { + unreachable!("Non-root assignment"); + }; + + match (slot, addr) { + (_, None) => { /* nop */ } + (slot @ None, addr) => *slot = addr, + (Some(existing), Some(new)) => { + if *existing != new { + return Err(unification_error(name.1, *existing, new)); + } + } + } + + Ok(()) + } + + /// Add the binding `a = b` to the table. Fails if this introduces a contradiction (A path + /// through bindings between two account addresses that are unequal to each other), and succeeds + /// otherwise. + pub fn unify(&mut self, a: QualifiedAddress, b: QualifiedAddress) -> Result<()> { + let ix = self.get_or_create_assignment(a); + let jx = self.get_or_create_assignment(b); + + let Some((ir, jr)) = self.get_mut_pair(ix, jx) else { + return Ok(()) + }; + + let Assignment::Assign(ia) = ir else { unreachable!("Non-root assignment"); }; + let Assignment::Assign(ja) = jr else { unreachable!("Non-root assignment"); }; + + match (ia, ja) { + (None, Some(_)) => *ir = Assignment::Linked(jx), + + (Some(_), None) | (None, None) => *jr = Assignment::Linked(ix), + + (Some(ia), Some(ja)) => { + if ia != ja { + return Err(unification_error(a.1, *ia, *ja)); + } + } + }; + + Ok(()) + } + + /// Returns the index of the "root" assignment (i.e. not a link to another assignment) for + /// `name` in this table, creating an empty assignment if one does not already exist. + /// + /// Performs path compression on the internal links to speed up future look-ups. + fn get_or_create_assignment(&mut self, name: QualifiedAddress) -> usize { + let Some(mut c) = self.redirection.get(&name).copied() else { + self.assignments.push(Assignment::Assign(None)); + self.redirection.insert(name, self.assignments.len() - 1); + return self.assignments.len() - 1; + }; + + let Assignment::Linked(mut p) = self.assignments[c] else { + return c; + }; + + let Assignment::Linked(mut gp) = self.assignments[p] else { + return p; + }; + + let mut chain = vec![c]; + while let Assignment::Linked(ggp) = self.assignments[gp] { + (c, p, gp) = (p, gp, ggp); + chain.push(c); + } + + for link in chain { + self.assignments[link] = Assignment::Linked(gp); + } + + gp + } + + /// Return a pair of mutable references into the `assignments` table at indices `ix` and `jx` if + /// they refer to different slots, or `None` if they are (i.e. if they are aliased.) + fn get_mut_pair(&mut self, ix: usize, jx: usize) -> Option<(&mut Assignment, &mut Assignment)> { + match ix.cmp(&jx) { + Ordering::Equal => None, + + Ordering::Greater => { + let (jr, ir) = self.get_mut_pair(jx, ix)?; + Some((ir, jr)) + } + + Ordering::Less => { + let (l, r) = self.assignments.split_at_mut(ix + 1); + Some((&mut l[ix], &mut r[jx - ix - 1])) + } + } + } + + /// Chase links from the assignent at index `ix` until a non-link `Assignment` is found, and + /// return a reference to that. + fn parent(&self, mut ix: usize) -> &Option { + loop { + match &self.assignments[ix] { + Assignment::Linked(p) => ix = *p, + Assignment::Assign(addr) => return addr, + } + } + } +} + +fn unification_error(name: NamedAddress, a: AccountAddress, b: AccountAddress) -> anyhow::Error { + anyhow!( + "Conflicting assignments for address '{name}': '0x{}' and '0x{}'.", + a.short_str_lossless(), + b.short_str_lossless(), + ) +} diff --git a/language/tools/move-package/tests/test_resolving_table.rs b/language/tools/move-package/tests/test_resolving_table.rs new file mode 100644 index 0000000000..dbc9438349 --- /dev/null +++ b/language/tools/move-package/tests/test_resolving_table.rs @@ -0,0 +1,140 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashSet; + +use move_core_types::account_address::AccountAddress; +use move_package::{ + resolution::resolving_table::ResolvingTable, + source_package::parsed_manifest::{NamedAddress, PackageName}, +}; + +#[test] +fn definitions() { + let mut table = ResolvingTable::new(); + + let p0 = PackageName::from("p0"); + let n0 = NamedAddress::from("n0"); + let n1 = NamedAddress::from("n1"); + let a0 = AccountAddress::random(); + let a1 = AccountAddress::random(); + assert_ne!(a0, a1); + + table.define((p0, n0), Some(a0)).expect("Definition"); + table.define((p0, n1), None).expect("Declaration"); + table.define((p0, n0), Some(a0)).expect("Redefinition"); + table.define((p0, n1), Some(a1)).expect("Prev declaration"); + + assert!( + table.define((p0, n0), Some(a1)).is_err(), + "Conflicting definition", + ); + + assert_eq!(Some(&a0), table.get((p0, n0))); + assert_eq!(Some(&a1), table.get((p0, n1))); +} + +#[test] +fn unification() { + let mut table = ResolvingTable::new(); + + let p0 = PackageName::from("p0"); + let n0 = NamedAddress::from("n0"); + let n1 = NamedAddress::from("n1"); + let n2 = NamedAddress::from("n2"); + let n3 = NamedAddress::from("n3"); + let n4 = NamedAddress::from("n4"); + let a0 = AccountAddress::random(); + let a1 = AccountAddress::random(); + assert_ne!(a0, a1); + + table.define((p0, n0), Some(a0)).expect("Definition"); + table.unify((p0, n0), (p0, n1)).expect("Unify fresh"); + + assert_eq!(Some(&a0), table.get((p0, n0))); + assert_eq!(Some(&a0), table.get((p0, n1))); + + table.define((p0, n2), None).expect("Declaration"); + table.unify((p0, n2), (p0, n3)).expect("Unify decl. 1"); + table.unify((p0, n3), (p0, n4)).expect("Unify decl. 2"); + table.define((p0, n4), Some(a1)).expect("Assign to chain"); + + assert_eq!(Some(&a1), table.get((p0, n2))); + assert_eq!(Some(&a1), table.get((p0, n3))); + assert_eq!(Some(&a1), table.get((p0, n4))); + + table + .unify((p0, n2), (p0, n3)) + .expect("Unify already unified"); + + assert!( + table.unify((p0, n3), (p0, n1)).is_err(), + "Conflicting definitions either side of unification", + ); +} + +#[test] +fn bindings() { + let mut table = ResolvingTable::new(); + + let p0 = PackageName::from("p0"); + let p1 = PackageName::from("p1"); + let n0 = NamedAddress::from("n0"); + let n1 = NamedAddress::from("n1"); + let n2 = NamedAddress::from("n2"); + let a0 = AccountAddress::random(); + let a1 = AccountAddress::random(); + + table.define((p0, n0), Some(a0)).unwrap(); + table.define((p0, n1), None).unwrap(); + table.define((p1, n2), Some(a1)).unwrap(); + + assert_eq!( + table.bindings(p0).collect::>(), + HashSet::from([(n0, &Some(a0)), (n1, &None)]), + "Bindings include unassigned addresses", + ); + + assert_eq!( + table.bindings(p1).collect::>(), + HashSet::from([(n2, &Some(a1))]), + ); + + table.unify((p0, n1), (p1, n2)).unwrap(); + table.unify((p0, n0), (p1, n0)).unwrap(); + + assert_eq!( + table.bindings(p0).collect::>(), + HashSet::from([(n0, &Some(a0)), (n1, &Some(a1))]), + "Bindings updated post unification", + ); + + assert_eq!( + table.bindings(p1).collect::>(), + HashSet::from([(n2, &Some(a1)), (n0, &Some(a0))]), + "Bindings updated post unification", + ); +} + +#[test] +fn contains() { + let mut table = ResolvingTable::new(); + + let p0 = PackageName::from("p0"); + let p1 = PackageName::from("p1"); + let n0 = NamedAddress::from("n0"); + let n1 = NamedAddress::from("n1"); + let a0 = AccountAddress::random(); + + table.define((p0, p1), Some(a0)).unwrap(); + table.define((p0, n0), None).unwrap(); + + // An assignment with a binding counts as contained. + assert!(table.contains((p0, p1))); + + // So does an assignment without a binding. + assert!(table.contains((p0, n0))); + + // But not a completely fresh address. + assert!(!table.contains((p0, n1))); +} From c1b2f816b82872e5da43388b15304dfa49093e2e Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Thu, 26 Jan 2023 17:13:00 +0000 Subject: [PATCH 7/8] fixup: Avoid double access into vector --- .../src/resolution/resolving_table.rs | 39 +++++++------------ 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/language/tools/move-package/src/resolution/resolving_table.rs b/language/tools/move-package/src/resolution/resolving_table.rs index 5e7f2f3291..2b82f0450c 100644 --- a/language/tools/move-package/src/resolution/resolving_table.rs +++ b/language/tools/move-package/src/resolution/resolving_table.rs @@ -1,7 +1,7 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use std::{cmp::Ordering, collections::BTreeMap}; +use std::collections::BTreeMap; use anyhow::{anyhow, Result}; use move_core_types::account_address::AccountAddress; @@ -94,21 +94,26 @@ impl ResolvingTable { let ix = self.get_or_create_assignment(a); let jx = self.get_or_create_assignment(b); - let Some((ir, jr)) = self.get_mut_pair(ix, jx) else { - return Ok(()) + if ix == jx { + return Ok(()); + } + + let Assignment::Assign(ia) = self.assignments[ix] else { + unreachable!("Non-root assignment"); }; - let Assignment::Assign(ia) = ir else { unreachable!("Non-root assignment"); }; - let Assignment::Assign(ja) = jr else { unreachable!("Non-root assignment"); }; + let Assignment::Assign(ja) = self.assignments[jx] else { + unreachable!("Non-root assignment"); + }; match (ia, ja) { - (None, Some(_)) => *ir = Assignment::Linked(jx), + (None, Some(_)) => self.assignments[ix] = Assignment::Linked(jx), - (Some(_), None) | (None, None) => *jr = Assignment::Linked(ix), + (Some(_), None) | (None, None) => self.assignments[jx] = Assignment::Linked(ix), (Some(ia), Some(ja)) => { if ia != ja { - return Err(unification_error(a.1, *ia, *ja)); + return Err(unification_error(a.1, ia, ja)); } } }; @@ -148,24 +153,6 @@ impl ResolvingTable { gp } - /// Return a pair of mutable references into the `assignments` table at indices `ix` and `jx` if - /// they refer to different slots, or `None` if they are (i.e. if they are aliased.) - fn get_mut_pair(&mut self, ix: usize, jx: usize) -> Option<(&mut Assignment, &mut Assignment)> { - match ix.cmp(&jx) { - Ordering::Equal => None, - - Ordering::Greater => { - let (jr, ir) = self.get_mut_pair(jx, ix)?; - Some((ir, jr)) - } - - Ordering::Less => { - let (l, r) = self.assignments.split_at_mut(ix + 1); - Some((&mut l[ix], &mut r[jx - ix - 1])) - } - } - } - /// Chase links from the assignent at index `ix` until a non-link `Assignment` is found, and /// return a reference to that. fn parent(&self, mut ix: usize) -> &Option { From c4d25b37912af7670ff9e886a799fcf581e4de39 Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Sun, 25 Dec 2022 18:24:58 +0000 Subject: [PATCH 8/8] [10/x][move-package/lock] Integrate DependencyGraph into ResolvedGraph Integrate `DependencyGraph` and `ResolvingTable` into the creation of `ResolvedGraph`. This change replaces the recursive dependency traversal logic of `ResolutionGraph` that `DependencyGraph` was based on with a linear traversal of the package's transitive dependencies, extracted from an existing `DependencyGraph`. This is part of the preparation for integrating third-party package managers into the package resolution process: They can now inject further transitive dependencies into the DependencyGraph which will be fetched during resolution to create the `ResolvedGraph`. Although some error message wording changes (particularly where it previously relied on recursive calls for building up error context), the logic is otherwise unchanged. Test Plan: Existing test cases: ``` move$ cargo-nextest nextest run -E 'package(move-cli) | package(move-package)' ``` --- language/tools/move-cli/src/base/test.rs | 2 +- .../build_tests/dependency_chain/args.exp | 2 +- .../build_tests/unbound_address/args.evm.exp | 14 +- .../build_tests/unbound_address/args.exp | 14 +- .../src/compilation/build_plan.rs | 24 +- .../src/compilation/compiled_package.rs | 19 +- .../src/compilation/model_builder.rs | 6 +- language/tools/move-package/src/lib.rs | 14 +- .../src/resolution/dependency_graph.rs | 10 +- .../src/resolution/resolution_graph.rs | 939 ++++++------------ .../package_hash_skips_non_move_files.rs | 2 +- .../tests/test_additional_addresses.rs | 111 ++- .../tests/test_dependency_graph.rs | 18 +- .../tools/move-package/tests/test_runner.rs | 4 +- .../test_sources/basic_no_deps/Move.resolved | 40 +- .../Move.resolved | 46 +- .../Move.resolved | 13 +- .../Move.resolved | 48 +- .../Move.resolved | 2 +- .../Move.resolved | 2 +- .../Move.resolved | 2 +- .../conflicting_dev_addresses/Move.resolved | 2 +- .../dep_dev_dep_diamond/Move.resolved | 228 ++--- .../dep_good_digest/Move.resolved | 98 +- .../Move.resolved | 178 ++-- .../diamond_problem_conflict/Move.resolved | 2 +- .../diamond_problem_no_conflict/Move.resolved | 184 ++-- .../Move.resolved | 2 +- .../multiple_deps_rename/Move.resolved | 142 +-- .../nested_deps_git_local/Move.resolved | 134 ++- .../tests/test_sources/one_dep/Move.resolved | 96 +- .../one_dep_assigned_address/Move.resolved | 90 +- .../one_dep_bad_digest/Move.resolved | 2 +- .../Move.resolved | 96 +- .../one_dep_reassigned_address/Move.resolved | 96 +- .../Move.resolved | 96 +- .../Move.resolved | 2 +- .../Move.resolved | 40 +- .../parsing_minimal_manifest/Move.resolved | 40 +- 39 files changed, 1133 insertions(+), 1727 deletions(-) diff --git a/language/tools/move-cli/src/base/test.rs b/language/tools/move-cli/src/base/test.rs index 901590752f..03b33c705a 100644 --- a/language/tools/move-cli/src/base/test.rs +++ b/language/tools/move-cli/src/base/test.rs @@ -193,7 +193,7 @@ pub fn run_move_unit_tests( .collect::>() }) .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 diff --git a/language/tools/move-cli/tests/build_tests/dependency_chain/args.exp b/language/tools/move-cli/tests/build_tests/dependency_chain/args.exp index ea1be9b5e0..229a055189 100644 --- a/language/tools/move-cli/tests/build_tests/dependency_chain/args.exp +++ b/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 diff --git a/language/tools/move-cli/tests/build_tests/unbound_address/args.evm.exp b/language/tools/move-cli/tests/build_tests/unbound_address/args.evm.exp index d3c437535e..691b91ea8c 100644 --- a/language/tools/move-cli/tests/build_tests/unbound_address/args.evm.exp +++ b/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' + ] diff --git a/language/tools/move-cli/tests/build_tests/unbound_address/args.exp b/language/tools/move-cli/tests/build_tests/unbound_address/args.exp index a89305d076..dde5aad952 100644 --- a/language/tools/move-cli/tests/build_tests/unbound_address/args.exp +++ b/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' + ] diff --git a/language/tools/move-package/src/compilation/build_plan.rs b/language/tools/move-package/src/compilation/build_plan.rs index 94fc299217..7f0536c7c6 100644 --- a/language/tools/move-package/src/compilation/build_plan.rs +++ b/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 { - 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()) diff --git a/language/tools/move-package/src/compilation/compiled_package.rs b/language/tools/move-package/src/compilation/compiled_package.rs index e3ea5a3d40..b25afadde5 100644 --- a/language/tools/move-package/src/compilation/compiled_package.rs +++ b/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: &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, @@ -918,7 +915,7 @@ pub(crate) fn make_source_and_deps_for_compiler( .collect::>>()?; 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)?; diff --git a/language/tools/move-package/src/compilation/model_builder.rs b/language/tools/move-package/src/compilation/model_builder.rs index b67fb4454d..0a60d9903a 100644 --- a/language/tools/move-package/src/compilation/model_builder.rs +++ b/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::>>()?; diff --git a/language/tools/move-package/src/lib.rs b/language/tools/move-package/src/lib.rs index 757ba43289..1c128cfb9c 100644 --- a/language/tools/move-package/src/lib.rs +++ b/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; +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, }; @@ -235,20 +234,15 @@ impl BuildConfig { // possibly be set by a different process in parallel. 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)?; 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) diff --git a/language/tools/move-package/src/resolution/dependency_graph.rs b/language/tools/move-package/src/resolution/dependency_graph.rs index cd3a94138f..7670f08d94 100644 --- a/language/tools/move-package/src/resolution/dependency_graph.rs +++ b/language/tools/move-package/src/resolution/dependency_graph.rs @@ -39,7 +39,7 @@ use super::{ /// /// In order to be `BuildConfig` agnostic, it contains `dev-dependencies` as well as `dependencies` /// and labels edges in the graph accordingly, as `DevOnly`, or `Always` dependencies. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct DependencyGraph { /// Path to the root package and its name (according to its manifest) pub root_path: PathBuf, @@ -58,13 +58,13 @@ pub struct DependencyGraph { pub always_deps: BTreeSet, } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Package { - kind: PM::DependencyKind, - version: Option, + pub kind: PM::DependencyKind, + pub version: Option, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Dependency { pub mode: DependencyMode, pub subst: Option, diff --git a/language/tools/move-package/src/resolution/resolution_graph.rs b/language/tools/move-package/src/resolution/resolution_graph.rs index c3bc4e99a7..65cdb221fe 100644 --- a/language/tools/move-package/src/resolution/resolution_graph.rs +++ b/language/tools/move-package/src/resolution/resolution_graph.rs @@ -2,56 +2,34 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use crate::{ - resolution::digest::compute_digest, - source_package::{ - layout::SourcePackageLayout, - parsed_manifest::{ - Dependency, FileName, NamedAddress, PackageDigest, PackageName, SourceManifest, - SubstOrRename, - }, - }, - BuildConfig, -}; use anyhow::{bail, Context, Result}; use move_command_line_common::files::{find_move_filenames, FileHash}; use move_core_types::account_address::AccountAddress; -use move_symbol_pool::Symbol; -use petgraph::{algo, graphmap::DiGraphMap, Outgoing}; use ptree::{print_tree, TreeBuilder}; use std::{ - cell::RefCell, collections::{BTreeMap, BTreeSet}, fs, io::Write, path::{Path, PathBuf}, - rc::Rc, }; -use super::{download_and_update_if_remote, parse_package_manifest}; - -pub type ResolvedTable = ResolutionTable; -pub type ResolvedPackage = ResolutionPackage; -pub type ResolvedGraph = ResolutionGraph; - -// rename_to => (from_package name, from_address_name) -pub type Renaming = BTreeMap; -pub type GraphIndex = PackageName; - -type ResolutionTable = BTreeMap; -type ResolvingTable = ResolutionTable; -type ResolvingGraph = ResolutionGraph; -type ResolvingPackage = ResolutionPackage; +use crate::{ + source_package::{ + layout::SourcePackageLayout, + manifest_parser::parse_move_manifest_from_file, + parsed_manifest::{ + FileName, NamedAddress, PackageDigest, PackageName, SourceManifest, SubstOrRename, + }, + }, + BuildConfig, +}; -#[derive(Debug, Clone)] -pub struct ResolvingNamedAddress { - value: Rc>>, -} +use super::{ + dependency_graph as DG, digest::compute_digest, download_and_update_if_remote, local_path, + resolving_table::ResolvingTable, +}; -/// A `ResolutionGraph` comes in two flavors: -/// 1. a `ResolutionGraph` during resolution (some named addresses may yet be instantiated) -/// 2. a `ResolvedGraph` which is a graph after resolution in which all named addresses have been -/// assigned a value. +/// The graph after resolution in which all named addresses have been assigned a value. /// /// Named addresses can be assigned values in a couple different ways: /// 1. They can be assigned a value in the declaring package. In this case the value of that @@ -59,593 +37,232 @@ pub struct ResolvingNamedAddress { /// 2. Can be left unassigned in the declaring package. In this case it can receive its value /// through unification across the package graph. /// -/// Named addresses can also be renamed in a package and will be re-exported under thes new names in this case. +/// Named addresses can also be renamed in a package and will be re-exported under thes new names in +/// this case. #[derive(Debug, Clone)] -pub struct ResolutionGraph { - pub root_package_path: PathBuf, +pub struct ResolvedGraph { + pub graph: DG::DependencyGraph, /// Build options pub build_options: BuildConfig, - /// Root package - pub root_package: SourceManifest, - /// Dependency graph - pub graph: DiGraphMap, /// A mapping of package name to its resolution - pub package_table: BTreeMap>, + pub package_table: PackageTable, } -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct ResolutionPackage { - /// Pointer into the `ResolutionGraph.graph` - pub resolution_graph_index: GraphIndex, - /// source manifest for this package +// rename_to => (from_package_name, from_address_name) +pub type Renaming = BTreeMap; +pub type ResolvedTable = BTreeMap; +type PackageTable = BTreeMap; + +#[derive(Debug, Clone)] +pub struct Package { + /// Source manifest for this package pub source_package: SourceManifest, /// Where this package is located on the filesystem pub package_path: PathBuf, /// The renaming of addresses performed by this package pub renaming: Renaming, - /// The mapping of addresses for this package (and that are in scope for it) - pub resolution_table: ResolutionTable, + /// The mapping of addresses that are in scope for this package. + pub resolved_table: ResolvedTable, /// The digest of the contents of all source files and manifest under the package root pub source_digest: PackageDigest, } -impl ResolvingGraph { - pub fn new( - root_package: SourceManifest, - root_package_path: PathBuf, +impl ResolvedGraph { + pub fn resolve( + graph: DG::DependencyGraph, mut build_options: BuildConfig, progress_output: &mut Progress, - ) -> Result { - if build_options.architecture.is_none() { - if let Some(info) = &root_package.build { - build_options.architecture = info.architecture; - } - } - let mut resolution_graph = Self { - root_package_path: root_package_path.clone(), - build_options, - root_package: root_package.clone(), - graph: DiGraphMap::new(), - package_table: BTreeMap::new(), - }; + ) -> Result { + let mut package_table = PackageTable::new(); + let mut resolving_table = ResolvingTable::new(); - resolution_graph - .build_resolution_graph( - root_package.clone(), - root_package_path, - true, - progress_output, - ) - .with_context(|| { - format!( - "Unable to resolve packages for package '{}'", - root_package.package.name - ) - })?; - Ok(resolution_graph) - } - - pub fn resolve(self) -> Result { - let ResolvingGraph { - root_package_path, - build_options, - root_package, - graph, - package_table, - } = self; - - let mut unresolved_addresses = Vec::new(); - - let resolved_package_table = package_table - .into_iter() - .map(|(name, package)| { - let ResolutionPackage { - resolution_graph_index, - source_package, - package_path, - renaming, - resolution_table, - source_digest, - } = package; - - let resolved_table = resolution_table - .into_iter() - .filter_map(|(addr_name, instantiation_opt)| { - match *instantiation_opt.value.borrow() { - None => { - unresolved_addresses.push(format!( - "Named address '{}' in package '{}'", - addr_name, name - )); - None - } - Some(addr) => Some((addr_name, addr)), - } - }) - .collect::>(); - let resolved_pkg = ResolvedPackage { - resolution_graph_index, - source_package, - package_path, - renaming, - resolution_table: resolved_table, - source_digest, - }; - (name, resolved_pkg) - }) - .collect::>(); - - if !unresolved_addresses.is_empty() { - bail!( - "Unresolved addresses found: [\n{}\n]\n\ - To fix this, add an entry for each unresolved address to the [addresses] section of {}/Move.toml: \ - e.g.,\n[addresses]\nStd = \"0x1\"\n\ - Alternatively, you can also define [dev-addresses] and call with the -d flag", - unresolved_addresses.join("\n"), - root_package_path.to_string_lossy() - ) - } - - Ok(ResolvedGraph { - root_package_path, - build_options, - root_package, - graph, - package_table: resolved_package_table, - }) - } - - fn build_resolution_graph( - &mut self, - package: SourceManifest, - package_path: PathBuf, - is_root_package: bool, - progress_output: &mut Progress, - ) -> Result<()> { - let package_name = package.package.name; - let package_node_id = match self.package_table.get(&package_name) { - None => self.get_or_add_node(package_name)?, - // Same package and we've already resolved it: OK, return early - Some(other) if other.source_package == package => return Ok(()), - // Different packages, with same name: Not OK - Some(other) => { - bail!( - "Conflicting dependencies found: package '{}' conflicts with '{}'", - other.source_package.package.name, - package.package.name, - ) - } - }; - - let mut renaming = BTreeMap::new(); - let mut resolution_table = self - .build_options - .additional_named_addresses - .clone() - .into_iter() - .map(|(name, addr)| { - ( - NamedAddress::from(name), - ResolvingNamedAddress::new(Some(addr)), - ) - }) - .collect(); - - // include dev dependencies if in dev mode - let additional_deps = if self.build_options.dev_mode { - package.dev_dependencies.clone() + let dep_mode = if build_options.dev_mode { + DG::DependencyMode::DevOnly } else { - BTreeMap::new() + DG::DependencyMode::Always }; - for (dep_name, dep) in package - .dependencies - .clone() - .into_iter() - .chain(additional_deps.into_iter()) - { - let dep_node_id = self.get_or_add_node(dep_name).with_context(|| { - format!( - "Cycle between packages {} and {} found", - package_name, dep_name - ) - })?; - self.graph.add_edge(package_node_id, dep_node_id, ()); - - let (dep_renaming, dep_resolution_table) = self - .process_dependency(dep_name, dep, package_path.clone(), progress_output) - .with_context(|| { - format!( - "While resolving dependency '{}' in package '{}'", - dep_name, package_name - ) - })?; - - ResolutionPackage::extend_renaming(&mut renaming, &dep_name, dep_renaming.clone()) - .with_context(|| { - format!( - "While resolving address renames in dependency '{}' in package '{}'", - dep_name, package_name - ) - })?; + // Resolve transitive dependencies in reverse topological order so that a package's + // dependencies get resolved before it does. + for pkg_name in graph.topological_order().into_iter().rev() { + // Skip dev-mode packages if not in dev-mode. + if !(build_options.dev_mode || graph.always_deps.contains(&pkg_name)) { + continue; + } - ResolutionPackage::extend_resolution_table( - &mut resolution_table, - &dep_name, - dep_resolution_table, - dep_renaming, - ) - .with_context(|| { - format!( - "Resolving named addresses for dependency '{}' in package '{}'", - dep_name, package_name + // Make sure the package is available locally. + let package_path = if pkg_name == graph.root_package { + graph.root_path.clone() + } else { + let pkg = &graph.package_table[&pkg_name]; + download_and_update_if_remote( + pkg_name, + &pkg.kind, + build_options.skip_fetch_latest_git_deps, + progress_output, ) - })?; - } - - self.unify_addresses_in_package(&package, &mut resolution_table, is_root_package)?; - - let source_digest = - ResolvingPackage::get_package_digest_for_config(&package_path, &self.build_options)?; - - let resolved_package = ResolutionPackage { - resolution_graph_index: package_node_id, - source_package: package, - package_path, - renaming, - resolution_table, - source_digest, - }; + .with_context(|| format!("Fetching '{pkg_name}'"))?; + graph.root_path.join(local_path(&pkg.kind)) + }; + + let mut resolved_pkg = Package::new(package_path, &build_options) + .with_context(|| format!("Resolving package '{pkg_name}'"))?; + + resolved_pkg + .define_addresses_in_package(&mut resolving_table) + .with_context(|| format!("Resolving addresses for '{pkg_name}'"))?; + + for (dep_name, dep, _pkg) in graph.immediate_dependencies(pkg_name, dep_mode) { + resolved_pkg + .process_dependency(dep_name, dep, &package_table, &mut resolving_table) + .with_context(|| { + format!("Processing dependency '{dep_name}' of '{pkg_name}'") + })?; + } - self.package_table.insert(package_name, resolved_package); - Ok(()) - } + package_table.insert(pkg_name, resolved_pkg); + } - fn unify_addresses_in_package( - &mut self, - package: &SourceManifest, - resolution_table: &mut ResolvingTable, - is_root_package: bool, - ) -> Result<()> { - let package_name = &package.package.name; - for (name, addr_opt) in package.addresses.clone().unwrap_or_default().into_iter() { - match resolution_table.get(&name) { - Some(other) => { - other.unify(addr_opt).with_context(|| { - format!( - "Unable to resolve named address '{}' in \ - package '{}' when resolving dependencies", - name, package_name - ) + // Add additional addresses to all package resolution tables. + for (name, addr) in &build_options.additional_named_addresses { + let name = NamedAddress::from(name.as_str()); + for pkg in package_table.keys() { + resolving_table + .define((*pkg, name), Some(*addr)) + .with_context(|| { + format!("Adding additional address '{name}' to package '{pkg}'") })?; - } - None => { - resolution_table.insert(name, ResolvingNamedAddress::new(addr_opt)); - } } } - if self.build_options.dev_mode && is_root_package { + let root_package = &package_table[&graph.root_package]; + + // Add dev addresses, but only for the root package + if build_options.dev_mode { let mut addr_to_name_mapping = BTreeMap::new(); - for (name, addr) in resolution_table - .iter() - .filter(|(_name, addr)| addr.value.borrow().is_some()) - { - let names = addr_to_name_mapping - .entry(addr.value.borrow().unwrap()) - .or_insert_with(Vec::new); - names.push(*name); + for (name, addr) in resolving_table.bindings(graph.root_package) { + if let Some(addr) = addr { + addr_to_name_mapping + .entry(*addr) + .or_insert_with(Vec::new) + .push(name); + }; } - for (name, addr) in package + for (name, addr) in root_package + .source_package .dev_address_assignments - .clone() - .unwrap_or_default() - .into_iter() + .iter() + .flatten() { - match resolution_table.get(&name) { - Some(other) => { - other.unify(Some(addr)).with_context(|| { - format!( - "Unable to resolve named address '{}' in\ - package '{}' when resolving dependencies in dev mode", - name, package_name - ) - })?; - } - None => { - bail!( - "Found unbound dev address assignment '{} = 0x{}' in root package '{}'. \ - Dev addresses cannot introduce new named addresses", - name, - addr.short_str_lossless(), - package_name - ); - } + let root_dev_addr = (graph.root_package, *name); + if !resolving_table.contains(root_dev_addr) { + bail!( + "Found unbound dev address assignment '{} = 0x{}' in root package '{}'. \ + Dev addresses cannot introduce new named addresses", + name, + addr.short_str_lossless(), + graph.root_package, + ); } - if let Some(conflicts) = addr_to_name_mapping.insert(addr, vec![name]) { + resolving_table + .define(root_dev_addr, Some(*addr)) + .with_context(|| { + format!( + "Unable to resolve named address '{}' in package '{}' when resolving \ + dependencies in dev mode", + name, graph.root_package, + ) + })?; + + if let Some(conflicts) = addr_to_name_mapping.insert(*addr, vec![*name]) { bail!( "Found non-unique dev address assignment '{name} = 0x{addr}' in root \ - package '{pkg}'. Dev address assignments must not conflict with any other \ - assignments in order to ensure that the package will compile with any \ - possible address assignment. \ - Assignment conflicts with previous assignments: {conflicts} = 0x{addr}", + package '{pkg}'. Dev address assignments must not conflict with any other \ + assignments in order to ensure that the package will compile with any \ + possible address assignment. \ + Assignment conflicts with previous assignments: {conflicts} = 0x{addr}", name = name, addr = addr.short_str_lossless(), - pkg = package_name, + pkg = graph.root_package, conflicts = conflicts - .into_iter() - .map(|n| n.to_string()) + .iter() + .map(NamedAddress::as_str) .collect::>() .join(", "), ) } } } - Ok(()) - } - - // Process a dependency. `dep_name_in_pkg` is the name assigned to the dependent package `dep` - // in the source manifest, and we check that this name matches the name of the dependency it is - // assigned to. - fn process_dependency( - &mut self, - dep_name_in_pkg: PackageName, - dep: Dependency, - root_path: PathBuf, - progress_output: &mut Progress, - ) -> Result<(Renaming, ResolvingTable)> { - download_and_update_if_remote( - dep_name_in_pkg, - &dep.kind, - self.build_options.skip_fetch_latest_git_deps, - progress_output, - )?; - let (dep_package, dep_package_dir) = - parse_package_manifest(&dep, &dep_name_in_pkg, root_path) - .with_context(|| format!("While processing dependency '{}'", dep_name_in_pkg))?; - self.build_resolution_graph(dep_package.clone(), dep_package_dir, false, progress_output) - .with_context(|| { - format!("Unable to resolve package dependency '{}'", dep_name_in_pkg) - })?; - - if dep_name_in_pkg != dep_package.package.name { - bail!("Name of dependency declared in package '{}' does not match dependency's package name '{}'", - dep_name_in_pkg, - dep_package.package.name - ); - } - - match dep.digest { - None => (), - Some(fixed_digest) => { - let resolved_pkg = self - .package_table - .get(&dep_name_in_pkg) - .context("Unable to find resolved package by name")?; - if fixed_digest != resolved_pkg.source_digest { - bail!( - "Source digest mismatch in dependency '{}'. Expected '{}' but got '{}'.", - dep_name_in_pkg, - fixed_digest, - resolved_pkg.source_digest - ) - } - } - } - - let resolving_dep = &self.package_table[&dep_name_in_pkg]; - let mut renaming = BTreeMap::new(); - let mut resolution_table = resolving_dep.resolution_table.clone(); - - // check that address being renamed exists in the dep that is being renamed/imported - if let Some(dep_subst) = dep.subst { - for (name, rename_from_or_assign) in dep_subst.into_iter() { - match rename_from_or_assign { - SubstOrRename::RenameFrom(ident) => { - // Make sure dep has the address that we're importing - if !resolving_dep.resolution_table.contains_key(&ident) { - bail!( - "Tried to rename named address {0} from package '{1}'.\ - However, {1} does not contain that address", - ident, - dep_name_in_pkg - ); - } - - // Apply the substitution, NB that the refcell for the address's value is kept! - if let Some(other_val) = resolution_table.remove(&ident) { - resolution_table.insert(name, other_val); - } - - if renaming.insert(name, (dep_name_in_pkg, ident)).is_some() { - bail!("Duplicate renaming of named address '{0}' found for dependency {1}", - name, - dep_name_in_pkg, - ); - } - } - SubstOrRename::Assign(value) => { - resolution_table - .get(&name) - .map(|named_addr| named_addr.unify(Some(value))) - .transpose() - .with_context(|| { - format!( - "Unable to assign value to named address {} in dependency {}", - name, dep_name_in_pkg - ) - })?; - } - } - } - } - - Ok((renaming, resolution_table)) - } - fn get_or_add_node(&mut self, package_name: PackageName) -> Result { - if self.graph.contains_node(package_name) { - // If we encounter a node that we've already added we should check for cycles - if algo::is_cyclic_directed(&self.graph) { - // get the first cycle. Exists because we found a cycle above. - let mut cycle = algo::kosaraju_scc(&self.graph)[0] - .iter() - .map(|node| node.as_str().to_string()) - .collect::>(); - // Add offending node at end to complete the cycle for display - cycle.push(package_name.as_str().to_string()); - bail!("Found cycle between packages: {}", cycle.join(" -> ")); - } - Ok(package_name) - } else { - Ok(self.graph.add_node(package_name)) - } - } -} - -impl ResolvingPackage { - // Extend and check for duplicate names in rename_to - fn extend_renaming( - renaming: &mut Renaming, - dep_name: &PackageName, - dep_renaming: Renaming, - ) -> Result<()> { - for (rename_to, rename_from) in dep_renaming.into_iter() { - // We cannot rename multiple named addresses to the same name. In the future we'll want - // to support this. - if renaming.insert(rename_to, rename_from).is_some() { - bail!( - "Duplicate renaming of named address '{}' found in dependency '{}'", - rename_to, - dep_name - ); + if build_options.architecture.is_none() { + if let Some(info) = &root_package.source_package.build { + build_options.architecture = info.architecture; } } - Ok(()) - } - // The resolution table contains the transitive closure of addresses that are known in that - // package. Extends the package's resolution table and checks for duplicate renamings that - // conflict during this process. - fn extend_resolution_table( - resolution_table: &mut ResolvingTable, - dep_name: &PackageName, - dep_resolution_table: ResolvingTable, - dep_renaming: Renaming, - ) -> Result<()> { - let renames = dep_renaming - .into_iter() - .map(|(rename_to, (_, rename_from))| (rename_from, rename_to)) - .collect::>(); - - for (addr_name, addr_value) in dep_resolution_table.into_iter() { - let addr_name = renames.get(&addr_name).cloned().unwrap_or(addr_name); - if let Some(other) = resolution_table.insert(addr_name, addr_value.clone()) { - // They need to be the same refcell so resolve to the same location if there are any - // possible reassignments - if other.value != addr_value.value { - bail!( - "Named address '{}' in dependency '{}' is already set to '{}' but was then reassigned to '{}'", - &addr_name, - dep_name, - match other.value.take() { - None => "unassigned".to_string(), - Some(addr) => format!("0x{}", addr.short_str_lossless()), - }, - match addr_value.value.take() { - None => "unassigned".to_string(), - Some(addr) => format!("0x{}", addr.short_str_lossless()), - } - ); - } - } + // Now that all address unification has happened, individual package resolution tables can + // be unified. + for pkg in package_table.values_mut() { + pkg.finalize_address_resolution(&resolving_table) + .with_context(|| { + format!( + "Unresolved addresses found. To fix this, add an entry for each unresolved \ + address to the [addresses] section of {}/Move.toml: e.g.,\n\n\ + \ + [addresses]\n\ + std = \"0x1\"\n\n\ + \ + Alternatively, you can also define [dev-addresses] and call with the -d \ + flag", + graph.root_path.display() + ) + })?; } - Ok(()) - } - - fn get_source_paths_for_config( - package_path: &Path, - config: &BuildConfig, - ) -> Result> { - let mut places_to_look = Vec::new(); - let mut add_path = |layout_path: SourcePackageLayout| { - let path = package_path.join(layout_path.path()); - if layout_path.is_optional() && !path.exists() { - return; - } - places_to_look.push(path) - }; - - add_path(SourcePackageLayout::Sources); - add_path(SourcePackageLayout::Scripts); - - if config.dev_mode { - add_path(SourcePackageLayout::Examples); - add_path(SourcePackageLayout::Tests); - } - Ok(places_to_look) + Ok(ResolvedGraph { + graph, + build_options, + package_table, + }) } - fn get_package_digest_for_config( - package_path: &Path, - config: &BuildConfig, - ) -> Result { - let mut source_paths = Self::get_source_paths_for_config(package_path, config)?; - source_paths.push(package_path.join(SourcePackageLayout::Manifest.path())); - compute_digest(source_paths.as_slice()) + pub fn root_package(&self) -> PackageName { + self.graph.root_package } -} -impl ResolvingNamedAddress { - pub fn new(address_opt: Option) -> Self { - Self { - value: Rc::new(RefCell::new(address_opt)), - } + pub fn get_package(&self, name: PackageName) -> &Package { + self.package_table.get(&name).unwrap() } - pub fn unify(&self, address_opt: Option) -> Result<()> { - match address_opt { - None => Ok(()), - Some(addr_val) => match &mut *self.value.borrow_mut() { - Some(current_value) if current_value != &addr_val => - bail!("Attempted to assign a different value '0x{}' to an a already-assigned named address '0x{}'", - addr_val.short_str_lossless(), current_value.short_str_lossless() - ), - Some(_) => Ok(()), - x @ None => { - *x = Some(addr_val); - Ok(()) - } - }, + /// Return the names of packages in this resolution graph in topological order. + pub fn topological_order(&self) -> Vec { + let mut order = self.graph.topological_order(); + if !self.build_options.dev_mode { + order.retain(|pkg| self.graph.always_deps.contains(pkg)); } - } -} - -impl ResolvedGraph { - pub fn get_package(&self, package_ident: &PackageName) -> &ResolvedPackage { - self.package_table.get(package_ident).unwrap() + order } fn print_info_dfs(&self, current_node: &PackageName, tree: &mut TreeBuilder) -> Result<()> { let pkg = self.package_table.get(current_node).unwrap(); - for (name, addr) in &pkg.resolution_table { + for (name, addr) in &pkg.resolved_table { tree.add_empty_child(format!("{}:0x{}", name, addr.short_str_lossless())); } - for node in self.graph.neighbors_directed(*current_node, Outgoing) { - tree.begin_child(node.to_string()); - self.print_info_dfs(&node, tree)?; + for dep in pkg.immediate_dependencies(self) { + tree.begin_child(dep.to_string()); + self.print_info_dfs(&dep, tree)?; tree.end_child(); } + Ok(()) } pub fn print_info(&self) -> Result<()> { - let root = self.root_package.package.name; + let root = self.root_package(); let mut tree = TreeBuilder::new(root.to_string()); self.print_info_dfs(&root, &mut tree)?; let tree = tree.build(); @@ -655,20 +272,16 @@ impl ResolvedGraph { pub fn extract_named_address_mapping( &self, - ) -> impl Iterator + '_ { - let rooot_package_name = &self.root_package.package.name; - let root_package = self - .package_table - .get(rooot_package_name) - .expect("Failed to find root package in package table -- this should never happen"); - - root_package - .resolution_table - .iter() - .map(|(name, addr)| (*name, *addr)) + ) -> impl Iterator { + self.package_table + .get(&self.root_package()) + .expect("Failed to find root package in package table -- this should never happen") + .resolved_table + .clone() + .into_iter() } - pub fn file_sources(&self) -> BTreeMap { + pub fn file_sources(&self) -> BTreeMap { self.package_table .iter() .flat_map(|(_, rpkg)| { @@ -676,7 +289,7 @@ impl ResolvedGraph { .unwrap() .iter() .map(|fname| { - let contents = fs::read_to_string(Path::new(fname.as_str())).unwrap(); + let contents = fs::read_to_string(fname.as_str()).unwrap(); let fhash = FileHash::new(&contents); (fhash, (*fname, contents)) }) @@ -686,61 +299,169 @@ impl ResolvedGraph { } } -impl ResolvedPackage { - pub fn get_sources(&self, config: &BuildConfig) -> Result> { - let places_to_look = - ResolvingPackage::get_source_paths_for_config(&self.package_path, config)? - .into_iter() - .map(|p| p.to_string_lossy().to_string()) - .collect::>(); - Ok(find_move_filenames(&places_to_look, false)? - .into_iter() - .map(Symbol::from) - .collect()) +impl Package { + fn new(package_path: PathBuf, config: &BuildConfig) -> Result { + Ok(Package { + source_package: parse_move_manifest_from_file(&package_path)?, + source_digest: package_digest_for_config(&package_path, config)?, + package_path, + renaming: Renaming::new(), + resolved_table: ResolvedTable::new(), + }) } - /// Returns the transitive dependencies of this package in dependency order - pub fn transitive_dependencies(&self, resolved_graph: &ResolvedGraph) -> BTreeSet { - let mut seen = BTreeSet::new(); - let resolve_package = |package_name: PackageName| { - let mut package_deps = resolved_graph - .package_table - .get(&package_name) - .unwrap() - .transitive_dependencies(resolved_graph); - package_deps.insert(package_name); - package_deps - }; + fn define_addresses_in_package(&self, resolving_table: &mut ResolvingTable) -> Result<()> { + let package = self.source_package.package.name; + for (name, addr) in self.source_package.addresses.iter().flatten() { + resolving_table.define((package, *name), *addr)?; + } + Ok(()) + } - let immediate_deps = self.immediate_dependencies(resolved_graph); - let transitive_deps: Vec<_> = immediate_deps - .into_iter() - .flat_map(resolve_package) + fn process_dependency( + &mut self, + dep_name: PackageName, + dep: &DG::Dependency, + package_table: &PackageTable, + resolving_table: &mut ResolvingTable, + ) -> Result<()> { + let pkg_name = self.source_package.package.name; + let mut dep_renaming = BTreeMap::new(); + + for (to, subst) in dep.subst.iter().flatten() { + match subst { + SubstOrRename::Assign(addr) => { + resolving_table.define((pkg_name, *to), Some(*addr))?; + } + + SubstOrRename::RenameFrom(from) => { + if !resolving_table.contains((dep_name, *from)) { + bail!( + "Tried to rename named address {0} from package '{1}', \ + however {1} does not contain that address", + from, + dep_name, + ) + } + + if let Some((prev_dep, prev_from)) = + self.renaming.insert(*to, (dep_name, *from)) + { + bail!( + "Duplicate renaming of named address '{to}' in dependencies of \ + '{pkg_name}'. Substituted with '{from}' from dependency '{dep_name}' \ + and '{prev_from}' from dependency '{prev_dep}'.", + ) + } + + dep_renaming.insert(*from, *to); + } + } + } + + let bound_in_dep: Vec<_> = resolving_table + .bindings(dep_name) + .map(|(from, _)| from) .collect(); - transitive_deps - .into_iter() - .filter(|ident| { - if !seen.contains(ident) { - seen.insert(*ident); - true - } else { - false + for from in bound_in_dep { + let to = *dep_renaming.get(&from).unwrap_or(&from); + resolving_table.unify((pkg_name, to), (dep_name, from))?; + } + + let Some(resolved_dep) = package_table.get(&dep_name) else { + bail!( + "Unable to find resolved information for dependency '{dep_name}' of \ + '{pkg_name}'", + ); + }; + + if let Some(digest) = dep.digest { + if digest != resolved_dep.source_digest { + bail!( + "Source digest mismatch in dependency '{dep_name}' of '{pkg_name}'. \ + Expected '{digest}' but got '{}'.", + resolved_dep.source_digest + ) + } + } + + Ok(()) + } + + fn finalize_address_resolution(&mut self, resolving_table: &ResolvingTable) -> Result<()> { + let mut unresolved_addresses = Vec::new(); + + let pkg_name = self.source_package.package.name; + for (name, addr) in resolving_table.bindings(pkg_name) { + match *addr { + Some(addr) => { + self.resolved_table.insert(name, addr); } - }) + None => { + unresolved_addresses + .push(format!(" Named address '{name}' in package '{pkg_name}'")); + } + } + } + + if !unresolved_addresses.is_empty() { + bail!( + "Unresolved addresses: [\n{}\n]", + unresolved_addresses.join("\n"), + ) + } + + Ok(()) + } + + pub fn immediate_dependencies(&self, graph: &ResolvedGraph) -> BTreeSet { + graph + .graph + .immediate_dependencies( + self.source_package.package.name, + if graph.build_options.dev_mode { + DG::DependencyMode::DevOnly + } else { + DG::DependencyMode::Always + }, + ) + .map(|(name, _, _)| name) .collect() } - pub fn immediate_dependencies(&self, resolved_graph: &ResolvedGraph) -> BTreeSet { - if resolved_graph.build_options.dev_mode { - self.source_package - .dependencies - .keys() - .chain(self.source_package.dev_dependencies.keys()) - .copied() - .collect() - } else { - self.source_package.dependencies.keys().copied().collect() + pub fn get_sources(&self, config: &BuildConfig) -> Result> { + let places_to_look = source_paths_for_config(&self.package_path, config); + Ok(find_move_filenames(&places_to_look, false)? + .into_iter() + .map(FileName::from) + .collect()) + } +} + +fn source_paths_for_config(package_path: &Path, config: &BuildConfig) -> Vec { + let mut places_to_look = Vec::new(); + let mut add_path = |layout_path: SourcePackageLayout| { + let path = package_path.join(layout_path.path()); + if layout_path.is_optional() && !path.exists() { + return; } + places_to_look.push(path) + }; + + add_path(SourcePackageLayout::Sources); + add_path(SourcePackageLayout::Scripts); + + if config.dev_mode { + add_path(SourcePackageLayout::Examples); + add_path(SourcePackageLayout::Tests); } + + places_to_look +} + +fn package_digest_for_config(package_path: &Path, config: &BuildConfig) -> Result { + let mut source_paths = source_paths_for_config(package_path, config); + source_paths.push(package_path.join(SourcePackageLayout::Manifest.path())); + compute_digest(&source_paths) } diff --git a/language/tools/move-package/tests/package_hash_skips_non_move_files.rs b/language/tools/move-package/tests/package_hash_skips_non_move_files.rs index fe96bc6bd9..384bb0cf5d 100644 --- a/language/tools/move-package/tests/package_hash_skips_non_move_files.rs +++ b/language/tools/move-package/tests/package_hash_skips_non_move_files.rs @@ -35,7 +35,7 @@ fn package_hash_skips_non_move_files() { std::fs::remove_file(&dummy_path).unwrap(); for (pkg, res_pkg) in pkg1.package_table { - let other_res_pkg = pkg2.get_package(&pkg); + let other_res_pkg = pkg2.get_package(pkg); assert_eq!( res_pkg.source_digest, other_res_pkg.source_digest, "source digests differ for {}", diff --git a/language/tools/move-package/tests/test_additional_addresses.rs b/language/tools/move-package/tests/test_additional_addresses.rs index fdaf81715c..fc62d2bb5d 100644 --- a/language/tools/move-package/tests/test_additional_addresses.rs +++ b/language/tools/move-package/tests/test_additional_addresses.rs @@ -4,97 +4,110 @@ use move_core_types::account_address::AccountAddress; use move_package::{ - resolution::resolution_graph as RG, source_package::manifest_parser as MP, BuildConfig, + resolution::{dependency_graph as DG, resolution_graph as RG}, + source_package::manifest_parser as MP, + BuildConfig, }; -use std::{collections::BTreeMap, path::Path}; +use std::{collections::BTreeMap, path::PathBuf}; use tempfile::tempdir; #[test] fn test_additonal_addresses() { - let path = - Path::new("tests/test_sources/basic_no_deps_address_not_assigned_with_dev_assignment"); - let pm = MP::parse_move_manifest_from_file(path).unwrap(); + let path: PathBuf = [ + "tests", + "test_sources", + "basic_no_deps_address_not_assigned_with_dev_assignment", + ] + .into_iter() + .collect(); - let mut additional_named_addresses = BTreeMap::new(); - additional_named_addresses.insert( - "A".to_string(), - AccountAddress::from_hex_literal("0x1").unwrap(), - ); + let pm = MP::parse_move_manifest_from_file(&path).unwrap(); - assert!(RG::ResolutionGraph::new( - pm.clone(), - path.parent().unwrap().to_path_buf(), + let mut sink = std::io::sink(); + let dg = DG::DependencyGraph::new( + &pm, path, /* skip_fetch_latest_git_deps */ true, &mut sink, + ) + .unwrap(); + + assert!(RG::ResolvedGraph::resolve( + dg.clone(), BuildConfig { install_dir: Some(tempdir().unwrap().path().to_path_buf()), - additional_named_addresses, + additional_named_addresses: BTreeMap::from([( + "A".to_string(), + AccountAddress::from_hex_literal("0x1").unwrap() + )]), ..Default::default() }, - &mut Vec::new() /* empty writer as no diags needed */ + &mut sink, ) - .unwrap() - .resolve() .is_ok()); - assert!(RG::ResolutionGraph::new( - pm, - path.parent().unwrap().to_path_buf(), + assert!(RG::ResolvedGraph::resolve( + dg, BuildConfig { install_dir: Some(tempdir().unwrap().path().to_path_buf()), ..Default::default() }, - &mut Vec::new() /* empty writer as no diags needed */ + &mut sink, ) - .unwrap() - .resolve() .is_err()); } #[test] fn test_additonal_addresses_already_assigned_same_value() { - let path = Path::new("tests/test_sources/basic_no_deps_address_assigned"); - let pm = MP::parse_move_manifest_from_file(path).unwrap(); + let path: PathBuf = ["tests", "test_sources", "basic_no_deps_address_assigned"] + .into_iter() + .collect(); + + let pm = MP::parse_move_manifest_from_file(&path).unwrap(); - let mut additional_named_addresses = BTreeMap::new(); - additional_named_addresses.insert( - "A".to_string(), - AccountAddress::from_hex_literal("0x0").unwrap(), - ); + let mut sink = std::io::sink(); + let dg = DG::DependencyGraph::new( + &pm, path, /* skip_fetch_latest_git_deps */ true, &mut sink, + ) + .unwrap(); - assert!(RG::ResolutionGraph::new( - pm, - path.parent().unwrap().to_path_buf(), + assert!(RG::ResolvedGraph::resolve( + dg, BuildConfig { install_dir: Some(tempdir().unwrap().path().to_path_buf()), - additional_named_addresses, + additional_named_addresses: BTreeMap::from([( + "A".to_string(), + AccountAddress::from_hex_literal("0x0").unwrap() + )]), ..Default::default() }, - &mut Vec::new() /* empty writer as no diags needed */ + &mut sink, ) - .unwrap() - .resolve() .is_ok()); } #[test] fn test_additonal_addresses_already_assigned_different_value() { - let path = Path::new("tests/test_sources/basic_no_deps_address_assigned"); - let pm = MP::parse_move_manifest_from_file(path).unwrap(); + let path: PathBuf = ["tests", "test_sources", "basic_no_deps_address_assigned"] + .into_iter() + .collect(); + + let pm = MP::parse_move_manifest_from_file(&path).unwrap(); - let mut additional_named_addresses = BTreeMap::new(); - additional_named_addresses.insert( - "A".to_string(), - AccountAddress::from_hex_literal("0x1").unwrap(), - ); + let mut sink = std::io::sink(); + let dg = DG::DependencyGraph::new( + &pm, path, /* skip_fetch_latest_git_deps */ true, &mut sink, + ) + .unwrap(); - assert!(RG::ResolutionGraph::new( - pm, - path.parent().unwrap().to_path_buf(), + assert!(RG::ResolvedGraph::resolve( + dg, BuildConfig { install_dir: Some(tempdir().unwrap().path().to_path_buf()), - additional_named_addresses, + additional_named_addresses: BTreeMap::from([( + "A".to_string(), + AccountAddress::from_hex_literal("0x1").unwrap() + )]), ..Default::default() }, - &mut Vec::new() /* empty writer as no diags needed */ + &mut sink, ) .is_err()); } diff --git a/language/tools/move-package/tests/test_dependency_graph.rs b/language/tools/move-package/tests/test_dependency_graph.rs index c698daec8a..57672e3c00 100644 --- a/language/tools/move-package/tests/test_dependency_graph.rs +++ b/language/tools/move-package/tests/test_dependency_graph.rs @@ -186,17 +186,17 @@ fn immediate_dependencies() { .collect::>() }; - assert_eq!(deps(r, DependencyMode::Always), BTreeSet::from([a, c]),); - assert_eq!(deps(a, DependencyMode::Always), BTreeSet::from([b]),); - assert_eq!(deps(b, DependencyMode::Always), BTreeSet::from([]),); - assert_eq!(deps(c, DependencyMode::Always), BTreeSet::from([]),); - assert_eq!(deps(d, DependencyMode::Always), BTreeSet::from([]),); + assert_eq!(deps(r, DependencyMode::Always), BTreeSet::from([a, c])); + assert_eq!(deps(a, DependencyMode::Always), BTreeSet::from([b])); + assert_eq!(deps(b, DependencyMode::Always), BTreeSet::from([])); + assert_eq!(deps(c, DependencyMode::Always), BTreeSet::from([])); + assert_eq!(deps(d, DependencyMode::Always), BTreeSet::from([])); assert_eq!(deps(r, DependencyMode::DevOnly), BTreeSet::from([a, b, c])); - assert_eq!(deps(a, DependencyMode::DevOnly), BTreeSet::from([b, d]),); - assert_eq!(deps(b, DependencyMode::DevOnly), BTreeSet::from([c]),); - assert_eq!(deps(c, DependencyMode::DevOnly), BTreeSet::from([]),); - assert_eq!(deps(d, DependencyMode::DevOnly), BTreeSet::from([]),); + assert_eq!(deps(a, DependencyMode::DevOnly), BTreeSet::from([b, d])); + assert_eq!(deps(b, DependencyMode::DevOnly), BTreeSet::from([c])); + assert_eq!(deps(c, DependencyMode::DevOnly), BTreeSet::from([])); + assert_eq!(deps(d, DependencyMode::DevOnly), BTreeSet::from([])); } fn no_dep_test_package() -> PathBuf { diff --git a/language/tools/move-package/tests/test_runner.rs b/language/tools/move-package/tests/test_runner.rs index 4b9c795dd8..8a28fa8461 100644 --- a/language/tools/move-package/tests/test_runner.rs +++ b/language/tools/move-package/tests/test_runner.rs @@ -12,7 +12,7 @@ use move_package::{ }, package_hooks, package_hooks::PackageHooks, - resolution::resolution_graph::ResolvedPackage, + resolution::resolution_graph::Package, source_package::parsed_manifest::{CustomDepInfo, PackageDigest}, BuildConfig, ModelConfig, }; @@ -177,7 +177,7 @@ fn scrub_compiled_package(pkg: &mut CompiledPackageInfo) { scrub_build_config(&mut pkg.build_flags); } -fn scrub_resolved_package(pkg: &mut ResolvedPackage) { +fn scrub_resolved_package(pkg: &mut Package) { pkg.package_path = PathBuf::from("ELIDED_FOR_TEST"); pkg.source_digest = PackageDigest::from("ELIDED_FOR_TEST"); } diff --git a/language/tools/move-package/tests/test_sources/basic_no_deps/Move.resolved b/language/tools/move-package/tests/test_sources/basic_no_deps/Move.resolved index 0e356dd656..5dc66b3741 100644 --- a/language/tools/move-package/tests/test_sources/basic_no_deps/Move.resolved +++ b/language/tools/move-package/tests/test_sources/basic_no_deps/Move.resolved @@ -1,5 +1,15 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/basic_no_deps", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/basic_no_deps", + root_package: "test", + package_graph: { + "test": [], + }, + package_table: {}, + always_deps: { + "test", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,30 +27,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "test", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: None, - dev_address_assignments: None, - build: None, - dependencies: {}, - dev_dependencies: {}, - }, - graph: { - "test": [], - }, package_table: { - "test": ResolutionPackage { - resolution_graph_index: "test", + "test": Package { source_package: SourceManifest { package: PackageInfo { name: "test", @@ -61,7 +49,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, }, diff --git a/language/tools/move-package/tests/test_sources/basic_no_deps_address_assigned/Move.resolved b/language/tools/move-package/tests/test_sources/basic_no_deps_address_assigned/Move.resolved index be663ffaa9..aec7cc237a 100644 --- a/language/tools/move-package/tests/test_sources/basic_no_deps_address_assigned/Move.resolved +++ b/language/tools/move-package/tests/test_sources/basic_no_deps_address_assigned/Move.resolved @@ -1,5 +1,15 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/basic_no_deps_address_assigned", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/basic_no_deps_address_assigned", + root_package: "test", + package_graph: { + "test": [], + }, + package_table: {}, + always_deps: { + "test", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,36 +27,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "test", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "A": Some( - 00000000000000000000000000000000, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: {}, - dev_dependencies: {}, - }, - graph: { - "test": [], - }, package_table: { - "test": ResolutionPackage { - resolution_graph_index: "test", + "test": Package { source_package: SourceManifest { package: PackageInfo { name: "test", @@ -73,7 +55,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000000, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned/Move.resolved b/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned/Move.resolved index 800db8c337..b712376e2a 100644 --- a/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned/Move.resolved +++ b/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned/Move.resolved @@ -1,7 +1,8 @@ -Unresolved addresses found: [ -Named address 'A' in package 'test' -] -To fix this, add an entry for each unresolved address to the [addresses] section of tests/test_sources/basic_no_deps_address_not_assigned/Move.toml: e.g., +Unresolved addresses found. To fix this, add an entry for each unresolved address to the [addresses] section of tests/test_sources/basic_no_deps_address_not_assigned/Move.toml: e.g., + [addresses] -Std = "0x1" -Alternatively, you can also define [dev-addresses] and call with the -d flag +std = "0x1" + +Alternatively, you can also define [dev-addresses] and call with the -d flag: Unresolved addresses: [ + Named address 'A' in package 'test' +] diff --git a/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned_with_dev_assignment/Move.resolved b/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned_with_dev_assignment/Move.resolved index 540b87af44..6c4ee0350b 100644 --- a/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned_with_dev_assignment/Move.resolved +++ b/language/tools/move-package/tests/test_sources/basic_no_deps_address_not_assigned_with_dev_assignment/Move.resolved @@ -1,5 +1,15 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/basic_no_deps_address_not_assigned_with_dev_assignment", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/basic_no_deps_address_not_assigned_with_dev_assignment", + root_package: "test", + package_graph: { + "test": [], + }, + package_table: {}, + always_deps: { + "test", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,38 +27,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "test", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "A": None, - }, - ), - dev_address_assignments: Some( - { - "A": 00000000000000000000000000000001, - }, - ), - build: None, - dependencies: {}, - dev_dependencies: {}, - }, - graph: { - "test": [], - }, package_table: { - "test": ResolutionPackage { - resolution_graph_index: "test", + "test": Package { source_package: SourceManifest { package: PackageInfo { name: "test", @@ -77,7 +57,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/basic_no_deps_invalid_dev_address_assignment/Move.resolved b/language/tools/move-package/tests/test_sources/basic_no_deps_invalid_dev_address_assignment/Move.resolved index d2746086e3..1cd54e6117 100644 --- a/language/tools/move-package/tests/test_sources/basic_no_deps_invalid_dev_address_assignment/Move.resolved +++ b/language/tools/move-package/tests/test_sources/basic_no_deps_invalid_dev_address_assignment/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'test': Found unbound dev address assignment 'B = 0x1' in root package 'test'. Dev addresses cannot introduce new named addresses +Found unbound dev address assignment 'B = 0x1' in root package 'test'. Dev addresses cannot introduce new named addresses diff --git a/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dep/Move.resolved b/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dep/Move.resolved index f04022e467..e921fef0ae 100644 --- a/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dep/Move.resolved +++ b/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dep/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'Root': Found non-unique dev address assignment 'B = 0x1' in root package 'Root'. Dev address assignments must not conflict with any other assignments in order to ensure that the package will compile with any possible address assignment. Assignment conflicts with previous assignments: A = 0x1 +Found non-unique dev address assignment 'B = 0x1' in root package 'Root'. Dev address assignments must not conflict with any other assignments in order to ensure that the package will compile with any possible address assignment. Assignment conflicts with previous assignments: A = 0x1 diff --git a/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dev_dep/Move.resolved b/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dev_dep/Move.resolved index f04022e467..e921fef0ae 100644 --- a/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dev_dep/Move.resolved +++ b/language/tools/move-package/tests/test_sources/conflicting_dev_address_with_dev_dep/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'Root': Found non-unique dev address assignment 'B = 0x1' in root package 'Root'. Dev address assignments must not conflict with any other assignments in order to ensure that the package will compile with any possible address assignment. Assignment conflicts with previous assignments: A = 0x1 +Found non-unique dev address assignment 'B = 0x1' in root package 'Root'. Dev address assignments must not conflict with any other assignments in order to ensure that the package will compile with any possible address assignment. Assignment conflicts with previous assignments: A = 0x1 diff --git a/language/tools/move-package/tests/test_sources/conflicting_dev_addresses/Move.resolved b/language/tools/move-package/tests/test_sources/conflicting_dev_addresses/Move.resolved index f04022e467..e921fef0ae 100644 --- a/language/tools/move-package/tests/test_sources/conflicting_dev_addresses/Move.resolved +++ b/language/tools/move-package/tests/test_sources/conflicting_dev_addresses/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'Root': Found non-unique dev address assignment 'B = 0x1' in root package 'Root'. Dev address assignments must not conflict with any other assignments in order to ensure that the package will compile with any possible address assignment. Assignment conflicts with previous assignments: A = 0x1 +Found non-unique dev address assignment 'B = 0x1' in root package 'Root'. Dev address assignments must not conflict with any other assignments in order to ensure that the package will compile with any possible address assignment. Assignment conflicts with previous assignments: A = 0x1 diff --git a/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.resolved b/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.resolved index f51eb92c55..c7df78289c 100644 --- a/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.resolved +++ b/language/tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.resolved @@ -1,5 +1,100 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/dep_dev_dep_diamond", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/dep_dev_dep_diamond", + root_package: "Root", + package_graph: { + "Root": [ + ( + "A", + Outgoing, + ), + ( + "C", + Outgoing, + ), + ( + "B", + Outgoing, + ), + ], + "B": [ + ( + "C", + Outgoing, + ), + ( + "A", + Incoming, + ), + ( + "Root", + Incoming, + ), + ], + "C": [ + ( + "B", + Incoming, + ), + ( + "Root", + Incoming, + ), + ], + "A": [ + ( + "B", + Outgoing, + ), + ( + "D", + Outgoing, + ), + ( + "Root", + Incoming, + ), + ], + "D": [ + ( + "A", + Incoming, + ), + ], + }, + package_table: { + "A": Package { + kind: Local( + "deps_only/A", + ), + version: None, + }, + "B": Package { + kind: Local( + "deps_only/B", + ), + version: None, + }, + "C": Package { + kind: Local( + "deps_only/C", + ), + version: None, + }, + "D": Package { + kind: Local( + "deps_only/D", + ), + version: None, + }, + }, + always_deps: { + "A", + "B", + "C", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,113 +112,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: None, - dev_address_assignments: None, - build: None, - dependencies: { - "A": Dependency { - kind: Local( - "./deps_only/A", - ), - subst: None, - version: None, - digest: None, - }, - "C": Dependency { - kind: Local( - "./deps_only/C", - ), - subst: None, - version: None, - digest: None, - }, - }, - dev_dependencies: { - "B": Dependency { - kind: Local( - "./deps_only/B", - ), - subst: None, - version: None, - digest: None, - }, - }, - }, - graph: { - "Root": [ - ( - "A", - Outgoing, - ), - ( - "C", - Outgoing, - ), - ( - "B", - Outgoing, - ), - ], - "A": [ - ( - "Root", - Incoming, - ), - ( - "B", - Outgoing, - ), - ( - "D", - Outgoing, - ), - ], - "B": [ - ( - "A", - Incoming, - ), - ( - "C", - Outgoing, - ), - ( - "Root", - Incoming, - ), - ], - "C": [ - ( - "B", - Incoming, - ), - ( - "Root", - Incoming, - ), - ], - "D": [ - ( - "A", - Incoming, - ), - ], - }, package_table: { - "A": ResolutionPackage { - resolution_graph_index: "A", + "A": Package { source_package: SourceManifest { package: PackageInfo { name: "A", @@ -162,11 +152,10 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, - "B": ResolutionPackage { - resolution_graph_index: "B", + "B": Package { source_package: SourceManifest { package: PackageInfo { name: "B", @@ -196,11 +185,10 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, - "C": ResolutionPackage { - resolution_graph_index: "C", + "C": Package { source_package: SourceManifest { package: PackageInfo { name: "C", @@ -221,11 +209,10 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, - "D": ResolutionPackage { - resolution_graph_index: "D", + "D": Package { source_package: SourceManifest { package: PackageInfo { name: "D", @@ -246,11 +233,10 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -297,7 +283,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, }, diff --git a/language/tools/move-package/tests/test_sources/dep_good_digest/Move.resolved b/language/tools/move-package/tests/test_sources/dep_good_digest/Move.resolved index d7830e7920..f33de8180f 100644 --- a/language/tools/move-package/tests/test_sources/dep_good_digest/Move.resolved +++ b/language/tools/move-package/tests/test_sources/dep_good_digest/Move.resolved @@ -1,5 +1,34 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/dep_good_digest", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/dep_good_digest", + root_package: "Root", + package_graph: { + "Root": [ + ( + "OtherDep", + Outgoing, + ), + ], + "OtherDep": [ + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "OtherDep": Package { + kind: Local( + "deps_only/other_dep", + ), + version: None, + }, + }, + always_deps: { + "OtherDep", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,64 +46,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "A": Some( - 00000000000000000000000000000001, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: { - "OtherDep": Dependency { - kind: Local( - "./deps_only/other_dep", - ), - subst: Some( - { - "A": RenameFrom( - "B", - ), - }, - ), - version: None, - digest: Some( - "6A88B7888D6049EB0121900E22B6FA2C0E702F042C8C8D4FD62AD5C990B9F9A8", - ), - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "OtherDep", - Outgoing, - ), - ], - "OtherDep": [ - ( - "Root", - Incoming, - ), - ], - }, package_table: { - "OtherDep": ResolutionPackage { - resolution_graph_index: "OtherDep", + "OtherDep": Package { source_package: SourceManifest { package: PackageInfo { name: "OtherDep", @@ -99,13 +72,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "B": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -154,7 +126,7 @@ ResolutionGraph { "B", ), }, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/diamond_problem_backflow_resolution/Move.resolved b/language/tools/move-package/tests/test_sources/diamond_problem_backflow_resolution/Move.resolved index 0070cfbeb2..10005443ee 100644 --- a/language/tools/move-package/tests/test_sources/diamond_problem_backflow_resolution/Move.resolved +++ b/language/tools/move-package/tests/test_sources/diamond_problem_backflow_resolution/Move.resolved @@ -1,5 +1,76 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/diamond_problem_backflow_resolution", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/diamond_problem_backflow_resolution", + root_package: "Root", + package_graph: { + "Root": [ + ( + "A", + Outgoing, + ), + ( + "B", + Outgoing, + ), + ], + "A": [ + ( + "C", + Outgoing, + ), + ( + "Root", + Incoming, + ), + ], + "C": [ + ( + "A", + Incoming, + ), + ( + "B", + Incoming, + ), + ], + "B": [ + ( + "C", + Outgoing, + ), + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "A": Package { + kind: Local( + "deps_only/A", + ), + version: None, + }, + "B": Package { + kind: Local( + "deps_only/B", + ), + version: None, + }, + "C": Package { + kind: Local( + "deps_only/C", + ), + version: None, + }, + }, + always_deps: { + "A", + "B", + "C", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,92 +88,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: None, - dev_address_assignments: None, - build: None, - dependencies: { - "A": Dependency { - kind: Local( - "./deps_only/A", - ), - subst: None, - version: None, - digest: None, - }, - "B": Dependency { - kind: Local( - "./deps_only/B", - ), - subst: Some( - { - "BA": Assign( - 00000000000000000000000000000001, - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "A", - Outgoing, - ), - ( - "B", - Outgoing, - ), - ], - "A": [ - ( - "Root", - Incoming, - ), - ( - "C", - Outgoing, - ), - ], - "C": [ - ( - "A", - Incoming, - ), - ( - "B", - Incoming, - ), - ], - "B": [ - ( - "Root", - Incoming, - ), - ( - "C", - Outgoing, - ), - ], - }, package_table: { - "A": ResolutionPackage { - resolution_graph_index: "A", + "A": Package { source_package: SourceManifest { package: PackageInfo { name: "A", @@ -143,13 +130,12 @@ ResolutionGraph { "A", ), }, - resolution_table: { + resolved_table: { "AA": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "B": ResolutionPackage { - resolution_graph_index: "B", + "B": Package { source_package: SourceManifest { package: PackageInfo { name: "B", @@ -190,13 +176,12 @@ ResolutionGraph { "A", ), }, - resolution_table: { + resolved_table: { "BA": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "C": ResolutionPackage { - resolution_graph_index: "C", + "C": Package { source_package: SourceManifest { package: PackageInfo { name: "C", @@ -221,13 +206,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -271,7 +255,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "AA": 00000000000000000000000000000001, "BA": 00000000000000000000000000000001, }, diff --git a/language/tools/move-package/tests/test_sources/diamond_problem_conflict/Move.resolved b/language/tools/move-package/tests/test_sources/diamond_problem_conflict/Move.resolved index 69488171f2..5990a4e83a 100644 --- a/language/tools/move-package/tests/test_sources/diamond_problem_conflict/Move.resolved +++ b/language/tools/move-package/tests/test_sources/diamond_problem_conflict/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'Root': While resolving dependency 'B' in package 'Root': Unable to assign value to named address BA in dependency B: Attempted to assign a different value '0x2' to an a already-assigned named address '0x1' +Processing dependency 'B' of 'Root': Conflicting assignments for address 'BA': '0x2' and '0x1'. diff --git a/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.resolved b/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.resolved index b22b98c8e8..8839a38f77 100644 --- a/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.resolved +++ b/language/tools/move-package/tests/test_sources/diamond_problem_no_conflict/Move.resolved @@ -1,5 +1,76 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/diamond_problem_no_conflict", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/diamond_problem_no_conflict", + root_package: "Root", + package_graph: { + "Root": [ + ( + "A", + Outgoing, + ), + ( + "B", + Outgoing, + ), + ], + "A": [ + ( + "C", + Outgoing, + ), + ( + "Root", + Incoming, + ), + ], + "C": [ + ( + "A", + Incoming, + ), + ( + "B", + Incoming, + ), + ], + "B": [ + ( + "C", + Outgoing, + ), + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "A": Package { + kind: Local( + "deps_only/A", + ), + version: None, + }, + "B": Package { + kind: Local( + "deps_only/B", + ), + version: None, + }, + "C": Package { + kind: Local( + "deps_only/C", + ), + version: None, + }, + }, + always_deps: { + "A", + "B", + "C", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,98 +88,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: None, - dev_address_assignments: None, - build: None, - dependencies: { - "A": Dependency { - kind: Local( - "./deps_only/A", - ), - subst: Some( - { - "AA": Assign( - 00000000000000000000000000000001, - ), - }, - ), - version: None, - digest: None, - }, - "B": Dependency { - kind: Local( - "./deps_only/B", - ), - subst: Some( - { - "BA": Assign( - 00000000000000000000000000000001, - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "A", - Outgoing, - ), - ( - "B", - Outgoing, - ), - ], - "A": [ - ( - "Root", - Incoming, - ), - ( - "C", - Outgoing, - ), - ], - "C": [ - ( - "A", - Incoming, - ), - ( - "B", - Incoming, - ), - ], - "B": [ - ( - "Root", - Incoming, - ), - ( - "C", - Outgoing, - ), - ], - }, package_table: { - "A": ResolutionPackage { - resolution_graph_index: "A", + "A": Package { source_package: SourceManifest { package: PackageInfo { name: "A", @@ -149,13 +130,12 @@ ResolutionGraph { "A", ), }, - resolution_table: { + resolved_table: { "AA": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "B": ResolutionPackage { - resolution_graph_index: "B", + "B": Package { source_package: SourceManifest { package: PackageInfo { name: "B", @@ -196,13 +176,12 @@ ResolutionGraph { "A", ), }, - resolution_table: { + resolved_table: { "BA": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "C": ResolutionPackage { - resolution_graph_index: "C", + "C": Package { source_package: SourceManifest { package: PackageInfo { name: "C", @@ -227,13 +206,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -283,7 +261,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "AA": 00000000000000000000000000000001, "BA": 00000000000000000000000000000001, }, diff --git a/language/tools/move-package/tests/test_sources/multiple_deps_no_rename_conflict/Move.resolved b/language/tools/move-package/tests/test_sources/multiple_deps_no_rename_conflict/Move.resolved index c77dcdaf2f..4167a7120c 100644 --- a/language/tools/move-package/tests/test_sources/multiple_deps_no_rename_conflict/Move.resolved +++ b/language/tools/move-package/tests/test_sources/multiple_deps_no_rename_conflict/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'test': Resolving named addresses for dependency 'D' in package 'test': Named address 'A' in dependency 'D' is already set to '0x1' but was then reassigned to '0x2' +Processing dependency 'D' of 'test': Conflicting assignments for address 'A': '0x1' and '0x2'. diff --git a/language/tools/move-package/tests/test_sources/multiple_deps_rename/Move.resolved b/language/tools/move-package/tests/test_sources/multiple_deps_rename/Move.resolved index 3b7cd97e5b..20bec436b8 100644 --- a/language/tools/move-package/tests/test_sources/multiple_deps_rename/Move.resolved +++ b/language/tools/move-package/tests/test_sources/multiple_deps_rename/Move.resolved @@ -1,5 +1,51 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/multiple_deps_rename", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/multiple_deps_rename", + root_package: "test", + package_graph: { + "test": [ + ( + "C", + Outgoing, + ), + ( + "D", + Outgoing, + ), + ], + "C": [ + ( + "test", + Incoming, + ), + ], + "D": [ + ( + "test", + Incoming, + ), + ], + }, + package_table: { + "C": Package { + kind: Local( + "deps_only/C", + ), + version: None, + }, + "D": Package { + kind: Local( + "deps_only/D", + ), + version: None, + }, + }, + always_deps: { + "C", + "D", + "test", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,86 +63,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "test", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "A": Some( - 00000000000000000000000000000003, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: { - "C": Dependency { - kind: Local( - "./deps_only/C", - ), - subst: Some( - { - "CA": RenameFrom( - "A", - ), - }, - ), - version: None, - digest: None, - }, - "D": Dependency { - kind: Local( - "./deps_only/D", - ), - subst: Some( - { - "DA": RenameFrom( - "A", - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "test": [ - ( - "C", - Outgoing, - ), - ( - "D", - Outgoing, - ), - ], - "C": [ - ( - "test", - Incoming, - ), - ], - "D": [ - ( - "test", - Incoming, - ), - ], - }, package_table: { - "C": ResolutionPackage { - resolution_graph_index: "C", + "C": Package { source_package: SourceManifest { package: PackageInfo { name: "C", @@ -123,13 +91,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "D": ResolutionPackage { - resolution_graph_index: "D", + "D": Package { source_package: SourceManifest { package: PackageInfo { name: "D", @@ -156,13 +123,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000002, }, source_digest: "ELIDED_FOR_TEST", }, - "test": ResolutionPackage { - resolution_graph_index: "test", + "test": Package { source_package: SourceManifest { package: PackageInfo { name: "test", @@ -227,7 +193,7 @@ ResolutionGraph { "A", ), }, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000003, "CA": 00000000000000000000000000000001, "DA": 00000000000000000000000000000002, diff --git a/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.resolved b/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.resolved index d9a0410fb6..1bfc00d3e1 100644 --- a/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.resolved +++ b/language/tools/move-package/tests/test_sources/nested_deps_git_local/Move.resolved @@ -1,5 +1,59 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/nested_deps_git_local", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/nested_deps_git_local", + root_package: "NestedDeps", + package_graph: { + "NestedDeps": [ + ( + "MoveNursery", + Outgoing, + ), + ], + "MoveNursery": [ + ( + "MoveStdlib", + Outgoing, + ), + ( + "NestedDeps", + Incoming, + ), + ], + "MoveStdlib": [ + ( + "MoveNursery", + Incoming, + ), + ], + }, + package_table: { + "MoveNursery": Package { + kind: Git( + GitInfo { + git_url: "https://github.com/move-language/move", + git_rev: "781c844", + subdir: "language/move-stdlib/nursery", + }, + ), + version: None, + }, + "MoveStdlib": Package { + kind: Git( + GitInfo { + git_url: "https://github.com/move-language/move", + git_rev: "781c844", + subdir: "language/move-stdlib", + }, + ), + version: None, + }, + }, + always_deps: { + "MoveNursery", + "MoveStdlib", + "NestedDeps", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,70 +71,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "NestedDeps", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "std": Some( - 00000000000000000000000000000001, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: { - "MoveNursery": Dependency { - kind: Git( - GitInfo { - git_url: "https://github.com/move-language/move", - git_rev: "781c844", - subdir: "language/move-stdlib/nursery", - }, - ), - subst: None, - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "NestedDeps": [ - ( - "MoveNursery", - Outgoing, - ), - ], - "MoveNursery": [ - ( - "NestedDeps", - Incoming, - ), - ( - "MoveStdlib", - Outgoing, - ), - ], - "MoveStdlib": [ - ( - "MoveNursery", - Incoming, - ), - ], - }, package_table: { - "MoveNursery": ResolutionPackage { - resolution_graph_index: "MoveNursery", + "MoveNursery": Package { source_package: SourceManifest { package: PackageInfo { name: "MoveNursery", @@ -114,13 +106,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "std": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "MoveStdlib": ResolutionPackage { - resolution_graph_index: "MoveStdlib", + "MoveStdlib": Package { source_package: SourceManifest { package: PackageInfo { name: "MoveStdlib", @@ -149,13 +140,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "std": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "NestedDeps": ResolutionPackage { - resolution_graph_index: "NestedDeps", + "NestedDeps": Package { source_package: SourceManifest { package: PackageInfo { name: "NestedDeps", @@ -195,7 +185,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "std": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/one_dep/Move.resolved b/language/tools/move-package/tests/test_sources/one_dep/Move.resolved index 172d5f0cf5..0ce3019d6a 100644 --- a/language/tools/move-package/tests/test_sources/one_dep/Move.resolved +++ b/language/tools/move-package/tests/test_sources/one_dep/Move.resolved @@ -1,5 +1,34 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/one_dep", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/one_dep", + root_package: "Root", + package_graph: { + "Root": [ + ( + "OtherDep", + Outgoing, + ), + ], + "OtherDep": [ + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "OtherDep": Package { + kind: Local( + "deps_only/other_dep", + ), + version: None, + }, + }, + always_deps: { + "OtherDep", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,62 +46,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "A": Some( - 00000000000000000000000000000001, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: { - "OtherDep": Dependency { - kind: Local( - "./deps_only/other_dep", - ), - subst: Some( - { - "A": RenameFrom( - "B", - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "OtherDep", - Outgoing, - ), - ], - "OtherDep": [ - ( - "Root", - Incoming, - ), - ], - }, package_table: { - "OtherDep": ResolutionPackage { - resolution_graph_index: "OtherDep", + "OtherDep": Package { source_package: SourceManifest { package: PackageInfo { name: "OtherDep", @@ -97,13 +72,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "B": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -150,7 +124,7 @@ ResolutionGraph { "B", ), }, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/one_dep_assigned_address/Move.resolved b/language/tools/move-package/tests/test_sources/one_dep_assigned_address/Move.resolved index 95012cda3d..2ed0b4e0df 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_assigned_address/Move.resolved +++ b/language/tools/move-package/tests/test_sources/one_dep_assigned_address/Move.resolved @@ -1,5 +1,34 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/one_dep_assigned_address", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/one_dep_assigned_address", + root_package: "Root", + package_graph: { + "Root": [ + ( + "OtherDep", + Outgoing, + ), + ], + "OtherDep": [ + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "OtherDep": Package { + kind: Local( + "deps_only/other_dep", + ), + version: None, + }, + }, + always_deps: { + "OtherDep", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,56 +46,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: None, - dev_address_assignments: None, - build: None, - dependencies: { - "OtherDep": Dependency { - kind: Local( - "./deps_only/other_dep", - ), - subst: Some( - { - "A": RenameFrom( - "B", - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "OtherDep", - Outgoing, - ), - ], - "OtherDep": [ - ( - "Root", - Incoming, - ), - ], - }, package_table: { - "OtherDep": ResolutionPackage { - resolution_graph_index: "OtherDep", + "OtherDep": Package { source_package: SourceManifest { package: PackageInfo { name: "OtherDep", @@ -93,13 +74,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "B": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -140,7 +120,7 @@ ResolutionGraph { "B", ), }, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.resolved b/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.resolved index 17a4ec7152..40d77916a8 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.resolved +++ b/language/tools/move-package/tests/test_sources/one_dep_bad_digest/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'Root': While resolving dependency 'OtherDep' in package 'Root': Source digest mismatch in dependency 'OtherDep'. Expected 'BAD_DIGEST' but got '0B4B841390F30CACBA194AD650968D2A362349F65D8EB2BF354AA4F95ED0B909'. +Processing dependency 'OtherDep' of 'Root': Source digest mismatch in dependency 'OtherDep' of 'Root'. Expected 'BAD_DIGEST' but got '0B4B841390F30CACBA194AD650968D2A362349F65D8EB2BF354AA4F95ED0B909'. diff --git a/language/tools/move-package/tests/test_sources/one_dep_multiple_of_same_name/Move.resolved b/language/tools/move-package/tests/test_sources/one_dep_multiple_of_same_name/Move.resolved index 58341c2b83..de8c2e3c9c 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_multiple_of_same_name/Move.resolved +++ b/language/tools/move-package/tests/test_sources/one_dep_multiple_of_same_name/Move.resolved @@ -1,5 +1,34 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/one_dep_multiple_of_same_name", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/one_dep_multiple_of_same_name", + root_package: "Root", + package_graph: { + "Root": [ + ( + "OtherDep", + Outgoing, + ), + ], + "OtherDep": [ + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "OtherDep": Package { + kind: Local( + "deps_only/other_dep", + ), + version: None, + }, + }, + always_deps: { + "OtherDep", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,62 +46,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "A": Some( - 00000000000000000000000000000001, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: { - "OtherDep": Dependency { - kind: Local( - "./deps_only/other_dep", - ), - subst: Some( - { - "A": RenameFrom( - "B", - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "OtherDep", - Outgoing, - ), - ], - "OtherDep": [ - ( - "Root", - Incoming, - ), - ], - }, package_table: { - "OtherDep": ResolutionPackage { - resolution_graph_index: "OtherDep", + "OtherDep": Package { source_package: SourceManifest { package: PackageInfo { name: "OtherDep", @@ -97,13 +72,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "B": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -150,7 +124,7 @@ ResolutionGraph { "B", ), }, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/one_dep_reassigned_address/Move.resolved b/language/tools/move-package/tests/test_sources/one_dep_reassigned_address/Move.resolved index 5a942543f1..232ee4a469 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_reassigned_address/Move.resolved +++ b/language/tools/move-package/tests/test_sources/one_dep_reassigned_address/Move.resolved @@ -1,5 +1,34 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/one_dep_reassigned_address", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/one_dep_reassigned_address", + root_package: "Root", + package_graph: { + "Root": [ + ( + "OtherDep", + Outgoing, + ), + ], + "OtherDep": [ + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "OtherDep": Package { + kind: Local( + "deps_only/other_dep", + ), + version: None, + }, + }, + always_deps: { + "OtherDep", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,62 +46,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "B": Some( - 00000000000000000000000000000002, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: { - "OtherDep": Dependency { - kind: Local( - "./deps_only/other_dep", - ), - subst: Some( - { - "A": RenameFrom( - "B", - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "OtherDep", - Outgoing, - ), - ], - "OtherDep": [ - ( - "Root", - Incoming, - ), - ], - }, package_table: { - "OtherDep": ResolutionPackage { - resolution_graph_index: "OtherDep", + "OtherDep": Package { source_package: SourceManifest { package: PackageInfo { name: "OtherDep", @@ -99,13 +74,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "B": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -152,7 +126,7 @@ ResolutionGraph { "B", ), }, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, "B": 00000000000000000000000000000002, }, diff --git a/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings/Move.resolved b/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings/Move.resolved index bbcce5da29..f200db7e8d 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings/Move.resolved +++ b/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings/Move.resolved @@ -1,5 +1,34 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/one_dep_unification_across_local_renamings", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/one_dep_unification_across_local_renamings", + root_package: "Root", + package_graph: { + "Root": [ + ( + "OtherDep", + Outgoing, + ), + ], + "OtherDep": [ + ( + "Root", + Incoming, + ), + ], + }, + package_table: { + "OtherDep": Package { + kind: Local( + "deps_only/other_dep", + ), + version: None, + }, + }, + always_deps: { + "OtherDep", + "Root", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,62 +46,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "Root", - version: ( - 0, - 0, - 0, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: Some( - { - "A": Some( - 00000000000000000000000000000001, - ), - }, - ), - dev_address_assignments: None, - build: None, - dependencies: { - "OtherDep": Dependency { - kind: Local( - "./deps_only/other_dep", - ), - subst: Some( - { - "A": RenameFrom( - "B", - ), - }, - ), - version: None, - digest: None, - }, - }, - dev_dependencies: {}, - }, - graph: { - "Root": [ - ( - "OtherDep", - Outgoing, - ), - ], - "OtherDep": [ - ( - "Root", - Incoming, - ), - ], - }, package_table: { - "OtherDep": ResolutionPackage { - resolution_graph_index: "OtherDep", + "OtherDep": Package { source_package: SourceManifest { package: PackageInfo { name: "OtherDep", @@ -97,13 +72,12 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: { + resolved_table: { "B": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", }, - "Root": ResolutionPackage { - resolution_graph_index: "Root", + "Root": Package { source_package: SourceManifest { package: PackageInfo { name: "Root", @@ -150,7 +124,7 @@ ResolutionGraph { "B", ), }, - resolution_table: { + resolved_table: { "A": 00000000000000000000000000000001, }, source_digest: "ELIDED_FOR_TEST", diff --git a/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings_with_resolution/Move.resolved b/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings_with_resolution/Move.resolved index fbde59a906..4359ec9178 100644 --- a/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings_with_resolution/Move.resolved +++ b/language/tools/move-package/tests/test_sources/one_dep_unification_across_local_renamings_with_resolution/Move.resolved @@ -1 +1 @@ -Unable to resolve packages for package 'Root': Unable to resolve named address 'A' in package 'Root' when resolving dependencies: Attempted to assign a different value '0x1' to an a already-assigned named address '0x2' +Processing dependency 'OtherDep' of 'Root': Conflicting assignments for address 'A': '0x1' and '0x2'. diff --git a/language/tools/move-package/tests/test_sources/parsing_invalid_identifier_package_name/Move.resolved b/language/tools/move-package/tests/test_sources/parsing_invalid_identifier_package_name/Move.resolved index 8a048c9d68..cc819eb44f 100644 --- a/language/tools/move-package/tests/test_sources/parsing_invalid_identifier_package_name/Move.resolved +++ b/language/tools/move-package/tests/test_sources/parsing_invalid_identifier_package_name/Move.resolved @@ -1,5 +1,15 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/parsing_invalid_identifier_package_name", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/parsing_invalid_identifier_package_name", + root_package: "®´∑œ", + package_graph: { + "®´∑œ": [], + }, + package_table: {}, + always_deps: { + "®´∑œ", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,30 +27,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "®´∑œ", - version: ( - 0, - 1, - 2, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: None, - dev_address_assignments: None, - build: None, - dependencies: {}, - dev_dependencies: {}, - }, - graph: { - "®´∑œ": [], - }, package_table: { - "®´∑œ": ResolutionPackage { - resolution_graph_index: "®´∑œ", + "®´∑œ": Package { source_package: SourceManifest { package: PackageInfo { name: "®´∑œ", @@ -61,7 +49,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, }, diff --git a/language/tools/move-package/tests/test_sources/parsing_minimal_manifest/Move.resolved b/language/tools/move-package/tests/test_sources/parsing_minimal_manifest/Move.resolved index 05dd712a37..6930565747 100644 --- a/language/tools/move-package/tests/test_sources/parsing_minimal_manifest/Move.resolved +++ b/language/tools/move-package/tests/test_sources/parsing_minimal_manifest/Move.resolved @@ -1,5 +1,15 @@ -ResolutionGraph { - root_package_path: "tests/test_sources/parsing_minimal_manifest", +ResolvedGraph { + graph: DependencyGraph { + root_path: "tests/test_sources/parsing_minimal_manifest", + root_package: "name", + package_graph: { + "name": [], + }, + package_table: {}, + always_deps: { + "name", + }, + }, build_options: BuildConfig { dev_mode: true, test_mode: false, @@ -17,30 +27,8 @@ ResolutionGraph { fetch_deps_only: false, skip_fetch_latest_git_deps: false, }, - root_package: SourceManifest { - package: PackageInfo { - name: "name", - version: ( - 0, - 1, - 2, - ), - authors: [], - license: None, - custom_properties: {}, - }, - addresses: None, - dev_address_assignments: None, - build: None, - dependencies: {}, - dev_dependencies: {}, - }, - graph: { - "name": [], - }, package_table: { - "name": ResolutionPackage { - resolution_graph_index: "name", + "name": Package { source_package: SourceManifest { package: PackageInfo { name: "name", @@ -61,7 +49,7 @@ ResolutionGraph { }, package_path: "ELIDED_FOR_TEST", renaming: {}, - resolution_table: {}, + resolved_table: {}, source_digest: "ELIDED_FOR_TEST", }, },