Install dependencies in reverse topological order (crystal-lang#369)

spec/integration/install_spec.cr

@@ -392,6 +392,18 @@ describe "install" do
     end
   end
 
+  it "runs install and postinstall in reverse topological order" do
+    with_shard({dependencies: {transitive_2: "*"}}) do
+      output = run "shards install --no-color"
+      install_lines = output.lines.select /^\w: (Installing|Postinstall)/
+      install_lines[0].should match(/Installing version /)
+      install_lines[1].should match(/Installing transitive /)
+      install_lines[2].should match(/Postinstall of transitive:/)
+      install_lines[3].should match(/Installing transitive_2 /)
+      install_lines[4].should match(/Postinstall of transitive_2:/)
+    end
+  end
+
   it "fails with circular dependencies" do
     create_git_repository "a"
     create_git_release "a", "0.1.0", "name: a\nversion: 0.1.0\ndependencies:\n  b:\n    git: #{git_path("b")}"

spec/integration/spec_helper.cr

@@ -92,6 +92,17 @@ private def setup_repositories
       postinstall: crystal build src/version.cr
     YAML
 
+  create_git_repository "transitive_2"
+  create_git_release "transitive_2", "0.1.0", <<-YAML
+    name: transitive_2
+    version: 0.1.0
+    dependencies:
+      transitive:
+        git: #{git_path(:transitive)}
+    scripts:
+      postinstall: ../transitive/version
+    YAML
+
   # dependencies with executables:
   create_git_repository "binary"
   create_file "binary", "bin/foobar", "#! /usr/bin/env sh\necho 'OK'", perm: 0o755

src/commands/install.cr

@@ -50,16 +50,20 @@ module Shards
       end
 
       private def install(packages : Array(Package))
-        # first install all dependencies:
-        installed = packages.compact_map { |package| install(package) }
+        # packages are returned by the solver in reverse topological order,
+        # so transitive dependencies are installed first
+        packages.each do |package|
+          # first install the dependency:
+          next unless install(package)
 
-        # then execute the postinstall script of installed dependencies (with
-        # access to all transitive dependencies):
-        installed.each(&.postinstall)
+          # then execute the postinstall script
+          # (with access to all transitive dependencies):
+          package.postinstall
 
-        # always install executables because the path resolver never actually
-        # installs dependencies:
-        packages.each(&.install_executables)
+          # always install executables because the path resolver never actually
+          # installs dependencies:
+          package.install_executables
+        end
       end
 
       private def install(package : Package)

src/molinillo_solver.cr

@@ -67,7 +67,7 @@ module Shards
           .resolve(deps, base)
 
       packages = [] of Package
-      result.each do |v|
+      tsort(result).each do |v|
         next unless v.payload
         spec = v.payload.as?(Spec) || raise "BUG: returned graph payload was not a Spec"
         v.requirements.each do |dependency|
@@ -91,6 +91,28 @@ module Shards
       packages
     end
 
+    private def tsort(graph)
+      sorted_vertices = typeof(graph.vertices).new
+
+      graph.vertices.values.each do |vertex|
+        if vertex.incoming_edges.empty?
+          tsort_visit(vertex, sorted_vertices)
+        end
+      end
+
+      sorted_vertices.values
+    end
+
+    private def tsort_visit(vertex, sorted_vertices)
+      vertex.successors.each do |succ|
+        unless sorted_vertices.has_key?(succ.name)
+          tsort_visit(succ, sorted_vertices)
+        end
+      end
+
+      sorted_vertices[vertex.name] = vertex
+    end
+
     def name_for(spec : Shards::Spec)
       spec.resolver.not_nil!.name
     end