Merge #7000

7000: Git based sources and custom sources deprecation r=deivid-rodriguez a=deivid-rodriguez

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

The problem was that our plan for deprecating using `git` instead of `https` for custom sources under the hood, and to deprecating those custom sources itself was unclear.

### What was your diagnosis of the problem?

My diagnosis was that we should give contradictory/confusing deprecation warnings to users, so we should do this is several steps.

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

My fix is to implement the following plan:

* In bundler 2 (next release, 2.1 for example), start using https by default for the custom sources (`:github`, `:gist`, `:bitbucket`), and deprecate setting the `github.https` setting to false.
* In bundler 3, make the setting `github.https` a noop, using `https` nevertheless, and go ahead and deprecate the sources.

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

I chose this fix because it's the friendlier path I could think of for our users.


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

6 files changed, 102 insertions, 63 deletions

diff --git a/lib/bundler/dsl.rb b/lib/bundler/dsl.rb
index c736fc9730..d16a50cb7e 100644
--- a/lib/bundler/dsl.rb
+++ b/lib/bundler/dsl.rb
@@ -290,37 +290,21 @@ module Bundler
         warn_deprecated_git_source(:github, <<-'RUBY'.strip, 'Change any "reponame" :github sources to "username/reponame".')
 "https://github.com/#{repo_name}.git"
         RUBY
-        # It would be better to use https instead of the git protocol, but this
-        # can break deployment of existing locked bundles when switching between
-        # different versions of Bundler. The change will be made in 2.0, which
-        # does not guarantee compatibility with the 1.x series.
-        #
-        # See https://github.com/bundler/bundler/pull/2569 for discussion
-        #
-        # This can be overridden by adding this code to your Gemfiles:
-        #
-        #   git_source(:github) do |repo_name|
-        #     repo_name = "#{repo_name}/#{repo_name}" unless repo_name.include?("/")
-        #     "https://github.com/#{repo_name}.git"
-        #   end
         repo_name = "#{repo_name}/#{repo_name}" unless repo_name.include?("/")
-        # TODO: 2.0 upgrade this setting to the default
         if Bundler.feature_flag.github_https?
-          Bundler::SharedHelpers.major_deprecation 2, "The `github.https` setting will be removed"
           "https://github.com/#{repo_name}.git"
         else
+          Bundler::SharedHelpers.major_deprecation 2, "Setting `github.https` to false is deprecated and won't be supported in the future."
           "git://github.com/#{repo_name}.git"
         end
       end
 
-      # TODO: 2.0 remove this deprecated git source
       git_source(:gist) do |repo_name|
         warn_deprecated_git_source(:gist, '"https://gist.github.com/#{repo_name}.git"')
 
         "https://gist.github.com/#{repo_name}.git"
       end
 
-      # TODO: 2.0 remove this deprecated git source
       git_source(:bitbucket) do |repo_name|
         warn_deprecated_git_source(:bitbucket, <<-'RUBY'.strip)
 user_name, repo_name = repo_name.split("/")
@@ -488,7 +472,6 @@ repo_name ||= user_name
     end
 
     def warn_deprecated_git_source(name, replacement, additional_message = nil)
-      # TODO: 2.0 remove deprecation
       additional_message &&= " #{additional_message}"
       replacement = if replacement.count("\n").zero?
         "{|repo_name| #{replacement} }"
@@ -496,8 +479,8 @@ repo_name ||= user_name
         "do |repo_name|\n#{replacement.to_s.gsub(/^/, "      ")}\n    end"
       end
 
