summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBundlerbot <bot@bundler.io>2020-02-27 22:00:32 +0000
committerBundlerbot <bot@bundler.io>2020-02-27 22:00:32 +0000
commit788a4071cf4e0b42f83e25ba2aedaf0b63546866 (patch)
tree3c214dbecf85e94ed499e6c842ec01b5197a7f32
parent0659182ff24faed7b4dc8479f1c056fae32815e4 (diff)
parenta123d2aec2ad70122a24e2dc96268051811c9987 (diff)
downloadbundler-788a4071cf4e0b42f83e25ba2aedaf0b63546866.tar.gz
Merge #7580
7580: Automultiplatform again r=indirect a=deivid-rodriguez This PR is a reintroduction of #7215, now that I consider the multiplatform feature usable enough. ### What was the end-user problem that led to this PR? The problem was that currently if the gemfile include gems for specific platforms, like the default `gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]` Rails uses, bundler will always print a noisy warning, but I don't think users are doing anything wrong, just running `bundle install` on it. Also, unless they run `bundle lock --add-platform x86-mingw32 x86-mswin32 x64-mingw32 java`, and commit the resulting lockfile, they will continue to get the same warning over and over again. ### What was your diagnosis of the problem? My diagnosis is that the fact that some gems will be unused under some platforms is inherent to the multiplatform feature itself, so we shouldn't warn about it because it's expected. Take the following Gemfile for example (a simplification of the default Gemfile Rails generators create): ```ruby source "https://rubygems.org" gem "rails" gem "tzinfo-data", platforms: "jruby" ``` If I type that Gemfile, it means that I'm explicitly opting into the multiplatform feature. So the behavior I would want as a user when I run `bundle install` on it is: * Resolve and lock the Gemfile for all platforms (`jruby` and whatever platform I'm running). * Install the resolution for the platform that I'm currently running. That way, when the other developers of my team using `jruby` install the `Gemfile`, they install a predictable, deterministic resolution. Currently, the only way to get that behavior is to run: ``` $ bundle install $ bundle lock --add-platform java ``` But there's no way to do it without getting a warning on the first `bundle install`, which annoys people because it makes you think you're doing something wrong. If you only plan to use MRI, you shouldn't specify any jruby-specific gems in your Gemfile and write instead: ```ruby source "https://rubygems.org" gem "rails" ``` If on the other hand you only plan to use jruby, then you should not specify any `platform` option at all and write ```ruby source "https://rubygems.org" gem "rails" gem "tzinfo-data" ``` So, to sum up, I think the range of platforms users expect to support can be guessed implicit from the Gemfile and the running platform. So we should resolve by default for all those platforms and don't warn about behavior that's most likely expected. ### What is your fix for the problem, implemented in this PR? My fix is to do the right thing by default, and not warn users at all. That is: * Resolve the gemfile and lock it for all platforms present in the Gemfile. * Install gems for the current platform if requested. ### Why did you choose this fix out of the possible options? I chose this fix because I think it's better for our users. We currently have a specific setting to avoid this warning, which I guess we added given the complaints. We no longer need that setting, nor the warning, so I removed both. Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
-rw-r--r--lib/bundler/cli/outdated.rb6
-rw-r--r--lib/bundler/definition.rb24
-rw-r--r--lib/bundler/inline.rb2
-rw-r--r--lib/bundler/settings.rb1
-rw-r--r--man/bundle-config.13
-rw-r--r--man/bundle-config.1.txt4
-rw-r--r--man/bundle-config.ronn2
-rw-r--r--spec/install/gemfile/platform_spec.rb37
-rw-r--r--spec/runtime/require_spec.rb2
9 files changed, 32 insertions, 49 deletions
diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb
index 3d4922f8b5..5f065654b1 100644
--- a/lib/bundler/cli/outdated.rb
+++ b/lib/bundler/cli/outdated.rb
@@ -76,6 +76,8 @@ module Bundler
next unless gems.empty? || gems.include?(current_spec.name)
active_spec = retrieve_active_spec(definition, current_spec)
+ next unless active_spec
+
next unless filter_options_patch.empty? || update_present_via_semver_portions(current_spec, active_spec, options)
gem_outdated = Gem::Version.new(active_spec.version) > Gem::Version.new(current_spec.version)
@@ -144,6 +146,8 @@ module Bundler
end
def retrieve_active_spec(definition, current_spec)
+ return unless current_spec.match_platform(Bundler.local_platform)
+
if strict
active_spec = definition.find_resolved_spec(current_spec)
else
@@ -231,8 +235,6 @@ module Bundler
end
def update_present_via_semver_portions(current_spec, active_spec, options)
- return false if active_spec.nil?
-
current_major = current_spec.version.segments.first
active_major = active_spec.version.segments.first
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index efd7618194..fca7d35aaa 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -117,7 +117,7 @@ module Bundler
end
@unlocking ||= @unlock[:ruby] ||= (!@locked_ruby_version ^ !@ruby_version)
- add_current_platform unless Bundler.frozen_bundle?
+ add_platforms unless Bundler.frozen_bundle?
converge_path_sources_to_gemspec_sources
@path_changes = converge_paths
@@ -523,10 +523,6 @@ module Bundler
raise InvalidOption, "Unable to remove the platform `#{platform}` since the only platforms are #{@platforms.join ", "}"
end
- def add_current_platform
- current_platforms.each {|platform| add_platform(platform) }
- end
-
def find_resolved_spec(current_spec)
specs.find_by_name_and_platform(current_spec.name, current_spec.platform)
end
@@ -548,6 +544,12 @@ module Bundler
private
+ def add_platforms
+ (@dependencies.flat_map(&:expanded_platforms) + current_platforms).uniq.each do |platform|
+ add_platform(platform)
+ end
+ end
+
def current_platforms
current_platform = Bundler.local_platform
[].tap do |platforms|
@@ -892,17 +894,7 @@ module Bundler
dependencies.each do |dep|
dep = Dependency.new(dep, ">= 0") unless dep.respond_to?(:name)
next if !remote && !dep.current_platform?
- platforms = dep.gem_platforms(sorted_platforms)
- if platforms.empty? && !Bundler.settings[:disable_platform_warnings]
- mapped_platforms = dep.expanded_platforms
- Bundler.ui.warn \
- "The dependency #{dep} will be unused by any of the platforms Bundler is installing for. " \
- "Bundler is installing for #{@platforms.join ", "} but the dependency " \
- "is only for #{mapped_platforms.join ", "}. " \
- "To add those platforms to the bundle, " \
- "run `bundle lock --add-platform #{mapped_platforms.join " "}`."
- end
- platforms.each do |p|
+ dep.gem_platforms(sorted_platforms).each do |p|
deps << DepProxy.new(dep, p) if remote || p == generic_local_platform
end
end
diff --git a/lib/bundler/inline.rb b/lib/bundler/inline.rb
index f1f77a7a9c..59211193d4 100644
--- a/lib/bundler/inline.rb
+++ b/lib/bundler/inline.rb
@@ -58,7 +58,7 @@ def gemfile(install = false, options = {}, &gemfile)
Bundler.ui = install ? ui : Bundler::UI::Silent.new
if install || definition.missing_specs?
- Bundler.settings.temporary(:inline => true, :disable_platform_warnings => true) do
+ Bundler.settings.temporary(:inline => true) do
installer = Bundler::Installer.install(Bundler.root, definition, :system => true)
installer.post_install_messages.each do |name, message|
Bundler.ui.info "Post-install message from #{name}:\n#{message}"
diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb
index afbb02397c..23ae66efa0 100644
--- a/lib/bundler/settings.rb
+++ b/lib/bundler/settings.rb
@@ -22,7 +22,6 @@ module Bundler
disable_exec_load
disable_local_branch_check
disable_multisource
- disable_platform_warnings
disable_shared_gems
disable_version_check
force_ruby_platform
diff --git a/man/bundle-config.1 b/man/bundle-config.1
index 58be36c5c6..9376693f7c 100644
--- a/man/bundle-config.1
+++ b/man/bundle-config.1
@@ -187,9 +187,6 @@ The following is a list of all configuration keys and their purpose\. You can le
\fBdisable_multisource\fR (\fBBUNDLE_DISABLE_MULTISOURCE\fR): When set, Gemfiles containing multiple sources will produce errors instead of warnings\. Use \fBbundle config unset disable_multisource\fR to unset\.
.
.IP "\(bu" 4
-\fBdisable_platform_warnings\fR (\fBBUNDLE_DISABLE_PLATFORM_WARNINGS\fR): Disable warnings during bundle install when a dependency is unused on the current platform\.
-.
-.IP "\(bu" 4
\fBdisable_shared_gems\fR (\fBBUNDLE_DISABLE_SHARED_GEMS\fR): Stop Bundler from accessing gems installed to RubyGems\' normal location\.
.
.IP "\(bu" 4
diff --git a/man/bundle-config.1.txt b/man/bundle-config.1.txt
index d6632792e5..135028fbe5 100644
--- a/man/bundle-config.1.txt
+++ b/man/bundle-config.1.txt
@@ -214,10 +214,6 @@ LIST OF AVAILABLE KEYS
files containing multiple sources will produce errors instead of
warnings. Use bundle config unset disable_multisource to unset.
- o disable_platform_warnings (BUNDLE_DISABLE_PLATFORM_WARNINGS): Dis-
- able warnings during bundle install when a dependency is unused on
- the current platform.
-
o disable_shared_gems (BUNDLE_DISABLE_SHARED_GEMS): Stop Bundler from
accessing gems installed to RubyGems' normal location.
diff --git a/man/bundle-config.ronn b/man/bundle-config.ronn
index 6b0e044cd3..b241bba505 100644
--- a/man/bundle-config.ronn
+++ b/man/bundle-config.ronn
@@ -181,8 +181,6 @@ learn more about their operation in [bundle install(1)](bundle-install.1.html).
When set, Gemfiles containing multiple sources will produce errors
instead of warnings.
Use `bundle config unset disable_multisource` to unset.
-* `disable_platform_warnings` (`BUNDLE_DISABLE_PLATFORM_WARNINGS`):
- Disable warnings during bundle install when a dependency is unused on the current platform.
* `disable_shared_gems` (`BUNDLE_DISABLE_SHARED_GEMS`):
Stop Bundler from accessing gems installed to RubyGems' normal location.
* `disable_version_check` (`BUNDLE_DISABLE_VERSION_CHECK`):
diff --git a/spec/install/gemfile/platform_spec.rb b/spec/install/gemfile/platform_spec.rb
index 52e1cf86fa..c637eeee0c 100644
--- a/spec/install/gemfile/platform_spec.rb
+++ b/spec/install/gemfile/platform_spec.rb
@@ -378,7 +378,7 @@ RSpec.describe "bundle install with platform conditionals" do
expect(out).not_to match(/Could not find gem 'some_gem/)
end
- it "prints a helpful warning when a dependency is unused on any platform" do
+ it "resolves all platforms by default and without warning messages" do
simulate_platform "ruby"
simulate_ruby_engine "ruby"
@@ -390,28 +390,27 @@ RSpec.describe "bundle install with platform conditionals" do
bundle! "install"
- expect(err).to include <<-O.strip
-The dependency #{Gem::Dependency.new("rack", ">= 0")} will be unused by any of the platforms Bundler is installing for. Bundler is installing for ruby but the dependency is only for x86-mingw32, x86-mswin32, x64-mingw32, java. To add those platforms to the bundle, run `bundle lock --add-platform x86-mingw32 x86-mswin32 x64-mingw32 java`.
- O
- end
-
- context "when disable_platform_warnings is true" do
- before { bundle! "config set disable_platform_warnings true" }
+ expect(err).to be_empty
- it "does not print the warning when a dependency is unused on any platform" do
- simulate_platform "ruby"
- simulate_ruby_engine "ruby"
-
- gemfile <<-G
- source "#{file_uri_for(gem_repo1)}"
+ lockfile_should_be <<-L
+ GEM
+ remote: #{file_uri_for(gem_repo1)}/
+ specs:
+ rack (1.0.0)
- gem "rack", :platform => [:mingw, :mswin, :x64_mingw, :jruby]
- G
+ PLATFORMS
+ java
+ ruby
+ x64-mingw32
+ x86-mingw32
+ x86-mswin32
- bundle! "install"
+ DEPENDENCIES
+ rack
- expect(out).not_to match(/The dependency (.*) will be unused/)
- end
+ BUNDLED WITH
+ #{Bundler::VERSION}
+ L
end
end
diff --git a/spec/runtime/require_spec.rb b/spec/runtime/require_spec.rb
index a2e6ba7244..3b9021b4fc 100644
--- a/spec/runtime/require_spec.rb
+++ b/spec/runtime/require_spec.rb
@@ -423,7 +423,7 @@ RSpec.describe "Bundler.require with platform specific dependencies" do
source "#{file_uri_for(gem_repo1)}"
platforms :#{not_local_tag} do
- gem "fail", :require => "omgomg"
+ gem "platform_specific", :require => "omgomg"
end
gem "rack", "1.0.0"