From 96495b4c6368b0696e543ca6ce8b98a1e9d379bf Mon Sep 17 00:00:00 2001
From: Russell Cohen <rcoh@amazon.com>
Date: Fri, 7 Jan 2022 17:38:34 -0500
Subject: [PATCH] Fix bug in publisher topological sort (#1049)

* Fix bug in publisher topological sort

* Update tools/publisher/src/sort.rs

Co-authored-by: Zelda Hessler <zhessler@amazon.com>

Co-authored-by: Zelda Hessler <zhessler@amazon.com>
---
 tools/publisher/src/sort.rs | 47 ++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/tools/publisher/src/sort.rs b/tools/publisher/src/sort.rs
index 637fe6527..017e21006 100644
--- a/tools/publisher/src/sort.rs
+++ b/tools/publisher/src/sort.rs
@@ -55,21 +55,19 @@ fn dependency_order_visit(
     visited: &mut BTreeSet<PackageHandle>,
     result: &mut Vec<PackageHandle>,
 ) -> Result<(), Error> {
-    visited.insert(package_handle.clone());
+    if visited.contains(package_handle) {
+        return Ok(());
+    }
+    if stack.contains(package_handle) {
+        return Err(Error::DependencyCycle);
+    }
     stack.insert(package_handle.clone());
-
     let local_dependencies = &packages[package_handle].local_dependencies;
     for dependency in local_dependencies {
-        if visited.contains(dependency) && stack.contains(dependency) {
-            return Err(Error::DependencyCycle);
-        }
-        if package_handle != dependency
-            && packages.contains_key(dependency)
-            && !visited.contains(dependency)
-        {
-            dependency_order_visit(dependency, packages, stack, visited, result)?;
-        }
+        dependency_order_visit(dependency, packages, stack, visited, result)?;
     }
+    stack.remove(package_handle);
+    visited.insert(package_handle.clone());
     result.push(package_handle.clone());
     Ok(())
 }
@@ -122,4 +120,31 @@ mod tests {
         let error = dependency_order(packages).err().expect("cycle");
         assert_eq!("dependency cycle detected", format!("{}", error));
     }
+
+    #[test]
+    pub fn complex_tree() {
+        let packages = vec![
+            package("codeexamples", &["aws-config", "aws-apigateway"]),
+            package(
+                "aws-apigateway",
+                &["aws-config", "aws-types", "aws-endpoint"],
+            ),
+            package("aws-config", &["aws-endpoint", "aws-types", "aws-sdk-sts"]),
+            package("aws-types", &[]),
+            package("aws-endpoint", &[]),
+            package("aws-sdk-sts", &[]),
+        ];
+        let result = dependency_order(packages).expect("ok");
+        assert_eq!(
+            result.iter().map(|p| &p.handle.name).collect::<Vec<_>>(),
+            vec![
+                "aws-endpoint",
+                "aws-sdk-sts",
+                "aws-types",
+                "aws-config",
+                "aws-apigateway",
+                "codeexamples"
+            ]
+        );
+    }
 }
-- 
GitLab