-      Bundler::SharedHelpers.major_deprecation 2, <<-EOS
-The :#{name} git source is deprecated, and will be removed in Bundler 2.0.#{additional_message} Add this code to the top of your Gemfile to ensure it continues to work:
+      Bundler::SharedHelpers.major_deprecation 3, <<-EOS
+The :#{name} git source is deprecated, and will be removed in the future.#{additional_message} Add this code to the top of your Gemfile to ensure it continues to work:
 
     git_source(:#{name}) #{replacement}
 
diff --git a/lib/bundler/feature_flag.rb b/lib/bundler/feature_flag.rb
index e98ed8dabf..8b7b836e42 100644
--- a/lib/bundler/feature_flag.rb
+++ b/lib/bundler/feature_flag.rb
@@ -50,7 +50,7 @@ module Bundler
     settings_flag(:prefer_gems_rb) { bundler_2_mode? }
     settings_flag(:print_only_version_number) { bundler_2_mode? }
     settings_flag(:setup_makes_kernel_gem_public) { !bundler_2_mode? }
-    settings_flag(:skip_default_git_sources) { bundler_2_mode? }
+    settings_flag(:skip_default_git_sources) { bundler_4_mode? }
     settings_flag(:specific_platform) { bundler_2_mode? }
     settings_flag(:suppress_install_using_messages) { bundler_2_mode? }
     settings_flag(:unlock_source_unlocks_spec) { !bundler_2_mode? }
diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb
index c9294ca801..bed5c5604e 100644
--- a/lib/bundler/settings.rb
+++ b/lib/bundler/settings.rb
@@ -35,6 +35,7 @@ module Bundler
       frozen
       gem.coc
       gem.mit
+      github.https
       global_path_appends_ruby_scope
       global_gem_cache
       ignore_messages
diff --git a/spec/bundler/dsl_spec.rb b/spec/bundler/dsl_spec.rb
index ce0544a573..bc40ab94b2 100644
--- a/spec/bundler/dsl_spec.rb
+++ b/spec/bundler/dsl_spec.rb
@@ -28,24 +28,51 @@ RSpec.describe Bundler::Dsl do
     context "github_https feature flag" do
       it "is true when github.https is true" do
         bundle "config set github.https true"
-        expect(Bundler.feature_flag.github_https?).to eq "true"
+        expect(Bundler.feature_flag.github_https?).to eq true
       end
     end
 
-    context "default hosts (git, gist)", :bundler => "< 2" do
-      context "when github.https config is true" do
-        before { bundle "config set github.https true" }
-        it "converts :github to :git using https" do
-          subject.gem("sparks", :github => "indirect/sparks")
-          github_uri = "https://github.com/indirect/sparks.git"
+    shared_examples_for "the github DSL" do |protocol|
+      context "when full repo is used" do
+        let(:repo) { "indirect/sparks" }
+
+        it "converts :github to URI using #{protocol}" do
+          subject.gem("sparks", :github => repo)
+          github_uri = "#{protocol}://github.com/#{repo}.git"
           expect(subject.dependencies.first.source.uri).to eq(github_uri)
         end
       end
 
-      it "converts :github to :git" do
-        subject.gem("sparks", :github => "indirect/sparks")
-        github_uri = "git://github.com/indirect/sparks.git"
-        expect(subject.dependencies.first.source.uri).to eq(github_uri)
+      context "when shortcut repo is used" do
+        let(:repo) { "rails" }
+
+        it "converts :github to URI using #{protocol}" do
+          subject.gem("sparks", :github => repo)
+          github_uri = "#{protocol}://github.com/#{repo}/#{repo}.git"
+          expect(subject.dependencies.first.source.uri).to eq(github_uri)
+        end
+      end
+    end
+
+    context "default hosts (git, gist)" do
+      context "when github.https config is true" do
+        before { bundle "config set github.https true" }
+
+        it_behaves_like "the github DSL", "https"
+      end
+
+      context "when github.https config is false" do
+        before { bundle "config set github.https false" }
+
+        it_behaves_like "the github DSL", "git"
+      end
+
+      context "by default", :bundler => "< 2" do
+        it_behaves_like "the github DSL", "git"
+      end
+
+      context "by default", :bundler => "2" do
+        it_behaves_like "the github DSL", "https"
       end
 
       it "converts numeric :gist to :git" do
@@ -60,12 +87,6 @@ RSpec.describe Bundler::Dsl do
         expect(subject.dependencies.first.source.uri).to eq(github_uri)
       end
 
-      it "converts 'rails' to 'rails/rails'" do
-        subject.gem("rails", :github => "rails")
-        github_uri = "git://github.com/rails/rails.git"
-        expect(subject.dependencies.first.source.uri).to eq(github_uri)
-      end
-
       it "converts :bitbucket to :git" do
         subject.gem("not-really-a-gem", :bitbucket => "mcorp/flatlab-rails")
         bitbucket_uri = "https://mcorp@bitbucket.org/mcorp/flatlab-rails.git"
@@ -79,7 +100,7 @@ RSpec.describe Bundler::Dsl do
       end
     end
 
-    context "default git sources", :bundler => "2" do
+    context "default git sources", :bundler => "4" do
       it "has none" do
         expect(subject.instance_variable_get(:@git_sources)).to eq({})
       end
@@ -260,6 +281,32 @@ RSpec.describe Bundler::Dsl do
 
     describe "#github", :bundler => "2" do
       it "from github" do
+        spree_gems = %w[spree_core spree_api spree_backend]
+        subject.github "spree" do
+          spree_gems.each {|spree_gem| subject.send :gem, spree_gem }
+        end
+
+        subject.dependencies.each do |d|
+          expect(d.source.uri).to eq("https://github.com/spree/spree.git")
+        end
+      end
+    end
+
+    describe "#github", :bundler => "3" do
+      it "from github" do
+        spree_gems = %w[spree_core spree_api spree_backend]
+        subject.github "spree" do
+          spree_gems.each {|spree_gem| subject.send :gem, spree_gem }
+        end
+
+        subject.dependencies.each do |d|
+          expect(d.source.uri).to eq("https://github.com/spree/spree.git")
+        end
+      end
+    end
+
+    describe "#github", :bundler => "4" do
+      it "from github" do
         expect do
           spree_gems = %w[spree_core spree_api spree_backend]
           subject.github "spree" do
diff --git a/spec/other/major_deprecation_spec.rb b/spec/other/major_deprecation_spec.rb
index 550808103f..5175e8d4c8 100644
--- a/spec/other/major_deprecation_spec.rb
+++ b/spec/other/major_deprecation_spec.rb
@@ -169,34 +169,39 @@ RSpec.describe "major deprecations" do
     end
   end
 
-  xdescribe Bundler::Dsl do
+  describe Bundler::Dsl do
+    let(:msg) do
+      <<-EOS
+The :github git source is deprecated, and will be removed in the future. Change any "reponame" :github sources to "username/reponame". Add this code to the top of your Gemfile to ensure it continues to work:
+
+    git_source(:github) {|repo_name| "https://github.com/\#{repo_name}.git" }
+
+      EOS
+    end
+
     before do
       @rubygems = double("rubygems")
       allow(Bundler::Source::Rubygems).to receive(:new) { @rubygems }
     end
 
     context "with github gems" do
-      it "warns about the https change" do
-        msg = <<-EOS
-The :github git source is deprecated, and will be removed in Bundler 2.0. Change any "reponame" :github sources to "username/reponame". Add this code to the top of your Gemfile to ensure it continues to work:
-
-    git_source(:github) {|repo_name| "https://github.com/\#{repo_name}.git" }
+      it "warns about the https change if people are opting out" do
+        Bundler.settings.temporary "github.https" => false
+        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(3, msg)
+        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(2, "Setting `github.https` to false is deprecated and won't be supported in the future.")
+        subject.gem("sparks", :github => "indirect/sparks")
+      end
 
-        EOS
-        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(2, msg)
+      it "upgrades to https by default", :bundler => "2" do
+        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(3, msg)
         subject.gem("sparks", :github => "indirect/sparks")
+        github_uri = "https://github.com/indirect/sparks.git"
+        expect(subject.dependencies.first.source.uri).to eq(github_uri)
       end
 
-      it "upgrades to https on request" do
+      it "upgrades to https on request", :bundler => "< 2" do
         Bundler.settings.temporary "github.https" => true
-        msg = <<-EOS
-The :github git source is deprecated, and will be removed in Bundler 2.0. Change any "reponame" :github sources to "username/reponame". Add this code to the top of your Gemfile to ensure it continues to work:
-
-    git_source(:github) {|repo_name| "https://github.com/\#{repo_name}.git" }
-
-        EOS
-        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(2, msg)
-        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(2, "The `github.https` setting will be removed")
+        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(3, msg)
         subject.gem("sparks", :github => "indirect/sparks")
         github_uri = "https://github.com/indirect/sparks.git"
         expect(subject.dependencies.first.source.uri).to eq(github_uri)
@@ -207,7 +212,7 @@ The :github git source is deprecated, and will be removed in Bundler 2.0. Change
       it "warns about removal" do
         allow(Bundler.ui).to receive(:deprecate)
         msg = <<-EOS
-The :bitbucket git source is deprecated, and will be removed in Bundler 2.0. Add this code to the top of your Gemfile to ensure it continues to work:
+The :bitbucket git source is deprecated, and will be removed in the future. Add this code to the top of your Gemfile to ensure it continues to work:
 
     git_source(:bitbucket) do |repo_name|
       user_name, repo_name = repo_name.split("/")
@@ -216,7 +221,7 @@ The :bitbucket git source is deprecated, and will be removed in Bundler 2.0. Add
     end
 
         EOS
-        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(2, msg)
+        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(3, msg)
         subject.gem("not-really-a-gem", :bitbucket => "mcorp/flatlab-rails")
       end
     end
@@ -224,11 +229,13 @@ The :bitbucket git source is deprecated, and will be removed in Bundler 2.0. Add
     context "with gist gems" do
       it "warns about removal" do
         allow(Bundler.ui).to receive(:deprecate)
-        msg = "The :gist git source is deprecated, and will be removed " \
-          "in Bundler 2.0. Add this code to the top of your Gemfile to ensure it " \
-          "continues to work:\n\n    git_source(:gist) {|repo_name| " \
-          "\"https://gist.github.com/\#{repo_name}.git\" }\n\n"
-        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(2, msg)
+        msg = <<-EOS
+The :gist git source is deprecated, and will be removed in the future. Add this code to the top of your Gemfile to ensure it continues to work:
+
+    git_source(:gist) {|repo_name| "https://gist.github.com/\#{repo_name}.git" }
+
+        EOS
+        expect(Bundler::SharedHelpers).to receive(:major_deprecation).with(3, msg)
         subject.gem("not-really-a-gem", :gist => "1234")
       end
     end
diff --git a/spec/quality_spec.rb b/spec/quality_spec.rb
index 6477a03fe4..3ddc775fae 100644
--- a/spec/quality_spec.rb
+++ b/spec/quality_spec.rb
@@ -172,6 +172,7 @@ RSpec.describe "The library itself" do
       forget_cli_options
       gem.coc
       gem.mit
+      github.https
       inline
       lockfile_uses_separate_rubygems_sources
       use_gem_version_promoter_for_major_updates