Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

[Installer] Skip eval’ing a Gemfile twice when there’s a lockfile #4952

Merged
merged 6 commits into from
Sep 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/bundler/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,17 @@ def missing_specs
missing
end

def missing_dependencies
def missing_dependencies?
missing = []
resolve.materialize(current_dependencies, missing)
missing
return false if missing.empty?
Bundler.ui.debug "The definition is missing #{missing.map(&:full_name)}"
true
rescue BundlerError => e
Bundler.ui.debug "The definition is missing dependencies, failed to resolve & materialize locally (#{e})"
true
ensure
@specs = nil
end

def requested_specs
Expand Down
18 changes: 9 additions & 9 deletions lib/bundler/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def run(options)
return
end

resolve_if_need(options)
resolve_if_needed(options)
install(options)

lock unless Bundler.settings[:frozen]
Expand Down Expand Up @@ -181,18 +181,18 @@ def create_bundle_path
"because of an invalid symlink. Remove the symlink so the directory can be created."
end

def resolve_if_need(options)
def resolve_if_needed(options)
if Bundler.default_lockfile.exist? && !options["update"]
local = Bundler.ui.silence do
begin
tmpdef = Definition.build(Bundler.default_gemfile, Bundler.default_lockfile, nil)
true unless tmpdef.new_platform? || tmpdef.missing_dependencies.any?
rescue BundlerError
end
if @definition.new_platform?
Bundler.ui.debug "Resolving since there is a new platform in the definition"
elsif @definition.missing_dependencies?
nil
else
Bundler.ui.debug "No need to re-resolve! The info in the lockfile is sufficient"
return
end
end

return if local
options["local"] ? @definition.resolve_with_cache! : @definition.resolve_remotely!
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/bundler/source/path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ def initialize(options)
end

def remote!
@local_specs = nil
@allow_remote = true
end

def cached!
@local_specs = nil
@allow_cached = true
end

Expand Down
2 changes: 2 additions & 0 deletions lib/bundler/source/rubygems.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ def initialize(options = {})
end

def remote!
@specs = nil
@allow_remote = true
end

def cached!
@specs = nil
@allow_cached = true
end

Expand Down
1 change: 1 addition & 0 deletions lib/bundler/source_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ def lock_sources
end

def get(source)
return unless source
source_list_for(source).find {|s| source == s }
end

Expand Down
4 changes: 4 additions & 0 deletions lib/bundler/spec_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ def merge(set)
SpecSet.new(arr)
end

def to_s
format("<%s:%p @specs=%s>", self.class, object_id, @specs.map(&:full_name))
end

private

def sorted
Expand Down
42 changes: 42 additions & 0 deletions spec/install/gemfile/lockfile_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true
require "spec_helper"

describe "bundle install with a lockfile present" do
let(:gf) { <<-G }
source "file://#{gem_repo1}"

gem "rack", "1.0.0"
G

before do
install_gemfile(gf)
end

context "gemfile evaluation" do
let(:gf) { super() + "\n\n File.open('evals', 'a') {|f| f << %(1\n) } unless ENV['BUNDLER_SPEC_NO_APPEND']" }

it "does not evaluate the gemfile twice" do
bundle! :install

with_env_vars("BUNDLER_SPEC_NO_APPEND" => "1") { should_be_installed "rack 1.0.0" }

# The first eval is from the initial install, we're testing that the
# second install doesn't double-eval
expect(bundled_app("evals").read.lines.size).to eq(2)
end

context "when the gem is not installed" do
before { FileUtils.rm_rf ".bundle" }

it "does not evaluate the gemfile twice" do
bundle! :install

with_env_vars("BUNDLER_SPEC_NO_APPEND" => "1") { should_be_installed "rack 1.0.0" }

# The first eval is from the initial install, we're testing that the
# second install doesn't double-eval
expect(bundled_app("evals").read.lines.size).to eq(2)
end
end
end
end