Use Gitlab::Git's Popen on that module's code

This allows the current Gitaly migration to depend on less code outside
of the Gitlab::Git module


Former-commit-id: b2ff24de0df2fed729c0c15a91f863687a58b35f

lib/gitlab/git/operation_service.rb

 module Gitlab
   module Git
     class OperationService
+      include Gitlab::Git::Popen
+
       attr_reader :user, :repository
       def initialize(user, new_repository)
@@ -147,7 +149,7 @@ module Gitlab
         # (and have!) accidentally reset the ref to an earlier state, clobbering
         # commits. See also https://github.com/libgit2/libgit2/issues/1534.
         command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
-        _, status = Gitlab::Popen.popen(
+        _, status = popen(
           command,
           repository.path) do |stdin|
           stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00")

lib/gitlab/git/popen.rb

@@ -5,17 +5,21 @@ require 'open3'
 module Gitlab
   module Git
     module Popen
-      def popen(cmd, path)
+      def popen(cmd, path, vars = {})
         unless cmd.is_a?(Array)
           raise "System commands must be given as an array of strings"
         end
-        vars = { "PWD" => path }
+        path ||= Dir.pwd
+        vars['PWD'] = path
         options = { chdir: path }
         @cmd_output = ""
         @cmd_status = 0
         Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+          yield(stdin) if block_given?
+          stdin.close
+
           @cmd_output << stdout.read
           @cmd_output << stderr.read
           @cmd_status = wait_thr.value.exitstatus

lib/gitlab/git/repository.rb

@@ -489,7 +489,7 @@ module Gitlab
             # Not found -> ["", 0]
             # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]
-            Gitlab::Popen.popen(args, @path).first.split.last
+            popen(args, @path).first.split.last
           end
         end
       end
@@ -672,9 +672,7 @@ module Gitlab
         end
         command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
-        message, status = Gitlab::Popen.popen(
-          command,
-          path) do |stdin|
+        message, status = popen(command, path) do |stdin|
           stdin.write(instructions.join)
         end

lib/gitlab/git/rev_list.rb

@@ -3,6 +3,8 @@
 module Gitlab
   module Git
     class RevList
+      include Gitlab::Git::Popen
+
       attr_reader :oldrev, :newrev, :path_to_repo
       def initialize(path_to_repo:, newrev:, oldrev: nil)
@@ -26,7 +28,7 @@ module Gitlab
       private
       def execute(args)
-        output, status = Gitlab::Popen.popen(args, nil, Gitlab::Git::Env.all.stringify_keys)
+        output, status = popen(args, nil, Gitlab::Git::Env.all.stringify_keys)
         unless status.zero?
           raise "Got a non-zero exit code while calling out `#{args.join(' ')}`."

spec/lib/gitlab/checks/force_push_spec.rb

@@ -5,13 +5,13 @@ describe Gitlab::Checks::ForcePush do
   context "exit code checking", skip_gitaly_mock: true do
     it "does not raise a runtime error if the `popen` call to git returns a zero exit code" do
-      allow(Gitlab::Popen).to receive(:popen).and_return(['normal output', 0])
+      allow_any_instance_of(Gitlab::Git::RevList).to receive(:popen).and_return(['normal output', 0])
       expect { described_class.force_push?(project, 'oldrev', 'newrev') }.not_to raise_error
     end
     it "raises a runtime error if the `popen` call to git returns a non-zero exit code" do
-      allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1])
+      allow_any_instance_of(Gitlab::Git::RevList).to receive(:popen).and_return(['error', 1])
       expect { described_class.force_push?(project, 'oldrev', 'newrev') }.to raise_error(RuntimeError)
     end

spec/lib/gitlab/git/repository_spec.rb

@@ -481,7 +481,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
     it 'raises an error if it failed' do
-      expect(Gitlab::Popen).to receive(:popen).and_return(['Error', 1])
+      expect(@repo).to receive(:popen).and_return(['Error', 1])
       expect do
         @repo.delete_refs('refs/heads/fix')

spec/lib/gitlab/git/rev_list_spec.rb

@@ -14,7 +14,7 @@ describe Gitlab::Git::RevList do
     let(:rev_list) { described_class.new(newrev: 'newrev', path_to_repo: project.repository.path_to_repo) }
     it 'calls out to `popen`' do
-      expect(Gitlab::Popen).to receive(:popen).with([
+      expect(rev_list).to receive(:popen).with([
         Gitlab.config.git.bin_path,
         "--git-dir=#{project.repository.path_to_repo}",
         'rev-list',
@@ -36,7 +36,7 @@ describe Gitlab::Git::RevList do
     let(:rev_list) { described_class.new(oldrev: 'oldrev', newrev: 'newrev', path_to_repo: project.repository.path_to_repo) }
     it 'calls out to `popen`' do
-      expect(Gitlab::Popen).to receive(:popen).with([
+      expect(rev_list).to receive(:popen).with([
         Gitlab.config.git.bin_path,
         "--git-dir=#{project.repository.path_to_repo}",
         'rev-list',