Fix: transitive dependencies in postinstall script

Postpones the execution of the postinstall script, so it's run after
all dependencies have been installed, to solve the availability of
transitive dependencies. Also postpones the installation of
executables, since they're likely to be built by the postinstall
script.

Links the project's `lib` folder as the shard's own `lib` folder,
to solve the accessibility of transitive dependencies (for
executable for the postinstall script). This solution was preferred
over manipulating the CRYSTAL_PATH environment variable, since a
wrapper script of the crystal executable could replace it (e.g. the
`bin/script` of crystal's repository does just that).

Author: ysbaddaden

src/commands/install.cr

@@ -44,19 +44,26 @@ module Shards
       end
 
       private def install(packages : Set)
-        packages.each { |package| install(package) }
+        packages
+          .compact_map { |package| install(package) }
+          .each(&.postinstall)
+
+        # always install executables because the path resolver never installs
+        # dependencies, but uses them as-is:
+        packages.each(&.install_executables)
       end
 
       private def install(package : Package, version = nil)
         version ||= package.version
 
         if package.installed?(version)
           Shards.logger.info "Using #{package.name} (#{package.report_version})"
-        else
-          Shards.logger.info "Installing #{package.name} (#{package.report_version})"
-          package.install(version)
-          package.install_executables
+          return
         end
+
+        Shards.logger.info "Installing #{package.name} (#{package.report_version})"
+        package.install(version)
+        package
       end
 
       private def generate_lockfile?

src/commands/update.cr

@@ -7,21 +7,34 @@ module Shards
       def run(*args)
         manager.resolve
 
-        manager.packages.each do |package|
-          if package.installed?
-            Shards.logger.info "Using #{package.name} (#{package.report_version})"
-          else
-            Shards.logger.info "Installing #{package.name} (#{package.report_version})"
-            package.install
-          end
-          package.install_executables
-        end
+        install(manager.packages)
 
         if generate_lockfile?
           manager.to_lock(lockfile_path)
         end
       end
 
+      private def install(packages : Set)
+        packages
+          .compact_map { |package| install(package) }
+          .each(&.postinstall)
+
+        # always install executables because the path resolver never installs
+        # dependencies, but uses them as-is:
+        packages.each(&.install_executables)
+      end
+
+      private def install(package : Package)
+        if package.installed?
+          Shards.logger.info "Using #{package.name} (#{package.report_version})"
+          return
+        end
+
+        Shards.logger.info "Installing #{package.name} (#{package.report_version})"
+        package.install
+        package
+      end
+
       private def generate_lockfile?
         !Shards.production? && manager.packages.any?
       end

src/package.cr

@@ -68,7 +68,19 @@ module Shards
     end
 
     def install(version = nil)
+      # install the shard:
       resolver.install(version || self.version)
+
+      # link the project's lib path as the shard's lib path, so the dependency
+      # can access transitive dependencies:
+      unless @dependency.path
+        lib_path = File.join(resolver.install_path, "lib")
+        Shards.logger.debug "Link #{Shards.install_path} to #{lib_path}"
+        File.symlink("../../lib", lib_path)
+      end
+    end
+
+    def postinstall
       resolver.run_script("postinstall")
     rescue ex : Script::Error
       resolver.cleanup_install_directory

test/integration/install_test.cr

@@ -291,6 +291,15 @@ class InstallCommandTest < Minitest::Test
     end
   end
 
+  def test_runs_postinstall_with_transitive_dependencies
+    with_shard({ dependencies: {transitive: "*"} }) do
+      run "shards install"
+      binary = File.join(application_path, "lib", "transitive", "version")
+      assert File.exists?(binary)
+      assert_equal "version @ 0.1.0\n", `#{binary}`
+    end
+  end
+
   def test_fails_when_shard_name_doesnt_match
     metadata = {
       dependencies: {

test/integration/update_test.cr

@@ -184,6 +184,15 @@ class UpdateCommandTest < Minitest::Test
     end
   end
 
+  def test_runs_postinstall_with_transitive_dependencies
+    with_shard({ dependencies: {transitive: "*"} }, {transitive: "0.1.0"}) do
+      run "shards update"
+      binary = File.join(application_path, "lib", "transitive", "version")
+      assert File.exists?(binary)
+      assert_equal "version @ 0.1.0\n", `#{binary}`
+    end
+  end
+
   def test_installs_executables
     metadata = {
       dependencies: {

test/integration_helper.cr

@@ -65,6 +65,23 @@ class Minitest::Test
     create_git_repository "unstable", "0.1.0", "0.2.0", "0.3.0.alpha", "0.3.0.beta"
     create_git_repository "preview", "0.1.0", "0.2.0", "0.3.0.a", "0.3.0.b", "0.3.0", "0.4.0.a"
 
+    # postinstall script with transitive dependency:
+    create_git_repository "version"
+    create_file "version", "src/version.cr", %(module Version; STRING = "version @ 0.1.0"; end)
+    create_git_release "version", "0.1.0"
+
+    create_git_repository "transitive"
+    create_file "transitive", "src/version.cr", %(require "version"; puts Version::STRING)
+    create_git_release "transitive", "0.2.0", <<-YAML
+name: transitive
+version: 0.2.0
+dependencies:
+  version:
+    git: #{git_path(:version)}
+scripts:
+  postinstall: crystal build src/version.cr
+YAML
+
     Minitest::Test.created_repositories!
   end