[rubygems/rubygems] Fix Gemfile.lock versions leaking to bundler/inline install output

The lockfile is completely ignored in inline mode, yet the previous
output would suggest it wasn't.

763125a745

lib/bundler/installer.rb

@@ -13,7 +13,7 @@ module Bundler
       Installer.ambiguous_gems = []
     end
 
-    attr_reader :post_install_messages
+    attr_reader :post_install_messages, :definition
 
     # Begins the installation process for Bundler.
     # For more information see the #run method on this class.

lib/bundler/installer/gem_installer.rb

@@ -51,7 +51,20 @@ module Bundler
     end
 
     def install
-      spec.source.install(spec, :force => force, :ensure_builtin_gems_cached => standalone, :build_args => Array(spec_settings))
+      spec.source.install(
+        spec,
+        :force => force,
+        :ensure_builtin_gems_cached => standalone,
+        :build_args => Array(spec_settings),
+        :previous_spec => previous_spec,
+      )
+    end
+
+    def previous_spec
+      locked_gems = installer.definition.locked_gems
+      return unless locked_gems
+
+      locked_gems.specs.find {|s| s.name == spec.name }
     end
 
     def out_of_space_message

lib/bundler/plugin/installer/git.rb

@@ -20,10 +20,6 @@ module Bundler
           end
         end
 
-        def version_message(spec)
-          "#{spec.name} #{spec.version}"
-        end
-
         def root
           Plugin.root
         end

lib/bundler/plugin/installer/rubygems.rb

@@ -4,10 +4,6 @@ module Bundler
   module Plugin
     class Installer
       class Rubygems < Bundler::Source::Rubygems
-        def version_message(spec)
-          "#{spec.name} #{spec.version}"
-        end
-
         private
 
         def requires_sudo?

lib/bundler/source.rb

@@ -15,13 +15,12 @@ module Bundler
       specs.unmet_dependency_names
     end
 
-    def version_message(spec)
+    def version_message(spec, locked_spec = nil)
       message = "#{spec.name} #{spec.version}"
       message += " (#{spec.platform})" if spec.platform != Gem::Platform::RUBY && !spec.platform.nil?
 
-      if Bundler.locked_gems
+      if locked_spec
-        locked_spec = Bundler.locked_gems.specs.find {|s| s.name == spec.name }
+        locked_spec_version = locked_spec.version
-        locked_spec_version = locked_spec.version if locked_spec
         if locked_spec_version && spec.version != locked_spec_version
           message += Bundler.ui.add_color(" (was #{locked_spec_version})", version_color(spec.version, locked_spec_version))
         end

lib/bundler/source/git.rb

@@ -181,7 +181,7 @@ module Bundler
       def install(spec, options = {})
         force = options[:force]
 
-        print_using_message "Using #{version_message(spec)} from #{self}"
+        print_using_message "Using #{version_message(spec, options[:previous_spec])} from #{self}"
 
         if (requires_checkout? && !@copied) || force
           Bundler.ui.debug "  * Checking out revision: #{ref}"

lib/bundler/source/path.rb

@@ -82,7 +82,7 @@ module Bundler
       end
 
       def install(spec, options = {})
-        using_message = "Using #{version_message(spec)} from #{self}"
+        using_message = "Using #{version_message(spec, options[:previous_spec])} from #{self}"
         using_message += " and installing its executables" unless spec.executables.empty?
         print_using_message using_message
         generate_bin(spec, :disable_extensions => true)

lib/bundler/source/rubygems.rb

@@ -162,7 +162,7 @@ module Bundler
           uris.uniq!
           Installer.ambiguous_gems << [spec.name, *uris] if uris.length > 1
 
-          path = fetch_gem(spec)
+          path = fetch_gem(spec, options[:previous_spec])
           begin
             s = Bundler.rubygems.spec_from_gem(path, Bundler.settings["trust-policy"])
             spec.__swap__(s)
@@ -173,7 +173,7 @@ module Bundler
         end
 
         unless Bundler.settings[:no_install]
-          message = "Installing #{version_message(spec)}"
+          message = "Installing #{version_message(spec, options[:previous_spec])}"
           message += " with native extensions" if spec.extensions.any?
           Bundler.ui.confirm message
 
@@ -458,7 +458,7 @@ module Bundler
         end
       end
 
