Merge #7006

7006: Fix sticky options deprecation r=deivid-rodriguez a=deivid-rodriguez

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

The problem was that deprecating sticky options is currently "useless" since we are removing the flags where the stickyness makes a difference at the same time. So, we don't help users adapt to (and understand) our changes. 

### What was your diagnosis of the problem?

My diagnosis was that we should do this change in two steps, so that first users get a explanation about why we are removing the flags and what is the preferred alternative for them, and once nobody is no longer using these flags, we deprecate sticky options in general, since they are surprising and no longer useful, since we don't depend on them. 

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

My fix is to:

* In bundler 2 (next release, 2.1 for example), print a deprecation message to warn about the removal of all the flags that currently depend on the options being remembered.

* In bundler 3, remove the options and deprecate remembering command line flags.

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

I chose this fix because it properly and gradually informs our users about the changes that we intend to make and why we intend to make them.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

4 files changed, 85 insertions, 24 deletions

diff --git a/lib/bundler/cli.rb b/lib/bundler/cli.rb
index affd7a78c1..6aad0fc822 100644
--- a/lib/bundler/cli.rb
+++ b/lib/bundler/cli.rb
@@ -61,11 +61,6 @@ module Bundler
       end
     end
 
-    def self.deprecated_option(*args, &blk)
-      return if Bundler.feature_flag.forget_cli_options?
-      method_option(*args, &blk)
-    end
-
     check_unknown_options!(:except => [:config, :exec])
     stop_on_unknown_option! :exec
 
@@ -142,7 +137,7 @@ module Bundler
       Gemfile to a gem with a gemspec, the --gemspec option will automatically add each
       dependency listed in the gemspec file to the newly created Gemfile.
     D
-    deprecated_option "gemspec", :type => :string, :banner => "Use the specified .gemspec to create the Gemfile"
+    method_option "gemspec", :type => :string, :banner => "Use the specified .gemspec to create the Gemfile"
     def init
       require "bundler/cli/init"
       Init.new(options.dup).run
@@ -188,13 +183,13 @@ module Bundler
 
       If the bundle has already been installed, bundler will tell you so and then exit.
     D
-    deprecated_option "binstubs", :type => :string, :lazy_default => "bin", :banner =>
+    method_option "binstubs", :type => :string, :lazy_default => "bin", :banner =>
       "Generate bin stubs for bundled gems to ./bin"
-    deprecated_option "clean", :type => :boolean, :banner =>
+    method_option "clean", :type => :boolean, :banner =>
       "Run bundle clean automatically after install"
-    deprecated_option "deployment", :type => :boolean, :banner =>
+    method_option "deployment", :type => :boolean, :banner =>
       "Install using defaults tuned for deployment environments"
-    deprecated_option "frozen", :type => :boolean, :banner =>
+    method_option "frozen", :type => :boolean, :banner =>
       "Do not allow the Gemfile.lock to be updated after this install"
     method_option "full-index", :type => :boolean, :banner =>
       "Fall back to using the single-file index of all gems"
@@ -204,32 +199,37 @@ module Bundler
       "Specify the number of jobs to run in parallel"
     method_option "local", :type => :boolean, :banner =>
       "Do not attempt to fetch gems remotely and use the gem cache instead"
-    deprecated_option "no-cache", :type => :boolean, :banner =>
+    method_option "no-cache", :type => :boolean, :banner =>
       "Don't update the existing gem cache."
     method_option "redownload", :type => :boolean, :aliases => "--force", :banner =>
       "Force downloading every gem."
-    deprecated_option "no-prune", :type => :boolean, :banner =>
+    method_option "no-prune", :type => :boolean, :banner =>
       "Don't remove stale gems from the cache."
-    deprecated_option "path", :type => :string, :banner =>
+    method_option "path", :type => :string, :banner =>
       "Specify a different path than the system default ($BUNDLE_PATH or $GEM_HOME). Bundler will remember this value for future installs on this machine"
     method_option "quiet", :type => :boolean, :banner =>
       "Only output warnings and errors."
-    deprecated_option "shebang", :type => :string, :banner =>
+    method_option "shebang", :type => :string, :banner =>
       "Specify a different shebang executable name than the default (usually 'ruby')"
     method_option "standalone", :type => :array, :lazy_default => [], :banner =>
       "Make a bundle that can work without the Bundler runtime"
-    deprecated_option "system", :type => :boolean, :banner =>
+    method_option "system", :type => :boolean, :banner =>
       "Install to the system location ($BUNDLE_PATH or $GEM_HOME) even if the bundle was previously installed somewhere else for this application"
     method_option "trust-policy", :alias => "P", :type => :string, :banner =>
       "Gem trust policy (like gem install -P). Must be one of " +
         Bundler.rubygems.security_policy_keys.join("|")
-    deprecated_option "without", :type => :array, :banner =>
+    method_option "without", :type => :array, :banner =>
       "Exclude gems that are part of the specified named group."
-    deprecated_option "with", :type => :array, :banner =>
+    method_option "with", :type => :array, :banner =>
       "Include gems that are part of the specified named group."
     map "i" => "install"
     def install
       SharedHelpers.major_deprecation(2, "The `--force` option has been renamed to `--redownload`") if ARGV.include?("--force")
