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

Commit

Permalink
Merge #6965
Browse files Browse the repository at this point in the history
6965: Turn on deprecations by default r=indirect a=deivid-rodriguez

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

The problem was we've had deprecations in place for a long time, but we've never displayed them to users by default. 

### What was your diagnosis of the problem?

My diagnosis was that the current strategy doesn't work because:

* Printing deprecations as an optin feature almost never get enabled, so most users don't know about the stuff we'll be deprecating in the future.

* Printing deprecations in "deprecation releases" doesn't work well either, because it's unclear how long the deprecation release should last and thus how long we need to hold the final release (that will inhibit installation of the deprecation release).

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

My fix is to remove the concept of deprecation releases, and to add a feature flag for printing major deprecations that it's enabled by default.

As a extra related change, I also reworded the deprecation messages, because I find the current message "[DEPRECATED FOR 2.0] <Message about the deprecation>" a bit confusing because it's unclear what the version printed is referring to (deprecation horizon? current running version?), so I changed it to just "[DEPRECATED] <Message about the deprecation>".

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

I chose this fix because it (once released) finally makes it so that users will know about our deprecations.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
  • Loading branch information
bundlerbot and deivid-rodriguez committed Feb 22, 2019
2 parents 6ce5322 + fda9db6 commit 552d095
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 55 deletions.
3 changes: 2 additions & 1 deletion lib/bundler/settings.rb
Expand Up @@ -41,7 +41,6 @@ class Settings
init_gems_rb
list_command
lockfile_uses_separate_rubygems_sources
major_deprecations
no_install
no_prune
only_update_to_newer_versions
Expand All @@ -52,6 +51,7 @@ class Settings
prefer_patch
print_only_version_number
setup_makes_kernel_gem_public
silence_deprecations
silence_root_warning
skip_default_git_sources
specific_platform
Expand All @@ -76,6 +76,7 @@ class Settings
].freeze

