[move-package] Support dependency overrides (#11181)

## Description 

When building a dependency graph, different versions of the same
(transitively) dependent package can be encountered. If this is indeed
the case, a single version must be chosen by the developer to be the
override, and this override must be specified in a manifest file whose
package dominates all the conflicting "uses" of the dependent package.
These overrides must taken into consideration during the dependency
graph construction and this PR implements the relevant changes to
dependency graph construction algorithm. For additional details see the
doc-comments in the code as well as the PR this one is based on
(move-language/move#1023) which contains a
discussion of issues encountered during development of this algorithm.

## Test Plan 

A comprehensive test suite is attached.
---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [x] user-visible impact

### Release notes

This change allows a developer to override transitive dependencies of a
package they are developing to avoid conflicts that could otherwise
arise.

tools/move-package/src/source_package/manifest_parser.rs

@@ -334,6 +334,11 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency>
         .transpose()?;
     let version = table.remove("version").map(parse_version).transpose()?;
     let digest = table.remove("digest").map(parse_digest).transpose()?;
+    let dep_override = table
+        .remove("override")
+        .map(parse_dep_override)
+        .transpose()?
+        .map_or(false, |o| o);
 
     let kind = match (
         table.remove("local"),
@@ -350,7 +355,10 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency>
                 bail!("Local source path not a string")
             };
 
-            PM::DependencyKind::Local(local)
+            PM::DependencyKind::Local(
+                // with allow_cwd_parent set to true, it never fails
+                PM::normalize_path(local, true /* allow_cwd_parent */).unwrap(),
+            )
         }
 
         (None, subdir, Some(git_url), None) => {
@@ -433,6 +441,7 @@ pub fn parse_dependency(dep_name: &str, mut tval: TV) -> Result<PM::Dependency>
         subst,
         version,
         digest,
+        dep_override,
     }))
 }
 
@@ -502,6 +511,13 @@ fn parse_digest(tval: TV) -> Result<PM::PackageDigest> {
     Ok(PM::PackageDigest::from(digest_str))
 }
 
