chore: some refactoring

spec/dedupe_lock_spec.cr

@@ -1,15 +1,17 @@
 require "./spec_helper"
-require "../src/utils/dedupe_lock"
+require "../src/utils/concurrent/dedupe_lock"
+
+alias DedupeLock = ::Zap::Utils::Concurrent::DedupeLock
 
 class Deduped
-  include Zap::Utils::DedupeLock(Int32)
+  include DedupeLock(Int32)
 end
 
 module GloballyDeduped
-  Zap::Utils::DedupeLock::Global.setup(:global, Int32)
+  DedupeLock::Global.setup(:global, Int32)
 end
 
-describe Zap::Utils::DedupeLock do
+describe DedupeLock do
   it "should lock and memoize a block" do
     s = Deduped.new
 
@@ -67,7 +69,7 @@ describe Zap::Utils::DedupeLock do
   end
 end
 
-describe Zap::Utils::DedupeLock::Global do
+describe DedupeLock::Global do
   it "should lock and memoize a block" do
     check = Atomic(Int32).new(0)
     results = Array(Int32).new(10, 0)

spec/parallel_spec.cr

@@ -1,10 +1,12 @@
 require "./spec_helper"
-require "../src/utils/parallel"
+require "../src/utils/concurrent/parallel"
 
-describe Zap::Utils::Parallel do
+alias Parallel = ::Zap::Utils::Concurrent::Parallel
+
+describe Parallel do
   it("should parallelize computations (index)") do
     10.times do
