From 01e5c607fc7ca08f815b6110ca692fab8cb47419 Mon Sep 17 00:00:00 2001
From: Matthias Koefferlein <matthias@koefferlein.de>
Date: Sun, 23 Apr 2017 18:22:06 +0200
Subject: [PATCH] WIP: sort packages by their dependencies

This fix ensures that packages satisfying dependencies
for other packages come before these in the flat package order.
This is in particular important for macro packages where those
supplying libraries need to be executed before the ones
that use these.
---
 src/lay/laySalt.cc        | 61 +++++++++++++++++++++++++++++++++++++--
 src/lay/laySalt.h         |  7 +++++
 src/unit_tests/laySalt.cc | 42 +++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/src/lay/laySalt.cc b/src/lay/laySalt.cc
index 9def56e49..001fc4354 100644
--- a/src/lay/laySalt.cc
+++ b/src/lay/laySalt.cc
@@ -56,6 +56,12 @@ Salt &Salt::operator= (const Salt &other)
   return *this;
 }
 
+SaltGrains &
+Salt::root ()
+{
+  return m_root;
+}
+
 Salt::flat_iterator
 Salt::begin_flat ()
 {
@@ -146,13 +152,32 @@ Salt::add_collection_to_flat (SaltGrains &gg)
 
 namespace {
 
-struct NameCompare
+struct NameAndTopoIndexCompare
 {
+  NameAndTopoIndexCompare (const std::map<std::string, int> &topo_index)
+    : mp_topo_index (&topo_index)
+  {
+    //  .. nothing yet ..
+  }
+
   bool operator () (lay::SaltGrain *a, lay::SaltGrain *b) const
   {
+    std::map<std::string, int>::const_iterator ti_a = mp_topo_index->find (a->name ());
+    std::map<std::string, int>::const_iterator ti_b = mp_topo_index->find (b->name ());
+
+    //  Reverse sorting by topological index as highest priority
+    if (ti_a != mp_topo_index->end () && ti_b != mp_topo_index->end ()) {
+      if (ti_a->second != ti_b->second) {
+        return ti_a->second > ti_b->second;
+      }
+    }
+
     //  TODO: UTF-8 support?
     return a->name () < b->name ();
   }
+
+private:
+  const std::map<std::string, int> *mp_topo_index;
 };
 
 }
@@ -169,9 +194,41 @@ Salt::validate ()
       m_grains_by_name.insert (std::make_pair ((*i)->name (), *i));
     }
 
+    //  Compute a set of topological indexes. Packages which serve depedencies of other packages have a higher
+    //  topological index. Later we sort the packages by descending topo index to ensure the packages which are
+    //  input to others come first.
+
+    std::map<std::string, int> topological_index;
+    for (std::map<std::string, SaltGrain *>::const_iterator g = m_grains_by_name.begin (); g != m_grains_by_name.end (); ++g) {
+      topological_index.insert (std::make_pair (g->first, 0));
+    }
+
+    //  NOTE: we allow max. 10 levels of dependencies. That should be sufficient. Limiting the levels of dependencies prevents
+    //  infinite recursion due to faulty recursive dependencies.
+    for (int n = 0; n < 10; ++n) {
+
+      bool any_updated = false;
+
+      for (std::map<std::string, SaltGrain *>::const_iterator g = m_grains_by_name.begin (); g != m_grains_by_name.end (); ++g) {
+        int index = topological_index [g->first];
+        for (std::vector<SaltGrain::Dependency>::const_iterator d = g->second->dependencies ().begin (); d != g->second->dependencies ().end (); ++d) {
+          std::map<std::string, int>::iterator ti = topological_index.find (d->name);
+          if (ti != topological_index.end () && ti->second < index + 1) {
+            ti->second = index + 1;
+            any_updated = true;
+          }
+        }
+      }
+
+      if (! any_updated) {
+        break;
+      }
+
+    }
+
     //  NOTE: we intentionally sort after the name list has been built - this way
     //  the first entry will win in the name to grain map.
-    std::sort (mp_flat_grains.begin (), mp_flat_grains.end (), NameCompare ());
+    std::sort (mp_flat_grains.begin (), mp_flat_grains.end (), NameAndTopoIndexCompare (topological_index));
 
   }
 }
diff --git a/src/lay/laySalt.h b/src/lay/laySalt.h
index c37766bfc..aef3da0ac 100644
--- a/src/lay/laySalt.h
+++ b/src/lay/laySalt.h
@@ -185,6 +185,13 @@ public:
    */
   bool create_grain (const SaltGrain &templ, SaltGrain &target);
 
+  /**
+   *  @brief Gets the root collection
+   *
+   *  This method is provided for test purposes mainly.
+   */
+  SaltGrains &root ();
+
 signals:
   /**
    *  @brief A signal triggered before one of the collections changed
diff --git a/src/unit_tests/laySalt.cc b/src/unit_tests/laySalt.cc
index 8edff9a14..7c5e6eb81 100644
--- a/src/unit_tests/laySalt.cc
+++ b/src/unit_tests/laySalt.cc
@@ -357,3 +357,45 @@ TEST (4)
   EXPECT_EQ (salt.grain_by_name ("b")->name (), "b");
   EXPECT_EQ (salt.grain_by_name ("c/c/v")->name (), "c/c/v");
 }
+
+TEST (5)
+{
+  lay::SaltGrains grains;
+
+  lay::SaltGrain g1;
+  g1.set_name ("g1");
+  lay::SaltGrain::Dependency dep;
+  dep.name = "g2";
+  g1.dependencies ().push_back (dep);
+  dep.name = "g3";
+  g1.dependencies ().push_back (dep);
+  grains.add_grain (g1);
+
+  lay::SaltGrains g34;
+
+  lay::SaltGrain g3;
+  g3.set_name ("g3");
+  g34.add_grain (g3);
+
+  lay::SaltGrain g4;
+  g4.set_name ("g4");
+  g34.add_grain (g4);
+
+  grains.add_collection (g34);
+
+  lay::SaltGrain g2;
+  g2.set_name ("g2");
+  dep.name = "g3";
+  g2.dependencies ().push_back (dep);
+  grains.add_grain (g2);
+
+  lay::Salt salt;
+  salt.root ().add_collection (grains);
+
+  std::vector<std::string> names;
+  for (lay::Salt::flat_iterator i = salt.begin_flat (); i != salt.end_flat (); ++i) {
+    names.push_back ((*i)->name ());
+  }
+
+  EXPECT_EQ (tl::join (names, ","), "g3,g2,g1,g4");
+}