+fn parse_dep_override(tval: TV) -> Result<PM::DepOverride> {
+    if !tval.is_bool() {
+        bail!("Invalid dependency override value");
+    }
+    Ok(tval.as_bool().unwrap())
+}
+
 // check that only recognized names are provided at the top-level
 fn warn_if_unknown_field_names(table: &toml::map::Map<String, TV>, known_names: &[&str]) {
     let mut unknown_names = BTreeSet::new();

tools/move-package/src/source_package/parsed_manifest.rs

@@ -16,6 +16,7 @@ pub type NamedAddress = Symbol;
 pub type PackageName = Symbol;
 pub type FileName = Symbol;
 pub type PackageDigest = Symbol;
+pub type DepOverride = bool;
 
 pub type AddressDeclarations = BTreeMap<NamedAddress, Option<AccountAddress>>;
 pub type DevAddressDeclarations = BTreeMap<NamedAddress, AccountAddress>;
@@ -55,6 +56,7 @@ pub struct InternalDependency {
     pub subst: Option<Substitution>,
     pub version: Option<Version>,
     pub digest: Option<PackageDigest>,
+    pub dep_override: DepOverride,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -154,7 +156,7 @@ impl Default for DependencyKind {
 /// or is prefixed by accesses to parent directories when `allow_cwd_parent` is false.
 ///
 /// Returns the normalized path on success.
-fn normalize_path(path: impl AsRef<Path>, allow_cwd_parent: bool) -> Result<PathBuf> {
+pub fn normalize_path(path: impl AsRef<Path>, allow_cwd_parent: bool) -> Result<PathBuf> {
     use Component::*;
 
     let mut stack = Vec::new();

tools/move-package/tests/test_dependency_graph.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use std::{
-    collections::BTreeSet,
+    collections::{BTreeMap, BTreeSet},
     fs::{self, File},
     io::Write,
     path::PathBuf,
@@ -184,7 +184,15 @@ fn merge_simple() {
     )
     .expect("Reading inner");
 
-    assert!(outer.merge(inner, Symbol::from("")).is_ok());
+    assert!(outer
+        .merge(
+            Symbol::from("A"),
+            Symbol::from("A"),
+            inner,
+            Symbol::from(""),
+            &BTreeMap::new(),
+        )
+        .is_ok());
 
     assert_eq!(
         outer.topological_order(),
@@ -214,7 +222,15 @@ fn merge_into_root() {
     )
     .expect("Reading inner");
 
-    assert!(outer.merge(inner, Symbol::from("")).is_ok());
+    assert!(outer
+        .merge(
+            Symbol::from("Root"),
+            Symbol::from("A"),
+            inner,
+            Symbol::from(""),
+            &BTreeMap::new(),
+        )
+        .is_ok());
 
     assert_eq!(
         outer.topological_order(),
@@ -243,7 +259,7 @@ fn merge_detached() {
     )
     .expect("Reading inner");
 
-    let Err(err) = outer.merge(inner, Symbol::from("")) else {
+    let Err(err) = outer.merge(Symbol::from("OtherDep"), Symbol::from("A"), inner, Symbol::from(""), &BTreeMap::new()) else {
         panic!("Inner's root is not part of outer's graph, so this should fail");
     };
 
@@ -267,7 +283,7 @@ fn merge_after_calculating_always_deps() {
     )
     .expect("Reading inner");
 
-    let Err(err) = outer.merge(inner, Symbol::from("")) else {
+    let Err(err) = outer.merge(Symbol::from("A"),Symbol::from("A"), inner, Symbol::from(""), &BTreeMap::new()) else {
         panic!("Outer's always deps have already been calculated so this should fail");
     };
 
@@ -295,7 +311,15 @@ fn merge_overlapping() {
     )
     .expect("Reading inner");
 
-    assert!(outer.merge(inner, Symbol::from("")).is_ok());
+    assert!(outer
+        .merge(
+            Symbol::from("B"),
+            Symbol::from("A"),
+            inner,
+            Symbol::from(""),
+            &BTreeMap::new(),
+        )
+        .is_ok());
 }
 
 #[test]
@@ -319,7 +343,7 @@ fn merge_overlapping_different_deps() {
     )
     .expect("Reading inner");
 
-    let Err(err) = outer.merge(inner, Symbol::from("")) else {
+    let Err(err) = outer.merge(Symbol::from("B"),Symbol::from("A"), inner, Symbol::from(""), &BTreeMap::new()) else {
         panic!("Outer and inner mention package A which has different dependencies in both.");
     };
 
@@ -347,7 +371,7 @@ fn merge_cyclic() {
     )
     .expect("Reading inner");
 
-    let Err(err) = outer.merge(inner, Symbol::from("")) else {
+    let Err(err) = outer.merge(Symbol::from("B"), Symbol::from("Root"), inner, Symbol::from(""), &BTreeMap::new()) else {
         panic!("Inner refers back to outer's root");
     };
 

tools/move-package/tests/test_sources/dep_dev_dep_diamond/Move.resolved

@@ -69,27 +69,31 @@ ResolvedGraph {
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
             "B": Package {
                 kind: Local(
                     "deps_only/B",
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
             "C": Package {
                 kind: Local(
                     "deps_only/C",
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
             "D": Package {
                 kind: Local(
                     "deps_only/D",
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
         },
         always_deps: {
@@ -142,6 +146,7 @@ ResolvedGraph {
                             subst: None,
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },
@@ -154,6 +159,7 @@ ResolvedGraph {
                             subst: None,
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },
@@ -189,6 +195,7 @@ ResolvedGraph {
                             subst: None,
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },
@@ -266,33 +273,36 @@ ResolvedGraph {
                     "A": Internal(
                         InternalDependency {
                             kind: Local(
-                                "./deps_only/A",
+                                "deps_only/A",
                             ),
                             subst: None,
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                     "C": Internal(
                         InternalDependency {
                             kind: Local(
-                                "./deps_only/C",
+                                "deps_only/C",
                             ),
                             subst: None,
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },
                 dev_dependencies: {
                     "B": Internal(
                         InternalDependency {
                             kind: Local(
-                                "./deps_only/B",
+                                "deps_only/B",
                             ),
                             subst: None,
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },

tools/move-package/tests/test_sources/dep_good_digest/Move.resolved

@@ -23,6 +23,7 @@ ResolvedGraph {
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
         },
         always_deps: {
@@ -104,7 +105,7 @@ ResolvedGraph {
                     "OtherDep": Internal(
                         InternalDependency {
                             kind: Local(
-                                "./deps_only/other_dep",
+                                "deps_only/other_dep",
                             ),
                             subst: Some(
                                 {
@@ -117,6 +118,7 @@ ResolvedGraph {
                             digest: Some(
                                 "6A88B7888D6049EB0121900E22B6FA2C0E702F042C8C8D4FD62AD5C990B9F9A8",
                             ),
+                            dep_override: false,
                         },
                     ),
                 },

tools/move-package/tests/test_sources/diamond_problem_backflow_resolution/Move.resolved

@@ -51,20 +51,23 @@ ResolvedGraph {
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
             "B": Package {
                 kind: Local(
                     "deps_only/B",
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
             "C": Package {
                 kind: Local(
                     "deps_only/C",
                 ),
                 version: None,
                 resolver: None,
+                overridden_path: false,
             },
         },
         always_deps: {
@@ -123,6 +126,7 @@ ResolvedGraph {
                             ),
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },
@@ -171,6 +175,7 @@ ResolvedGraph {
                             ),
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },
@@ -238,17 +243,18 @@ ResolvedGraph {
                     "A": Internal(
                         InternalDependency {
                             kind: Local(
-                                "./deps_only/A",
+                                "deps_only/A",
                             ),
                             subst: None,
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                     "B": Internal(
                         InternalDependency {
                             kind: Local(
-                                "./deps_only/B",
+                                "deps_only/B",
                             ),
                             subst: Some(
                                 {
@@ -259,6 +265,7 @@ ResolvedGraph {
                             ),
                             version: None,
                             digest: None,
+                            dep_override: false,
                         },
                     ),
                 },

tools/move-package/tests/test_sources/diamond_problem_dep_conflict/Move.resolved

@@ -0,0 +1,3 @@
+Failed to resolve dependencies for package 'Root': Resolving dependencies for package 'B': Conflicting dependencies found:
+C = { local = "deps_only/C", version = "2.0.0" }
+C = { local = "deps_only/C", version = "1.0.0" }

tools/move-package/tests/test_sources/diamond_problem_dep_conflict/Move.toml

@@ -0,0 +1,7 @@
+[package]
+name = "Root"
+version = "0.0.0"
+
+[dependencies]
+A = { local = "./deps_only/A" }
+B = { local = "./deps_only/B" }

tools/move-package/tests/test_sources/diamond_problem_dep_conflict/deps_only/A/Move.toml

@@ -0,0 +1,6 @@
+[package]
+name = "A"
+version = "0.0.0"
+
+[dependencies]
+C = { local = "../C", version = "2.0.0" }

tools/move-package/tests/test_sources/diamond_problem_dep_conflict/deps_only/B/Move.toml

@@ -0,0 +1,6 @@
+[package]
+name = "B"
+version = "0.0.0"
+
+[dependencies]
+C = { local = "../C", version = "1.0.0" }

tools/move-package/tests/test_sources/diamond_problem_dep_conflict/deps_only/C/Move.toml

@@ -0,0 +1,3 @@
+[package]
+name = "C"
+version = "0.0.0"

tools/move-package/tests/test_sources/diamond_problem_dep_external_conflict/Move.resolved

@@ -0,0 +1,3 @@
+Failed to resolve dependencies for package 'Root': Adding dependencies from ../resolvers/successful.sh for dependency 'A' in 'Root': Conflicting dependencies found:
+ADep = { local = "deps_only/ADep", version = "1.0.0" }
+ADep = { local = "deps_only/ADep" } # Resolved by ../resolvers/successful.sh