-      results = Zap::Utils::Parallel.parallelize(10) { |i|
+      results = Parallel.parallelize(10) { |i|
         # Shuffle the fibers
         sleep(Random.rand(10).milliseconds)
         # Perform the computation
@@ -17,7 +19,7 @@ describe Zap::Utils::Parallel do
 
   it("should parallelize computations (iterable)") do
     10.times do
-      results = Zap::Utils::Parallel.parallelize(1..10) { |i|
+      results = Parallel.parallelize(1..10) { |i|
         # Shuffle the fibers
         sleep(Random.rand(10).milliseconds)
         # Perform the computation
@@ -29,7 +31,7 @@ describe Zap::Utils::Parallel do
   end
 
   it("should raise when one of the computation throws an exception") do
-    parallel = Zap::Utils::Parallel(Int32).new(1..10) { |i|
+    parallel = Parallel(Int32).new(1..10) { |i|
       raise "error" if i == 5
       raise "unraised error" if i == 7
       i

spec/workspaces_spec.cr

@@ -1,9 +1,9 @@
 require "./spec_helper"
 require "../src/workspaces"
 
-alias Workspaces = Zap::Workspaces
-alias Workspace = Zap::Workspaces::Workspace
-alias Package = Zap::Package
+alias Workspaces = ::Zap::Workspaces
+alias Workspace = ::Zap::Workspaces::Workspace
+alias Package = ::Zap::Package
 
 WORKSPACE_A = Workspace.new(
   package: Package.from_json(%({

src/installer/backend/backend.cr → src/backend/backend.cr

@@ -1,6 +1,8 @@
 require "file_utils"
 
 module Zap::Backend
+  alias Pipeline = ::Zap::Utils::Concurrent::Pipeline
+
   enum Backends
     CloneFile
     CopyFile

src/cli/install.cr

@@ -1,5 +1,5 @@
 require "../commands/install/config"
-require "../installer/backend"
+require "../backend"
 
 class Zap::CLI
   alias InstallConfig = Commands::Install::Config

src/commands/install/install.cr

@@ -8,6 +8,8 @@ require "../../installer/pnp"
 require "../../workspaces"
 
 module Zap::Commands::Install
+  alias Pipeline = Utils::Concurrent::Pipeline
+
   record State,
     config : Zap::Config,
     install_config : Install::Config,
@@ -196,7 +198,7 @@ module Zap::Commands::Install
 
   private def self.strategy_check(config : Zap::Config, install_config : Install::Config, lockfile : Lockfile, reporter : Reporter)
     if !config.global && lockfile.strategy && lockfile.strategy != install_config.strategy
-      Log.debug { "Install strategy changed from #{lockfile.strategy} to #{install_config.strategy}" if lockfile.strategy}
+      Log.debug { "Install strategy changed from #{lockfile.strategy} to #{install_config.strategy}" if lockfile.strategy }
       reporter.info "Install strategy changed from #{lockfile.strategy} to #{install_config.strategy}." if lockfile.strategy
       if ::File.exists?(config.node_modules)
         reporter.info "Removing the existing `node_modules` folder…"

src/config.cr

@@ -1,6 +1,6 @@
 require "./utils/filter"
 require "./utils/from_env"
-require "./installer/backend"
+require "./backend"
 require "./package"
 
 module Zap

src/installer/classic/classic.cr

@@ -1,5 +1,5 @@
 require "../installer"
-require "../backend/*"
+require "../../backend/*"
 
 class Zap::Installer::Classic < Zap::Installer::Base
   record DependencyItem,
@@ -92,14 +92,7 @@ class Zap::Installer::Classic < Zap::Installer::Base
         Log.debug { "(#{dependency.key}) Installing package…" }
 
         # Raise if the architecture is not supported
-        begin
-          dependency.match_os_and_cpu!
-        rescue e
-          # If the package is optional, skip it
-          next if dependency_item.optional
-          # Else, raise the error
-          raise e
-        end
+        check_os_and_cpu!(dependency, early: :next, optional: dependency_item.optional)
 
         # Install a dependency and get the new cache to pass to the subdeps
         install_location, did_install = install_dependency(
@@ -125,21 +118,10 @@ class Zap::Installer::Classic < Zap::Installer::Base
         ancestors = dependency_item.ancestors.dup.push(dependency)
         # Process each child dependency
         dependency.each_dependency(include_dev: false) do |name, version_or_alias, type|
-          # Apply overrides
+          # Apply override
           pkg = state.lockfile.get_package?(name, version_or_alias)
           next unless pkg
-          if overrides = state.lockfile.overrides
-            if override = overrides.override?(pkg, ancestors)
-              # maybe enable logging with a verbose flag?
-              # ancestors_str = ancestors.map { |a| "#{a.name}@#{a.version}" }.join(" > ")
-              # state.reporter.log("#{"Overriden:".colorize.bold.yellow} #{"#{override.name}@"}#{override.specifier.colorize.blue} (was: #{pkg.version}) #{"(#{ancestors_str})".colorize.dim}")
-              pkg = state.lockfile.packages["#{override.name}@#{override.specifier}"]
-              Log.debug {
-                ancestors_str = ancestors.map { |a| "#{a.name}@#{a.version}" }.join(" > ")
-                "(#{pkg.key}) Overriden dependency: #{"#{override.name}@"}#{override.specifier} (was: #{pkg.version}) (#{ancestors_str})"
-              }
-            end
-          end
+          pkg = apply_override(state, pkg, ancestors)
           # Queue child dependency
           dependency_queue << DependencyItem.new(
             dependency: pkg,

src/installer/installer.cr

@@ -3,6 +3,24 @@ module Zap::Installer
 
   Log = Zap::Log.for(self)
 
+  # Check if a package is already installed on the filesystem
+  def self.package_already_installed?(dependency : Package, path : Path)
+    if exists = Dir.exists?(path)
+      metadata_path = path / METADATA_FILE_NAME
+      unless File.readable?(metadata_path)
+        FileUtils.rm_rf(path)
+        exists = false
+      else
+        key = File.read(metadata_path)
+        if key != dependency.key
+          FileUtils.rm_rf(path)
+          exists = false
+        end
+      end
+    end
+    exists
+  end
+
   abstract class Base
     getter state : Commands::Install::State
     getter main_package : Package
@@ -17,6 +35,7 @@ module Zap::Installer
 
     abstract def install : Nil
 
+    # Remove a set of direct dependencies from the filesystem
     def remove(dependencies : Set({String, String | Package::Alias, String})) : Nil
       dependencies.each do |(name, version_or_alias, root_name)|
         workspace = state.context.workspaces.try &.find { |w| w.package.name == root_name }
@@ -33,6 +52,7 @@ module Zap::Installer
       end
     end
 
+    # Prune unused dependencies from the filesystem
     def prune_orphan_modules
       prune_workspace_orphans(Path.new(state.config.node_modules))
       state.context.workspaces.try &.each do |workspace|
@@ -159,22 +179,42 @@ module Zap::Installer
       end
       result
     end
-  end
 
-  def self.package_already_installed?(dependency : Package, path : Path)
-    if exists = Dir.exists?(path)
-      metadata_path = path / METADATA_FILE_NAME
-      unless File.readable?(metadata_path)
-        FileUtils.rm_rf(path)
-        exists = false
-      else
-        key = File.read(metadata_path)
-        if key != dependency.key
-          FileUtils.rm_rf(path)
-          exists = false
+    # Check if a package is overriden and return the override as a Package if any.
+    # Otherwise return the original package.
+    protected def apply_override(
+      state : Commands::Install::State,
+      package : Package,
+      ancestors : Enumerable(Package | Lockfile::Root),
+      *,
+      reverse_ancestors? : Bool = false
+    ) : Package
+      if overrides = state.lockfile.overrides
+        ancestors = ancestors.to_a.reverse if reverse_ancestors?
+        if override = overrides.override?(package, ancestors)
+          # maybe enable logging with a verbose flag?
+          # ancestors_str = ancestors.map { |a| "#{a.name}@#{a.version}" }.join(" > ")
+          # state.reporter.log("#{"Overriden:".colorize.bold.yellow} #{"#{override.name}@"}#{override.specifier.colorize.blue} (was: #{package.version}) #{"(#{ancestors_str})".colorize.dim}")
+          Log.debug {
+            ancestors_str = ancestors.select(&.is_a?(Package)).map { |a| "#{a.as(Package).name}@#{a.as(Package).version}" }.join(" > ")
+            "(#{package.key}) Overriden dependency: #{"#{override.name}@"}#{override.specifier} (was: #{package.version}) (#{ancestors_str})"
+          }
+          return state.lockfile.packages["#{override.name}@#{override.specifier}"]
         end
       end
+      package
+    end
+
+    # Raise if the architecture is not supported. If the package is optional, skip it.
+    macro check_os_and_cpu!(package, *, early, optional = nil)
+      begin
+        {{package}}.match_os_and_cpu!
+      rescue e
+        # If the package is optional, skip it
+        {{early.id}} if {{optional}}
+        # Raise the error unless the package is an optional dependency
+        raise e
+      end
     end
-    exists
   end
 end

src/installer/isolated/isolated.cr

@@ -1,5 +1,5 @@
 require "../installer"
-require "../backend/*"
+require "../../backend/*"
 
 class Zap::Installer::Isolated < Zap::Installer::Base
   # See: https://github.com/npm/rfcs/blob/main/accepted/0042-isolated-mode.md
@@ -57,19 +57,15 @@ class Zap::Installer::Isolated < Zap::Installer::Base
     ancestors : Ancestors,
     root_path : Path? = nil,
     optional : Bool = false
-  ) : Path
+  ) : Path?
     resolved_peers = nil
     overrides = nil
 
     if package.is_a?(Package)
       Log.debug { "(#{package.key}) Installing package…" }
-      # Raise if the architecture is not supported
-      begin
-        package.match_os_and_cpu!
-      rescue e
-        # Raise the error unless the package is an optional dependency
-        raise e unless optional
-      end
+
+      # Raise if the architecture is not supported - unless the package is optional
+      check_os_and_cpu!(package, early: :return, optional: optional)
 
       # Links/Workspaces are easy, we just need to return the target path
       if package.kind.link?
@@ -163,20 +159,8 @@ class Zap::Installer::Isolated < Zap::Installer::Base
       # Add to the ancestors
       ancestors.unshift(package)
 
-      # Apply overrides
-      if overrides = state.lockfile.overrides
-        reversed_ancestors = ancestors.to_a.reverse
-        if override = overrides.override?(dependency, reversed_ancestors)
-          # maybe enable logging with a verbose flag?
-          # ancestors_str = reversed_ancestors.select(&.is_a?(Package)).map { |a| "#{a.as(Package).name}@#{a.as(Package).version}" }.join(" > ")
-          # state.reporter.log("#{"Overriden:".colorize.bold.yellow} #{"#{override.name}@"}#{override.specifier.colorize.blue} (was: #{dependency.version}) #{"(#{ancestors_str})".colorize.dim}")
-          dependency = state.lockfile.packages["#{override.name}@#{override.specifier}"]
-          Log.debug {
-            ancestors_str = reversed_ancestors.select(&.is_a?(Package)).map { |a| "#{a.as(Package).name}@#{a.as(Package).version}" }.join(" > ")
-            "(#{dependency.key}) Overriden dependency: #{"#{override.name}@"}#{override.specifier} (was: #{dependency.version}) (#{ancestors_str})"
-          }
-        end
-      end
+      # Apply override
+      dependency = apply_override(state, dependency, ancestors, reverse_ancestors?: true)
 
       # Install the dependency to its own folder
       source = install_package(
@@ -186,6 +170,9 @@ class Zap::Installer::Isolated < Zap::Installer::Base
       )
       ancestors.shift
 
+      # Skip if the dependency is optional and was not installed
+      next unless source
+
       # Link it to the parent package
       target = install_path / name
       Log.debug { "(#{package.is_a?(Package) ? package.key : package.name}) Linking #{dependency.key}: #{source} -> #{target}" }

src/installer/pnp/pnp.cr

@@ -55,7 +55,7 @@ class Zap::Installer::PnP < Zap::Installer::Base
     ancestors : Ancestors,
     optional : Bool = false,
     workspace_or_main_package : (Workspaces::Workspace | Package)? = nil
-  ) : PackageReference
+  ) : PackageReference?
     resolved_peers = nil
     overrides = nil
     root = ancestors.last?
@@ -67,13 +67,9 @@ class Zap::Installer::PnP < Zap::Installer::Base
       reference = package.key
 
       Log.debug { "(#{package.key}) Installing package…" }
-      # Raise if the architecture is not supported
-      begin
-        package.match_os_and_cpu!
-      rescue e
-        # Raise the error unless the package is an optional dependency
-        raise e unless optional
-      end
+
+      # Raise if the architecture is not supported - unless the package is optional
+      check_os_and_cpu!(package, early: :return, optional: optional)
 
       # Shortcut for links, no need to check its dependencies
       if package.kind.link?
@@ -217,20 +213,8 @@ class Zap::Installer::PnP < Zap::Installer::Base
       # Add to the ancestors
       ancestors.unshift(package_or_root)
 
-      # Apply overrides
-      if overrides = state.lockfile.overrides
-        reversed_ancestors = ancestors.to_a.reverse
-        if override = overrides.override?(dependency, reversed_ancestors)
-          # maybe enable logging with a verbose flag?
-          # ancestors_str = reversed_ancestors.select(&.is_a?(Package)).map { |a| "#{a.as(Package).name}@#{a.as(Package).version}" }.join(" > ")
-          # state.reporter.log("#{"Overriden:".colorize.bold.yellow} #{"#{override.name}@"}#{override.specifier.colorize.blue} (was: #{dependency.version}) #{"(#{ancestors_str})".colorize.dim}")
-          dependency = state.lockfile.packages["#{override.name}@#{override.specifier}"]
-          Log.debug {
-            ancestors_str = reversed_ancestors.select(&.is_a?(Package)).map { |a| "#{a.as(Package).name}@#{a.as(Package).version}" }.join(" > ")
-            "(#{dependency.key}) Overriden dependency: #{"#{override.name}@"}#{override.specifier} (was: #{dependency.version}) (#{ancestors_str})"
-          }
-        end
-      end
+      # Apply override
+      dependency = apply_override(state, dependency, ancestors, reverse_ancestors?: true)
 
       # Install the dependency to its own folder
       dependency_reference = install_package(
@@ -240,6 +224,9 @@ class Zap::Installer::PnP < Zap::Installer::Base
       )
       ancestors.shift
 
+      # Skip if the dependency is optional and was not installed
+      next unless dependency_reference
+
       # Link it to the parent package
       Log.debug { "(#{package_or_root.is_a?(Package) ? package_or_root.key : package_or_root.name}) Linking #{dependency.key}: #{dependency_reference} -> #{reference}" }
       package_dependencies << Manifest::Data::PackageDependency.new(

src/lockfile.cr

@@ -2,7 +2,7 @@ require "yaml"
 require "digest"
 require "./utils/**"
 
-alias DependencyType = Zap::Package::DependencyType
+alias DependencyType = ::Zap::Package::DependencyType
 
 class Zap::Lockfile
   include YAML::Serializable