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 #5818 - bundler:seg-bundler-2-specific-platform, r=indi…
Browse files Browse the repository at this point in the history
…rect

[2.0] Enable specific_platform by default on 2.0

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

The problem was that Bundler has somewhat suspect handling of multi-platform gems. We'd assume that different platform versions of gems were generally interchangeable, so if Bundler resolved to the "ruby" platform gem we'd just blindly try to swap in the gem for the local platform, which could lead to issues (say if the sets of dependencies were different).

### Was was your diagnosis of the problem?

My diagnosis was that we needed to stop only working with the notion of "generic" platforms, which mapped everything to (basically) either java, pure ruby, and windows, and instead keep track of the actual platforms a bundle was being used on, and resolve for those specific platforms.

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

My fix enables the changes made in #4836 by default on Bundler 2.

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

I chose this fix because it means Bundler will default to more correct platforms semantics out of the box.
  • Loading branch information
bundlerbot committed Jul 5, 2017
2 parents 5c62240 + 81ac8ab commit b088392
Show file tree
Hide file tree
Showing 21 changed files with 142 additions and 94 deletions.
5 changes: 3 additions & 2 deletions lib/bundler/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ def remove_platform(platform)

def add_current_platform
current_platform = Bundler.local_platform
add_platform(current_platform) if Bundler.settings[:specific_platform]
add_platform(current_platform) if Bundler.feature_flag.specific_platform?
add_platform(generic(current_platform))
end

Expand Down Expand Up @@ -847,11 +847,12 @@ def concat_ruby_version_requirements(ruby_version, ruby_versions = [])
end

def expand_dependencies(dependencies, remote = false)
sorted_platforms = Resolver.sort_platforms(@platforms)
deps = []
dependencies.each do |dep|
dep = Dependency.new(dep, ">= 0") unless dep.respond_to?(:name)
next if !remote && !dep.current_platform?
platforms = dep.gem_platforms(@platforms)
platforms = dep.gem_platforms(sorted_platforms)
if platforms.empty?
mapped_platforms = dep.platforms.map {|p| Dependency::PLATFORM_MAP[p] }
Bundler.ui.warn \
Expand Down
12 changes: 5 additions & 7 deletions lib/bundler/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,14 @@ def initialize(name, version, options = {}, &blk)
@autorequire = Array(options["require"] || []) if options.key?("require")
end

# Returns the platforms this dependency is valid for, in the same order as
# passed in the `valid_platforms` parameter
def gem_platforms(valid_platforms)
return valid_platforms if @platforms.empty?

platforms = []
@platforms.each do |p|
platform = PLATFORM_MAP[p]
next unless valid_platforms.include?(platform)
platforms |= [platform]
end
platforms
@gem_platforms ||= @platforms.map {|pl| PLATFORM_MAP[pl] }.compact.uniq

valid_platforms & @gem_platforms
end

def should_include?
Expand Down
1 change: 1 addition & 0 deletions lib/bundler/feature_flag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def self.settings_method(name, key, &default)
settings_flag(:plugins) { @bundler_version >= Gem::Version.new("1.14") }
settings_flag(:prefer_gems_rb) { bundler_2_mode? }
settings_flag(:skip_default_git_sources) { bundler_2_mode? }
settings_flag(:specific_platform) { bundler_2_mode? }
settings_flag(:suppress_install_using_messages) { bundler_2_mode? }
settings_flag(:unlock_source_unlocks_spec) { !bundler_2_mode? }
settings_flag(:update_requires_all_flag) { bundler_2_mode? }
Expand Down
2 changes: 1 addition & 1 deletion lib/bundler/lazy_specification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def to_lock
end

def __materialize__
search_object = Bundler.settings[:specific_platform] || Bundler.settings[:force_ruby_platform] ? self : Dependency.new(name, version)
search_object = Bundler.feature_flag.specific_platform? || Bundler.settings[:force_ruby_platform] ? self : Dependency.new(name, version)
@specification = if source.is_a?(Source::Gemspec) && source.gemspec.name == name
source.gemspec.tap {|s| s.source = source }
else
Expand Down
18 changes: 17 additions & 1 deletion lib/bundler/resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ def message
end.min_by(&:size)
trees.reject! {|t| !maximal.include?(t.last) } if maximal

trees = trees.sort_by {|t| t.flatten.map(&:to_s) }
trees.uniq! {|t| t.flatten.map {|dep| [dep.name, dep.requirement] } }

o << trees.sort_by {|t| t.reverse.map(&:name) }.map do |tree|
t = String.new
depth = 2
Expand Down Expand Up @@ -379,10 +382,23 @@ def sort_dependencies(dependencies, activated, conflicts)
amount_constrained(dependency),
conflicts[name] ? 0 : 1,
activated.vertex_named(name).payload ? 0 : search_for(dependency).count,
self.class.platform_sort_key(dependency.__platform),
]
end
end

# Sort platforms from most general to most specific
def self.sort_platforms(platforms)
platforms.sort_by do |platform|
platform_sort_key(platform)
end
end

def self.platform_sort_key(platform)
return ["", "", ""] if Gem::Platform::RUBY == platform
platform.to_a.map {|part| part || "" }
end

private

