Fix plugin installation from gemfile

Author: ccutrer

Manifest.txt

@@ -162,6 +162,7 @@ bundler/lib/bundler/plugin/api/source.rb
 bundler/lib/bundler/plugin/dsl.rb
 bundler/lib/bundler/plugin/events.rb
 bundler/lib/bundler/plugin/index.rb
+bundler/lib/bundler/plugin/index_definition.rb
 bundler/lib/bundler/plugin/installer.rb
 bundler/lib/bundler/plugin/installer/git.rb
 bundler/lib/bundler/plugin/installer/path.rb

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,6 +31,8 @@ def run
 
       conservative = options[:conservative]
 
+      Plugin.gemfile_install(Bundler.default_gemfile, :unlock => full_update && !conservative) if Bundler.feature_flag.plugins?
+
       if full_update
         if conservative
           Bundler.definition(conservative: conservative)

bundler/lib/bundler/definition.rb

@@ -56,7 +56,8 @@ def self.build(gemfile, lockfile, unlock)
     #   to be updated or true if all gems should be updated
     # @param ruby_version [Bundler::RubyVersion, nil] Requested Ruby Version
     # @param optional_groups [Array(String)] A list of optional groups
-    def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, optional_groups = [], gemfiles = [])
+    # @param lockfile_contents [String, nil] The contents of the lockfile
+    def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, optional_groups = [], gemfiles = [], lockfile_contents = nil)
       if [true, false].include?(unlock)
         @unlocking_bundler = false
         @unlocking = unlock
@@ -76,7 +77,6 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
       @gemfiles        = gemfiles
 
       @lockfile               = lockfile
-      @lockfile_contents      = String.new
 
       @locked_bundler_version = nil
       @resolved_bundler_version = nil
@@ -85,8 +85,8 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
       @new_platform = nil
       @removed_platform = nil
 
-      if lockfile_exists?
-        @lockfile_contents = Bundler.read_file(lockfile)
+      if lockfile_exists? || lockfile_contents
+        @lockfile_contents = lockfile_contents || Bundler.read_file(lockfile)
         @locked_gems = LockfileParser.new(@lockfile_contents)
         @locked_platforms = @locked_gems.platforms
         @platforms = @locked_platforms.dup
@@ -106,6 +106,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
           @locked_sources = []
         end
       else
+        @lockfile_contents = String.new
         @unlock         = {}
         @platforms      = []
         @locked_gems    = nil

bundler/lib/bundler/dsl.rb

@@ -226,9 +226,9 @@ def github(repo, options = {})
       with_source(git_source) { yield }
     end
 
-    def to_definition(lockfile, unlock)
+    def to_definition(lockfile, unlock, lockfile_contents: nil)
       check_primary_source_safety
-      Definition.new(lockfile, @dependencies, @sources, unlock, @ruby_version, @optional_groups, @gemfiles)
+      Definition.new(lockfile, @dependencies, @sources, unlock, @ruby_version, @optional_groups, @gemfiles, lockfile_contents)
     end
 
     def group(*args, &blk)

bundler/lib/bundler/plugin.rb

@@ -4,10 +4,11 @@
 
 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 :DSL, File.expand_path("plugin/dsl", __dir__)
+    autoload :Events, File.expand_path("plugin/events", __dir__)
+    autoload :Index, File.expand_path("plugin/index", __dir__)
+    autoload :IndexDefinition, File.expand_path("plugin/index_definition", __dir__)
+    autoload :Installer, File.expand_path("plugin/installer", __dir__)
     autoload :SourceList, File.expand_path("plugin/source_list", __dir__)
 
     class MalformattedPlugin < PluginError; end
@@ -100,21 +101,25 @@ def list
     #
     # @param [Pathname] gemfile path
     # @param [Proc] block that can be evaluated for (inline) Gemfile
