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 <plugin>` just like any other gem.

Co-authored-by: Diogo Fernandes <diogofernandesop@gmail.com>

Author: ccutrer

Manifest.txt

@@ -161,6 +161,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

bundler/lib/bundler/cli/install.rb

@@ -59,7 +59,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!

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"]

bundler/lib/bundler/definition.rb

@@ -105,6 +105,8 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
           @locked_specs   = SpecSet.new([])
           @locked_sources = []
         end
+
+        remove_plugin_dependencies_if_necessary
       else
         @unlock         = {}
         @platforms      = []
@@ -228,6 +230,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
@@ -421,6 +427,7 @@ def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false)
     def validate_runtime!
       validate_ruby!
       validate_platforms!
+      validate_plugins!
     end
 
     def validate_ruby!
@@ -456,6 +463,19 @@ def validate_platforms!
         "Add the current platform to the lockfile with\n`bundle lock --add-platform #{local_platform}` and try again."
     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)
 
@@ -1083,5 +1103,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

bundler/lib/bundler/dependency.rb

@@ -29,7 +29,15 @@ 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
 
       @autorequire    = nil
       @groups         = Array(options["group"] || :default).map(&:to_sym)

bundler/lib/bundler/dsl.rb

@@ -160,9 +160,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-<type>,
+        # 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"
@@ -271,7 +277,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)

bundler/lib/bundler/feature_flag.rb

@@ -35,6 +35,7 @@ def self.settings_method(name, key, &default)
     settings_flag(:global_gem_cache) { 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? }

bundler/lib/bundler/man/bundle-config.1

@@ -163,6 +163,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\.

bundler/lib/bundler/man/bundle-config.1.ronn

@@ -234,6 +234,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`):

bundler/lib/bundler/plugin.rb

@@ -4,18 +4,20 @@
 
 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
     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,29 +103,44 @@ 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)
         else
           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<String>] 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