# returns an integer \in (-\infty, 0]
Expand Down Expand Up @@ -432,7 +448,7 @@ def verify_gemfile_dependencies_are_found!(requirements)
elsif source = @source_requirements[name]
specs = source.specs[name]
versions_with_platforms = specs.map {|s| [s.version, s.platform] }
message = String.new("Could not find gem '#{requirement}' in #{source}#{cache_message}.\n")
message = String.new("Could not find gem '#{SharedHelpers.pretty_dependency(requirement)}' in #{source}#{cache_message}.\n")
message << if versions_with_platforms.any?
"The source contains '#{name}' at: #{formatted_versions_with_platforms(versions_with_platforms)}"
else
Expand Down
1 change: 1 addition & 0 deletions lib/bundler/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Settings
prefer_gems_rb
silence_root_warning
skip_default_git_sources
specific_platform
suppress_install_using_messages
unlock_source_unlocks_spec
update_requires_all_flag
Expand Down
6 changes: 3 additions & 3 deletions spec/bundler/definition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
rack (= 1.0)
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
foo!
Expand Down Expand Up @@ -171,7 +171,7 @@
rack (= 1.0)
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
foo!
Expand All @@ -197,7 +197,7 @@
foo (1.0)
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
foo
Expand Down
2 changes: 1 addition & 1 deletion spec/commands/check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def lock_with(bundler_version = nil)
rack (1.0.0)
PLATFORMS
#{generic_local_platform}
#{lockfile_platforms}
DEPENDENCIES
rack
Expand Down
4 changes: 2 additions & 2 deletions spec/commands/install_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@
specs:
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
Expand All @@ -425,7 +425,7 @@
specs:
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
Expand Down
16 changes: 8 additions & 8 deletions spec/commands/lock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def read_lockfile(file = "Gemfile.lock")
with_license (1.0)
PLATFORMS
#{local}
#{lockfile_platforms}
DEPENDENCIES
foo
Expand All @@ -58,7 +58,7 @@ def read_lockfile(file = "Gemfile.lock")
it "prints a lockfile when there is no existing lockfile with --print" do
bundle "lock --print"

expect(out).to include(@lockfile)
expect(out).to eq(@lockfile)
end

it "prints a lockfile when there is an existing lockfile with --print" do
Expand Down Expand Up @@ -166,13 +166,13 @@ def read_lockfile(file = "Gemfile.lock")
bundle! "lock --add-platform java x86-mingw32"

lockfile = Bundler::LockfileParser.new(read_lockfile)
expect(lockfile.platforms).to eq([java, local, mingw])
expect(lockfile.platforms).to match_array(local_platforms.unshift(java, mingw).uniq)
end

it "supports adding the `ruby` platform" do
bundle! "lock --add-platform ruby"
lockfile = Bundler::LockfileParser.new(read_lockfile)
expect(lockfile.platforms).to eq([local, "ruby"].uniq)
expect(lockfile.platforms).to match_array(local_platforms.unshift("ruby").uniq)
end

it "warns when adding an unknown platform" do
Expand All @@ -184,17 +184,17 @@ def read_lockfile(file = "Gemfile.lock")
bundle! "lock --add-platform java x86-mingw32"

lockfile = Bundler::LockfileParser.new(read_lockfile)
expect(lockfile.platforms).to eq([java, local, mingw])
expect(lockfile.platforms).to match_array(local_platforms.unshift(java, mingw).uniq)

bundle! "lock --remove-platform java"

lockfile = Bundler::LockfileParser.new(read_lockfile)
expect(lockfile.platforms).to eq([local, mingw])
expect(lockfile.platforms).to match_array(local_platforms.unshift(mingw).uniq)
end

it "errors when removing all platforms" do
bundle "lock --remove-platform #{local}"
expect(out).to include("Removing all platforms from the bundle is not allowed")
bundle "lock --remove-platform #{local_platforms.join(" ")}"
expect(last_command.bundler_err).to include("Removing all platforms from the bundle is not allowed")
end

# from https://github.com/bundler/bundler/issues/4896
Expand Down
6 changes: 3 additions & 3 deletions spec/commands/update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@
specs:
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
Expand All @@ -546,7 +546,7 @@
specs:
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
Expand Down Expand Up @@ -590,7 +590,7 @@
specs:
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
Expand Down
2 changes: 1 addition & 1 deletion spec/install/gemfile/platform_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
expect(the_bundle).to include_gems "nokogiri 1.4.2 JAVA", "weakling 0.0.3"
end

it "works with gems that have extra platform-specific runtime dependencies" do
it "works with gems that have extra platform-specific runtime dependencies", :bundler => "< 2" do
simulate_platform x64_mac

update_repo2 do
Expand Down
2 changes: 1 addition & 1 deletion spec/install/gems/flex_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@
specs:
PLATFORMS
ruby
#{lockfile_platforms}
DEPENDENCIES
rack
Expand Down
2 changes: 2 additions & 0 deletions spec/lock/lockfile_bundler_1_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@
PLATFORMS
#{generic_local_platform}
#{specific_local_platform}
DEPENDENCIES
rack
Expand Down Expand Up @@ -299,6 +300,7 @@
PLATFORMS
#{generic_local_platform}
#{specific_local_platform}
DEPENDENCIES
rack
Expand Down
Loading

0 comments on commit b088392

Please sign in to comment.