-      def fetch_gem(spec)
+      def fetch_gem(spec, previous_spec = nil)
         return false unless spec.remote
 
         spec.fetch_platform
@@ -476,7 +476,7 @@ module Bundler
         SharedHelpers.filesystem_access(download_cache_path) do |p|
           FileUtils.mkdir_p(p)
         end
-        download_gem(spec, download_cache_path)
+        download_gem(spec, download_cache_path, previous_spec)
 
         if requires_sudo?
           SharedHelpers.filesystem_access(cache_path) do |p|
@@ -521,9 +521,12 @@ module Bundler
       # @param  [String] download_cache_path
       #         the local directory the .gem will end up in.
       #
-      def download_gem(spec, download_cache_path)
+      # @param  [Specification] previous_spec
+      #         the spec previously locked
+      #
+      def download_gem(spec, download_cache_path, previous_spec = nil)
         uri = spec.remote.uri
-        Bundler.ui.confirm("Fetching #{version_message(spec)}")
+        Bundler.ui.confirm("Fetching #{version_message(spec, previous_spec)}")
         Bundler.rubygems.download_gem(spec, uri, download_cache_path)
       end
 

spec/bundler/bundler/installer/gem_installer_spec.rb

@@ -3,7 +3,8 @@
 require "bundler/installer/gem_installer"
 
 RSpec.describe Bundler::GemInstaller do
-  let(:installer) { instance_double("Installer") }
+  let(:definition) { instance_double("Definition", :locked_gems => nil) }
+  let(:installer) { instance_double("Installer", :definition => definition) }
   let(:spec_source) { instance_double("SpecSource") }
   let(:spec) { instance_double("Specification", :name => "dummy", :version => "0.0.1", :loaded_from => "dummy", :source => spec_source) }
 
@@ -11,7 +12,7 @@ RSpec.describe Bundler::GemInstaller do
 
   context "spec_settings is nil" do
     it "invokes install method with empty build_args" do
-      allow(spec_source).to receive(:install).with(spec, :force => false, :ensure_builtin_gems_cached => false, :build_args => [])
+      allow(spec_source).to receive(:install).with(spec, :force => false, :ensure_builtin_gems_cached => false, :build_args => [], :previous_spec => nil)
       subject.install_from_spec
     end
   end
@@ -22,7 +23,7 @@ RSpec.describe Bundler::GemInstaller do
       allow(Bundler.settings).to receive(:[]).with(:inline)
       allow(Bundler.settings).to receive(:[]).with(:forget_cli_options)
       allow(Bundler.settings).to receive(:[]).with("build.dummy").and_return("--with-dummy-config=dummy")
-      expect(spec_source).to receive(:install).with(spec, :force => false, :ensure_builtin_gems_cached => false, :build_args => ["--with-dummy-config=dummy"])
+      expect(spec_source).to receive(:install).with(spec, :force => false, :ensure_builtin_gems_cached => false, :build_args => ["--with-dummy-config=dummy"], :previous_spec => nil)
       subject.install_from_spec
     end
   end
@@ -33,7 +34,13 @@ RSpec.describe Bundler::GemInstaller do
       allow(Bundler.settings).to receive(:[]).with(:inline)
       allow(Bundler.settings).to receive(:[]).with(:forget_cli_options)
       allow(Bundler.settings).to receive(:[]).with("build.dummy").and_return("--with-dummy-config=dummy --with-another-dummy-config")
-      expect(spec_source).to receive(:install).with(spec, :force => false, :ensure_builtin_gems_cached => false, :build_args => ["--with-dummy-config=dummy", "--with-another-dummy-config"])
+      expect(spec_source).to receive(:install).with(
+        spec,
+        :force => false,
+        :ensure_builtin_gems_cached => false,
+        :build_args => ["--with-dummy-config=dummy", "--with-another-dummy-config"],
+        :previous_spec => nil
+      )
       subject.install_from_spec
     end
   end

spec/bundler/bundler/source_spec.rb

@@ -30,17 +30,7 @@ RSpec.describe Bundler::Source do
     end
 
     context "when there are locked gems" do
-      let(:locked_gems) { double(:locked_gems) }
-
-      before { allow(Bundler).to receive(:locked_gems).and_return(locked_gems) }
-
       context "that contain the relevant gem spec" do
