diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-11-21 18:29:18 +0100 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-11-26 16:44:26 +0100 |
commit | 67f2faa72d53096bf71cbe126b5b6d0830d42886 (patch) | |
tree | e31e7486e5da779542a3346dedff3134b0c44f55 | |
parent | 5e9921d547501fc8be2ebd16b63eec712154db10 (diff) | |
download | gitlab-ce-67f2faa72d53096bf71cbe126b5b6d0830d42886.tar.gz |
Don't specify `shell` for Gitaly write-ref
Use shelling out to git to write refs instead of rugged, hoping to
avoid creating invalid refs.
To update HEAD we switched to using `git symbolic-ref`.
-rw-r--r-- | app/models/project.rb | 2 | ||||
-rw-r--r-- | app/models/repository.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/bvl-use-shell-writeref.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/repository_service.rb | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 13 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 6 |
7 files changed, 24 insertions, 13 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 4d1917b9ab2..b85ec90f3ca 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1394,7 +1394,7 @@ class Project < ActiveRecord::Base def change_head(branch) if repository.branch_exists?(branch) repository.before_change_head - repository.raw_repository.write_ref('HEAD', "refs/heads/#{branch}", shell: false) + repository.raw_repository.write_ref('HEAD', "refs/heads/#{branch}") repository.copy_gitattributes(branch) repository.after_change_head reload_default_branch diff --git a/app/models/repository.rb b/app/models/repository.rb index a77fa8f2ce7..427dac99b79 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -259,7 +259,7 @@ class Repository next if kept_around?(sha) # This will still fail if the file is corrupted (e.g. 0 bytes) - raw_repository.write_ref(keep_around_ref_name(sha), sha, shell: false) + raw_repository.write_ref(keep_around_ref_name(sha), sha) rescue Gitlab::Git::CommandError => ex Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}" end diff --git a/changelogs/unreleased/bvl-use-shell-writeref.yml b/changelogs/unreleased/bvl-use-shell-writeref.yml new file mode 100644 index 00000000000..682d428e8c5 --- /dev/null +++ b/changelogs/unreleased/bvl-use-shell-writeref.yml @@ -0,0 +1,5 @@ +--- +title: Avoid creating invalid refs using rugged, shelling out for writing refs +merge_request: 23286 +author: +type: fixed diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 9b48031b2d3..0a541031884 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -723,11 +723,11 @@ module Gitlab delete_refs(tmp_ref) end - def write_ref(ref_path, ref, old_ref: nil, shell: true) + def write_ref(ref_path, ref, old_ref: nil) ref_path = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref_path}" unless ref_path.start_with?("refs/") || ref_path == "HEAD" wrapped_gitaly_errors do - gitaly_repository_client.write_ref(ref_path, ref, old_ref, shell) + gitaly_repository_client.write_ref(ref_path, ref, old_ref) end end diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index c0044d7addb..8a1abfbf874 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -251,12 +251,11 @@ module Gitlab ) end - def write_ref(ref_path, ref, old_ref, shell) + def write_ref(ref_path, ref, old_ref) request = Gitaly::WriteRefRequest.new( repository: @gitaly_repo, ref: ref_path.b, - revision: ref.b, - shell: shell + revision: ref.b ) request.old_revision = old_ref.b unless old_ref.nil? diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 1fe73c12fc0..852ee9c96af 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1469,6 +1469,19 @@ describe Gitlab::Git::Repository, :seed_helper do end end end + + it 'writes the HEAD' do + repository.write_ref('HEAD', 'refs/heads/feature') + + expect(repository.commit('HEAD')).to eq(repository.commit('feature')) + expect(repository.root_ref).to eq('feature') + end + + it 'writes other refs' do + repository.write_ref('refs/heads/feature', SeedRepo::Commit::ID) + + expect(repository.commit('feature').sha).to eq(SeedRepo::Commit::ID) + end end describe '#write_config' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 51278836604..d23bdbc2c30 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2203,12 +2203,6 @@ describe Project do project.change_head(project.default_branch) end - it 'creates the new reference with rugged' do - expect(project.repository.raw_repository).to receive(:write_ref).with('HEAD', "refs/heads/#{project.default_branch}", shell: false) - - project.change_head(project.default_branch) - end - it 'copies the gitattributes' do expect(project.repository).to receive(:copy_gitattributes).with(project.default_branch) project.change_head(project.default_branch) |