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

Commit

Permalink
Auto merge of #6495 - bundler:segiddins/6491-extra-gem-platform-in-lo…
Browse files Browse the repository at this point in the history
…ckfile, r=segiddins

[Definition] Filter out unneeded gem platforms after resolving

### What was the end-user problem that led to this PR?

The problem was the lockfile would contain platform-specific gems that it didn't need

Closes #6491

### What was your diagnosis of the problem?

My diagnosis was the resolver sometimes activates platforms it doesn't need, since it can't know it won't need them

### What is your fix for the problem, implemented in this PR?

My fix is to use `SpecSet#for` to filter out gems that won't ever be used

### Why did you choose this fix out of the possible options?

I chose this fix because it isn't re-inventing the wheel!

(cherry picked from commit 7b603f3)
  • Loading branch information
bundlerbot authored and colby-swandale committed Oct 4, 2018
1 parent 17c7bb6 commit 3001ce2
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 15 deletions.
27 changes: 16 additions & 11 deletions lib/bundler/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,22 @@ def specs_for(groups)
def resolve
@resolve ||= begin
last_resolve = converge_locked_specs
if Bundler.frozen_bundle?
Bundler.ui.debug "Frozen, using resolution from the lockfile"
last_resolve
elsif !unlocking? && nothing_changed?
Bundler.ui.debug("Found no changes, using resolution from the lockfile")
last_resolve
else
# Run a resolve against the locally available gems
Bundler.ui.debug("Found changes from the lockfile, re-resolving dependencies because #{change_reason}")
last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, gem_version_promoter, additional_base_requirements_for_resolve, platforms)
end
resolve =
if Bundler.frozen_bundle?
Bundler.ui.debug "Frozen, using resolution from the lockfile"
last_resolve
elsif !unlocking? && nothing_changed?
Bundler.ui.debug("Found no changes, using resolution from the lockfile")
last_resolve
else
# Run a resolve against the locally available gems
Bundler.ui.debug("Found changes from the lockfile, re-resolving dependencies because #{change_reason}")
last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, gem_version_promoter, additional_base_requirements_for_resolve, platforms)
end

# filter out gems that _can_ be installed on multiple platforms, but don't need
# to be
resolve.for(expand_dependencies(dependencies, true), [], false, false, false)
end
end

Expand Down
4 changes: 0 additions & 4 deletions lib/bundler/resolver/spec_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ def dependencies_for_activated_platforms
dependencies.concat(metadata_dependencies).flatten
end

def platforms_for_dependency_named(dependency)
__dependencies.select {|_, deps| deps.map(&:name).include? dependency }.keys
end

def ==(other)
return unless other.is_a?(SpecGroup)
name == other.name &&
Expand Down
143 changes: 143 additions & 0 deletions spec/install/gemfile/platform_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,149 @@
expect(the_bundle).not_to include_gems "weakling"
end

it "does not keep unneeded platforms for gems that are used" do
build_repo4 do
build_gem "empyrean", "0.1.0"
build_gem "coderay", "1.1.2"
build_gem "method_source", "0.9.0"
build_gem("spoon", "0.0.6") {|s| s.add_runtime_dependency "ffi" }
build_gem "pry", "0.11.3" do |s|
s.platform = "java"
s.add_runtime_dependency "coderay", "~> 1.1.0"
s.add_runtime_dependency "method_source", "~> 0.9.0"
s.add_runtime_dependency "spoon", "~> 0.0"
end
build_gem "pry", "0.11.3" do |s|
s.add_runtime_dependency "coderay", "~> 1.1.0"
s.add_runtime_dependency "method_source", "~> 0.9.0"
end
build_gem("ffi", "1.9.23") {|s| s.platform = "java" }
build_gem("ffi", "1.9.23")
end

simulate_platform java

install_gemfile! <<-G
source "file://localhost/#{gem_repo4}"
gem "empyrean", "0.1.0"
gem "pry"
G

expect(the_bundle.lockfile).to read_as strip_whitespace(<<-L)
GEM
remote: file://localhost/#{gem_repo4}/
specs:
coderay (1.1.2)
empyrean (0.1.0)
ffi (1.9.23-java)
method_source (0.9.0)
pry (0.11.3-java)
coderay (~> 1.1.0)
method_source (~> 0.9.0)
spoon (~> 0.0)
spoon (0.0.6)
ffi
PLATFORMS
java
DEPENDENCIES
empyrean (= 0.1.0)
pry
BUNDLED WITH
#{Bundler::VERSION}
L

bundle! "lock --add-platform ruby"

good_lockfile = strip_whitespace(<<-L)
GEM
remote: file://localhost/#{gem_repo4}/
specs:
coderay (1.1.2)
empyrean (0.1.0)
ffi (1.9.23-java)
method_source (0.9.0)
pry (0.11.3)
coderay (~> 1.1.0)
method_source (~> 0.9.0)
pry (0.11.3-java)
coderay (~> 1.1.0)
method_source (~> 0.9.0)
spoon (~> 0.0)
spoon (0.0.6)
ffi
PLATFORMS
java
ruby
DEPENDENCIES
empyrean (= 0.1.0)
pry
BUNDLED WITH
#{Bundler::VERSION}
L

expect(the_bundle.lockfile).to read_as good_lockfile

bad_lockfile = strip_whitespace <<-L
GEM
remote: file://localhost/#{gem_repo4}/
specs:
coderay (1.1.2)
empyrean (0.1.0)
ffi (1.9.23)
ffi (1.9.23-java)
method_source (0.9.0)
pry (0.11.3)
coderay (~> 1.1.0)
method_source (~> 0.9.0)
pry (0.11.3-java)
coderay (~> 1.1.0)
method_source (~> 0.9.0)
spoon (~> 0.0)
spoon (0.0.6)
ffi
PLATFORMS
java
ruby
DEPENDENCIES
empyrean (= 0.1.0)
pry
BUNDLED WITH
#{Bundler::VERSION}
L

aggregate_failures do
lockfile bad_lockfile
bundle! :install
expect(the_bundle.lockfile).to read_as good_lockfile

lockfile bad_lockfile
bundle! :update, :all => true
expect(the_bundle.lockfile).to read_as good_lockfile

lockfile bad_lockfile
bundle! "update ffi"
expect(the_bundle.lockfile).to read_as good_lockfile

lockfile bad_lockfile
bundle! "update empyrean"
expect(the_bundle.lockfile).to read_as good_lockfile

lockfile bad_lockfile
bundle! :lock
expect(the_bundle.lockfile).to read_as good_lockfile
end
end

it "works the other way with gems that have different dependencies" do
simulate_platform "ruby"
install_gemfile <<-G
Expand Down

0 comments on commit 3001ce2

Please sign in to comment.