Fix git source lockfile unstability

We have some flags that limit running git commit commands under certain
situations, for example, when running under `--local`. However, those
should only affect remote git operations, not local read-only operations
like `git --version`, or `git rev-parse --abbrev-ref HEAD`.

This commit refactors things to achieve that.

By doing this, the `#to_s` representation of a source is more
consistent, since we don't get any errors when reading the checked out
branch, and we avoid some flip-flop lockfile issues.

bundler/lib/bundler/source/git/git_proxy.rb

@@ -67,8 +67,8 @@ def revision
         end
 
         def current_branch
-          @current_branch ||= allowed_with_path do
-            git("rev-parse", "--abbrev-ref", "HEAD", :dir => path).strip
+          @current_branch ||= with_path do
+            git_local("rev-parse", "--abbrev-ref", "HEAD", :dir => path).strip
           end
         end
 
@@ -84,7 +84,7 @@ def version
         end
 
         def full_version
-          @full_version ||= git("--version").sub(/git version\s*/, "").strip
+          @full_version ||= git_local("--version").sub(/git version\s*/, "").strip
         end
 
         def checkout
@@ -253,15 +253,15 @@ def git_retry(*command, dir: nil)
         end
 
         def git(*command, dir: nil)
-          command_with_no_credentials = check_allowed(command)
-
-          out, err, status = capture(command, dir)
-
-          raise GitCommandError.new(command_with_no_credentials, dir || SharedHelpers.pwd, err) unless status.success?
-
-          Bundler.ui.warn err unless err.empty?
+          run_command(*command, :dir => dir) do |unredacted_command|
+            check_allowed(unredacted_command)
+          end
+        end
 
-          out
+        def git_local(*command, dir: nil)
+          run_command(*command, :dir => dir) do |unredacted_command|
+            redact_and_check_presence(unredacted_command)
+          end
         end
 
         def has_revision_cached?
@@ -330,12 +330,30 @@ def allowed_with_path
         end
 
         def check_allowed(command)
-          require "shellwords"
-          command_with_no_credentials = URICredentialsFilter.credential_filtered_string("git #{command.shelljoin}", uri)
+          command_with_no_credentials = redact_and_check_presence(command)
           raise GitNotAllowedError.new(command_with_no_credentials) unless allow?
           command_with_no_credentials
         end
 
+        def redact_and_check_presence(command)
+          raise GitNotInstalledError.new unless Bundler.git_present?
+
+          require "shellwords"
+          URICredentialsFilter.credential_filtered_string("git #{command.shelljoin}", uri)
+        end
+
+        def run_command(*command, dir: nil)
+          command_with_no_credentials = yield(command)
+
+          out, err, status = capture(command, dir)
+
+          raise GitCommandError.new(command_with_no_credentials, dir || SharedHelpers.pwd, err) unless status.success?
+
+          Bundler.ui.warn err unless err.empty?
+
+          out
+        end
+
         def capture(cmd, dir, ignore_err: false)
           SharedHelpers.with_clean_git_env do
             require "open3"

bundler/spec/bundler/env_spec.rb

@@ -217,7 +217,7 @@ def with_clear_paths(env_var, env_value)
 
     context "when the git version is OS specific" do
       it "includes OS specific information with the version number" do
-        expect(git_proxy_stub).to receive(:git).with("--version").
+        expect(git_proxy_stub).to receive(:git_local).with("--version").
           and_return("git version 1.2.3 (Apple Git-BS)")
         expect(Bundler::Source::Git::GitProxy).to receive(:new).and_return(git_proxy_stub)
 

bundler/spec/bundler/source/git/git_proxy_spec.rb

@@ -13,23 +13,23 @@
   context "with configured credentials" do
     it "adds username and password to URI" do
       Bundler.settings.temporary(uri => "u:p") do
-        allow(subject).to receive(:git).with("--version").and_return("git version 2.14.0")
+        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
         expect(subject).to receive(:capture).with([*base_clone_args, "--", "https://u:p@github.com/rubygems/rubygems.git", path.to_s], nil).and_return(["", "", clone_result])
         subject.checkout
       end
     end
 
     it "adds username and password to URI for host" do
       Bundler.settings.temporary("github.com" => "u:p") do
-        allow(subject).to receive(:git).with("--version").and_return("git version 2.14.0")
+        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
         expect(subject).to receive(:capture).with([*base_clone_args, "--", "https://u:p@github.com/rubygems/rubygems.git", path.to_s], nil).and_return(["", "", clone_result])
         subject.checkout
       end
     end
 
     it "does not add username and password to mismatched URI" do
       Bundler.settings.temporary("https://u:p@github.com/rubygems/rubygems-mismatch.git" => "u:p") do
-        allow(subject).to receive(:git).with("--version").and_return("git version 2.14.0")
+        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
         expect(subject).to receive(:capture).with([*base_clone_args, "--", uri, path.to_s], nil).and_return(["", "", clone_result])
         subject.checkout
       end
@@ -39,7 +39,7 @@
       Bundler.settings.temporary("github.com" => "u:p") do
         original = "https://orig:info@github.com/rubygems/rubygems.git"
         subject = described_class.new(Pathname("path"), original, "HEAD")
-        allow(subject).to receive(:git).with("--version").and_return("git version 2.14.0")
+        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
         expect(subject).to receive(:capture).with([*base_clone_args, "--", original, path.to_s], nil).and_return(["", "", clone_result])
         subject.checkout
       end
@@ -49,7 +49,7 @@
   describe "#version" do
     context "with a normal version number" do
       before do
-        expect(subject).to receive(:git).with("--version").
+        expect(subject).to receive(:git_local).with("--version").
           and_return("git version 1.2.3")
       end
 
@@ -64,7 +64,7 @@
 
     context "with a OSX version number" do
       before do
-        expect(subject).to receive(:git).with("--version").
+        expect(subject).to receive(:git_local).with("--version").
           and_return("git version 1.2.3 (Apple Git-BS)")
       end
 
@@ -79,7 +79,7 @@
 
     context "with a msysgit version number" do
       before do
-        expect(subject).to receive(:git).with("--version").
+        expect(subject).to receive(:git_local).with("--version").
           and_return("git version 1.2.3.msysgit.0")
       end
 
@@ -96,7 +96,7 @@
   describe "#full_version" do
     context "with a normal version number" do
       before do
-        expect(subject).to receive(:git).with("--version").
+        expect(subject).to receive(:git_local).with("--version").
           and_return("git version 1.2.3")
       end
 
@@ -107,7 +107,7 @@
 
     context "with a OSX version number" do
       before do
-        expect(subject).to receive(:git).with("--version").
+        expect(subject).to receive(:git_local).with("--version").
           and_return("git version 1.2.3 (Apple Git-BS)")
       end
 
@@ -118,7 +118,7 @@
 
     context "with a msysgit version number" do
       before do
-        expect(subject).to receive(:git).with("--version").
+        expect(subject).to receive(:git_local).with("--version").
           and_return("git version 1.2.3.msysgit.0")
       end