From 69ec0edc280fa01ecd5598e2573b04aac32553f3 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 13 Jun 2024 13:15:58 -0600 Subject: [PATCH 1/4] Fix plugin installation from gemfile Several things are fixed: * Don't re-install a plugin referenced from the gemfile with every call to `bundle install` * If the version of a plugin referenced in the gemfile conflicts with what's in the plugin index, _do_ re-install it * If plugins aren't installed yet, don't throw cryptic errors from commands that don't implicitly install gems, such as `bundle check` and `bundle info`. This also applies if the plugin index references a system gem, and that gem is removed. This is all accomplished by actuallying including plugins as regular dependencies in the Gemfile, so that they end up in the lockfile, and then just using the regular lockfile with the plugins-only pass of the gemfile in the Plugin infrastructure. This also means that non-specific version constraints can be used for plugins, and you can update them with `bundle update ` just like any other gem. Co-authored-by: Diogo Fernandes --- Manifest.txt | 1 + bundler/lib/bundler/cli/install.rb | 2 +- bundler/lib/bundler/cli/update.rb | 26 +- bundler/lib/bundler/definition.rb | 29 ++ bundler/lib/bundler/dependency.rb | 10 +- bundler/lib/bundler/dsl.rb | 16 +- bundler/lib/bundler/feature_flag.rb | 1 + bundler/lib/bundler/man/bundle-config.1 | 2 + bundler/lib/bundler/man/bundle-config.1.ronn | 2 + bundler/lib/bundler/plugin.rb | 75 +++-- bundler/lib/bundler/plugin/dsl.rb | 8 +- bundler/lib/bundler/plugin/dummy_source.rb | 9 + bundler/lib/bundler/plugin/installer.rb | 18 +- bundler/lib/bundler/plugin/installer/path.rb | 2 +- bundler/lib/bundler/settings.rb | 1 + bundler/spec/bundler/plugin_spec.rb | 19 +- bundler/spec/bundler/source_list_spec.rb | 2 +- bundler/spec/other/major_deprecation_spec.rb | 4 +- bundler/spec/plugins/install_spec.rb | 287 ++++++++++++++++++- bundler/spec/plugins/list_spec.rb | 2 +- bundler/spec/plugins/source/example_spec.rb | 6 + bundler/spec/support/matchers.rb | 11 + 22 files changed, 467 insertions(+), 66 deletions(-) create mode 100644 bundler/lib/bundler/plugin/dummy_source.rb diff --git a/Manifest.txt b/Manifest.txt index a4391b9fedc8..bd7885261223 100644 --- a/Manifest.txt +++ b/Manifest.txt @@ -164,6 +164,7 @@ bundler/lib/bundler/plugin.rb bundler/lib/bundler/plugin/api.rb bundler/lib/bundler/plugin/api/source.rb bundler/lib/bundler/plugin/dsl.rb +bundler/lib/bundler/plugin/dummy_source.rb bundler/lib/bundler/plugin/events.rb bundler/lib/bundler/plugin/index.rb bundler/lib/bundler/plugin/installer.rb diff --git a/bundler/lib/bundler/cli/install.rb b/bundler/lib/bundler/cli/install.rb index b0b354cf1076..d012f7a4879f 100644 --- a/bundler/lib/bundler/cli/install.rb +++ b/bundler/lib/bundler/cli/install.rb @@ -64,7 +64,7 @@ def run removed_message: "The --binstubs option have been removed in favor of `bundle binstubs --all`" end - Plugin.gemfile_install(Bundler.default_gemfile) if Bundler.feature_flag.plugins? + Plugin.gemfile_install(Bundler.default_gemfile, Bundler.default_lockfile) if Bundler.feature_flag.plugins? definition = Bundler.definition definition.validate_runtime! diff --git a/bundler/lib/bundler/cli/update.rb b/bundler/lib/bundler/cli/update.rb index 985e8db05191..ac79f28b91ae 100644 --- a/bundler/lib/bundler/cli/update.rb +++ b/bundler/lib/bundler/cli/update.rb @@ -15,8 +15,6 @@ def run Bundler.self_manager.update_bundler_and_restart_with_it_if_needed(update_bundler) if update_bundler - Plugin.gemfile_install(Bundler.default_gemfile) if Bundler.feature_flag.plugins? - sources = Array(options[:source]) groups = Array(options[:group]).map(&:to_sym) @@ -33,29 +31,39 @@ def run conservative = options[:conservative] - if full_update + unlock = if full_update if conservative - Bundler.definition(conservative: conservative) + { conservative: conservative } else - Bundler.definition(true) + true end else unless Bundler.default_lockfile.exist? raise GemfileLockNotFound, "This Bundle hasn't been installed yet. " \ "Run `bundle install` to update and install the bundled gems." end - Bundler::CLI::Common.ensure_all_gems_in_lockfile!(gems) + explicit_gems = gems if groups.any? deps = Bundler.definition.dependencies.select {|d| (d.groups & groups).any? } gems.concat(deps.map(&:name)) end - Bundler.definition(gems: gems, sources: sources, ruby: options[:ruby], - conservative: conservative, - bundler: update_bundler) + { + gems: gems, + sources: sources, + ruby: options[:ruby], + conservative: conservative, + bundler: update_bundler, + } end + Plugin.gemfile_install(Bundler.default_gemfile, Bundler.default_lockfile, unlock.dup) if Bundler.feature_flag.plugins? + + Bundler::CLI::Common.ensure_all_gems_in_lockfile!(explicit_gems) if explicit_gems + + Bundler.definition(unlock) + Bundler::CLI::Common.configure_gem_version_promoter(Bundler.definition, options) Bundler::Fetcher.disable_endpoint = options["full-index"] diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index f0d7cf8d2ad2..074b9c2d03ac 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -116,6 +116,8 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti @locked_specs = @originally_locked_specs @locked_sources = @locked_gems.sources end + + remove_plugin_dependencies_if_necessary else @locked_gems = nil @locked_platforms = [] @@ -273,6 +275,10 @@ def requested_dependencies dependencies_for(requested_groups) end + def plugin_dependencies + requested_dependencies.select {|dep| dep.type == :plugin } + end + def current_dependencies filter_relevant(dependencies) end @@ -429,6 +435,7 @@ def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false) def validate_runtime! validate_ruby! validate_platforms! + validate_plugins! end def validate_ruby! @@ -470,6 +477,19 @@ def normalize_platforms @resolve = SpecSet.new(resolve.for(current_dependencies, @platforms)) end + def validate_plugins! + missing_plugins_list = [] + plugin_dependencies.each do |plugin| + missing_plugins_list << plugin unless Plugin.installed?(plugin.name) + end + missing_plugins_list.map! {|p| "#{p.name} (#{p.requirement})" } + if missing_plugins_list.size > 1 + raise GemNotFound, "Plugins #{missing_plugins_list.join(", ")} are not installed" + elsif missing_plugins_list.any? + raise GemNotFound, "Plugin #{missing_plugins_list.join(", ")} is not installed" + end + end + def add_platform(platform) return if @platforms.include?(platform) @@ -1166,5 +1186,14 @@ def spec_set_incomplete_for_platform?(spec_set, platform) def source_map @source_map ||= SourceMap.new(sources, dependencies, @locked_specs) end + + def remove_plugin_dependencies_if_necessary + return if Bundler.feature_flag.plugins_in_lockfile? + # we already have plugin dependencies in the lockfile; continue to do so regardless + # of the current setting + return if @dependencies.any? {|d| d.type == :plugin && @locked_deps.key?(d.name) } + + @dependencies.reject! {|d| d.type == :plugin } + end end end diff --git a/bundler/lib/bundler/dependency.rb b/bundler/lib/bundler/dependency.rb index e81696ff42fc..2b0f63671382 100644 --- a/bundler/lib/bundler/dependency.rb +++ b/bundler/lib/bundler/dependency.rb @@ -7,7 +7,15 @@ module Bundler class Dependency < Gem::Dependency def initialize(name, version, options = {}, &blk) type = options["type"] || :runtime - super(name, version, type) + if type == :plugin + # RubyGems doesn't support plugin type, which only + # makes sense in the context of Bundler, so bypass + # the RubyGems validation + super(name, version, :runtime) + @type = type + else + super(name, version, type) + end @options = options end diff --git a/bundler/lib/bundler/dsl.rb b/bundler/lib/bundler/dsl.rb index 32f45d97ec03..aedb0d6b2152 100644 --- a/bundler/lib/bundler/dsl.rb +++ b/bundler/lib/bundler/dsl.rb @@ -108,9 +108,15 @@ 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 + # Implicitly add a dependency on source plugins who are named bundler-source-, + # and aren't already mentioned in the Gemfile. + # See also Plugin::DSL#source + if source_plugin.start_with?("bundler-source-") && !@dependencies.any? {|d| d.name == source_plugin } + plugin(source_plugin) + end unless block_given? raise InvalidOption, "You need to pass a block to #source with :type option" @@ -218,7 +224,13 @@ def env(name) end def plugin(*args) - # Pass on + options = args.last.is_a?(Hash) ? args.pop.dup : {} + + normalize_hash(options) + options["type"] = :plugin + options["require"] = false + + gem(*args, options) end def method_missing(name, *args) diff --git a/bundler/lib/bundler/feature_flag.rb b/bundler/lib/bundler/feature_flag.rb index b19cf42cc37c..d881c8dd2aef 100644 --- a/bundler/lib/bundler/feature_flag.rb +++ b/bundler/lib/bundler/feature_flag.rb @@ -36,6 +36,7 @@ def self.settings_method(name, key, &default) settings_flag(:lockfile_checksums) { bundler_3_mode? } settings_flag(:path_relative_to_cwd) { bundler_3_mode? } settings_flag(:plugins) { @bundler_version >= Gem::Version.new("1.14") } + settings_flag(:plugins_in_lockfile) { bundler_3_mode? } settings_flag(:print_only_version_number) { bundler_3_mode? } settings_flag(:setup_makes_kernel_gem_public) { !bundler_3_mode? } settings_flag(:update_requires_all_flag) { bundler_4_mode? } diff --git a/bundler/lib/bundler/man/bundle-config.1 b/bundler/lib/bundler/man/bundle-config.1 index 190177eb37e2..175be8ab6228 100644 --- a/bundler/lib/bundler/man/bundle-config.1 +++ b/bundler/lib/bundler/man/bundle-config.1 @@ -165,6 +165,8 @@ The following is a list of all configuration keys and their purpose\. You can le .IP "\(bu" 4 \fBplugins\fR (\fBBUNDLE_PLUGINS\fR): Enable Bundler's experimental plugin system\. .IP "\(bu" 4 +\fBplugins_in_lockfile\fR (\fBBUNDLE_PLUGINS_IN_LOCKFILE\fR): Include plugins as regular dependencies in the lockfile\. +.IP "\(bu" 4 \fBprefer_patch\fR (BUNDLE_PREFER_PATCH): Prefer updating only to next patch version during updates\. Makes \fBbundle update\fR calls equivalent to \fBbundler update \-\-patch\fR\. .IP "\(bu" 4 \fBprint_only_version_number\fR (\fBBUNDLE_PRINT_ONLY_VERSION_NUMBER\fR): Print only version number from \fBbundler \-\-version\fR\. diff --git a/bundler/lib/bundler/man/bundle-config.1.ronn b/bundler/lib/bundler/man/bundle-config.1.ronn index 44c31cd10d84..a3fe7650c18a 100644 --- a/bundler/lib/bundler/man/bundle-config.1.ronn +++ b/bundler/lib/bundler/man/bundle-config.1.ronn @@ -241,6 +241,8 @@ learn more about their operation in [bundle install(1)](bundle-install.1.html). Makes `--path` relative to the CWD instead of the `Gemfile`. * `plugins` (`BUNDLE_PLUGINS`): Enable Bundler's experimental plugin system. +* `plugins_in_lockfile` (`BUNDLE_PLUGINS_IN_LOCKFILE`): + Include plugins as regular dependencies in the lockfile. * `prefer_patch` (BUNDLE_PREFER_PATCH): Prefer updating only to next patch version during updates. Makes `bundle update` calls equivalent to `bundler update --patch`. * `print_only_version_number` (`BUNDLE_PRINT_ONLY_VERSION_NUMBER`): diff --git a/bundler/lib/bundler/plugin.rb b/bundler/lib/bundler/plugin.rb index 44129cc0ff50..6d9fd3586418 100644 --- a/bundler/lib/bundler/plugin.rb +++ b/bundler/lib/bundler/plugin.rb @@ -4,11 +4,12 @@ module Bundler module Plugin - autoload :DSL, File.expand_path("plugin/dsl", __dir__) - autoload :Events, File.expand_path("plugin/events", __dir__) - autoload :Index, File.expand_path("plugin/index", __dir__) - autoload :Installer, File.expand_path("plugin/installer", __dir__) - autoload :SourceList, File.expand_path("plugin/source_list", __dir__) + autoload :DSL, File.expand_path("plugin/dsl", __dir__) + autoload :DummySource, File.expand_path("plugin/dummy_source", __dir__) + autoload :Events, File.expand_path("plugin/events", __dir__) + autoload :Index, File.expand_path("plugin/index", __dir__) + autoload :Installer, File.expand_path("plugin/installer", __dir__) + autoload :SourceList, File.expand_path("plugin/source_list", __dir__) class MalformattedPlugin < PluginError; end class UndefinedCommandError < PluginError; end @@ -16,6 +17,7 @@ class UnknownSourceError < PluginError; end class PluginInstallError < PluginError; end PLUGIN_FILE_NAME = "plugins.rb" + @gemfile_parse = false module_function @@ -26,6 +28,7 @@ def reset! @commands = {} @hooks_by_event = Hash.new {|h, k| h[k] = [] } @loaded_plugin_names = [] + @index = nil end reset! @@ -40,7 +43,7 @@ def install(names, options) specs = Installer.new.install(names, options) - save_plugins names, specs + save_plugins specs rescue PluginError specs_to_delete = specs.select {|k, _v| names.include?(k) && !index.commands.values.include?(k) } specs_to_delete.each_value {|spec| Bundler.rm_rf(spec.full_gem_path) } @@ -100,8 +103,22 @@ def list # # @param [Pathname] gemfile path # @param [Proc] block that can be evaluated for (inline) Gemfile - def gemfile_install(gemfile = nil, &inline) - Bundler.settings.temporary(frozen: false, deployment: false) do + def gemfile_install(gemfile = nil, lockfile = nil, unlock = {}, &inline) + # skip the update if unlocking specific gems, but none of them are our plugins + if unlock.is_a?(Hash) && unlock[:gems] && !unlock[:gems].empty? && + (unlock[:gems] & index.installed_plugins).empty? + unlock = {} + end + + @gemfile_parse = true + # plugins_in_lockfile is the user facing setting to force plugins to be + # included in the lockfile as regular dependencies. But during this + # first pass over the Gemfile where we're installing the plugins, we + # need that setting to be set, so that we can find the plugins and + # install them. We don't persist a lockfile during this pass, so it won't + # have any user-facing impact. + Bundler.settings.temporary(plugins_in_lockfile: true) do + Bundler.configure builder = DSL.new if block_given? builder.instance_eval(&inline) @@ -109,20 +126,21 @@ def gemfile_install(gemfile = nil, &inline) builder.eval_gemfile(gemfile) end builder.check_primary_source_safety - definition = builder.to_definition(nil, true) + definition = builder.to_definition(lockfile, unlock) return if definition.dependencies.empty? - plugins = definition.dependencies.map(&:name).reject {|p| index.installed? p } installed_specs = Installer.new.install_definition(definition) - save_plugins plugins, installed_specs, builder.inferred_plugins + save_plugins installed_specs, builder.inferred_plugins end rescue RuntimeError => e unless e.is_a?(GemfileError) Bundler.ui.error "Failed to install plugin: #{e.message}\n #{e.backtrace[0]}" end raise + ensure + @gemfile_parse = false end # The index object used to store the details about the plugin @@ -183,12 +201,17 @@ def add_source(source, cls) # Checks if any plugin declares the source def source?(name) - !index.source_plugin(name.to_s).nil? + !!source_plugin(name) + end + + # Returns the plugin that handles the source +name+ if any + def source_plugin(name) + index.source_plugin(name.to_s) 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(name) load_plugin(index.source_plugin(name)) unless @sources.key? name @@ -199,9 +222,14 @@ def source(name) # @return [API::Source] the instance of the class that handles the source # type passed in locked_opts def from_lock(locked_opts) + opts = locked_opts.merge("uri" => locked_opts["remote"]) + # when reading the lockfile while doing the plugin-install-from-gemfile phase, + # we need to ignore any plugin sources + return DummySource.new(opts) if @gemfile_parse + src = source(locked_opts["type"]) - src.new(locked_opts.merge("uri" => locked_opts["remote"])) + src.new(opts) end # To be called via the API to register a hooks and corresponding block that @@ -237,7 +265,9 @@ def hook(event, *args, &arg_blk) # # @return [String, nil] installed path def installed?(plugin) - Index.new.installed?(plugin) + (path = index.installed?(plugin)) && + index.plugin_path(plugin).join(PLUGIN_FILE_NAME).file? && + path end # @return [true, false] whether the plugin is loaded @@ -251,12 +281,8 @@ def loaded?(plugin) # @param [Hash] specs of plugins mapped to installation path (currently they # contain all the installed specs, including plugins) # @param [Array] names of inferred source plugins that can be ignored - def save_plugins(plugins, specs, optional_plugins = []) - plugins.each do |name| - next if index.installed?(name) - - spec = specs[name] - + def save_plugins(specs, optional_plugins = []) + specs.each do |name, spec| save_plugin(name, spec, optional_plugins.include?(name)) end end @@ -281,7 +307,10 @@ def validate_plugin!(plugin_path) # # @raise [PluginInstallError] if validation or registration raises any error def save_plugin(name, spec, optional_plugin = false) - validate_plugin! Pathname.new(spec.full_gem_path) + path = Pathname.new(spec.full_gem_path) + return if index.installed?(name) && index.plugin_path(name) == path + + validate_plugin!(path) installed = register_plugin(name, spec, optional_plugin) Bundler.ui.info "Installed plugin #{name}" if installed rescue PluginError => e @@ -316,7 +345,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| 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..b4ae76fd77b2 100644 --- a/bundler/lib/bundler/plugin/dsl.rb +++ b/bundler/lib/bundler/plugin/dsl.rb @@ -5,12 +5,11 @@ 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 # So that we don't have to override all there methods to dummy ones # explicitly. # They will be handled by method_missing - [:gemspec, :gem, :install_if, :platforms, :env].each {|m| undef_method m } + [:gemspec, :install_if, :platforms, :env].each {|m| undef_method m } # This lists the plugins that was added automatically and not specified by # the user. @@ -24,12 +23,11 @@ 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) + def gem(*args) + super if args.last.is_a?(Hash) && args.last["type"] == :plugin end def method_missing(name, *args) diff --git a/bundler/lib/bundler/plugin/dummy_source.rb b/bundler/lib/bundler/plugin/dummy_source.rb new file mode 100644 index 000000000000..63e63a683349 --- /dev/null +++ b/bundler/lib/bundler/plugin/dummy_source.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Bundler + module Plugin + class DummySource + include API::Source + end + end +end diff --git a/bundler/lib/bundler/plugin/installer.rb b/bundler/lib/bundler/plugin/installer.rb index ac3c3ea7f312..86d19f83e1d1 100644 --- a/bundler/lib/bundler/plugin/installer.rb +++ b/bundler/lib/bundler/plugin/installer.rb @@ -32,12 +32,18 @@ def install(names, options) # # @param [Definition] definition object # @return [Hash] map of names to their specs they are installed with - def install_definition(definition) + def install_definition(definition, latest = false) def definition.lock(*); end - definition.remotely! + + if latest || definition.missing_specs? + definition.remotely! + else + definition.with_cache! + end + specs = definition.specs - install_from_specs specs + install_from_specs(specs) end private @@ -99,14 +105,14 @@ def install_rubygems(names, version, sources) end def install_all_sources(names, version, source_list, source = nil) - deps = names.map {|name| Dependency.new(name, version, { "source" => source }) } + deps = names.map {|name| Dependency.new(name, version, { "source" => source, "type" => :plugin }) } Bundler.configure_gem_home_and_path(Plugin.root) Bundler.settings.temporary(deployment: false, frozen: false) do definition = Definition.new(nil, deps, source_list, true) - install_definition(definition) + install_definition(definition, true) end end @@ -120,6 +126,8 @@ def install_from_specs(specs) paths = {} specs.each do |spec| + next if spec.name == "bundler" + spec.source.install spec paths[spec.name] = spec diff --git a/bundler/lib/bundler/plugin/installer/path.rb b/bundler/lib/bundler/plugin/installer/path.rb index 58c4924eb0cb..02ec62d41749 100644 --- a/bundler/lib/bundler/plugin/installer/path.rb +++ b/bundler/lib/bundler/plugin/installer/path.rb @@ -5,7 +5,7 @@ module Plugin class Installer class Path < Bundler::Source::Path def root - SharedHelpers.in_bundle? ? Bundler.root : Plugin.root + Plugin.root end def eql?(other) diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index cde01e0181ed..4fbab9bab070 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -38,6 +38,7 @@ class Settings path_relative_to_cwd path.system plugins + plugins_in_lockfile prefer_patch print_only_version_number setup_makes_kernel_gem_public diff --git a/bundler/spec/bundler/plugin_spec.rb b/bundler/spec/bundler/plugin_spec.rb index fea392500067..ae4304ca38f2 100644 --- a/bundler/spec/bundler/plugin_spec.rb +++ b/bundler/spec/bundler/plugin_spec.rb @@ -115,6 +115,7 @@ let(:gemfile) { bundled_app_gemfile } before do + allow(Bundler).to receive(:configure) allow(Plugin::DSL).to receive(:new) { builder } allow(builder).to receive(:eval_gemfile).with(gemfile) allow(builder).to receive(:check_primary_source_safety) @@ -139,7 +140,7 @@ before do allow(index).to receive(:installed?) { nil } - allow(definition).to receive(:dependencies) { [Bundler::Dependency.new("new-plugin", ">=0"), Bundler::Dependency.new("another-plugin", ">=0")] } + allow(definition).to receive(:dependencies) { [Bundler::Dependency.new("new-plugin", ">=0", "type" => :plugin), Bundler::Dependency.new("another-plugin", ">=0", "type" => :plugin)] } allow(installer).to receive(:install_definition) { plugin_specs } end @@ -187,18 +188,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 value 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/other/major_deprecation_spec.rb b/bundler/spec/other/major_deprecation_spec.rb index 036c855c4e3f..c168f505826a 100644 --- a/bundler/spec/other/major_deprecation_spec.rb +++ b/bundler/spec/other/major_deprecation_spec.rb @@ -104,9 +104,9 @@ pending "is removed and shows a helpful error message about it", bundler: "3" end - describe "bundle update --quiet" do + describe "bundle update --all --quiet" do it "does not print any deprecations" do - bundle :update, quiet: true, raise_on_error: false + bundle :update, all: true, quiet: true, raise_on_error: false expect(deprecations).to be_empty end end diff --git a/bundler/spec/plugins/install_spec.rb b/bundler/spec/plugins/install_spec.rb index d0de607e6c80..871a28e90c17 100644 --- a/bundler/spec/plugins/install_spec.rb +++ b/bundler/spec/plugins/install_spec.rb @@ -291,6 +291,157 @@ def exec(command, args) expect(out).to include("Bundle complete!") end + it "installs plugins in included groups" do + gemfile <<-G + source 'https://gem.repo2' + group :development do + plugin 'foo' + end + gem 'myrack', "1.0.0" + G + + bundle "install" + + expect(out).to include("Installed plugin foo") + + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed("foo") + end + + it "does not install plugins in excluded groups" do + gemfile <<-G + source 'https://gem.repo2' + group :development do + plugin 'foo' + end + gem 'myrack', "1.0.0" + G + + bundle "config set --local without development" + bundle "install" + + expect(out).not_to include("Installed plugin foo") + + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_not_be_installed("foo") + end + + it "upgrade plugins version listed in gemfile" do + update_repo2 do + build_plugin "foo", "1.4.0" + build_plugin "foo", "1.5.0" + end + + gemfile <<-G + source 'https://gem.repo2' + plugin 'foo', "1.4.0" + gem 'myrack', "1.0.0" + G + + bundle "install" + + expect(out).to include("Installing foo 1.4.0") + expect(out).to include("Installed plugin foo") + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed_with_version("foo", "1.4.0") + + gemfile <<-G + source 'https://gem.repo2' + plugin 'foo', "1.5.0" + gem 'myrack', "1.0.0" + G + + bundle "install" + + expect(out).to include("Installing foo 1.5.0") + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed_with_version("foo", "1.5.0") + end + + it "downgrade plugins version listed in gemfile" do + update_repo2 do + build_plugin "foo", "1.4.0" + build_plugin "foo", "1.5.0" + end + + gemfile <<-G + source 'https://gem.repo2' + plugin 'foo', "1.5.0" + gem 'myrack', "1.0.0" + G + + bundle "install" + + expect(out).to include("Installing foo 1.5.0") + expect(out).to include("Installed plugin foo") + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed_with_version("foo", "1.5.0") + + gemfile <<-G + source 'https://gem.repo2' + plugin 'foo', "1.4.0" + gem 'myrack', "1.0.0" + G + + bundle "install" + + expect(out).to include("Installing foo 1.4.0") + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed_with_version("foo", "1.4.0") + end + + it "install only plugins not installed yet listed in gemfile" do + gemfile <<-G + source 'https://gem.repo2' + plugin 'foo' + gem 'myrack', "1.0.0" + G + + 2.times { bundle "install" } + + expect(out).to_not include("Fetching gem metadata") + expect(out).to_not include("Fetching foo") + expect(out).to_not include("Installed plugin foo") + + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed("foo") + + gemfile <<-G + source 'https://gem.repo2' + plugin 'foo' + plugin 'kung-foo' + gem 'myrack', "1.0.0" + G + + bundle "install" + + expect(out).to include("Installing kung-foo") + expect(out).to include("Installed plugin kung-foo") + + expect(out).to_not include("Fetching foo") + expect(out).to_not include("Installed plugin foo") + + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed("foo") + plugin_should_be_installed("kung-foo") + end + it "accepts git sources" do build_git "ga-plugin" do |s| s.write "plugins.rb" @@ -338,24 +489,148 @@ def exec(command, args) it "installs plugins" do install_gemfile <<-G source 'https://gem.repo2' + plugin 'foo' gem 'myrack', "1.0.0" G + expect(out).to include("Installed plugin foo") + bundle "config set --local deployment true" + bundle "install" + + expect(out).to include("Bundle complete!") + + expect(the_bundle).to include_gems("myrack 1.0.0") + plugin_should_be_installed("foo") + end + end + + context "with plugins_in_lockfile" do + it "includes plugins as dependencies for a new lockfile" do install_gemfile <<-G source 'https://gem.repo2' plugin 'foo' gem 'myrack', "1.0.0" G - expect(out).to include("Installed plugin foo") + expect(the_bundle).to include_gems("foo 1.0.0") + end - expect(out).to include("Bundle complete!") + it "does not include plugins as dependencies for an existing lockfile", bundler: "< 3" do + install_gemfile <<-G + source 'https://gem.repo2' + gem 'myrack', "1.0.0" + G - expect(the_bundle).to include_gems("myrack 1.0.0") - plugin_should_be_installed("foo") + expect(the_bundle).not_to include_gems("foo 1.0.0") + + install_gemfile <<-G + source 'https://gem.repo2' + plugin 'foo' + gem 'myrack', "1.0.0" + G + + expect(the_bundle).not_to include_gems("foo 1.0.0") + + # it adds the plugins to the lockfile when specifically instructed + bundle "config set --local plugins_in_lockfile true" + bundle "install" + + expect(the_bundle).to include_gems("foo 1.0.0") + + # but will not remove them, once they're there, regardless of the setting + bundle "config set --local plugins_in_lockfile false" + bundle "install" + + expect(the_bundle).to include_gems("foo 1.0.0") + end + + it "includes plugins as dependencies for an existing lockfile", bundler: "3" do + install_gemfile <<-G + source 'https://gem.repo2' + gem 'myrack', "1.0.0" + G + + expect(the_bundle).not_to include_gems("foo 1.0.0") + + install_gemfile <<-G + source 'https://gem.repo2' + plugin 'foo' + gem 'myrack', "1.0.0" + G + + expect(the_bundle).to include_gems("foo 1.0.0") + end + end + end + + it "fails bundle commands if plugins are not yet installed" do + allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile) + + gemfile <<-G + source 'https://gem.repo2' + group :development do + plugin 'foo' end + source 'https://gem.repo1' do + gem 'rake' + end + G + + plugin_should_not_be_installed("foo") + + bundle "check", raise_on_error: false + expect(err).to include("Plugin foo (>= 0) is not installed") + + bundle "exec rake", raise_on_error: false + expect(err).to include("Plugin foo (>= 0) is not installed") + + bundle "config set --local without development" + bundle "install" + bundle "config unset --local without" + + plugin_should_not_be_installed("foo") + + bundle "check", raise_on_error: false + expect(err).to include("Plugin foo (>= 0) is not installed") + + bundle "exec rake", raise_on_error: false + expect(err).to include("Plugin foo (>= 0) is not installed") + + plugin_should_not_be_installed("foo") + + bundle "install" + plugin_should_be_installed("foo") + + bundle "check" + bundle "exec rake -T", raise_on_error: false + expect(err).not_to include("Plugin foo (>= 0) is not installed") + end + + it "fails bundle commands gracefully when a plugin index reference is left dangling" do + allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile) + + build_lib "ga-plugin" do |s| + s.write "plugins.rb" end + + install_gemfile <<-G + source "https://gem.repo1" + plugin 'ga-plugin', :path => "#{lib_path("ga-plugin-1.0")}" + G + + expect(out).to include("Installed plugin ga-plugin") + plugin_should_be_installed("ga-plugin") + + FileUtils.rm_rf(lib_path("ga-plugin-1.0")) + + plugin_should_not_be_installed("ga-plugin") + + bundle "check", raise_on_error: false + expect(err).to include("Plugin ga-plugin (>= 0) is not installed") + + bundle "exec rake -T", raise_on_error: false + expect(err).to include("Plugin ga-plugin (>= 0) is not installed") end context "inline gemfiles" do @@ -370,7 +645,9 @@ def exec(command, args) RUBY ruby code, artifice: "compact_index", env: { "BUNDLER_VERSION" => Bundler::VERSION } - expect(local_plugin_gem("foo-1.0", "plugins.rb")).to exist + + allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile) + plugin_should_be_installed("foo") end end diff --git a/bundler/spec/plugins/list_spec.rb b/bundler/spec/plugins/list_spec.rb index 30e3f82467e6..74ede33f5495 100644 --- a/bundler/spec/plugins/list_spec.rb +++ b/bundler/spec/plugins/list_spec.rb @@ -53,7 +53,7 @@ def exec(command, args) plugin_should_be_installed("foo", "bar") bundle "plugin list" - expected_output = "foo\n-----\n shout\n\nbar\n-----\n scream" + expected_output = "bar\n-----\n scream\n\nfoo\n-----\n shout" expect(out).to include(expected_output) end end diff --git a/bundler/spec/plugins/source/example_spec.rb b/bundler/spec/plugins/source/example_spec.rb index f9624463144a..85276330c861 100644 --- a/bundler/spec/plugins/source/example_spec.rb +++ b/bundler/spec/plugins/source/example_spec.rb @@ -72,6 +72,7 @@ def install(spec, opts) checksums = checksums_section_when_enabled do |c| c.no_checksum "a-path-gem", "1.0" + c.checksum gem_repo2, "bundler-source-mpath", "1.0" end expect(lockfile).to eq <<~G @@ -84,12 +85,14 @@ def install(spec, opts) GEM remote: https://gem.repo2/ specs: + bundler-source-mpath (1.0) PLATFORMS #{lockfile_platforms} DEPENDENCIES a-path-gem! + bundler-source-mpath #{checksums} BUNDLED WITH #{Bundler::VERSION} @@ -341,6 +344,7 @@ def installed? bundle "install" checksums = checksums_section_when_enabled do |c| + c.checksum gem_repo2, "bundler-source-gitp", "1.0" c.no_checksum "ma-gitp-gem", "1.0" end @@ -355,11 +359,13 @@ def installed? GEM remote: https://gem.repo2/ specs: + bundler-source-gitp (1.0) PLATFORMS #{lockfile_platforms} DEPENDENCIES + bundler-source-gitp ma-gitp-gem! #{checksums} BUNDLED WITH diff --git a/bundler/spec/support/matchers.rb b/bundler/spec/support/matchers.rb index 9f311fc0d77c..4675c2990cc1 100644 --- a/bundler/spec/support/matchers.rb +++ b/bundler/spec/support/matchers.rb @@ -211,6 +211,7 @@ def indent(string, padding = 4, indent_character = " ") RSpec::Matchers.alias_matcher :include_gem, :include_gems def plugin_should_be_installed(*names) + Bundler::Plugin.instance_variable_set(:@index, nil) names.each do |name| expect(Bundler::Plugin).to be_installed(name) path = Pathname.new(Bundler::Plugin.installed?(name)) @@ -218,7 +219,17 @@ def plugin_should_be_installed(*names) end end + def plugin_should_be_installed_with_version(name, version) + Bundler::Plugin.instance_variable_set(:@index, nil) + expect(Bundler::Plugin).to be_installed(name) + path = Pathname.new(Bundler::Plugin.installed?(name)) + + expect(File.basename(path)).to eq("#{name}-#{version}") + expect(path + "plugins.rb").to exist + end + def plugin_should_not_be_installed(*names) + Bundler::Plugin.instance_variable_set(:@index, nil) names.each do |name| expect(Bundler::Plugin).not_to be_installed(name) end From 8c0fccf763872caa275c23d985b80d28f9a434a4 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Wed, 9 Apr 2025 09:46:13 -0600 Subject: [PATCH 2/4] Fix plugin dep handling Since #8480, we can't use the undocumented "type" option, but the add_dependency helper was added that lets us easily validate first, then add our custom options, then directly add the now custom dependency. This conveniently simplifies the Plugin version of the DSL, because `plugin` no longer flows through `gem`, so it doesn't need to special case it. --- bundler/lib/bundler/dsl.rb | 7 ++++--- bundler/lib/bundler/plugin/dsl.rb | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/bundler/lib/bundler/dsl.rb b/bundler/lib/bundler/dsl.rb index aedb0d6b2152..c8b1e60ade73 100644 --- a/bundler/lib/bundler/dsl.rb +++ b/bundler/lib/bundler/dsl.rb @@ -223,14 +223,15 @@ def env(name) @env = old end - def plugin(*args) + def plugin(name, *args) options = args.last.is_a?(Hash) ? args.pop.dup : {} + version = args || [">= 0"] - normalize_hash(options) + normalize_options(name, version, options) options["type"] = :plugin options["require"] = false - gem(*args, options) + add_dependency(name, version, options) end def method_missing(name, *args) diff --git a/bundler/lib/bundler/plugin/dsl.rb b/bundler/lib/bundler/plugin/dsl.rb index b4ae76fd77b2..b2140934ff3e 100644 --- a/bundler/lib/bundler/plugin/dsl.rb +++ b/bundler/lib/bundler/plugin/dsl.rb @@ -27,7 +27,7 @@ def initialize end def gem(*args) - super if args.last.is_a?(Hash) && args.last["type"] == :plugin + # Ignore regular dependencies when doing the plugins-only pre-parse end def method_missing(name, *args) From 2477e15fee3bd0e507f7d775ffe105df340febdc Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Wed, 9 Apr 2025 10:28:10 -0600 Subject: [PATCH 3/4] Change how plugin deps are identified Since #8486, hax to make any dependency type work inside bundler have been removed, so we mark plugins as type: :plugin. Instead, follow that PR's lead and just make it a dedicated option to Bundler::Dependency. --- bundler/lib/bundler/definition.rb | 6 +++--- bundler/lib/bundler/dependency.rb | 14 +++++--------- bundler/lib/bundler/dsl.rb | 2 +- bundler/lib/bundler/plugin/installer.rb | 2 +- bundler/spec/bundler/plugin_spec.rb | 2 +- 5 files changed, 11 insertions(+), 15 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 074b9c2d03ac..4decb39ccf64 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -276,7 +276,7 @@ def requested_dependencies end def plugin_dependencies - requested_dependencies.select {|dep| dep.type == :plugin } + requested_dependencies.select(&:plugin?) end def current_dependencies @@ -1191,9 +1191,9 @@ def remove_plugin_dependencies_if_necessary return if Bundler.feature_flag.plugins_in_lockfile? # we already have plugin dependencies in the lockfile; continue to do so regardless # of the current setting - return if @dependencies.any? {|d| d.type == :plugin && @locked_deps.key?(d.name) } + return if @dependencies.any? {|d| d.plugin? && @locked_deps.key?(d.name) } - @dependencies.reject! {|d| d.type == :plugin } + @dependencies.reject!(&:plugin?) end end end diff --git a/bundler/lib/bundler/dependency.rb b/bundler/lib/bundler/dependency.rb index 2b0f63671382..57c431c5bb92 100644 --- a/bundler/lib/bundler/dependency.rb +++ b/bundler/lib/bundler/dependency.rb @@ -7,15 +7,7 @@ module Bundler class Dependency < Gem::Dependency def initialize(name, version, options = {}, &blk) type = options["type"] || :runtime - if type == :plugin - # RubyGems doesn't support plugin type, which only - # makes sense in the context of Bundler, so bypass - # the RubyGems validation - super(name, version, :runtime) - @type = type - else - super(name, version, type) - end + super(name, version, type) @options = options end @@ -126,6 +118,10 @@ def gemfile_dep? !gemspec_dev_dep? end + def plugin? + @plugin ||= @options.fetch("plugin", false) + end + def current_env? return true unless env if env.is_a?(Hash) diff --git a/bundler/lib/bundler/dsl.rb b/bundler/lib/bundler/dsl.rb index c8b1e60ade73..45b554a4bd25 100644 --- a/bundler/lib/bundler/dsl.rb +++ b/bundler/lib/bundler/dsl.rb @@ -228,7 +228,7 @@ def plugin(name, *args) version = args || [">= 0"] normalize_options(name, version, options) - options["type"] = :plugin + options["plugin"] = true options["require"] = false add_dependency(name, version, options) diff --git a/bundler/lib/bundler/plugin/installer.rb b/bundler/lib/bundler/plugin/installer.rb index 86d19f83e1d1..48fc3b82e3ae 100644 --- a/bundler/lib/bundler/plugin/installer.rb +++ b/bundler/lib/bundler/plugin/installer.rb @@ -105,7 +105,7 @@ def install_rubygems(names, version, sources) end def install_all_sources(names, version, source_list, source = nil) - deps = names.map {|name| Dependency.new(name, version, { "source" => source, "type" => :plugin }) } + deps = names.map {|name| Dependency.new(name, version, { "source" => source, "plugin" => true }) } Bundler.configure_gem_home_and_path(Plugin.root) diff --git a/bundler/spec/bundler/plugin_spec.rb b/bundler/spec/bundler/plugin_spec.rb index ae4304ca38f2..29cf78cf8917 100644 --- a/bundler/spec/bundler/plugin_spec.rb +++ b/bundler/spec/bundler/plugin_spec.rb @@ -140,7 +140,7 @@ before do allow(index).to receive(:installed?) { nil } - allow(definition).to receive(:dependencies) { [Bundler::Dependency.new("new-plugin", ">=0", "type" => :plugin), Bundler::Dependency.new("another-plugin", ">=0", "type" => :plugin)] } + allow(definition).to receive(:dependencies) { [Bundler::Dependency.new("new-plugin", ">=0", "plugin" => true), Bundler::Dependency.new("another-plugin", ">=0", "plugin" => true)] } allow(installer).to receive(:install_definition) { plugin_specs } end From 2542e7146c6dcf37be25339219f4498d6b260e16 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Wed, 9 Apr 2025 10:32:14 -0600 Subject: [PATCH 4/4] remove validate_plugins! it's not core to this PR, and can be debated separately --- bundler/lib/bundler/definition.rb | 14 ------ bundler/spec/plugins/install_spec.rb | 69 ---------------------------- 2 files changed, 83 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 4decb39ccf64..b853cfb5503e 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -435,7 +435,6 @@ def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false) def validate_runtime! validate_ruby! validate_platforms! - validate_plugins! end def validate_ruby! @@ -477,19 +476,6 @@ def normalize_platforms @resolve = SpecSet.new(resolve.for(current_dependencies, @platforms)) end - def validate_plugins! - missing_plugins_list = [] - plugin_dependencies.each do |plugin| - missing_plugins_list << plugin unless Plugin.installed?(plugin.name) - end - missing_plugins_list.map! {|p| "#{p.name} (#{p.requirement})" } - if missing_plugins_list.size > 1 - raise GemNotFound, "Plugins #{missing_plugins_list.join(", ")} are not installed" - elsif missing_plugins_list.any? - raise GemNotFound, "Plugin #{missing_plugins_list.join(", ")} is not installed" - end - end - def add_platform(platform) return if @platforms.include?(platform) diff --git a/bundler/spec/plugins/install_spec.rb b/bundler/spec/plugins/install_spec.rb index 871a28e90c17..338d8746ac67 100644 --- a/bundler/spec/plugins/install_spec.rb +++ b/bundler/spec/plugins/install_spec.rb @@ -564,75 +564,6 @@ def exec(command, args) end end - it "fails bundle commands if plugins are not yet installed" do - allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile) - - gemfile <<-G - source 'https://gem.repo2' - group :development do - plugin 'foo' - end - source 'https://gem.repo1' do - gem 'rake' - end - G - - plugin_should_not_be_installed("foo") - - bundle "check", raise_on_error: false - expect(err).to include("Plugin foo (>= 0) is not installed") - - bundle "exec rake", raise_on_error: false - expect(err).to include("Plugin foo (>= 0) is not installed") - - bundle "config set --local without development" - bundle "install" - bundle "config unset --local without" - - plugin_should_not_be_installed("foo") - - bundle "check", raise_on_error: false - expect(err).to include("Plugin foo (>= 0) is not installed") - - bundle "exec rake", raise_on_error: false - expect(err).to include("Plugin foo (>= 0) is not installed") - - plugin_should_not_be_installed("foo") - - bundle "install" - plugin_should_be_installed("foo") - - bundle "check" - bundle "exec rake -T", raise_on_error: false - expect(err).not_to include("Plugin foo (>= 0) is not installed") - end - - it "fails bundle commands gracefully when a plugin index reference is left dangling" do - allow(Bundler::SharedHelpers).to receive(:find_gemfile).and_return(bundled_app_gemfile) - - build_lib "ga-plugin" do |s| - s.write "plugins.rb" - end - - install_gemfile <<-G - source "https://gem.repo1" - plugin 'ga-plugin', :path => "#{lib_path("ga-plugin-1.0")}" - G - - expect(out).to include("Installed plugin ga-plugin") - plugin_should_be_installed("ga-plugin") - - FileUtils.rm_rf(lib_path("ga-plugin-1.0")) - - plugin_should_not_be_installed("ga-plugin") - - bundle "check", raise_on_error: false - expect(err).to include("Plugin ga-plugin (>= 0) is not installed") - - bundle "exec rake -T", raise_on_error: false - expect(err).to include("Plugin ga-plugin (>= 0) is not installed") - end - context "inline gemfiles" do it "installs the listed plugins" do code = <<-RUBY