[packages] Fix dependency override prunning issue (#13921)

## Description 

Fixes an issue related to pruning overridden dependencies during
dependency graph construction. More concretely, when tryin to find
dependency paths in a pruned sub-graph, one could encounter no target
node if this node was representing a pruned overridden package. The
proposed solution is to simply also search for the target node in the
overrides map (if a search for it in the sub-graph fails)

## Test Plan 

Added a test for this specific scenario, plus all existing tests must
pass.

---
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)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

tools/move-package/src/resolution/dependency_graph.rs

@@ -270,7 +270,7 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
             .add_node(combined_graph.root_package);
 
         // get overrides
-        let overrides = collect_overrides(parent, &root_manifest.dependencies)?;
+        let mut overrides = collect_overrides(parent, &root_manifest.dependencies)?;
         let dev_overrides = collect_overrides(parent, &root_manifest.dev_dependencies)?;
 
         for (
@@ -296,7 +296,10 @@ impl<Progress: Write> DependencyGraphBuilder<Progress> {
         let mut all_deps = root_manifest.dependencies.clone();
         all_deps.extend(root_manifest.dev_dependencies.clone());
 
-        combined_graph.merge(dep_graphs, parent, &all_deps)?;
+        // we can mash overrides together as the sets cannot overlap (it's asserted during pruning)
+        overrides.extend(dev_overrides);
+
+        combined_graph.merge(dep_graphs, parent, &all_deps, &overrides)?;
 
         combined_graph.check_acyclic()?;
         combined_graph.discover_always_deps();
@@ -563,6 +566,7 @@ impl DependencyGraph {
         mut dep_graphs: BTreeMap<PM::PackageName, DependencyGraphInfo>,
         parent: &PM::DependencyKind,
         dependencies: &PM::Dependencies,
+        overrides: &BTreeMap<PM::PackageName, Package>,
     ) -> Result<()> {
         if !self.always_deps.is_empty() {
             bail!("Merging dependencies into a graph after calculating its 'always' dependencies");
@@ -664,6 +668,7 @@ impl DependencyGraph {
                         &dep_graphs,
                         graph_info.is_external,
                     ),
+                    overrides,
                 ) {
                     Ok(_) => continue,
                     Err((existing_pkg_deps, pkg_deps)) => {
@@ -715,7 +720,7 @@ impl DependencyGraph {
     /// available in a package tables of a respective (dependent) subgraph. This function return the
     /// right package table, depending on whether conflict was detected between pre-populated
     /// combined graph and another sub-graph or between two separate sub-graphs. If we tried to use
-    /// combined graphs's package table "as is" we would get an error in all cases similar to the on
+    /// combined graphs's package table "as is" we would get an error in all cases similar to the one
     /// in the direct_and_indirect_dep test where A is a direct dependency of Root (as C would be
     /// missing from the combined graph's table):
     ///
@@ -1452,25 +1457,46 @@ fn deps_equal<'a>(
     graph1_pkg_table: &'a BTreeMap<PM::PackageName, Package>,
     graph2: &'a DependencyGraph,
     graph2_pkg_table: &'a BTreeMap<PM::PackageName, Package>,
+    overrides: &'a BTreeMap<PM::PackageName, Package>,
 ) -> std::result::Result<
     (),
     (
         Vec<(&'a Dependency, PM::PackageName, &'a Package)>,
         Vec<(&'a Dependency, PM::PackageName, &'a Package)>,
     ),
 > {
-    let graph1_edges = BTreeSet::from_iter(
-        graph1
-            .package_graph
-            .edges(pkg_name)
-            .map(|(_, pkg, dep)| (dep, pkg, graph1_pkg_table.get(&pkg).unwrap())),
-    );
-    let graph2_edges = BTreeSet::from_iter(
-        graph2
-            .package_graph
-            .edges(pkg_name)
-            .map(|(_, pkg, dep)| (dep, pkg, graph2_pkg_table.get(&pkg).unwrap())),
-    );
+    // Unwraps in the code below are safe as these edges (and target nodes) must exist either in the
+    // sub-graph or in the pre-populated combined graph (see pkg_table_for_deps_compare's doc
+    // comment for a more detailed explanation). If these were to fail, it would indicate a bug in
+    // the algorithm so it's OK to panic here.
+    let graph1_edges: BTreeSet<_> = graph1
+        .package_graph
+        .edges(pkg_name)
+        .map(|(_, pkg, dep)| {
+            (
+                dep,
+                pkg,
+                graph1_pkg_table
+                    .get(&pkg)
+                    .or_else(|| overrides.get(&pkg))
+                    .unwrap(),
+            )
+        })
+        .collect();
+    let graph2_edges: BTreeSet<_> = graph2
+        .package_graph
+        .edges(pkg_name)
+        .map(|(_, pkg, dep)| {
+            (
+                dep,
+                pkg,
+                graph2_pkg_table
+                    .get(&pkg)
+                    .or_else(|| overrides.get(&pkg))
+                    .unwrap(),
+            )
+        })
+        .collect();
 
     let (graph1_pkgs, graph2_pkgs): (Vec<_>, Vec<_>) = graph1_edges
         .symmetric_difference(&graph2_edges)

tools/move-package/tests/test_dependency_graph.rs

@@ -234,7 +234,12 @@ fn merge_simple() {
         }),
     )]);
     assert!(outer
-        .merge(dep_graphs, &DependencyKind::default(), dependencies,)
+        .merge(
+            dep_graphs,
+            &DependencyKind::default(),
+            dependencies,
+            &BTreeMap::new()
+        )
         .is_ok(),);
     assert_eq!(
         outer.topological_order(),
@@ -280,7 +285,12 @@ fn merge_into_root() {
         }),
     )]);
     assert!(outer
-        .merge(dep_graphs, &DependencyKind::default(), dependencies,)
+        .merge(
+            dep_graphs,
+            &DependencyKind::default(),
+            dependencies,
+            &BTreeMap::new()
+        )
         .is_ok());
 
     assert_eq!(
@@ -320,6 +330,7 @@ fn merge_detached() {
         dep_graphs,
         &DependencyKind::default(),
         &BTreeMap::new(),
+        &BTreeMap::new()
     ) else {
         panic!("Inner's root is not part of outer's graph, so this should fail");
     };
@@ -354,6 +365,7 @@ fn merge_after_calculating_always_deps() {
         dep_graphs,
         &DependencyKind::default(),
         &BTreeMap::new(),
+        &BTreeMap::new()
     ) else {
         panic!("Outer's always deps have already been calculated so this should fail");
     };
@@ -426,7 +438,12 @@ fn merge_overlapping() {
         ),
     ]);
     assert!(outer
-        .merge(dep_graphs, &DependencyKind::default(), dependencies,)
+        .merge(
+            dep_graphs,
+            &DependencyKind::default(),
+            dependencies,
+            &BTreeMap::new()
+        )
         .is_ok());
 }
 
