Merge pull request #6957 from ccutrer/plugin-install

Fix plugin installation from gemfile

Author: hsbt

Manifest.txt

@@ -168,6 +168,7 @@ bundler/lib/bundler/plugin/installer/git.rb
 bundler/lib/bundler/plugin/installer/path.rb
 bundler/lib/bundler/plugin/installer/rubygems.rb
 bundler/lib/bundler/plugin/source_list.rb
+bundler/lib/bundler/plugin/unloaded_source.rb
 bundler/lib/bundler/process_lock.rb
 bundler/lib/bundler/remote_specification.rb
 bundler/lib/bundler/resolver.rb

bundler/lib/bundler/cli/install.rb

@@ -38,7 +38,7 @@ def run
 
       Bundler::Fetcher.disable_endpoint = options["full-index"]
 
-      Plugin.gemfile_install(Bundler.default_gemfile) if Bundler.settings[:plugins]
+      Plugin.gemfile_install(Bundler.default_gemfile, Bundler.default_lockfile) if Bundler.settings[:plugins]
 
       # For install we want to enable strict validation
       # (rather than some optimizations we perform at app 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.settings[: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.dup
 
         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.settings[: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/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

@@ -124,9 +124,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"
@@ -256,8 +262,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/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 :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 :UnloadedSource, File.expand_path("plugin/unloaded_source", __dir__)
 
     class MalformattedPlugin < PluginError; end
     class UndefinedCommandError < PluginError; end
@@ -17,6 +18,14 @@ class PluginInstallError < PluginError; end
 
     PLUGIN_FILE_NAME = "plugins.rb"
 
+    # Module-level flag set while .gemfile_install parses the Gemfile and
+    # consulted by .from_lock to substitute plugin sources with
+    # UnloadedSource. It relies on definitions being built one at a time in
+    # a single thread; if they are ever built concurrently or reentrantly,
+    # this needs to be replaced by explicit state passed down to the
+    # lockfile parser.
+    @gemfile_parse = false
+
     module_function
 
     def reset!
@@ -26,6 +35,7 @@ def reset!
       @commands = {}
       @hooks_by_event = Hash.new {|h, k| h[k] = [] }
       @loaded_plugin_names = []
+      @index = nil
     end
 
     reset!
@@ -40,7 +50,7 @@ def install(names, options)
 
       specs = Installer.new.install(names, options)
 
-      save_plugins names, specs
+      save_plugins specs.slice(*names)
     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 +110,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
-        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)
-
-        return if definition.dependencies.empty?
+    def gemfile_install(gemfile = nil, lockfile = nil, unlock = {}, &inline)
+      @gemfile_parse = true
+      Bundler.configure
+      builder = DSL.new
+      if block_given?
+        builder.instance_eval(&inline)
+      else
+        builder.eval_gemfile(gemfile)
+      end
+      builder.check_primary_source_safety
 
-        plugins = definition.dependencies.map(&:name)
-        installed_specs = Installer.new.install_definition(definition)
+      plugins = builder.dependencies.map(&:name)
+      return if plugins.empty?
 
-        save_plugins plugins, installed_specs, builder.inferred_plugins
+      # skip the update if unlocking specific gems, but none of them are plugins
+      # declared in the Gemfile
+      if unlock.is_a?(Hash) && unlock[:gems] && !unlock[:gems].empty? &&
+         (unlock[:gems] & plugins).empty?
+        unlock = {}
       end
+
+      # resolve remotely when unlocking, so that plugins can be updated.
+      # Definition#initialize consumes the unlock hash, so this must be decided
+      # before building the definition.
+      updating = unlock == true || (unlock.is_a?(Hash) && !unlock.empty?)
+
+      definition = builder.to_definition(lockfile, unlock)
+
+      installed_specs = Installer.new.install_definition(definition, updating)
+
+      save_plugins installed_specs.slice(*plugins), builder.inferred_plugins
     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 +208,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 +229,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 UnloadedSource.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 +272,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
@@ -247,19 +284,11 @@ def loaded?(plugin)
 
     # Post installation processing and registering with index
     #
-    # @param [Array<String>] plugins list to be installed
     # @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|
-        spec = specs[name]
-
-        # It's possible that the `plugin` found in the Gemfile don't appear in the specs. For instance when
-        # calling `BUNDLE_WITHOUT=default bundle install`, the plugins will not get installed.
-        next if spec.nil?
-        next if index.up_to_date?(spec)
-
+    def save_plugins(specs, optional_plugins = [])
+      specs.each do |name, spec|
         save_plugin(name, spec, optional_plugins.include?(name))
       end
     end
@@ -284,6 +313,8 @@ def validate_plugin!(plugin_path)
     #
     # @raise [PluginInstallError] if validation or registration raises any error
     def save_plugin(name, spec, optional_plugin = false)
+      return if index.up_to_date?(spec)
+
       validate_plugin! Pathname.new(spec.full_gem_path)
       installed = register_plugin(name, spec, optional_plugin)
       Bundler.ui.info "Installed plugin #{name}" if installed
@@ -319,7 +350,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/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
@@ -89,14 +95,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, "plugin" => true }) }
 
         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
 
@@ -110,6 +116,8 @@ def install_from_specs(specs)
         paths = {}
 
         specs.each do |spec|
+          next if spec.name == "bundler"
+
           spec.source.download(spec)
           spec.source.install(spec)