Commit 70dc7e9a authored by Douwe Maan's avatar Douwe Maan

Merge branch 'gitlab-git-popen' into 'master'

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

Closes gitaly#597

See merge request gitlab-org/gitlab-ce!14237
parents 2ce49b53 34eeac61
module Gitlab module Gitlab
module Git module Git
class OperationService class OperationService
include Gitlab::Git::Popen
WithBranchResult = Struct.new(:newrev, :repo_created, :branch_created) do WithBranchResult = Struct.new(:newrev, :repo_created, :branch_created) do
alias_method :repo_created?, :repo_created alias_method :repo_created?, :repo_created
alias_method :branch_created?, :branch_created alias_method :branch_created?, :branch_created
...@@ -150,7 +152,7 @@ module Gitlab ...@@ -150,7 +152,7 @@ module Gitlab
# (and have!) accidentally reset the ref to an earlier state, clobbering # (and have!) accidentally reset the ref to an earlier state, clobbering
# commits. See also https://github.com/libgit2/libgit2/issues/1534. # commits. See also https://github.com/libgit2/libgit2/issues/1534.
command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z] command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
_, status = Gitlab::Popen.popen( _, status = popen(
command, command,
repository.path) do |stdin| repository.path) do |stdin|
stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00") stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00")
......
...@@ -5,17 +5,21 @@ require 'open3' ...@@ -5,17 +5,21 @@ require 'open3'
module Gitlab module Gitlab
module Git module Git
module Popen module Popen
def popen(cmd, path) def popen(cmd, path, vars = {})
unless cmd.is_a?(Array) unless cmd.is_a?(Array)
raise "System commands must be given as an array of strings" raise "System commands must be given as an array of strings"
end end
vars = { "PWD" => path } path ||= Dir.pwd
vars['PWD'] = path
options = { chdir: path } options = { chdir: path }
@cmd_output = "" @cmd_output = ""
@cmd_status = 0 @cmd_status = 0
Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
yield(stdin) if block_given?
stdin.close
@cmd_output << stdout.read @cmd_output << stdout.read
@cmd_output << stderr.read @cmd_output << stderr.read
@cmd_status = wait_thr.value.exitstatus @cmd_status = wait_thr.value.exitstatus
......
...@@ -490,7 +490,7 @@ module Gitlab ...@@ -490,7 +490,7 @@ module Gitlab
# Not found -> ["", 0] # Not found -> ["", 0]
# Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 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 end
end end
...@@ -792,9 +792,7 @@ module Gitlab ...@@ -792,9 +792,7 @@ module Gitlab
end end
command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z] command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
message, status = Gitlab::Popen.popen( message, status = popen(command, path) do |stdin|
command,
path) do |stdin|
stdin.write(instructions.join) stdin.write(instructions.join)
end end
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
module Gitlab module Gitlab
module Git module Git
class RevList class RevList
include Gitlab::Git::Popen
attr_reader :oldrev, :newrev, :path_to_repo attr_reader :oldrev, :newrev, :path_to_repo
def initialize(path_to_repo:, newrev:, oldrev: nil) def initialize(path_to_repo:, newrev:, oldrev: nil)
...@@ -26,7 +28,7 @@ module Gitlab ...@@ -26,7 +28,7 @@ module Gitlab
private private
def execute(args) 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? unless status.zero?
raise "Got a non-zero exit code while calling out `#{args.join(' ')}`." raise "Got a non-zero exit code while calling out `#{args.join(' ')}`."
......
...@@ -5,13 +5,13 @@ describe Gitlab::Checks::ForcePush do ...@@ -5,13 +5,13 @@ describe Gitlab::Checks::ForcePush do
context "exit code checking", skip_gitaly_mock: true 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 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 expect { described_class.force_push?(project, 'oldrev', 'newrev') }.not_to raise_error
end end
it "raises a runtime error if the `popen` call to git returns a non-zero exit code" do 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) expect { described_class.force_push?(project, 'oldrev', 'newrev') }.to raise_error(RuntimeError)
end end
......
...@@ -481,7 +481,7 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -481,7 +481,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
end end
it 'raises an error if it failed' do 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 expect do
@repo.delete_refs('refs/heads/fix') @repo.delete_refs('refs/heads/fix')
......
...@@ -14,7 +14,7 @@ describe Gitlab::Git::RevList do ...@@ -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) } let(:rev_list) { described_class.new(newrev: 'newrev', path_to_repo: project.repository.path_to_repo) }
it 'calls out to `popen`' do 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, Gitlab.config.git.bin_path,
"--git-dir=#{project.repository.path_to_repo}", "--git-dir=#{project.repository.path_to_repo}",
'rev-list', 'rev-list',
...@@ -36,7 +36,7 @@ describe Gitlab::Git::RevList do ...@@ -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) } let(:rev_list) { described_class.new(oldrev: 'oldrev', newrev: 'newrev', path_to_repo: project.repository.path_to_repo) }
it 'calls out to `popen`' do 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, Gitlab.config.git.bin_path,
"--git-dir=#{project.repository.path_to_repo}", "--git-dir=#{project.repository.path_to_repo}",
'rev-list', 'rev-list',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment