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

Add config variable and check for platform warnings #6309

Merged
merged 4 commits into from Mar 1, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/bundler/definition.rb
Expand Up @@ -878,7 +878,7 @@ def expand_dependencies(dependencies, remote = false)
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?
if platforms.empty? && !Bundler.settings[:disable_platform_warnings]
mapped_platforms = dep.platforms.map {|p| Dependency::PLATFORM_MAP[p] }
Bundler.ui.warn \
"The dependency #{dep} will be unused by any of the platforms Bundler is installing for. " \
Expand Down
1 change: 1 addition & 0 deletions lib/bundler/settings.rb
Expand Up @@ -26,6 +26,7 @@ class Settings
disable_exec_load
disable_local_branch_check
disable_multisource
disable_platform_warnings
disable_shared_gems
disable_version_check
error_on_stderr
Expand Down
2 changes: 2 additions & 0 deletions man/bundle-config.ronn
Expand Up @@ -166,6 +166,8 @@ learn more about their operation in [bundle install(1)][bundle-install(1)].
When set, Gemfiles containing multiple sources will produce errors
instead of warnings.
Use `bundle config --delete disable_multisource` to unset.
* `disable_platform_warnings` (`BUNDLE_DISABLE_PLATFORM_WARNINGS`):
Whether Bundler should show platform warnings.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would reword this to be a bit more explicit, something like:

Disable warnings during bundle install when a dependency is unused on the current platform

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 😄

* `disable_shared_gems` (`BUNDLE_DISABLE_SHARED_GEMS`):
Stop Bundler from accessing gems installed to RubyGems' normal location.
* `disable_version_check` (`BUNDLE_DISABLE_VERSION_CHECK`):
Expand Down
47 changes: 36 additions & 11 deletions spec/install/gemfile/platform_spec.rb
Expand Up @@ -230,21 +230,46 @@
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
simulate_platform "ruby"
simulate_ruby_engine "ruby"
context "when disable_platform_warnings is false (by default)" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't need a context blocked wrapping around it as the setting is set in Bundler automatically.

before { bundle! "config disable_platform_warnings false" }

gemfile <<-G
source "file://#{gem_repo1}"
it "prints a helpful warning when a dependency is unused on any platform" do
simulate_platform "ruby"
simulate_ruby_engine "ruby"

gem "rack", :platform => [:mingw, :mswin, :x64_mingw, :jruby]
G
gemfile <<-G
source "file://#{gem_repo1}"

gem "rack", :platform => [:mingw, :mswin, :x64_mingw, :jruby]
G

bundle! "install"
bundle! "install"

expect(out).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
expect(out).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
end

context "when disable_platform_warnings is true" do
before { bundle! "config disable_platform_warnings true" }

it "doesnot print the warning when a dependency is unused on any platform" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does not

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad!

simulate_platform "ruby"
simulate_ruby_engine "ruby"

gemfile <<-G
source "file://#{gem_repo1}"

gem "rack", :platform => [:mingw, :mswin, :x64_mingw, :jruby]
G

bundle! "install"

expect(out).not_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`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot to match exactly against in the output which could easily be missed if we change the test or behavior. I would match with:

expect(out).to_not include(/The dependency (.*) will be unused/)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Will change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, using include is throwing a TypeError

TypeError:
       no implicit conversion of Regexp into String

How about using match?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that was a typo on my part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using

expect(out).not_to match(/The dependency (.*) will be unused/)

does the trick. Is it good?

O
end
end
end

Expand Down