+
+      %w[clean deployment frozen no-cache no-prune path shebang system without with].each do |option|
+        remembered_flag_deprecation(option)
+      end
+
       require "bundler/cli/install"
       Bundler.settings.temporary(:no_install => false) do
         Install.new(options.dup).run
@@ -786,5 +786,22 @@ module Bundler
     rescue RuntimeError
       nil
     end
+
+    def remembered_flag_deprecation(name)
+      option = current_command.options[name]
+      flag_name = option.switch_name
+
+      name_index = ARGV.find {|arg| flag_name == arg }
+      return unless name_index
+
+      value = options[name]
+      value = value.join(" ").to_s if option.type == :array
+
+      Bundler::SharedHelpers.major_deprecation 2,\
+        "The `#{flag_name}` flag is deprecated because it relied on being " \
+        "remembered accross bundler invokations, which bundler will no longer " \
+        "do in future versions. Instead please use `bundle config #{name} " \
+        "'#{value}'`, and stop using this flag"
+    end
   end
 end
diff --git a/lib/bundler/feature_flag.rb b/lib/bundler/feature_flag.rb
index 8b7b836e42..e01676f97d 100644
--- a/lib/bundler/feature_flag.rb
+++ b/lib/bundler/feature_flag.rb
@@ -38,7 +38,7 @@ module Bundler
     settings_flag(:deployment_means_frozen) { bundler_2_mode? }
     settings_flag(:disable_multisource) { bundler_2_mode? }
     settings_flag(:error_on_stderr) { bundler_2_mode? }
-    settings_flag(:forget_cli_options) { bundler_2_mode? }
+    settings_flag(:forget_cli_options) { bundler_3_mode? }
     settings_flag(:global_path_appends_ruby_scope) { bundler_2_mode? }
     settings_flag(:global_gem_cache) { bundler_2_mode? }
     settings_flag(:init_gems_rb) { bundler_2_mode? }
diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb
index bed5c5604e..c708659e1c 100644
--- a/lib/bundler/settings.rb
+++ b/lib/bundler/settings.rb
@@ -116,11 +116,19 @@ module Bundler
           "bundle config set #{key} #{Array(value).join(":")}"
         end
 
-        Bundler::SharedHelpers.major_deprecation 2,\
-          "flags passed to commands " \
-          "will no longer be automatically remembered. Instead please set flags " \
-          "you want remembered between commands using `bundle config set " \
-          "<setting name> <setting value>`, i.e. `#{command}`"
+        unless Bundler.settings[:forget_cli_options] == false
+          Bundler::SharedHelpers.major_deprecation 3,\
+            "flags passed to commands " \
+            "will no longer be automatically remembered. Instead please set flags " \
+            "you want remembered between commands using `bundle config set " \
+            "<setting name> <setting value>`, i.e. `#{command}`. Once you are " \
+            "ready for the new behavior, use `bundle config set forget_cli_options " \
+            "true` to get rid of this message. Or if you want to get rid of " \
+            "this message and stick with the old behavior for now, run " \
+            "`bundle config set forget_cli_options false`, but keep in mind " \
+            "that this behavior will be fully removed in future versions of " \
+            "bundler."
+        end
 
         set_local(key, value)
       end
diff --git a/spec/other/major_deprecation_spec.rb b/spec/other/major_deprecation_spec.rb
index 5175e8d4c8..de9c05ec67 100644
--- a/spec/other/major_deprecation_spec.rb
+++ b/spec/other/major_deprecation_spec.rb
@@ -111,7 +111,7 @@ RSpec.describe "major deprecations" do
     end
 
     context "with flags" do
-      xit "should print a deprecation warning about autoremembering flags" do
+      it "should print a deprecation warning about autoremembering flags", :bundler => "3" do
         install_gemfile <<-G, :path => "vendor/bundle"
           source "file://#{gem_repo1}"
           gem "rack"
@@ -121,6 +121,42 @@ RSpec.describe "major deprecations" do
           "flags passed to commands will no longer be automatically remembered."
         )
       end
+
+      {
+        :clean => true,
+        :deployment => true,
+        :frozen => true,
+        :"no-cache" => true,
+        :"no-prune" => true,
+        :path => "vendor/bundle",
+        :shebang => "ruby27",
+        :system => true,
+        :without => "development",
+        :with => "development",
+      }.each do |name, value|
+        flag_name = "--#{name}"
+
+        context "with the #{flag_name} flag", :bundler => "2" do
+          it "should print a deprecation warning" do
+            bundle "install #{flag_name} #{value}"
+
+            expect(warnings).to have_major_deprecation(
+              "The `#{flag_name}` flag is deprecated because it relied on " \
+              "being remembered accross bundler invokations, which bundler " \
+              "will no longer do in future versions. Instead please use " \
+              "`bundle config #{name} '#{value}'`, and stop using this flag"
+            )
+          end
+        end
+
+        context "with the #{flag_name} flag", :bundler => "< 2" do
+          it "should not print a deprecation warning" do
+            bundle "install #{flag_name} #{value}"
+
+            expect(warnings).not_to have_major_deprecation
+          end
+        end
+      end
     end
   end