From 0bcf7492d2d633be5b1d71f976f74216b37e08ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 13 Oct 2020 18:34:00 +0200 Subject: [PATCH 1/2] Enable `specific_platform` by default To properly support gems providing native platform versions with different dependencies than their pure ruby counterparts. Nokogiri will start doing this soon. In particular, for each version, the pure ruby variant will not have a `required_ruby_version` upper bound, but the native variant will. Bundler should always install a valid bundle in this case. --- bundler/lib/bundler/definition.rb | 5 +- bundler/lib/bundler/feature_flag.rb | 1 - bundler/lib/bundler/lazy_specification.rb | 8 +-- bundler/lib/bundler/man/bundle-config.1.ronn | 8 --- bundler/lib/bundler/settings.rb | 1 - bundler/man/bundle-config.1 | 3 - bundler/spec/install/gemfile/gemspec_spec.rb | 2 - bundler/spec/install/gemfile/platform_spec.rb | 26 --------- .../install/gemfile/specific_platform_spec.rb | 6 +- bundler/spec/install/gems/resolving_spec.rb | 8 +-- bundler/spec/lock/lockfile_spec.rb | 33 +---------- bundler/spec/plugins/source/example_spec.rb | 56 +------------------ bundler/spec/support/platforms.rb | 6 +- bundler/test_gems.rb.lock | 1 + 14 files changed, 9 insertions(+), 155 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 9760887eeadd..1b03676d079f 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -559,10 +559,7 @@ def add_current_platform end def current_platforms - [].tap do |platforms| - platforms << local_platform if Bundler.feature_flag.specific_platform? - platforms << generic_local_platform - end + [local_platform, generic_local_platform] end def change_reason diff --git a/bundler/lib/bundler/feature_flag.rb b/bundler/lib/bundler/feature_flag.rb index fd76d9e5ba91..a92ec8756d9e 100644 --- a/bundler/lib/bundler/feature_flag.rb +++ b/bundler/lib/bundler/feature_flag.rb @@ -41,7 +41,6 @@ def self.settings_method(name, key, &default) settings_flag(:plugins) { @bundler_version >= Gem::Version.new("1.14") } settings_flag(:print_only_version_number) { bundler_3_mode? } settings_flag(:setup_makes_kernel_gem_public) { !bundler_3_mode? } - settings_flag(:specific_platform) { bundler_3_mode? } settings_flag(:suppress_install_using_messages) { bundler_3_mode? } settings_flag(:unlock_source_unlocks_spec) { !bundler_3_mode? } settings_flag(:update_requires_all_flag) { bundler_4_mode? } diff --git a/bundler/lib/bundler/lazy_specification.rb b/bundler/lib/bundler/lazy_specification.rb index 279d00afa743..22bf0f3f6f0b 100644 --- a/bundler/lib/bundler/lazy_specification.rb +++ b/bundler/lib/bundler/lazy_specification.rb @@ -82,7 +82,7 @@ def __materialize__ search_object = if source.is_a?(Source::Path) Dependency.new(name, version) else - Bundler.feature_flag.specific_platform? || Bundler.settings[:force_ruby_platform] ? self : Dependency.new(name, version) + self end platform_object = Gem::Platform.new(platform) candidates = source.specs.search(search_object) @@ -90,12 +90,6 @@ def __materialize__ MatchPlatform.platforms_match?(spec.platform, platform_object) end search = same_platform_candidates.last || candidates.last - if search && Gem::Platform.new(search.platform) != platform_object && !search.runtime_dependencies.-(dependencies.reject {|d| d.type == :development }).empty? - Bundler.ui.warn "Unable to use the platform-specific (#{search.platform}) version of #{name} (#{version}) " \ - "because it has different dependencies from the #{platform} version. " \ - "To use the platform-specific version of the gem, run `bundle config set --local specific_platform true` and install again." - search = source.specs.search(self).last - end search.dependencies = dependencies if search && (search.is_a?(RemoteSpecification) || search.is_a?(EndpointSpecification)) search end diff --git a/bundler/lib/bundler/man/bundle-config.1.ronn b/bundler/lib/bundler/man/bundle-config.1.ronn index 0bab01af903d..462edf784449 100644 --- a/bundler/lib/bundler/man/bundle-config.1.ronn +++ b/bundler/lib/bundler/man/bundle-config.1.ronn @@ -250,14 +250,6 @@ learn more about their operation in [bundle install(1)](bundle-install.1.html). be changed in the next major version. * `silence_root_warning` (`BUNDLE_SILENCE_ROOT_WARNING`): Silence the warning Bundler prints when installing gems as root. -* `specific_platform` (`BUNDLE_SPECIFIC_PLATFORM`): - Allow bundler to resolve for the specific running platform and store it in - the lockfile, instead of only using a generic platform. - A specific platform is the exact platform triple reported by - `Gem::Platform.local`, such as `x86_64-darwin-16` or `universal-java-1.8`. - On the other hand, generic platforms are those such as `ruby`, `mswin`, or - `java`. In this example, `x86_64-darwin-16` would map to `ruby` and - `universal-java-1.8` to `java`. * `ssl_ca_cert` (`BUNDLE_SSL_CA_CERT`): Path to a designated CA certificate file or folder containing multiple certificates for trusted CAs in PEM format. diff --git a/bundler/lib/bundler/settings.rb b/bundler/lib/bundler/settings.rb index 6ce81b5006ec..1c669491f64f 100644 --- a/bundler/lib/bundler/settings.rb +++ b/bundler/lib/bundler/settings.rb @@ -42,7 +42,6 @@ class Settings setup_makes_kernel_gem_public silence_deprecations silence_root_warning - specific_platform suppress_install_using_messages unlock_source_unlocks_spec update_requires_all_flag diff --git a/bundler/man/bundle-config.1 b/bundler/man/bundle-config.1 index d9f2e3b7942e..bf63e388ffb8 100644 --- a/bundler/man/bundle-config.1 +++ b/bundler/man/bundle-config.1 @@ -262,9 +262,6 @@ The following is a list of all configuration keys and their purpose\. You can le \fBsilence_root_warning\fR (\fBBUNDLE_SILENCE_ROOT_WARNING\fR): Silence the warning Bundler prints when installing gems as root\. . .IP "\(bu" 4 -\fBspecific_platform\fR (\fBBUNDLE_SPECIFIC_PLATFORM\fR): Allow bundler to resolve for the specific running platform and store it in the lockfile, instead of only using a generic platform\. A specific platform is the exact platform triple reported by \fBGem::Platform\.local\fR, such as \fBx86_64\-darwin\-16\fR or \fBuniversal\-java\-1\.8\fR\. On the other hand, generic platforms are those such as \fBruby\fR, \fBmswin\fR, or \fBjava\fR\. In this example, \fBx86_64\-darwin\-16\fR would map to \fBruby\fR and \fBuniversal\-java\-1\.8\fR to \fBjava\fR\. -. -.IP "\(bu" 4 \fBssl_ca_cert\fR (\fBBUNDLE_SSL_CA_CERT\fR): Path to a designated CA certificate file or folder containing multiple certificates for trusted CAs in PEM format\. . .IP "\(bu" 4 diff --git a/bundler/spec/install/gemfile/gemspec_spec.rb b/bundler/spec/install/gemfile/gemspec_spec.rb index 766b62ec3188..b41a083d9298 100644 --- a/bundler/spec/install/gemfile/gemspec_spec.rb +++ b/bundler/spec/install/gemfile/gemspec_spec.rb @@ -422,8 +422,6 @@ end end - bundle "config specific_platform false" - %w[ruby jruby].each do |platform| simulate_platform(platform) do install_gemfile <<-G diff --git a/bundler/spec/install/gemfile/platform_spec.rb b/bundler/spec/install/gemfile/platform_spec.rb index 729b6e0106fd..1a3794dffed2 100644 --- a/bundler/spec/install/gemfile/platform_spec.rb +++ b/bundler/spec/install/gemfile/platform_spec.rb @@ -255,32 +255,6 @@ 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", :bundler => "< 3" do - simulate_platform x64_mac - - update_repo2 do - build_gem "facter", "2.4.6" - build_gem "facter", "2.4.6" do |s| - s.platform = "universal-darwin" - s.add_runtime_dependency "CFPropertyList" - end - build_gem "CFPropertyList" - end - - install_gemfile <<-G - source "#{file_uri_for(gem_repo2)}" - - gem "facter" - G - - expect(err).to include "Unable to use the platform-specific (universal-darwin) version of facter (2.4.6) " \ - "because it has different dependencies from the ruby version. " \ - "To use the platform-specific version of the gem, run `bundle config set --local specific_platform true` and install again." - - expect(the_bundle).to include_gem "facter 2.4.6" - expect(the_bundle).not_to include_gem "CFPropertyList" - end - it "works with gems with platform-specific dependency having different requirements order" do simulate_platform x64_mac diff --git a/bundler/spec/install/gemfile/specific_platform_spec.rb b/bundler/spec/install/gemfile/specific_platform_spec.rb index 42127cc9e0ec..82492e78cbd5 100644 --- a/bundler/spec/install/gemfile/specific_platform_spec.rb +++ b/bundler/spec/install/gemfile/specific_platform_spec.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true -RSpec.describe "bundle install with specific_platform enabled" do - before do - bundle "config set specific_platform true" - end - +RSpec.describe "bundle install with specific platforms" do let(:google_protobuf) { <<-G } source "#{file_uri_for(gem_repo2)}" gem "google-protobuf" diff --git a/bundler/spec/install/gems/resolving_spec.rb b/bundler/spec/install/gems/resolving_spec.rb index 307376f11918..1c01ce588bdc 100644 --- a/bundler/spec/install/gems/resolving_spec.rb +++ b/bundler/spec/install/gems/resolving_spec.rb @@ -239,13 +239,7 @@ let(:ruby_requirement) { %("#{RUBY_VERSION}") } let(:error_message_requirement) { "~> #{RUBY_VERSION}.0" } - let(:error_message_platform) do - if Bundler.feature_flag.specific_platform? - " #{Bundler.local_platform}" - else - "" - end - end + let(:error_message_platform) { " #{Bundler.local_platform}" } shared_examples_for "ruby version conflicts" do it "raises an error during resolution" do diff --git a/bundler/spec/lock/lockfile_spec.rb b/bundler/spec/lock/lockfile_spec.rb index f8fc2b1d6cb8..bcae748227e5 100644 --- a/bundler/spec/lock/lockfile_spec.rb +++ b/bundler/spec/lock/lockfile_spec.rb @@ -981,38 +981,7 @@ G end - it "persists the spec's platform to the lockfile", :bundler => "< 3" do - build_repo2 do - build_gem "platform_specific", "1.0" do |s| - s.platform = Gem::Platform.new("universal-java-16") - end - end - - simulate_platform "universal-java-16" - - install_gemfile <<-G - source "#{file_uri_for(gem_repo2)}" - gem "platform_specific" - G - - lockfile_should_be <<-G - GEM - remote: #{file_uri_for(gem_repo2)}/ - specs: - platform_specific (1.0-java) - - PLATFORMS - java - - DEPENDENCIES - platform_specific - - BUNDLED WITH - #{Bundler::VERSION} - G - end - - it "persists the spec's platform and specific platform to the lockfile", :bundler => "3" do + it "persists the spec's platform and specific platform to the lockfile" do build_repo2 do build_gem "platform_specific", "1.0" do |s| s.platform = Gem::Platform.new("universal-java-16") diff --git a/bundler/spec/plugins/source/example_spec.rb b/bundler/spec/plugins/source/example_spec.rb index c3f7c7214a6a..e2bab9c1999d 100644 --- a/bundler/spec/plugins/source/example_spec.rb +++ b/bundler/spec/plugins/source/example_spec.rb @@ -67,32 +67,7 @@ def install(spec, opts) expect(the_bundle).to include_gems("a-path-gem 1.0") end - it "writes to lock file", :bundler => "< 3" do - bundle "install" - - lockfile_should_be <<-G - PLUGIN SOURCE - remote: #{lib_path("a-path-gem-1.0")} - type: mpath - specs: - a-path-gem (1.0) - - GEM - remote: #{file_uri_for(gem_repo2)}/ - specs: - - PLATFORMS - #{generic_local_platform} - - DEPENDENCIES - a-path-gem! - - BUNDLED WITH - #{Bundler::VERSION} - G - end - - it "writes to lock file", :bundler => "3" do + it "writes to lock file" do bundle "install" lockfile_should_be <<-G @@ -363,34 +338,7 @@ def installed? expect(the_bundle).to include_gems("ma-gitp-gem 1.0") end - it "writes to lock file", :bundler => "< 3" do - revision = revision_for(lib_path("ma-gitp-gem-1.0")) - bundle "install" - - lockfile_should_be <<-G - PLUGIN SOURCE - remote: #{file_uri_for(lib_path("ma-gitp-gem-1.0"))} - type: gitp - revision: #{revision} - specs: - ma-gitp-gem (1.0) - - GEM - remote: #{file_uri_for(gem_repo2)}/ - specs: - - PLATFORMS - #{generic_local_platform} - - DEPENDENCIES - ma-gitp-gem! - - BUNDLED WITH - #{Bundler::VERSION} - G - end - - it "writes to lock file", :bundler => "3" do + it "writes to lock file" do revision = revision_for(lib_path("ma-gitp-gem-1.0")) bundle "install" diff --git a/bundler/spec/support/platforms.rb b/bundler/spec/support/platforms.rb index a6ebd7510fb4..ab203919e71c 100644 --- a/bundler/spec/support/platforms.rb +++ b/bundler/spec/support/platforms.rb @@ -94,11 +94,7 @@ def lockfile_platforms end def local_platforms - if Bundler.feature_flag.specific_platform? - [local, specific_local_platform] - else - [local] - end + [local, specific_local_platform].uniq end end end diff --git a/bundler/test_gems.rb.lock b/bundler/test_gems.rb.lock index 0cd61430213c..95145a74a594 100644 --- a/bundler/test_gems.rb.lock +++ b/bundler/test_gems.rb.lock @@ -25,6 +25,7 @@ PLATFORMS java ruby x64-mingw32 + x86_64-linux DEPENDENCIES artifice (~> 0.6.0) From 91cfcfe0d28eac75d7611c7caa78c20b967fb742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 13 Oct 2020 20:48:28 +0200 Subject: [PATCH 2/2] Probably better fix to previous issue With specific platforms by default, it sounds like we can fix a previous "duplicated spec groups" issue by calling uniq on the array of definition platforms. --- bundler/lib/bundler/definition.rb | 2 +- bundler/lib/bundler/resolver.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 1b03676d079f..de5e11d50f87 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -559,7 +559,7 @@ def add_current_platform end def current_platforms - [local_platform, generic_local_platform] + [local_platform, generic_local_platform].uniq end def change_reason diff --git a/bundler/lib/bundler/resolver.rb b/bundler/lib/bundler/resolver.rb index 3a68d7579362..327c170fbe3d 100644 --- a/bundler/lib/bundler/resolver.rb +++ b/bundler/lib/bundler/resolver.rb @@ -158,9 +158,8 @@ def search_for(dependency) # spec group. sg_ruby = sg.copy_for(Gem::Platform::RUBY) selected_sgs << sg_ruby if sg_ruby - all_platforms = @platforms + [platform] - next if all_platforms.to_a == [Gem::Platform::RUBY] sg_all_platforms = nil + all_platforms = @platforms + [platform] self.class.sort_platforms(all_platforms).reverse_each do |other_platform| if sg_all_platforms.nil? sg_all_platforms = sg.copy_for(other_platform)