-        before do
-          specs = double(:specs)
-          allow(locked_gems).to receive(:specs).and_return(specs)
-          allow(specs).to receive(:find).and_return(locked_gem)
-        end
-
         context "without a version" do
           let(:locked_gem) { double(:locked_gem, :name => "nokogiri", :version => nil) }
 
@@ -62,7 +52,7 @@ RSpec.describe Bundler::Source do
             end
 
             it "should return a string with the spec name and version and locked spec version" do
-              expect(subject.version_message(spec)).to eq("nokogiri >= 1.6\e[32m (was < 1.5)\e[0m")
+              expect(subject.version_message(spec, locked_gem)).to eq("nokogiri >= 1.6\e[32m (was < 1.5)\e[0m")
             end
           end
 
@@ -74,7 +64,7 @@ RSpec.describe Bundler::Source do
             end
 
             it "should return a string with the spec name and version and locked spec version" do
-              expect(subject.version_message(spec)).to eq("nokogiri >= 1.6 (was < 1.5)")
+              expect(subject.version_message(spec, locked_gem)).to eq("nokogiri >= 1.6 (was < 1.5)")
             end
           end
         end
@@ -89,7 +79,7 @@ RSpec.describe Bundler::Source do
             end
 
             it "should return a string with the locked spec version in yellow" do
-              expect(subject.version_message(spec)).to eq("nokogiri 1.6.1\e[33m (was 1.7.0)\e[0m")
+              expect(subject.version_message(spec, locked_gem)).to eq("nokogiri 1.6.1\e[33m (was 1.7.0)\e[0m")
             end
           end
 
@@ -101,7 +91,7 @@ RSpec.describe Bundler::Source do
             end
 
             it "should return a string with the locked spec version in yellow" do
-              expect(subject.version_message(spec)).to eq("nokogiri 1.6.1 (was 1.7.0)")
+              expect(subject.version_message(spec, locked_gem)).to eq("nokogiri 1.6.1 (was 1.7.0)")
             end
           end
         end
@@ -116,7 +106,7 @@ RSpec.describe Bundler::Source do
             end
 
             it "should return a string with the locked spec version in green" do
-              expect(subject.version_message(spec)).to eq("nokogiri 1.7.1\e[32m (was 1.7.0)\e[0m")
+              expect(subject.version_message(spec, locked_gem)).to eq("nokogiri 1.7.1\e[32m (was 1.7.0)\e[0m")
             end
           end
 
@@ -128,27 +118,11 @@ RSpec.describe Bundler::Source do
             end
 
             it "should return a string with the locked spec version in yellow" do
-              expect(subject.version_message(spec)).to eq("nokogiri 1.7.1 (was 1.7.0)")
+              expect(subject.version_message(spec, locked_gem)).to eq("nokogiri 1.7.1 (was 1.7.0)")
             end
           end
         end
       end
-
-      context "that do not contain the relevant gem spec" do
-        before do
-          specs = double(:specs)
-          allow(locked_gems).to receive(:specs).and_return(specs)
-          allow(specs).to receive(:find).and_return(nil)
-        end
-
-        it_behaves_like "the lockfile specs are not relevant"
-      end
-    end
-
-    context "when there are no locked gems" do
-      before { allow(Bundler).to receive(:locked_gems).and_return(nil) }
-
-      it_behaves_like "the lockfile specs are not relevant"
     end
   end
 

spec/bundler/runtime/inline_spec.rb

@@ -239,6 +239,40 @@ RSpec.describe "bundler/inline#gemfile" do
     expect(err).to be_empty
   end
 
+  it "does not leak Gemfile.lock versions to the installation output" do
+    gemfile <<-G
+      source "https://notaserver.com"
+      gem "rake"
+    G
+
+    lockfile <<-G
+      GEM
+        remote: https://rubygems.org/
+        specs:
+          rake (11.3.0)
+
+      PLATFORMS
+        ruby
+
+      DEPENDENCIES
+        rake
+
+      BUNDLED WITH
+         #{Bundler::VERSION}
+    G
+
+    script <<-RUBY
+      gemfile(true) do
+        source "#{file_uri_for(gem_repo1)}"
+        gem "rake", "~> 13.0"
+      end
+    RUBY
+
+    expect(out).to include("Installing rake 13.0")
+    expect(out).not_to include("was 11.3.0")
+    expect(err).to be_empty
+  end
+
   it "installs inline gems when frozen is set" do
     script <<-RUBY, :env => { "BUNDLE_FROZEN" => "true" }
       gemfile do