From 296c2a538ffad75f6e0af57e0d23e50ae5318d90 Mon Sep 17 00:00:00 2001 From: Julien Elbaz Date: Wed, 13 Nov 2024 21:36:06 +0100 Subject: [PATCH] various fixes --- packages/commands/dlx/dlx.cr | 3 ++ packages/commands/install/install.cr | 18 ++++++---- .../install/linker/classic/classic.cr | 3 ++ .../install/linker/isolated/isolated.cr | 4 ++- packages/commands/install/linker/linker.cr | 2 +- packages/commands/install/linker/pnp/pnp.cr | 3 ++ .../commands/install/protocol/alias/alias.cr | 3 ++ .../commands/install/protocol/file/file.cr | 3 ++ packages/commands/install/protocol/git/git.cr | 3 ++ .../install/protocol/registry/registry.cr | 3 ++ .../install/protocol/registry/resolver.cr | 5 ++- .../commands/install/protocol/resolver.cr | 2 ++ .../protocol/tarball_url/tarball_url.cr | 3 ++ .../install/protocol/workspace/workspace.cr | 3 ++ packages/commands/install/registry_clients.cr | 34 ++++++++++--------- packages/commands/install/resolver.cr | 2 +- packages/commands/why/why.cr | 5 +++ packages/data/package/fields/utility.cr | 2 +- packages/data/package/scripts/scripts.cr | 8 ++--- packages/fetch/cache.cr | 2 ++ packages/fetch/fetch.cr | 3 +- packages/git/remote.cr | 6 ++-- packages/reporter/interactive.cr | 4 +-- packages/reporter/reporter.cr | 1 + packages/semver/range.cr | 9 ++++- 25 files changed, 97 insertions(+), 37 deletions(-) diff --git a/packages/commands/dlx/dlx.cr b/packages/commands/dlx/dlx.cr index 12f63f6..0ad8dce 100644 --- a/packages/commands/dlx/dlx.cr +++ b/packages/commands/dlx/dlx.cr @@ -1,7 +1,10 @@ +require "log" require "shared/constants" require "./config" module Commands::Dlx + Log = ::Log.for("zap.commands.dlx") + def self.run( config : Core::Config, dlx_config : Dlx::Config diff --git a/packages/commands/install/install.cr b/packages/commands/install/install.cr index 62890f8..ce5a8cc 100644 --- a/packages/commands/install/install.cr +++ b/packages/commands/install/install.cr @@ -1,4 +1,5 @@ require "benchmark" +require "log" require "concurrency/pipeline" require "reporter/reporter" require "reporter/null" @@ -15,6 +16,8 @@ require "./linker/isolated" require "./linker/pnp" module Commands::Install + Log = ::Log.for("zap.commands.install") + alias Pipeline = Concurrency::Pipeline def self.run( @@ -65,10 +68,10 @@ module Commands::Install config = self.strategy_check(config, install_config, lockfile, inferred_context, reporter) # Force hoisting if the hoisting options have changed - self.hoisting_check(install_config, lockfile, inferred_context, reporter) + install_config = self.hoisting_check(install_config, lockfile, inferred_context, reporter) # Force metadata retrieval if the package extensions options have changed - self.package_extensions_check(install_config, lockfile, inferred_context, reporter) + install_config = self.package_extensions_check(install_config, lockfile, inferred_context, reporter) # Init state struct state = State.new( @@ -119,6 +122,7 @@ module Commands::Install # Do not edit lockfile or package.json files in global mode or if the save flag is false unless state.config.global || !state.install_config.save # Write lockfile + Log.debug { "• Writing the lockfile" } state.lockfile.write(format: config.lockfile_format) # Edit and write the package.json files if the flags have been set in the config @@ -236,7 +240,7 @@ module Commands::Install config end - private def self.hoisting_check(install_config : Install::Config, lockfile : Data::Lockfile, inferred_context : Core::Config::InferredContext, reporter : Reporter) + private def self.hoisting_check(install_config : Install::Config, lockfile : Data::Lockfile, inferred_context : Core::Config::InferredContext, reporter : Reporter) : Install::Config if lockfile.update_hoisting_shasum(inferred_context.main_package) if install_config.frozen_lockfile # If the lockfile is frozen, raise an error @@ -246,12 +250,13 @@ module Commands::Install if lockfile.read_status.from_disk? Log.debug { "Detected a change in hoisting options in the package.json file" } reporter.info("Hoisting options were modified. The packages will be re-installed.") - install_config = install_config.copy_with(refresh_install: true) + return install_config.copy_with(refresh_install: true) end end + install_config end - private def self.package_extensions_check(install_config : Install::Config, lockfile : Data::Lockfile, inferred_context : Core::Config::InferredContext, reporter : Reporter) + private def self.package_extensions_check(install_config : Install::Config, lockfile : Data::Lockfile, inferred_context : Core::Config::InferredContext, reporter : Reporter) : Install::Config if lockfile.update_package_extensions_shasum(inferred_context.main_package) if install_config.frozen_lockfile # If the lockfile is frozen, raise an error @@ -261,9 +266,10 @@ module Commands::Install if lockfile.read_status.from_disk? Log.debug { "Detected a change in package extensions options in the package.json file" } reporter.info("Package extensions have been modified. Package metadata will forcefully be fetched from the registry and packages will be re-installed.") - install_config = install_config.copy_with(force_metadata_retrieval: true, refresh_install: true) + return install_config.copy_with(force_metadata_retrieval: true, refresh_install: true) end end + install_config end private def self.remove_packages(state : State) diff --git a/packages/commands/install/linker/classic/classic.cr b/packages/commands/install/linker/classic/classic.cr index 7f6c482..6594cc4 100644 --- a/packages/commands/install/linker/classic/classic.cr +++ b/packages/commands/install/linker/classic/classic.cr @@ -1,7 +1,10 @@ +require "log" require "utils/macros" require "../linker" class Commands::Install::Linker::Classic < Commands::Install::Linker::Base + Log = ::Log.for("zap.commands.install.linker.classic") + record DependencyItem, # the dependency to install dependency : Data::Package, diff --git a/packages/commands/install/linker/isolated/isolated.cr b/packages/commands/install/linker/isolated/isolated.cr index c8c0468..89a2448 100644 --- a/packages/commands/install/linker/isolated/isolated.cr +++ b/packages/commands/install/linker/isolated/isolated.cr @@ -1,3 +1,4 @@ +require "log" require "semver" require "shared/constants" require "utils/directories" @@ -15,6 +16,8 @@ class Commands::Install::Linker::Isolated < Commands::Install::Linker::Base @public_hoist_patterns : Array(Regex) @installed_packages : Set(String) = Set(String).new + Log = ::Log.for("zap.commands.install.linker.isolated") + def initialize( state, *, @@ -103,7 +106,6 @@ class Commands::Install::Linker::Isolated < Commands::Install::Linker::Base end # If the package folder exists, we assume that the package dependencies were already installed too - package_path = install_path / package.name if File.directory?(install_path) # If there is no need to perform a full pass, we can just return the package path and skip the dependencies unless state.install_config.refresh_install diff --git a/packages/commands/install/linker/linker.cr b/packages/commands/install/linker/linker.cr index b9e4ef5..0714e3c 100644 --- a/packages/commands/install/linker/linker.cr +++ b/packages/commands/install/linker/linker.cr @@ -3,7 +3,7 @@ require "data/package" require "../state" module Commands::Install::Linker - Log = ::Log.for("zap.linker") + Log = ::Log.for("zap.commands.install.linker") abstract class Base getter state : Commands::Install::State diff --git a/packages/commands/install/linker/pnp/pnp.cr b/packages/commands/install/linker/pnp/pnp.cr index 30351cc..674eabf 100644 --- a/packages/commands/install/linker/pnp/pnp.cr +++ b/packages/commands/install/linker/pnp/pnp.cr @@ -1,3 +1,4 @@ +require "log" require "utils/macros" require "../linker" @@ -9,6 +10,8 @@ class Commands::Install::Linker::PnP < Commands::Install::Linker::Base @relative_modules_store : Path @manifest : Manifest = Manifest.new + Log = ::Log.for("zap.commands.install.linker.pnp") + def initialize(state : Commands::Install::State) super(state) @node_modules = Path.new(state.config.node_modules) diff --git a/packages/commands/install/protocol/alias/alias.cr b/packages/commands/install/protocol/alias/alias.cr index 068b472..471b198 100644 --- a/packages/commands/install/protocol/alias/alias.cr +++ b/packages/commands/install/protocol/alias/alias.cr @@ -1,3 +1,4 @@ +require "log" require "utils/misc" require "extensions/object" require "../../resolver" @@ -5,6 +6,8 @@ require "../base" require "../registry" struct Commands::Install::Protocol::Alias < Commands::Install::Protocol::Base + Log = ::Log.for("zap.commands.install.protocol.alias") + def self.normalize?(str : String, path_info : PathInfo?) : {String?, String?}? if (parts = str.split("@npm:")).size > 1 # @npm: diff --git a/packages/commands/install/protocol/file/file.cr b/packages/commands/install/protocol/file/file.cr index 3ab1864..cee5c3a 100644 --- a/packages/commands/install/protocol/file/file.cr +++ b/packages/commands/install/protocol/file/file.cr @@ -1,8 +1,11 @@ +require "log" require "../../resolver" require "../base" require "./resolver" struct Commands::Install::Protocol::File < Commands::Install::Protocol::Base + Log = ::Log.for("zap.commands.install.protocol.file") + def self.normalize?(str : String, path_info : PathInfo?) : {String?, String?}? return nil unless path_info path_str = path_info.path.to_s diff --git a/packages/commands/install/protocol/git/git.cr b/packages/commands/install/protocol/git/git.cr index 8c49ddb..c7991be 100644 --- a/packages/commands/install/protocol/git/git.cr +++ b/packages/commands/install/protocol/git/git.cr @@ -1,9 +1,12 @@ +require "log" require "../base" require "./resolver" require "shared/constants" require "concurrency/dedupe_lock" struct Commands::Install::Protocol::Git < Commands::Install::Protocol::Base + Log = ::Log.for("zap.commands.install.protocol.git") + Concurrency::DedupeLock::Global.setup(:clone, Data::Package) def self.normalize?(str : String, path_info : PathInfo?) : {String?, String?}? diff --git a/packages/commands/install/protocol/registry/registry.cr b/packages/commands/install/protocol/registry/registry.cr index 6496218..40b5f82 100644 --- a/packages/commands/install/protocol/registry/registry.cr +++ b/packages/commands/install/protocol/registry/registry.cr @@ -1,7 +1,10 @@ +require "log" require "../base" require "./resolver" struct Commands::Install::Protocol::Registry < Commands::Install::Protocol::Base + Log = ::Log.for("zap.commands.install.protocol.registry") + # [<@scope>/] # [<@scope>/]@ # [<@scope>/]@ diff --git a/packages/commands/install/protocol/registry/resolver.cr b/packages/commands/install/protocol/registry/resolver.cr index 3eb36b6..1100353 100644 --- a/packages/commands/install/protocol/registry/resolver.cr +++ b/packages/commands/install/protocol/registry/resolver.cr @@ -1,3 +1,4 @@ +require "log" require "fetch" require "shared/constants" require "../base" @@ -9,6 +10,8 @@ struct Commands::Install::Protocol::Registry < Commands::Install::Protocol::Base end struct Commands::Install::Protocol::Registry::Resolver < Commands::Install::Protocol::Resolver + Log = ::Log.for("zap.commands.install.protocol.registry.resolver") + @clients : RegistryClients @client_pool : Fetch(Manifest) @package_name : String @@ -116,7 +119,7 @@ struct Commands::Install::Protocol::Registry::Resolver < Commands::Install::Prot end rescue e state.store.remove_package(metadata) - raise e + raise Exception.new("Unable to download package #{metadata.name}@#{metadata.version} from #{tarball_url}: #{e.message}", e) end true end diff --git a/packages/commands/install/protocol/resolver.cr b/packages/commands/install/protocol/resolver.cr index f8aac9a..1d87831 100644 --- a/packages/commands/install/protocol/resolver.cr +++ b/packages/commands/install/protocol/resolver.cr @@ -7,6 +7,8 @@ require "./aliased" require "../state" abstract struct Commands::Install::Protocol::Resolver + Log = ::Log.for("zap.commands.install.protocol.resolver") + alias Specifier = String | Semver::Range getter name : (String | Aliased)? diff --git a/packages/commands/install/protocol/tarball_url/tarball_url.cr b/packages/commands/install/protocol/tarball_url/tarball_url.cr index c57c89a..822e148 100644 --- a/packages/commands/install/protocol/tarball_url/tarball_url.cr +++ b/packages/commands/install/protocol/tarball_url/tarball_url.cr @@ -1,7 +1,10 @@ +require "log" require "../base" require "./resolver" struct Commands::Install::Protocol::TarballUrl < Commands::Install::Protocol::Base + Log = ::Log.for("zap.commands.install.protocol.tarball_url") + def self.normalize?(str : String, path_info : PathInfo?) : {String?, String?}? # if str.starts_with?("https://") || str.starts_with?("http://") diff --git a/packages/commands/install/protocol/workspace/workspace.cr b/packages/commands/install/protocol/workspace/workspace.cr index a44e759..86e435a 100644 --- a/packages/commands/install/protocol/workspace/workspace.cr +++ b/packages/commands/install/protocol/workspace/workspace.cr @@ -1,7 +1,10 @@ +require "log" require "../base" require "./resolver" struct Commands::Install::Protocol::Workspace < Commands::Install::Protocol::Base + Log = ::Log.for("zap.commands.install.protocol.workspace") + def self.normalize?(str : String, path_info : PathInfo?) : {String?, String?}? nil end diff --git a/packages/commands/install/registry_clients.cr b/packages/commands/install/registry_clients.cr index d00ab9e..cfec19d 100644 --- a/packages/commands/install/registry_clients.cr +++ b/packages/commands/install/registry_clients.cr @@ -6,9 +6,9 @@ require "fetch" # The pools are lazily initialized and cached. class Commands::Install::RegistryClients # The pool of clients for each registry - @client_pool_by_registry : Hash(String, Fetch(Manifest)) = Hash(String, Fetch(Manifest)).new + @@client_pool_by_registry : Hash(String, Fetch(Manifest)) = Hash(String, Fetch(Manifest)).new # Lock to synchronize access to the pools - @client_pool_by_registry_lock = Mutex.new + @@client_pool_by_registry_lock = Mutex.new # Initialize a new registry clients pool with the following arguments: # - store_path: path to the store where the metadata will be cached @@ -26,27 +26,29 @@ class Commands::Install::RegistryClients # Returns the client pool for the given registry url or creates a new one if it doesn't exist. def get_or_init_pool(url : String) : Fetch(Manifest) - @client_pool_by_registry_lock.synchronize do - @client_pool_by_registry[url] ||= init_client_pool(url) + @@client_pool_by_registry_lock.synchronize do + @@client_pool_by_registry[url] ||= init_client_pool(url) end end # Attempts to find a matching client pool for the given (tarball) url or creates a new one if it doesn't exist. # Strips the path from the url before matching. def find_or_init_pool(url : String) : {String, Fetch(Manifest)} - # Find if an existing pool matches the url - @client_pool_by_registry.find do |registry_url, _| - url.starts_with?(registry_url) - end || begin - # Otherwise create a new pool for the url hostname - uri = URI.parse(url) - # Remove the path - because it is impossible to infer based on the tarball url - uri.path = "/" - uri_str = uri.to_s - pool = init_client_pool(uri.to_s).tap do |pool| - @client_pool_by_registry[pool.base_url] = pool + @@client_pool_by_registry_lock.synchronize do + # Find if an existing pool matches the url + @@client_pool_by_registry.find do |registry_url, _| + url.starts_with?(registry_url) + end || begin + # Otherwise create a new pool for the url hostname + uri = URI.parse(url) + # Remove the path - because it is impossible to infer based on the tarball url + uri.path = "/" + uri_str = uri.to_s + pool = init_client_pool(uri.to_s).tap do |pool| + @@client_pool_by_registry[pool.base_url] = pool + end + {uri_str, pool} end - {uri_str, pool} end end diff --git a/packages/commands/install/resolver.cr b/packages/commands/install/resolver.cr index a3d7392..86e5b8d 100644 --- a/packages/commands/install/resolver.cr +++ b/packages/commands/install/resolver.cr @@ -11,7 +11,7 @@ require "./protocol/resolver" require "./protocol" module Commands::Install::Resolver - Log = ::Log.for("zap.resolver") + Log = ::Log.for("zap.commands.install.resolver") alias Pipeline = ::Concurrency::Pipeline diff --git a/packages/commands/why/why.cr b/packages/commands/why/why.cr index 2613ada..928de3c 100644 --- a/packages/commands/why/why.cr +++ b/packages/commands/why/why.cr @@ -1,4 +1,9 @@ +require "log" +require "data/lockfile" + module Commands::Why + Log = ::Log.for("zap.commands.why") + alias PackageResult = {root: Data::Lockfile::Root, ancestors: Deque({Data::Package, DependencyType}), type: DependencyType} DEPENDS_ON_CHAR = '←' diff --git a/packages/data/package/fields/utility.cr b/packages/data/package/fields/utility.cr index c2e3fbb..78dbc38 100644 --- a/packages/data/package/fields/utility.cr +++ b/packages/data/package/fields/utility.cr @@ -100,7 +100,7 @@ class Data::Package # A more path-friendly key. internal { getter hashed_key : String do - "#{name}@#{version}__#{dist.class.to_s.split("::").last.downcase}:#{Digest::SHA1.hexdigest(key)}" + "#{name}@#{version}__#{dist.class.to_s.split("::").last.downcase}:#{Digest::SHA1.hexdigest(key)}".gsub(':', '_') end } internal { safe_property transitive_overrides : Concurrency::SafeSet(Overrides::Override)? = nil } diff --git a/packages/data/package/scripts/scripts.cr b/packages/data/package/scripts/scripts.cr index 6e8428e..34904e8 100644 --- a/packages/data/package/scripts/scripts.cr +++ b/packages/data/package/scripts/scripts.cr @@ -146,14 +146,14 @@ class Data::Package pipeline.await end + # See: https://docs.npmjs.com/cli/commands/npm-run-script def self.run_script(command : String, chdir : Path | String, config : Core::Config, raise_on_error_code = true, output_io = nil, stdin = Process::Redirect::Close, **args, &block : String, Symbol ->) return if command.empty? - Log.debug { - "Running script: #{command} #{Utils::Macros.args_str}" - } output = (config.silent ? nil : output_io) || IO::Memory.new - # See: https://docs.npmjs.com/cli/commands/npm-run-script env = make_env(chdir, config) + Log.debug { + "Running script: #{command}\n#{Utils::Macros.args_str}\nenvironment: #{env}" + } yield command, :before status = Process.run(command, **args, shell: true, env: env, chdir: chdir.to_s, output: output, input: stdin, error: output) if !status.success? && raise_on_error_code diff --git a/packages/fetch/cache.cr b/packages/fetch/cache.cr index 8cf991e..ec1fb3c 100644 --- a/packages/fetch/cache.cr +++ b/packages/fetch/cache.cr @@ -4,6 +4,8 @@ require "utils/misc" class Fetch(T) abstract class Cache(T) + Log = ::Log.for("zap.fetch.cache") + abstract def get(key_str : String, etag : String?) : T? abstract def get(key_str : String, &etag : -> String?) : T? abstract def set(key_str : String, value : T, expiry : Time::Span?, etag : String?) : Nil diff --git a/packages/fetch/fetch.cr b/packages/fetch/fetch.cr index ff4333e..c4b640e 100644 --- a/packages/fetch/fetch.cr +++ b/packages/fetch/fetch.cr @@ -23,6 +23,7 @@ class Fetch(T) &block : HTTP::Client -> ) @pool = Concurrency::Pool(HTTP::Client).new(@pool_max_size) do + Log.debug { "Creating new client for #{@base_url}" } HTTP::Client.new(URI.parse(base_url)).tap do |client| block.call(client) end @@ -41,7 +42,7 @@ class Fetch(T) begin break yield client rescue e - ::Log.debug { e.message.colorize.red.to_s + Shared::Constants::NEW_LINE + e.backtrace.map { |line| "\t#{line}" }.join(Shared::Constants::NEW_LINE).colorize.red.to_s } + Log.debug { e.message.colorize.red.to_s + Shared::Constants::NEW_LINE + e.backtrace.map { |line| "\t#{line}" }.join(Shared::Constants::NEW_LINE).colorize.red.to_s } client.close sleep 0.5.seconds * retry_count raise e if retry_count >= retry_attempts diff --git a/packages/git/remote.cr b/packages/git/remote.cr index e112c53..c54393e 100644 --- a/packages/git/remote.cr +++ b/packages/git/remote.cr @@ -1,6 +1,8 @@ require "log" class Git::Remote + Log = ::Log.for("zap.git.remote") + # See: https://docs.npmjs.com/cli/v9/configuring-npm/package-json#git-urls-as-dependencies # ://[[:]@][:][:][/][# | #semver:] GIT_URL_REGEX = /(?:git\+)?(?git|ssh|http|https|file):\/\/(?:(?[^:@]+)?(:(?[^@]+))?@)?(?[^:\/]+)(:(?\d+))?[\/:](?[^#]+)((?:#semver:(?[^:]+))|(?:#(?[^:]+)))?/ @@ -118,7 +120,7 @@ class Git::Remote def self.run(command : String, stdio : Process::Stdio? = nil, **extra) : Nil command_and_args = command.split(/\s+/) output = stdio || Process::Redirect::Inherit - ::Log.debug { "Spawning: #{command_and_args} (#{extra})" } + Log.debug { "Spawning: #{command_and_args} (#{extra})" } status = Process.run(command_and_args[0], **extra, args: command_and_args[1..]? || nil, output: output, error: output) unless status.success? Fiber.yield @@ -130,7 +132,7 @@ class Git::Remote command_and_args = command.split(/\s+/) stderr = IO::Memory.new stdout = IO::Memory.new - ::Log.debug { "Spawning: #{command_and_args} (#{extra})" } + Log.debug { "Spawning: #{command_and_args} (#{extra})" } status = Process.run(command_and_args[0], **extra, args: command_and_args[1..]? || nil, output: stdout, error: stderr) unless status.success? raise stderr.to_s diff --git a/packages/reporter/interactive.cr b/packages/reporter/interactive.cr index f348a4f..567927c 100644 --- a/packages/reporter/interactive.cr +++ b/packages/reporter/interactive.cr @@ -284,9 +284,9 @@ class Reporter::Interactive < Reporter end if install_version - %("#{name}@#{install_version}") + %("#{name}@#{install_version.reduce}") else - incompatible_versions << {name, versions.map { |v, _| " #{v}" }.join(Shared::Constants::NEW_LINE)} + incompatible_versions << {name, versions.map { |v, peers| " #{v} #{"(#{peers.join(", ")})".colorize.dim}" }.join(Shared::Constants::NEW_LINE)} nil end end.compact! diff --git a/packages/reporter/reporter.cr b/packages/reporter/reporter.cr index 25b8d6b..d2f35aa 100644 --- a/packages/reporter/reporter.cr +++ b/packages/reporter/reporter.cr @@ -1,6 +1,7 @@ require "shared/constants" abstract class Reporter + Log = ::Log.for("zap.reporter") getter io_lock = Mutex.new abstract def output : IO diff --git a/packages/semver/range.cr b/packages/semver/range.cr index 6aee30f..51a9f26 100644 --- a/packages/semver/range.cr +++ b/packages/semver/range.cr @@ -24,6 +24,13 @@ struct Semver::Range protected def initialize(@comparator_sets : Array(ComparatorSet)) end + # Merges multiple ranges into a new range. + def merge(*others : self) : self + others.reduce(self) { |acc, range| + Range.new(acc.comparator_sets + range.comparator_sets) + } + end + # Parses a string into a Range object, returning nil if parsing fails. def self.parse?(str : String) : Range? parse(str) @@ -36,7 +43,7 @@ struct Semver::Range def self.parse(str : String) : Range range_set = Range.new - if (str.empty? || str == "*") + if (str.empty? || str == "*" || str == "latest") comparator_set = ComparatorSet.new range_set << comparator_set comparator_set << Comparator.new(Operator::GreaterThanOrEqual)