From 9f5f8a9883cdc72a43345772d5eb7f690c3b8fca Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Fri, 24 May 2024 13:50:10 -0600 Subject: [PATCH] fix specs for bundler 3 importantly, bundler 3 automatically caches, cleans, and prunes gems by default, so need to always keep track of plugins, even for the regular gemfile run, and include plugins in the list of known gems when cleaning up --- bundler/lib/bundler/definition.rb | 52 ++++++----- bundler/lib/bundler/dsl.rb | 107 ++++++++++------------- bundler/lib/bundler/plugin.rb | 28 +++--- bundler/lib/bundler/plugin/dsl.rb | 7 +- bundler/lib/bundler/plugin/index.rb | 32 ++++--- bundler/lib/bundler/plugin/installer.rb | 4 + bundler/lib/bundler/runtime.rb | 5 +- bundler/spec/bundler/plugin_spec.rb | 16 ++-- bundler/spec/bundler/source_list_spec.rb | 2 +- bundler/spec/plugins/install_spec.rb | 5 +- 10 files changed, 131 insertions(+), 127 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 00a8b8e644c5..1141c02427fc 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -14,7 +14,7 @@ class << self attr_reader( :dependencies, :locked_deps, - :plugins, + :plugin_dependencies, :locked_gems, :platforms, :ruby_version, @@ -58,7 +58,8 @@ def self.build(gemfile, lockfile, unlock) # @param ruby_version [Bundler::RubyVersion, nil] Requested Ruby Version # @param optional_groups [Array(String)] A list of optional groups # @param lockfile_contents [String, nil] The contents of the lockfile - # @param plugins [Array(Bundler::Dependency)] array of plugin dependencies from Gemfile + # @param plugin_dependencies [Array(Bundler::Dependency)] array of plugin dependencies from Gemfile + # @param plugin_sources [Bundler::SourceList] def initialize(lockfile, dependencies, sources, @@ -67,7 +68,8 @@ def initialize(lockfile, optional_groups = [], gemfiles = [], lockfile_contents = nil, - plugins = []) + plugin_dependencies = [], + plugin_sources = SourceList.new) if [true, false].include?(unlock) @unlocking_bundler = false @unlocking = unlock @@ -76,15 +78,16 @@ def initialize(lockfile, @unlocking = unlock.any? {|_k, v| !Array(v).empty? } end - @dependencies = dependencies - @plugins = plugins - @sources = sources - @unlock = unlock - @optional_groups = optional_groups - @prefer_local = false - @specs = nil - @ruby_version = ruby_version - @gemfiles = gemfiles + @dependencies = dependencies + @sources = sources + @plugin_dependencies = plugin_dependencies + @plugin_sources = plugin_sources + @unlock = unlock + @optional_groups = optional_groups + @prefer_local = false + @specs = nil + @ruby_version = ruby_version + @gemfiles = gemfiles @lockfile = lockfile @@ -240,16 +243,16 @@ def requested_dependencies dependencies_for(requested_groups) end - def requested_plugins - plugins_for(requested_groups) + def requested_plugin_dependencies + plugin_dependencies_for(requested_groups) end def current_dependencies filter_relevant(dependencies) end - def current_plugins - filter_relevant(plugins) + def current_plugin_dependencies + filter_relevant(plugin_dependencies) end def current_locked_dependencies @@ -294,9 +297,9 @@ def dependencies_for(groups) deps end - def plugins_for(groups) + def plugin_dependencies_for(groups) groups.map!(&:to_sym) - plugins = current_plugins # always returns a new array + plugins = current_plugin_dependencies # always returns a new array plugins.select! do |d| if RUBY_VERSION >= "3.1" d.groups.intersect?(groups) @@ -340,11 +343,14 @@ def resolve end def spec_git_paths - sources.git_sources.map {|s| File.realpath(s.path) if File.exist?(s.path) }.compact + # plugin sources from the main Gemfile run we never instructed that they are cached, + # but they were, by the plugin run of the gemfile + plugin_sources.git_sources.each(&:cached!) + (sources.git_sources + plugin_sources.git_sources).uniq.filter_map {|s| File.realpath(s.path) if File.exist?(s.path) } end def groups - (dependencies + plugins).map(&:groups).flatten.uniq + (dependencies + plugin_dependencies).map(&:groups).flatten.uniq end def lock(file_or_preserve_unknown_sections = false, preserve_unknown_sections_or_unused = false) @@ -488,7 +494,7 @@ def validate_platforms! def validate_plugins! missing_plugins_list = [] - requested_plugins.each do |plugin| + requested_plugin_dependencies.each do |plugin| missing_plugins_list << plugin unless Plugin.installed?(plugin.name) end if missing_plugins_list.size > 1 @@ -516,8 +522,8 @@ def most_specific_locked_platform end end - attr_reader :sources - private :sources + attr_reader :sources, :plugin_sources + private :sources, :plugin_sources def nothing_changed? return false unless lockfile_exists? diff --git a/bundler/lib/bundler/dsl.rb b/bundler/lib/bundler/dsl.rb index a60dd36ca85e..abe9fa410e68 100644 --- a/bundler/lib/bundler/dsl.rb +++ b/bundler/lib/bundler/dsl.rb @@ -27,9 +27,12 @@ def self.evaluate(gemfile, lockfile, unlock) def initialize @source = nil @sources = SourceList.new + @plugin_sources = SourceList.new + @current_sources = @sources @git_sources = {} @dependencies = [] - @plugins = [] + @plugin_dependencies = [] + @current_dependencies = @dependencies @groups = [] @install_conditionals = [] @optional_groups = [] @@ -98,12 +101,12 @@ def gem(name, *args) options["gemfile"] = @gemfile version = args || [">= 0"] - normalize_options(name, version, true, options) + normalize_options(name, version, options) dep = Dependency.new(name, version, options) # if there's already a dependency with this name we try to prefer one - if current = @dependencies.find {|d| d.name == dep.name } + if current = @current_dependencies.find {|d| d.name == dep.name } if current.requirement != dep.requirement current_requirement_open = current.requirements_list.include?(">= 0") @@ -136,7 +139,7 @@ def gem(name, *args) # Always prefer the dependency from the Gemfile if current.gemspec_dev_dep? - @dependencies.delete(current) + @current_dependencies.delete(current) elsif dep.gemspec_dev_dep? return elsif current.source != dep.source @@ -151,7 +154,7 @@ def gem(name, *args) end end - @dependencies << dep + @current_dependencies << dep end def source(source, *args, &blk) @@ -161,7 +164,7 @@ def source(source, *args, &blk) if options.key?("type") options["type"] = options["type"].to_s - unless Plugin.source?(options["type"]) + unless (source_plugin = Plugin.source_plugin(options["type"])) raise InvalidOption, "No plugin sources available for #{options["type"]}" end @@ -169,12 +172,14 @@ def source(source, *args, &blk) raise InvalidOption, "You need to pass a block to #source with :type option" end + plugin(source_plugin) unless @plugin_dependencies.any? {|d| d.name == source_plugin } + source_opts = options.merge("uri" => source) - with_source(@sources.add_plugin_source(options["type"], source_opts), &blk) + with_source(add_source(:add_plugin_source, options["type"], source_opts), &blk) elsif block_given? - with_source(@sources.add_rubygems_source("remotes" => source), &blk) + with_source(add_source(:add_rubygems_source, "remotes" => source), &blk) else - @sources.add_global_rubygems_remote(source) + add_source(:add_global_rubygems_remote, source) end end @@ -200,8 +205,7 @@ def path(path, options = {}, &blk) source_options["global"] = true unless block_given? - source = @sources.add_path_source(source_options) - with_source(source, &blk) + with_source(add_source(:add_path_source, source_options), &blk) end def git(uri, options = {}, &blk) @@ -216,20 +220,29 @@ def git(uri, options = {}, &blk) raise DeprecatedError, msg end - with_source(@sources.add_git_source(normalize_hash(options).merge("uri" => uri)), &blk) + options = normalize_hash(options).merge("uri" => uri) + with_source(add_source(:add_git_source, options), &blk) end - def github(repo, options = {}) + def github(repo, options = {}, &blk) raise ArgumentError, "GitHub sources require a block" unless block_given? - github_uri = @git_sources["github"].call(repo) - git_options = normalize_hash(options).merge("uri" => github_uri) - git_source = @sources.add_git_source(git_options) - with_source(git_source) { yield } + github_uri = @git_sources["github"].call(repo) + options = normalize_hash(options).merge("uri" => github_uri) + with_source(add_source(:add_git_source, options), &blk) end def to_definition(lockfile, unlock, lockfile_contents: nil) check_primary_source_safety - Definition.new(lockfile, @dependencies, @sources, unlock, @ruby_version, @optional_groups, @gemfiles, lockfile_contents, @plugins) + Definition.new(lockfile, + @dependencies, + @sources, + unlock, + @ruby_version, + @optional_groups, + @gemfiles, + lockfile_contents, + @plugin_dependencies, + @plugin_sources) end def group(*args, &blk) @@ -272,41 +285,12 @@ def env(name) end def plugin(name, *args) - options = args.last.is_a?(Hash) ? args.pop.dup : {} - options["gemfile"] = @gemfile - version = args || [">= 0"] - - # We don't care to add sources for plugins in this pass over the gemfile - # since we're not installing plugins here (they should already be - # installed), only keeping track of them so that we can verify they - # are currently installed. This is important because otherwise sources - # unique to the plugin (like a git source) would end up in the lockfile, - # which we don't want. - normalize_options(name, version, false, options) - - dep = Dependency.new(name, version, options) - - # if there's already a plugin with this name we try to prefer one - if current = @plugins.find {|d| d.name == dep.name } - if current.requirement != dep.requirement - raise GemfileError, "You cannot specify the same plugin twice with different version requirements.\n" \ - "You specified: #{current.name} (#{current.requirement}) and #{dep.name} (#{dep.requirement})" \ - "#{update_prompt}" - end - - if current.source != dep.source - raise GemfileError, "You cannot specify the same plugin twice coming from different sources.\n" \ - "You specified that #{dep.name} (#{dep.requirement}) should come from " \ - "#{current.source || "an unspecified source"} and #{dep.source}\n" - else - Bundler.ui.warn "Your Gemfile lists the plugin #{current.name} (#{current.requirement}) more than once.\n" \ - "You should keep only one of them.\n" \ - "Remove any duplicate entries and specify the plugin only once." \ - "While it's not a problem now, it could cause errors if you change the version of one of them later." - end - end - - @plugins << dep + @current_sources = @plugin_sources + @current_dependencies = @plugin_dependencies + gem(name, *args) + ensure + @current_sources = @sources + @current_dependencies = @dependencies end def method_missing(name, *args) @@ -320,6 +304,11 @@ def check_primary_source_safety private + def add_source(method, *args) + @plugin_sources.send(method, *args) unless @plugin_sources.equal?(@current_sources) + @current_sources.send(method, *args) + end + def add_git_sources git_source(:github) do |repo_name| if repo_name =~ GITHUB_PULL_REQUEST_URL @@ -382,7 +371,7 @@ def valid_keys @valid_keys ||= VALID_KEYS end - def normalize_options(name, version, add_to_sources, opts) + def normalize_options(name, version, opts) if name.is_a?(Symbol) raise GemfileError, %(You need to specify gem names as Strings. Use 'gem "#{name}"' instead) end @@ -417,9 +406,9 @@ def normalize_options(name, version, add_to_sources, opts) end # Save sources passed in a key - if opts.key?("source") && add_to_sources + if opts.key?("source") source = normalize_source(opts["source"]) - opts["source"] = @sources.add_rubygems_source("remotes" => source) + opts["source"] = @current_sources.add_rubygems_source("remotes" => source) end git_name = (git_names & opts.keys).last @@ -438,10 +427,8 @@ def normalize_options(name, version, add_to_sources, opts) else options = opts.dup end - if add_to_sources - source = send(type, param, options) {} - opts["source"] = source - end + source = send(type, param, options) {} + opts["source"] = source end opts["source"] ||= @source diff --git a/bundler/lib/bundler/plugin.rb b/bundler/lib/bundler/plugin.rb index d7c52674de64..44d37cdfc07b 100644 --- a/bundler/lib/bundler/plugin.rb +++ b/bundler/lib/bundler/plugin.rb @@ -120,16 +120,16 @@ def gemfile_install(gemfile = nil, unlock: false, &inline) definition = builder.to_definition(nil, unlock || {}, lockfile_contents: lockfile_contents) unless definition.no_resolve_needed? Installer.new.install_definition(definition) + + plugins = definition.requested_dependencies.select(&:should_include?).map(&:name) + installed_specs = plugins.to_h {|p| [p, definition.specs[p].first] } + + save_plugins plugins, installed_specs, builder.inferred_plugins end # we may have cached the non-deployment gem path, so paths need to be # reset in preparation for the regular gem installation phase that # needs to use the deployment gem path Bundler.reset_paths! if was_deployment - - plugins = definition.requested_dependencies.select(&:should_include?).map(&:name) - installed_specs = plugins.to_h {|p| [p, definition.specs[p].first] } - - save_plugins plugins, installed_specs, builder.inferred_plugins end rescue RuntimeError => e unless e.is_a?(GemfileError) @@ -138,6 +138,12 @@ def gemfile_install(gemfile = nil, unlock: false, &inline) raise end + # returns an array of specs of installed plugins from a runtime Definition + # @param [Definition] definition + def installed_specs(definition) + index.specs(definition.send(:plugin_sources), definition.plugin_dependencies, :specification).grep(Gem::Specification) + end + # The index object used to store the details about the plugin def index @index ||= Index.new @@ -194,16 +200,16 @@ def add_source(source, cls) @sources[source] = cls end - # Checks if any plugin declares the source - def source?(name) - !index.source_plugin(name.to_s).nil? + # Returns the plugin that handles sources of this name + def source_plugin(name) + index.source_plugin(name) end # @return [Class] that handles the source. The class includes API::Source def source(name) - raise UnknownSourceError, "Source #{name} not found" unless source? name + raise UnknownSourceError, "Source #{name} not found" unless (source_plugin = source_plugin(name)) - load_plugin(index.source_plugin(name)) unless @sources.key? name + load_plugin(source_plugin) unless @sources.key? name @sources[name] end @@ -329,7 +335,7 @@ def register_plugin(name, spec, optional_plugin = false) raise MalformattedPlugin, "#{e.class}: #{e.message}" end - if optional_plugin && @sources.keys.any? {|s| source? s } + if optional_plugin && @sources.keys.any? {|s| index.source_plugin(s) } Bundler.rm_rf(path) false else diff --git a/bundler/lib/bundler/plugin/dsl.rb b/bundler/lib/bundler/plugin/dsl.rb index da751d1774ec..37368fb13c31 100644 --- a/bundler/lib/bundler/plugin/dsl.rb +++ b/bundler/lib/bundler/plugin/dsl.rb @@ -5,7 +5,7 @@ module Plugin # Dsl to parse the Gemfile looking for plugins to install class DSL < Bundler::Dsl class PluginGemfileError < PluginError; end - alias_method :_gem, :gem # To use for plugin installation as gem + alias_method :plugin, :gem # To use for plugin installation as gem # So that we don't have to override all there methods to dummy ones # explicitly. @@ -24,14 +24,9 @@ class PluginGemfileError < PluginError; end def initialize super - @sources = Plugin::SourceList.new @inferred_plugins = [] # The source plugins inferred from :type end - def plugin(name, *args) - _gem(name, *args) - end - def method_missing(name, *args) raise PluginGemfileError, "Undefined local variable or method `#{name}' for Gemfile" unless Bundler::Dsl.method_defined? name end diff --git a/bundler/lib/bundler/plugin/index.rb b/bundler/lib/bundler/plugin/index.rb index edd6f069a339..59ebb1c6d6d0 100644 --- a/bundler/lib/bundler/plugin/index.rb +++ b/bundler/lib/bundler/plugin/index.rb @@ -123,10 +123,6 @@ def plugin_commands(plugin) @commands.find_all {|_, n| n == plugin }.map(&:first) end - def source?(source) - @sources.key? source - end - def source_plugin(name) @sources[name] end @@ -146,16 +142,24 @@ def installed_in_plugin_root?(name) # generate an in-memory lockfile from the index def generate_lockfile(sources, dependencies) - specs = [] + specs = specs(sources, dependencies, :lazy) + + require_relative "../lockfile_generator" + LockfileGenerator.generate(IndexDefinition.new(sources, specs, dependencies)) + end + + def specs(sources, dependencies, type) sources.cached! default_source = sources.global_rubygems_source - installed_plugins.each do |plugin| + installed_plugins.filter_map do |plugin| path = plugin_path(plugin) # path gems may have a gemspec, which is the most trustworthy # way to determine the version version = if (gemspec = path.join("#{plugin}.gemspec")).file? - Gem::Specification.load(gemspec.to_s).version + spec = Gem::Specification.load(gemspec.to_s) + spec.full_gem_path = path.to_s + spec.version elsif (version_index = path.to_s.index("#{plugin}-")) path.to_s[(version_index + plugin.length + 1)..] end @@ -165,14 +169,16 @@ def generate_lockfile(sources, dependencies) dep = dependencies.find {|d| d.name == plugin } next unless dep - spec = LazySpecification.new(plugin, version, nil, dep.source || default_source) - next unless spec.satisfies?(dep) + lazy_spec = LazySpecification.new(plugin, version, nil, dep.source || default_source) + next unless lazy_spec.satisfies?(dep) - specs << spec + if type == :lazy + lazy_spec + else + spec.source = dep.source || default_source + spec + end end - - require_relative "../lockfile_generator" - LockfileGenerator.generate(IndexDefinition.new(sources, specs, dependencies)) end private diff --git a/bundler/lib/bundler/plugin/installer.rb b/bundler/lib/bundler/plugin/installer.rb index 4f60862bb47f..ef976b3784d2 100644 --- a/bundler/lib/bundler/plugin/installer.rb +++ b/bundler/lib/bundler/plugin/installer.rb @@ -120,6 +120,10 @@ def install_from_specs(specs) paths = {} specs.each do |spec| + # bundler is always an implicit gem, but we don't need to try and install it + next if spec.name == "bundler" + + # spec.source.cache(spec) if spec.source.respond_to?(:cache) spec.source.install spec paths[spec.name] = spec diff --git a/bundler/lib/bundler/runtime.rb b/bundler/lib/bundler/runtime.rb index 54aa30ce0bd0..b415194464bc 100644 --- a/bundler/lib/bundler/runtime.rb +++ b/bundler/lib/bundler/runtime.rb @@ -151,7 +151,7 @@ def prune_cache(cache_path) SharedHelpers.filesystem_access(cache_path) do |p| FileUtils.mkdir_p(p) end unless File.exist?(cache_path) - resolve = @definition.resolve + resolve = @definition.resolve.to_a + Plugin.installed_specs(@definition) prune_gem_cache(resolve, cache_path) prune_git_and_path_cache(resolve, cache_path) end @@ -172,7 +172,8 @@ def clean(dry_run = false) spec_cache_paths = [] spec_gemspec_paths = [] spec_extension_paths = [] - Bundler.rubygems.add_default_gems_to(specs).values.each do |spec| + all_specs = Bundler.rubygems.add_default_gems_to(specs).values + Plugin.installed_specs(@definition) + all_specs.each do |spec| spec_gem_paths << spec.full_gem_path # need to check here in case gems are nested like for the rails git repo md = %r{(.+bundler/gems/.+-[a-f0-9]{7,12})}.match(spec.full_gem_path) diff --git a/bundler/spec/bundler/plugin_spec.rb b/bundler/spec/bundler/plugin_spec.rb index ecded6813961..eb1045be7857 100644 --- a/bundler/spec/bundler/plugin_spec.rb +++ b/bundler/spec/bundler/plugin_spec.rb @@ -192,18 +192,16 @@ end end - describe "#source?" do - it "returns true value for sources in index" do + describe "#source_plugin" do + it "returns the plugin for sources in index" do allow(index). - to receive(:command_plugin).with("foo-source") { "my-plugin" } - result = subject.command? "foo-source" - expect(result).to be_truthy + to receive(:source_plugin).with("foo-source") { "my-plugin" } + expect(subject.source_plugin("foo-source")).to eql "my-plugin" end - it "returns false value for source not in index" do - allow(index).to receive(:command_plugin).with("foo-source") { nil } - result = subject.command? "foo-source" - expect(result).to be_falsy + it "returns nil for source not in index" do + allow(index).to receive(:source_plugin).with("foo-source") { nil } + expect(subject.source_plugin("foo-source")).to be_nil end end diff --git a/bundler/spec/bundler/source_list_spec.rb b/bundler/spec/bundler/source_list_spec.rb index 13453cb2a33a..41be1121f6fe 100644 --- a/bundler/spec/bundler/source_list_spec.rb +++ b/bundler/spec/bundler/source_list_spec.rb @@ -6,7 +6,7 @@ stub_const "ASourcePlugin", Class.new(Bundler::Plugin::API) ASourcePlugin.source "new_source" - allow(Bundler::Plugin).to receive(:source?).with("new_source").and_return(true) + allow(Bundler::Plugin).to receive(:source_plugin).with("new_source").and_return("new_source") end subject(:source_list) { Bundler::SourceList.new } diff --git a/bundler/spec/plugins/install_spec.rb b/bundler/spec/plugins/install_spec.rb index 30b605be4c30..ca0c49251cca 100644 --- a/bundler/spec/plugins/install_spec.rb +++ b/bundler/spec/plugins/install_spec.rb @@ -319,7 +319,8 @@ def exec(command, args) gem 'rack', "1.0.0" G - bundle "install --without development" + bundle "config set --local without development" + bundle "install" expect(out).not_to include("Installed plugin foo") @@ -431,7 +432,7 @@ def exec(command, args) expect(out).to include("Installing kung-foo") expect(out).to include("Installed plugin kung-foo") - expect(out).to_not include("Installing foo") + expect(out).to_not include("Fetching foo") expect(out).to_not include("Installed plugin foo") expect(out).to include("Bundle complete!")