diff --git a/common.mk b/common.mk index 349d2a87e30906..374b722349c3f1 100644 --- a/common.mk +++ b/common.mk @@ -1550,7 +1550,11 @@ yes-update-default-gemspecs: $(PRECHECK_BUNDLED_GEMS:yes=main) -e "end" \ -e "spec.files.clear" \ -e "spec.extensions.clear" \ - -e "File.binwrite(File.join(destdir, spec.full_name+'.gemspec'), spec.to_ruby)" \ + -e "src = spec.to_ruby" \ + -e "src.sub!(/^$$/) {" \ + -e "%[# default: #{g} #{File.mtime(g).strftime(%[%s.%N])}\n]" \ + -e "}" \ + -e "File.binwrite(File.join(destdir, spec.full_name+'.gemspec'), src)" \ -e "end" \ -e "end" \ -- .bundle/specifications lib ext diff --git a/defs/gmake.mk b/defs/gmake.mk index 087aa1b549676b..718131e937a4ae 100644 --- a/defs/gmake.mk +++ b/defs/gmake.mk @@ -453,8 +453,10 @@ include $(top_srcdir)/defs/jit.mk # Query on the generated rdoc # # $ make rdoc:Integer#+ -rdoc\:%: PHONY - $(Q)$(RUNRUBY) $(srcdir)/libexec/ri --no-standard-docs --doc-dir=$(RDOCOUT) $(patsubst rdoc:%,%,$@) +rdoc\:%: PHONY programs $(RDOCOUT) update-default-gemspecs + $(Q)$(RUNRUBY) $(RUNOPT0) -I$(tooldir)/lib -rbundled_gem \ + -e "load BundledGem.command('rdoc', 'ri')" -- \ + --no-standard-docs --doc-dir=$(RDOCOUT) $(patsubst rdoc:%,%,$@) test_%.rb test/%: programs PHONY $(Q)$(exec) $(RUNRUBY) "$(TESTSDIR)/runner.rb" --ruby="$(RUNRUBY)" $(TEST_EXCLUDES) $(TESTOPTS) -- $(patsubst test/%,%,$@) diff --git a/lib/bundler/installer/gem_installer.rb b/lib/bundler/installer/gem_installer.rb index 5c4fa7825325c4..f3b43c31ee090a 100644 --- a/lib/bundler/installer/gem_installer.rb +++ b/lib/bundler/installer/gem_installer.rb @@ -25,6 +25,20 @@ def install_from_spec [false, specific_failure_message(e)] end + def download + spec.source.download( + spec, + force: force, + local: local, + build_args: Array(spec_settings), + previous_spec: previous_spec, + ) + + [true, nil] + rescue Bundler::BundlerError => e + [false, specific_failure_message(e)] + end + private def specific_failure_message(e) diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index d10e5ec92403ed..79489a6a6c95f6 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -32,6 +32,12 @@ def ready_to_enqueue? state == :none end + def ready_to_install?(installed_specs) + return false unless state == :downloaded + + spec.extensions.none? || dependencies_installed?(installed_specs) + end + def has_post_install_message? !post_install_message.empty? end @@ -84,6 +90,7 @@ def initialize(installer, all_specs, size, standalone, force, local: false, skip def call if @rake + do_download(@rake, 0) do_install(@rake, 0) Gem::Specification.reset end @@ -107,26 +114,54 @@ def failed_specs end def install_with_worker - enqueue_specs - process_specs until finished_installing? + installed_specs = {} + enqueue_specs(installed_specs) + + process_specs(installed_specs) until finished_installing? end def install_serially until finished_installing? raise "failed to find a spec to enqueue while installing serially" unless spec_install = @specs.find(&:ready_to_enqueue?) spec_install.state = :enqueued + do_download(spec_install, 0) do_install(spec_install, 0) end end def worker_pool @worker_pool ||= Bundler::Worker.new @size, "Parallel Installer", lambda {|spec_install, worker_num| - do_install(spec_install, worker_num) + case spec_install.state + when :enqueued + do_download(spec_install, worker_num) + when :installable + do_install(spec_install, worker_num) + else + spec_install + end } end - def do_install(spec_install, worker_num) + def do_download(spec_install, worker_num) Plugin.hook(Plugin::Events::GEM_BEFORE_INSTALL, spec_install) + + gem_installer = Bundler::GemInstaller.new( + spec_install.spec, @installer, @standalone, worker_num, @force, @local + ) + + success, message = gem_installer.download + + if success + spec_install.state = :downloaded + else + spec_install.error = "#{message}\n\n#{require_tree_for_spec(spec_install.spec)}" + spec_install.state = :failed + end + + spec_install + end + + def do_install(spec_install, worker_num) gem_installer = Bundler::GemInstaller.new( spec_install.spec, @installer, @standalone, worker_num, @force, @local ) @@ -147,9 +182,19 @@ def do_install(spec_install, worker_num) # Some specs might've had to wait til this spec was installed to be # processed so the call to `enqueue_specs` is important after every # dequeue. - def process_specs - worker_pool.deq - enqueue_specs + def process_specs(installed_specs) + spec = worker_pool.deq + + if spec.installed? + installed_specs[spec.name] = true + return + elsif spec.failed? + return + elsif spec.ready_to_install?(installed_specs) + spec.state = :installable + end + + worker_pool.enq(spec) end def finished_installing? @@ -185,18 +230,15 @@ def require_tree_for_spec(spec) # Later we call this lambda again to install specs that depended on # previously installed specifications. We continue until all specs # are installed. - def enqueue_specs - installed_specs = {} - @specs.each do |spec| - next unless spec.installed? - installed_specs[spec.name] = true - end - + def enqueue_specs(installed_specs) @specs.each do |spec| - if spec.ready_to_enqueue? && spec.dependencies_installed?(installed_specs) - spec.state = :enqueued - worker_pool.enq spec + if spec.installed? + installed_specs[spec.name] = true + next end + + spec.state = :enqueued + worker_pool.enq spec end end end diff --git a/lib/bundler/plugin/api/source.rb b/lib/bundler/plugin/api/source.rb index 6c888d037350b9..798326673ad788 100644 --- a/lib/bundler/plugin/api/source.rb +++ b/lib/bundler/plugin/api/source.rb @@ -74,6 +74,14 @@ def options_to_lock {} end + # Download the gem specified by the spec at appropriate path. + # + # A source plugin can implement this method to split the download and the + # installation of a gem. + # + # @return [Boolean] Whether the download of the gem succeeded. + def download(spec, opts); end + # Install the gem specified by the spec at appropriate path. # `install_path` provides a sufficient default, if the source can only # satisfy one gem, but is not binding. diff --git a/lib/bundler/plugin/installer.rb b/lib/bundler/plugin/installer.rb index 853ad9edca4f6f..9be8b36843bbba 100644 --- a/lib/bundler/plugin/installer.rb +++ b/lib/bundler/plugin/installer.rb @@ -110,7 +110,8 @@ def install_from_specs(specs) paths = {} specs.each do |spec| - spec.source.install spec + spec.source.download(spec) + spec.source.install(spec) paths[spec.name] = spec end diff --git a/lib/bundler/self_manager.rb b/lib/bundler/self_manager.rb index 1db77fd46b3fef..82efbf56a4fb80 100644 --- a/lib/bundler/self_manager.rb +++ b/lib/bundler/self_manager.rb @@ -63,6 +63,7 @@ def install_and_restart_with(version) end def install(spec) + spec.source.download(spec) spec.source.install(spec) end diff --git a/lib/bundler/source.rb b/lib/bundler/source.rb index 2b90a0eff1bfbb..cf71be8801256b 100644 --- a/lib/bundler/source.rb +++ b/lib/bundler/source.rb @@ -31,6 +31,8 @@ def version_message(spec, locked_spec = nil) message end + def download(*); end + def can_lock?(spec) spec.source == self end diff --git a/lib/bundler/source/rubygems.rb b/lib/bundler/source/rubygems.rb index e679010e97dc05..877343b5fb4d3f 100644 --- a/lib/bundler/source/rubygems.rb +++ b/lib/bundler/source/rubygems.rb @@ -9,6 +9,7 @@ class Rubygems < Source # Ask for X gems per API request API_REQUEST_SIZE = 100 + REQUIRE_MUTEX = Mutex.new attr_accessor :remotes @@ -21,6 +22,8 @@ def initialize(options = {}) @allow_local = options["allow_local"] || false @prefer_local = false @checksum_store = Checksum::Store.new + @gem_installers = {} + @gem_installers_mutex = Mutex.new Array(options["remotes"]).reverse_each {|r| add_remote(r) } @@ -162,49 +165,40 @@ def specs end end - def install(spec, options = {}) + def download(spec, options = {}) if (spec.default_gem? && !cached_built_in_gem(spec, local: options[:local])) || (installed?(spec) && !options[:force]) - print_using_message "Using #{version_message(spec, options[:previous_spec])}" - return nil # no post-install message + return true end - path = fetch_gem_if_possible(spec, options[:previous_spec]) - raise GemNotFound, "Could not find #{spec.file_name} for installation" unless path - - return if Bundler.settings[:no_install] - - install_path = rubygems_dir - bin_path = Bundler.system_bindir - - require_relative "../rubygems_gem_installer" - - installer = Bundler::RubyGemsGemInstaller.at( - path, - security_policy: Bundler.rubygems.security_policies[Bundler.settings["trust-policy"]], - install_dir: install_path.to_s, - bin_dir: bin_path.to_s, - ignore_dependencies: true, - wrappers: true, - env_shebang: true, - build_args: options[:build_args], - bundler_extension_cache_path: extension_cache_path(spec) - ) + installer = rubygems_gem_installer(spec, options) if spec.remote s = begin installer.spec rescue Gem::Package::FormatError - Bundler.rm_rf(path) + Bundler.rm_rf(installer.gem) raise rescue Gem::Security::Exception => e raise SecurityError, - "The gem #{File.basename(path, ".gem")} can't be installed because " \ + "The gem #{installer.gem} can't be installed because " \ "the security policy didn't allow it, with the message: #{e.message}" end spec.__swap__(s) end + spec + end + + def install(spec, options = {}) + if (spec.default_gem? && !cached_built_in_gem(spec, local: options[:local])) || (installed?(spec) && !options[:force]) + print_using_message "Using #{version_message(spec, options[:previous_spec])}" + return nil # no post-install message + end + + return if Bundler.settings[:no_install] + + installer = rubygems_gem_installer(spec, options) spec.source.checksum_store.register(spec, installer.gem_checksum) message = "Installing #{version_message(spec, options[:previous_spec])}" @@ -516,6 +510,34 @@ def extension_cache_slug(spec) return unless remote = spec.remote remote.cache_slug end + + # We are using a mutex to reaed and write from/to the hash. + # The reason this double synchronization was added is for performance + # and lock the mutex for the shortest possible amount of time. Otherwise, + # all threads are fighting over this mutex and when it gets acquired it gets locked + # until a threads finishes downloading a gem, leaving the other threads waiting + # doing nothing. + def rubygems_gem_installer(spec, options) + @gem_installers_mutex.synchronize { @gem_installers[spec.name] } || begin + path = fetch_gem_if_possible(spec, options[:previous_spec]) + raise GemNotFound, "Could not find #{spec.file_name} for installation" unless path + + REQUIRE_MUTEX.synchronize { require_relative "../rubygems_gem_installer" } + + installer = Bundler::RubyGemsGemInstaller.at( + path, + security_policy: Bundler.rubygems.security_policies[Bundler.settings["trust-policy"]], + install_dir: rubygems_dir.to_s, + bin_dir: Bundler.system_bindir.to_s, + ignore_dependencies: true, + wrappers: true, + env_shebang: true, + build_args: options[:build_args], + bundler_extension_cache_path: extension_cache_path(spec) + ) + @gem_installers_mutex.synchronize { @gem_installers[spec.name] ||= installer } + end + end end end end diff --git a/lib/rubygems/specification_policy.rb b/lib/rubygems/specification_policy.rb index 8ab82ee4a9a1b9..478e294e0997ca 100644 --- a/lib/rubygems/specification_policy.rb +++ b/lib/rubygems/specification_policy.rb @@ -192,15 +192,14 @@ def validate_duplicate_dependencies # :nodoc: # Checks that the gem does not depend on itself. def validate_dependencies # :nodoc: - warning_messages = [] + error_messages = [] @specification.dependencies.each do |dep| - if dep.name == @specification.name # warn on self reference - warning_messages << "Self referencing dependency is unnecessary and strongly discouraged." + if dep.name == @specification.name # error on self reference + error_messages << "Dependencies of this gem include a self-reference." end end - if warning_messages.any? - warning_messages.each {|warning_message| warning warning_message } - end + + error error_messages.join if error_messages.any? end def validate_required_ruby_version diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index cf01a40b8c6416..4d9661396f4030 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -2812,14 +2812,13 @@ def test_validate_self_referencing_dependencies Dir.chdir @tempdir do @a1.add_dependency @a1.name, "1" - use_ui @ui do + e = assert_raise Gem::InvalidSpecificationException do @a1.validate end - assert_equal <<-EXPECTED, @ui.error -#{w}: Self referencing dependency is unnecessary and strongly discouraged. -#{w}: See https://guides.rubygems.org/specification-reference/ for help - EXPECTED + expected = "Dependencies of this gem include a self-reference." + + assert_equal expected, e.message end end diff --git a/tool/lib/bundled_gem.rb b/tool/lib/bundled_gem.rb index d2ed61a508fdde..22d03112ed1e0f 100644 --- a/tool/lib/bundled_gem.rb +++ b/tool/lib/bundled_gem.rb @@ -16,6 +16,13 @@ module BundledGem "psych" # rdoc ] + def self.command(gem, cmd) + if stub = Gem::Specification.latest_spec_for(gem) + spec = stub.spec + File.join(spec.gem_dir, spec.bindir, cmd) + end + end + module_function def unpack(file, *rest) diff --git a/tool/outdate-bundled-gems.rb b/tool/outdate-bundled-gems.rb index 47ee80bc8924ea..2772702ff19eea 100755 --- a/tool/outdate-bundled-gems.rb +++ b/tool/outdate-bundled-gems.rb @@ -60,6 +60,7 @@ class Removal def initialize(base = nil) @base = (File.join(base, "/") if base) @remove = {} + @defaults = nil end def prefixed(name) @@ -92,10 +93,22 @@ def rmdir(name) @remove[slash(stripped(name))] = :rm_rf end - def glob(pattern, *rest) - Dir.glob(prefixed(pattern), *rest) {|n| - yield stripped(n) - } + def glob(pattern, *rest, &block) + Dir.glob(pattern, *rest, base: @base, &block) + end + + def default_gem?(spec) + (@defaults ||= {}).fetch(spec) do + File.open(prefixed(spec)) do |f| + if /^# default: (\S+) (\d+\.\d+)/ =~ f.gets("") + File.mtime(prefixed($1)) <= Time.at(Rational($2)) + else + false + end + rescue + false + end + end end def sorted @@ -133,12 +146,14 @@ def each_directory srcdir.glob(".bundle/specifications/*.gemspec") do |spec| unless srcdir.directory?(".bundle/gems/#{File.basename(spec, '.gemspec')}/") + next if srcdir.default_gem?(srcdir.prefixed(spec)) srcdir.unlink(spec) end end curdir.glob(".bundle/specifications/*.gemspec") do |spec| unless srcdir.directory?(".bundle/gems/#{File.basename(spec, '.gemspec')}") + next if srcdir.default_gem?(curdir.prefixed(spec)) curdir.unlink(spec) end end