Fix plugin installation from gemfile

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

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!

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

@@ -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(&:plugin?)
+    end
+
     def current_dependencies
       filter_relevant(dependencies)
     end
@@ -1166,5 +1172,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.plugin? && @locked_deps.key?(d.name) }
+
+      @dependencies.reject!(&:plugin?)
+    end
   end
 end

bundler/lib/bundler/dependency.rb

@@ -118,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)

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-<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"
@@ -217,8 +223,15 @@ def env(name)
       @env = old
     end
 
-    def plugin(*args)
-      # Pass on
+    def plugin(name, *args)
+      options = args.last.is_a?(Hash) ? args.pop.dup : {}
+      version = args || [">= 0"]
+
+      normalize_options(name, version, options)
+      options["plugin"] = true
+      options["require"] = false
+
+      add_dependency(name, version, options)
     end
 
     def method_missing(name, *args)

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? }

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\.

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`):

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

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)
+        # Ignore regular dependencies when doing the plugins-only pre-parse
       end
 
       def method_missing(name, *args)

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