-    def gemfile_install(gemfile = nil, &inline)
+    def gemfile_install(gemfile = nil, unlock: false, &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?
+        return if builder.dependencies.empty?
 
-        plugins = definition.dependencies.map(&:name).reject {|p| index.installed? p }
-        installed_specs = Installer.new.install_definition(definition)
+        lockfile_contents = index.generate_lockfile(builder.instance_variable_get(:@sources), builder.dependencies)
+        definition = builder.to_definition(nil, unlock || {}, :lockfile_contents => lockfile_contents)
+        unless definition.no_resolve_needed?
+          Installer.new.install_definition(definition)
+        end
+
+        plugins = definition.dependencies.map(&:name)
+        installed_specs = plugins.to_h {|p| [p, definition.specs[p].first] }
 
         save_plugins plugins, installed_specs, builder.inferred_plugins
       end
@@ -253,8 +258,6 @@ def loaded?(plugin)
     # @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]
 
         save_plugin(name, spec, optional_plugins.include?(name))
@@ -302,6 +305,8 @@ def register_plugin(name, spec, optional_plugin = false)
       sources = @sources
       hooks = @hooks_by_event
 
+      return false if index.installed?(name) == spec.full_gem_path
+
       @commands = {}
       @sources = {}
       @hooks_by_event = Hash.new {|h, k| h[k] = [] }

bundler/lib/bundler/plugin/index.rb

@@ -144,6 +144,37 @@ def installed_in_plugin_root?(name)
         path.start_with?("#{Plugin.root}/")
       end
 
+      # generate an in-memory lockfile from the index
+      def generate_lockfile(sources, dependencies)
+        specs = []
+        sources.cached!
+        default_source = sources.global_rubygems_source
+
+        installed_plugins.each do |plugin|
+          path = plugin_path(plugin)
+          # path gems may have a gemspec, which is the most trustworthy
+          # way to determine the version
+          version = if (gemspec = path.join("#{plugin}.gemspec")).file?
+            Gem::Specification.load(gemspec.to_s).version
+          elsif (version_index = path.to_s.index("#{plugin}-"))
+            path.to_s[(version_index + plugin.length + 1)..]
+          end
+
+          next unless version
+
+          dep = dependencies.find {|d| d.name == plugin }
+          next unless dep
+
+          spec = LazySpecification.new(plugin, version, nil, dep.source || default_source)
+          next unless spec.satisfies?(dep)
+
+          specs << spec
+        end
+
+        require_relative "../lockfile_generator"
+        LockfileGenerator.generate(IndexDefinition.new(sources, specs, dependencies))
+      end
+
       private
 
       # Reads the index file from the directory and initializes the instance

bundler/lib/bundler/plugin/index_definition.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module Bundler
+  module Plugin
+    # duck-type of Definition for feeding to LockfileGenerator
+    IndexDefinition = Struct.new(:sources, :specs, :dependencies) do
+      def platforms
+        [Bundler.local_platform]
+      end
+
+      def locked_ruby_version; end
+
+      def bundler_version_to_lock
+        VERSION
+      end
+
+      alias_method :resolve, :specs
+    end
+  end
+end

bundler/spec/plugins/install_spec.rb

@@ -263,6 +263,118 @@ def exec(command, args)
       expect(out).to include("Bundle complete!")
     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 '#{file_uri_for(gem_repo2)}'
+        plugin 'foo', "1.4.0"
+        gem 'rack', "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("rack 1.0.0")
+      plugin_should_be_installed_with_version("foo", "1.4.0")
+
+      gemfile <<-G
+        source '#{file_uri_for(gem_repo2)}'
+        plugin 'foo', "1.5.0"
+        gem 'rack', "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("rack 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 '#{file_uri_for(gem_repo2)}'
+        plugin 'foo', "1.5.0"
+        gem 'rack', "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("rack 1.0.0")
+      plugin_should_be_installed_with_version("foo", "1.5.0")
+
+      gemfile <<-G
+        source '#{file_uri_for(gem_repo2)}'
+        plugin 'foo', "1.4.0"
+        gem 'rack', "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("rack 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 '#{file_uri_for(gem_repo2)}'
+        plugin 'foo'
+        gem 'rack', "1.0.0"
+      G
+
+      2.times { bundle "install" }
+
+      expect(out).to_not include("Fetching gem metadata")
+      expect(out).to_not include("Installing foo")
+      expect(out).to_not include("Installed plugin foo")
+
+      expect(out).to include("Bundle complete!")
+
+      expect(the_bundle).to include_gems("rack 1.0.0")
+      plugin_should_be_installed("foo")
+
+      gemfile <<-G
+        source '#{file_uri_for(gem_repo2)}'
+        plugin 'foo'
+        plugin 'kung-foo'
+        gem 'rack', "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("Installing foo")
+      expect(out).to_not include("Installed plugin foo")
+
+      expect(out).to include("Bundle complete!")
+
+      expect(the_bundle).to include_gems("rack 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"
@@ -327,7 +439,9 @@ def exec(command, args)
       RUBY
 
       ruby code, 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
 

bundler/spec/support/matchers.rb

@@ -216,6 +216,14 @@ def plugin_should_be_installed(*names)
       end
     end
 
+    def plugin_should_be_installed_with_version(name, version)
+      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)
       names.each do |name|
         expect(Bundler::Plugin).not_to be_installed(name)