@@ -498,6 +515,7 @@ fn merge_overlapping_different_deps() {
         dep_graphs,
         &DependencyKind::default(),
         dependencies,
+        &BTreeMap::new()
     ) else {
         panic!("Outer and inner mention package A which has different dependencies in both.");
     };

tools/move-package/tests/test_sources/nested_deps_shared_override/Move.locked

@@ -0,0 +1,33 @@
+# @generated by Move, please check-in and do not edit manually.
+
+[move]
+version = 0
+manifest_digest = "D2FFFC63D2AF7BF7028F583317C7ACF0EBB9E6B9F0DFC6D8FB46732AF7B552E4"
+deps_digest = "060AD7E57DFB13104F21BE5F5C3759D03F0553FC3229247D9A7A6B45F50D03A3"
+
+dependencies = [
+  { name = "More" },
+  { name = "Nested" },
+  { name = "Shared" },
+]
+
+[[move.package]]
+name = "More"
+source = { local = "deps_only/more" }
+
+dependencies = [
+  { name = "Shared" },
+]
+
+[[move.package]]
+name = "Nested"
+source = { local = "deps_only/nested" }
+
+dependencies = [
+  { name = "More" },
+  { name = "Shared" },
+]
+
+[[move.package]]
+name = "Shared"
+source = { local = "deps_only/shared" }