DEFAULT_CONFIG = {
:silence_deprecations => false,
:disable_version_check => true,
:prefer_patch => false,
:redirect => 5,
Expand Down
9 changes: 4 additions & 5 deletions lib/bundler/shared_helpers.rb
Expand Up @@ -144,13 +144,13 @@ def major_deprecation(major_version, message)
bundler_major_version = Bundler.bundler_major_version
if bundler_major_version > major_version
require "bundler/errors"
raise DeprecatedError, "[REMOVED FROM #{major_version.succ}.0] #{message}"
raise DeprecatedError, "[REMOVED] #{message}"
end

return unless bundler_major_version >= major_version || prints_major_deprecations?
return unless bundler_major_version >= major_version && prints_major_deprecations?
@major_deprecation_ui ||= Bundler::UI::Shell.new("no-color" => true)
ui = Bundler.ui.is_a?(@major_deprecation_ui.class) ? Bundler.ui : @major_deprecation_ui
ui.warn("[DEPRECATED FOR #{major_version}.0] #{message}")
ui.warn("[DEPRECATED] #{message}")
end

def print_major_deprecations!
Expand Down Expand Up @@ -374,8 +374,7 @@ def resolve_path(path)

def prints_major_deprecations?
require "bundler"
deprecation_release = Bundler::VERSION.split(".").drop(1).include?("99")
return false if !deprecation_release && !Bundler.settings[:major_deprecations]
return false if Bundler.settings[:silence_deprecations]
require "bundler/deprecate"
return false if Bundler::Deprecate.skip
true
Expand Down
6 changes: 3 additions & 3 deletions man/bundle-config.ronn
Expand Up @@ -210,9 +210,6 @@ learn more about their operation in [bundle install(1)](bundle-install.1.html).
The number of gems Bundler can install in parallel. Defaults to 1.
* `list_command` (`BUNDLE_LIST_COMMAND`)
Enable new list command feature
* `major_deprecations` (`BUNDLE_MAJOR_DEPRECATIONS`):
Whether Bundler should print deprecation warnings for behavior that will
be changed in the next major version.
* `no_install` (`BUNDLE_NO_INSTALL`):
Whether `bundle package` should skip installing gems.
* `no_prune` (`BUNDLE_NO_PRUNE`):
Expand Down Expand Up @@ -247,6 +244,9 @@ learn more about their operation in [bundle install(1)](bundle-install.1.html).
* `shebang` (`BUNDLE_SHEBANG`):
The program name that should be invoked for generated binstubs. Defaults to
the ruby install name used to generate the binstub.
* `silence_deprecations` (`BUNDLE_SILENCE_DEPRECATIONS`):
Whether Bundler should silence deprecation warnings for behavior that will
be changed in the next major version.
* `silence_root_warning` (`BUNDLE_SILENCE_ROOT_WARNING`):
Silence the warning Bundler prints when installing gems as root.
* `skip_default_git_sources` (`BUNDLE_SKIP_DEFAULT_GIT_SOURCES`):
Expand Down
8 changes: 4 additions & 4 deletions spec/commands/show_spec.rb
Expand Up @@ -32,7 +32,7 @@

it "prints deprecation", :bundler => "2" do
bundle "show rails"
expect(err).to eq("[DEPRECATED FOR 2.0] use `bundle info rails` instead of `bundle show rails`")
expect(err).to eq("[DEPRECATED] use `bundle info rails` instead of `bundle show rails`")
end

it "prints path if gem exists in bundle (with --paths option)" do
Expand All @@ -42,7 +42,7 @@

it "prints deprecation when called with a gem and the --paths option", :bundler => "2" do
bundle "show rails --paths"
expect(err).to eq("[DEPRECATED FOR 2.0] use `bundle info rails --path` instead of `bundle show rails --paths`")
expect(err).to eq("[DEPRECATED] use `bundle info rails --path` instead of `bundle show rails --paths`")
end

it "warns if path no longer exists on disk" do
Expand All @@ -61,7 +61,7 @@

it "prints deprecation when called with bundler", :bundler => "2" do
bundle "show bundler"
expect(err).to eq("[DEPRECATED FOR 2.0] use `bundle info bundler` instead of `bundle show bundler`")
expect(err).to eq("[DEPRECATED] use `bundle info bundler` instead of `bundle show bundler`")
end
it "complains if gem not in bundle" do
bundle "show missing"
Expand All @@ -82,7 +82,7 @@
it "prints a deprecation when called with the --paths option", :bundler => 2 do
bundle "show --paths"

expect(err).to eq("[DEPRECATED FOR 2.0] use `bundle list` instead of `bundle show --paths`")
expect(err).to eq("[DEPRECATED] use `bundle list` instead of `bundle show --paths`")
end

it "prints summary of gems" do
Expand Down
3 changes: 0 additions & 3 deletions spec/install/gemfile/sources_spec.rb
Expand Up @@ -25,7 +25,6 @@
gem "rack-obama"
gem "rack"
G
bundle "config set major_deprecations true"
end

it "warns about ambiguous gems, but installs anyway, prioritizing sources last to first", :bundler => "< 2" do
Expand Down Expand Up @@ -55,7 +54,6 @@
gem "rack-obama"
gem "rack", "1.0.0" # force it to install the working version in repo1
G
bundle "config set major_deprecations true"
end

it "warns about ambiguous gems, but installs anyway", :bundler => "< 2" do
Expand Down Expand Up @@ -249,7 +247,6 @@
end

it "installs from the other source and warns about ambiguous gems", :bundler => "< 2" do
bundle "config set major_deprecations true"
bundle :install
expect(out).to have_major_deprecation a_string_including("Your Gemfile contains multiple primary sources.")
expect(out).to include("Warning: the gem 'rack' was found in multiple sources.")
Expand Down
12 changes: 6 additions & 6 deletions spec/install/redownload_spec.rb
Expand Up @@ -63,22 +63,22 @@

it "shows a deprecation when single flag passed", :bundler => 2 do
bundle! "install --force"
expect(err).to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "shows a deprecation when multiple flags passed", :bundler => 2 do
bundle! "install --no-color --force"
expect(err).to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "does not show a deprecation when single flag passed", :bundler => "< 2" do
bundle! "install --force"
expect(out).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(out).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "does not show a deprecation when multiple flags passed", :bundler => "< 2" do
bundle! "install --no-color --force"
expect(out).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(out).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end
end

Expand All @@ -89,12 +89,12 @@

it "does not show a deprecation when single flag passed" do
bundle! "install --redownload"
expect(err).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "does not show a deprecation when single multiple flags passed" do
bundle! "install --no-color --redownload"
expect(err).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end
end
end
18 changes: 0 additions & 18 deletions spec/other/major_deprecation_spec.rb
Expand Up @@ -4,25 +4,7 @@
let(:warnings) { last_command.bundler_err } # change to err in 2.0
let(:warnings_without_version_messages) { warnings.gsub(/#{Spec::Matchers::MAJOR_DEPRECATION}Bundler will only support ruby(gems)? >= .*/, "") }

context "in a .99 version" do
before do
simulate_bundler_version "1.99.1"
bundle "config unset major_deprecations"
end

it "prints major deprecations without being configured" do
ruby <<-R
require "bundler"
Bundler::SharedHelpers.major_deprecation(Bundler::VERSION)
R

expect(warnings).to have_major_deprecation("1.99.1")
end
end

before do
bundle "config set major_deprecations true"

create_file "gems.rb", <<-G
source "file:#{gem_repo1}"
ruby #{RUBY_VERSION.dump}
Expand Down
16 changes: 8 additions & 8 deletions spec/runtime/with_unbundled_env_spec.rb
Expand Up @@ -106,17 +106,17 @@ def build_bundler_context
it "prints a deprecation", :bundler => 2 do
code = "Bundler.clean_env"
bundle_exec_ruby! code.dump
expect(last_command.stdboth).to include(
"[DEPRECATED FOR 2.0] `Bundler.clean_env` has been deprecated in favor of `Bundler.unbundled_env`. " \
expect(err).to include(
"[DEPRECATED] `Bundler.clean_env` has been deprecated in favor of `Bundler.unbundled_env`. " \
"If you instead want the environment before bundler was originally loaded, use `Bundler.original_env`"
)
end

it "does not print a deprecation", :bundler => "< 2" do
code = "Bundler.clean_env"
bundle_exec_ruby! code.dump
expect(last_command.stdboth).not_to include(
"[DEPRECATED FOR 2.0] `Bundler.clean_env` has been deprecated in favor of `Bundler.unbundled_env`. " \
expect(out).not_to include(
"[DEPRECATED] `Bundler.clean_env` has been deprecated in favor of `Bundler.unbundled_env`. " \
"If you instead want the environment before bundler was originally loaded, use `Bundler.original_env`"
)
end
Expand Down Expand Up @@ -156,17 +156,17 @@ def build_bundler_context
it "prints a deprecation", :bundler => 2 do
code = "Bundler.with_clean_env {}"
bundle_exec_ruby! code.dump
expect(last_command.stdboth).to include(
"[DEPRECATED FOR 2.0] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. " \
expect(err).to include(
"[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. " \
"If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env`"
)
end

it "does not print a deprecation", :bundler => "< 2" do
code = "Bundler.with_clean_env {}"
bundle_exec_ruby! code.dump
expect(last_command.stdboth).not_to include(
"[DEPRECATED FOR 2.0] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. " \
expect(out).not_to include(
"[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. " \
"If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env`"
)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/support/matchers.rb
Expand Up @@ -60,7 +60,7 @@ def self.define_compound_matcher(matcher, preconditions, &declarations)
end
end

MAJOR_DEPRECATION = /^\[DEPRECATED FOR 2\.0\]\s*/
MAJOR_DEPRECATION = /^\[DEPRECATED\]\s*/

RSpec::Matchers.define :eq_err do |expected|
diffable
Expand Down
12 changes: 6 additions & 6 deletions spec/update/redownload_spec.rb
Expand Up @@ -11,34 +11,34 @@
describe "with --force" do
it "shows a deprecation when single flag passed", :bundler => 2 do
bundle! "update rack --force"
expect(err).to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "shows a deprecation when multiple flags passed", :bundler => 2 do
bundle! "update rack --no-color --force"
expect(err).to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "does not show a deprecation when single flag passed", :bundler => "< 2" do
bundle! "update rack --force"
expect(out).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(out).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "does not show a deprecation when multiple flags passed", :bundler => "< 2" do
bundle! "update rack --no-color --force"
expect(out).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(out).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end
end

describe "with --redownload" do
it "does not show a deprecation when single flag passed" do
bundle! "update rack --redownload"
expect(out).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end

it "does not show a deprecation when single multiple flags passed" do
bundle! "update rack --no-color --redownload"
expect(out).not_to include "[DEPRECATED FOR 2.0] The `--force` option has been renamed to `--redownload`"
expect(err).not_to include "[DEPRECATED] The `--force` option has been renamed to `--redownload`"
end
end
end

0 comments on commit 552d095

Please sign in to comment.