diff options
author | Nick Thomas <nick@gitlab.com> | 2018-03-07 02:02:24 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-03-07 02:02:24 +0000 |
commit | 0649a1f8359930b84d7295272b1f8e32c32c0d2c (patch) | |
tree | 65783a2593cf0286d02650e3f26f666001937339 | |
parent | ff00cfe45c744393de5bfcafdcfbd12108cc664f (diff) | |
parent | e9fad3e501f6c8fa7ebc58011e5bf9fff379617e (diff) | |
download | gitlab-ce-0649a1f8359930b84d7295272b1f8e32c32c0d2c.tar.gz |
Merge branch 'sh-make-prune-optional-in-git-fetch' into 'master'43962-add-wip-to-merge-requests-titles-by-default43958-include-adding-a-specific-runner-to-another-project-from-the-admin-area-to-docs
Make --prune a configurable parameter in fetching a git remote
See merge request gitlab-org/gitlab-ce!17346
-rw-r--r-- | app/models/repository.rb | 8 | ||||
-rw-r--r-- | changelogs/unreleased/sh-make-prune-optional-in-git-fetch.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/git/gitlab_projects.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/repository_service.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/shell.rb | 10 | ||||
-rw-r--r-- | spec/lib/gitlab/git/gitlab_projects_spec.rb | 20 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/repository_service_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/shell_spec.rb | 32 |
8 files changed, 68 insertions, 30 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb index 1a14afb951a..e6b88320110 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -866,20 +866,20 @@ class Repository raw_repository.ancestor?(ancestor_id, descendant_id) end - def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil) + def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, prune: true) unless remote_name remote_name = "tmp-#{SecureRandom.hex}" tmp_remote_name = true end add_remote(remote_name, url, mirror_refmap: refmap) - fetch_remote(remote_name, forced: forced) + fetch_remote(remote_name, forced: forced, prune: prune) ensure remove_remote(remote_name) if tmp_remote_name end - def fetch_remote(remote, forced: false, ssh_auth: nil, no_tags: false) - gitlab_shell.fetch_remote(raw_repository, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) + def fetch_remote(remote, forced: false, ssh_auth: nil, no_tags: false, prune: true) + gitlab_shell.fetch_remote(raw_repository, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, prune: prune) end def fetch_source_branch!(source_repository, source_branch, local_ref) diff --git a/changelogs/unreleased/sh-make-prune-optional-in-git-fetch.yml b/changelogs/unreleased/sh-make-prune-optional-in-git-fetch.yml new file mode 100644 index 00000000000..e961a23a031 --- /dev/null +++ b/changelogs/unreleased/sh-make-prune-optional-in-git-fetch.yml @@ -0,0 +1,5 @@ +--- +title: Make --prune a configurable parameter in fetching a git remote +merge_request: +author: +type: performance diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb index e5a747cb987..5e1e22ae65c 100644 --- a/lib/gitlab/git/gitlab_projects.rb +++ b/lib/gitlab/git/gitlab_projects.rb @@ -63,11 +63,12 @@ module Gitlab end end - def fetch_remote(name, timeout, force:, tags:, ssh_key: nil, known_hosts: nil) + def fetch_remote(name, timeout, force:, tags:, ssh_key: nil, known_hosts: nil, prune: true) tags_option = tags ? '--tags' : '--no-tags' logger.info "Fetching remote #{name} for repository #{repository_absolute_path}." - cmd = %W(git fetch #{name} --prune --quiet) + cmd = %W(git fetch #{name} --quiet) + cmd << '--prune' if prune cmd << '--force' if force cmd << tags_option diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index fdb3247cf4d..e1bc2f9ab61 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -45,10 +45,10 @@ module Gitlab GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request) end - def fetch_remote(remote, ssh_auth:, forced:, no_tags:, timeout:) + def fetch_remote(remote, ssh_auth:, forced:, no_tags:, timeout:, prune: true) request = Gitaly::FetchRemoteRequest.new( repository: @gitaly_repo, remote: remote, force: forced, - no_tags: no_tags, timeout: timeout + no_tags: no_tags, timeout: timeout, no_prune: !prune ) if ssh_auth&.ssh_import? diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 4ba44e0feef..dda7afc0999 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -125,13 +125,13 @@ module Gitlab # Ex. # fetch_remote(my_repo, "upstream") # - def fetch_remote(repository, remote, ssh_auth: nil, forced: false, no_tags: false) + def fetch_remote(repository, remote, ssh_auth: nil, forced: false, no_tags: false, prune: true) gitaly_migrate(:fetch_remote) do |is_enabled| if is_enabled - repository.gitaly_repository_client.fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, timeout: git_timeout) + repository.gitaly_repository_client.fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, timeout: git_timeout, prune: prune) else storage_path = Gitlab.config.repositories.storages[repository.storage]["path"] - local_fetch_remote(storage_path, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) + local_fetch_remote(storage_path, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, prune: prune) end end end @@ -428,8 +428,8 @@ module Gitlab ) end - def local_fetch_remote(storage_path, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false) - vars = { force: forced, tags: !no_tags } + def local_fetch_remote(storage_path, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false, prune: true) + vars = { force: forced, tags: !no_tags, prune: prune } if ssh_auth&.ssh_import? if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb index f4b964e1ee9..45bcd730332 100644 --- a/spec/lib/gitlab/git/gitlab_projects_spec.rb +++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb @@ -61,10 +61,11 @@ describe Gitlab::Git::GitlabProjects do let(:remote_name) { 'remote-name' } let(:branch_name) { 'master' } let(:force) { false } + let(:prune) { true } let(:tags) { true } - let(:args) { { force: force, tags: tags }.merge(extra_args) } + let(:args) { { force: force, tags: tags, prune: prune }.merge(extra_args) } let(:extra_args) { {} } - let(:cmd) { %W(git fetch #{remote_name} --prune --quiet --tags) } + let(:cmd) { %W(git fetch #{remote_name} --quiet --prune --tags) } subject { gl_projects.fetch_remote(remote_name, 600, args) } @@ -97,7 +98,7 @@ describe Gitlab::Git::GitlabProjects do context 'with --force' do let(:force) { true } - let(:cmd) { %W(git fetch #{remote_name} --prune --quiet --force --tags) } + let(:cmd) { %W(git fetch #{remote_name} --quiet --prune --force --tags) } it 'executes the command with forced option' do stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) @@ -108,7 +109,18 @@ describe Gitlab::Git::GitlabProjects do context 'with --no-tags' do let(:tags) { false } - let(:cmd) { %W(git fetch #{remote_name} --prune --quiet --no-tags) } + let(:cmd) { %W(git fetch #{remote_name} --quiet --prune --no-tags) } + + it 'executes the command' do + stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) + + is_expected.to be_truthy + end + end + + context 'with no prune' do + let(:prune) { false } + let(:cmd) { %W(git fetch #{remote_name} --quiet --tags) } it 'executes the command' do stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb index c50e73cecfc..1c41dbcb9ef 100644 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -85,6 +85,20 @@ describe Gitlab::GitalyClient::RepositoryService do end end + describe '#fetch_remote' do + let(:ssh_auth) { double(:ssh_auth, ssh_import?: true, ssh_key_auth?: false, ssh_known_hosts: nil) } + let(:import_url) { 'ssh://example.com' } + + it 'sends a fetch_remote_request message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:fetch_remote) + .with(gitaly_request_with_params(no_prune: false), kind_of(Hash)) + .and_return(double(value: true)) + + client.fetch_remote(import_url, ssh_auth: ssh_auth, forced: false, no_tags: false, timeout: 60) + end + end + describe '#rebase_in_progress?' do let(:rebase_id) { 1 } diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 4506cbc3982..56b45d8da3c 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -508,8 +508,8 @@ describe Gitlab::Shell do end shared_examples 'fetch_remote' do |gitaly_on| - def fetch_remote(ssh_auth = nil) - gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', ssh_auth: ssh_auth) + def fetch_remote(ssh_auth = nil, prune = true) + gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', ssh_auth: ssh_auth, prune: prune) end def expect_gitlab_projects(fail = false, options = {}) @@ -555,27 +555,33 @@ describe Gitlab::Shell do end it 'returns true when the command succeeds' do - expect_call(false, force: false, tags: true) + expect_call(false, force: false, tags: true, prune: true) expect(fetch_remote).to be_truthy end + it 'returns true when the command succeeds' do + expect_call(false, force: false, tags: true, prune: false) + + expect(fetch_remote(nil, false)).to be_truthy + end + it 'raises an exception when the command fails' do - expect_call(true, force: false, tags: true) + expect_call(true, force: false, tags: true, prune: true) expect { fetch_remote }.to raise_error(Gitlab::Shell::Error) end it 'allows forced and no_tags to be changed' do - expect_call(false, force: true, tags: false) + expect_call(false, force: true, tags: false, prune: true) - result = gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', forced: true, no_tags: true) + result = gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', forced: true, no_tags: true, prune: true) expect(result).to be_truthy end context 'SSH auth' do it 'passes the SSH key if specified' do - expect_call(false, force: false, tags: true, ssh_key: 'foo') + expect_call(false, force: false, tags: true, prune: true, ssh_key: 'foo') ssh_auth = build_ssh_auth(ssh_key_auth?: true, ssh_private_key: 'foo') @@ -583,7 +589,7 @@ describe Gitlab::Shell do end it 'does not pass an empty SSH key' do - expect_call(false, force: false, tags: true) + expect_call(false, force: false, tags: true, prune: true) ssh_auth = build_ssh_auth(ssh_key_auth: true, ssh_private_key: '') @@ -591,7 +597,7 @@ describe Gitlab::Shell do end it 'does not pass the key unless SSH key auth is to be used' do - expect_call(false, force: false, tags: true) + expect_call(false, force: false, tags: true, prune: true) ssh_auth = build_ssh_auth(ssh_key_auth: false, ssh_private_key: 'foo') @@ -599,7 +605,7 @@ describe Gitlab::Shell do end it 'passes the known_hosts data if specified' do - expect_call(false, force: false, tags: true, known_hosts: 'foo') + expect_call(false, force: false, tags: true, prune: true, known_hosts: 'foo') ssh_auth = build_ssh_auth(ssh_known_hosts: 'foo') @@ -607,7 +613,7 @@ describe Gitlab::Shell do end it 'does not pass empty known_hosts data' do - expect_call(false, force: false, tags: true) + expect_call(false, force: false, tags: true, prune: true) ssh_auth = build_ssh_auth(ssh_known_hosts: '') @@ -615,7 +621,7 @@ describe Gitlab::Shell do end it 'does not pass known_hosts data unless SSH is to be used' do - expect_call(false, force: false, tags: true) + expect_call(false, force: false, tags: true, prune: true) ssh_auth = build_ssh_auth(ssh_import?: false, ssh_known_hosts: 'foo') @@ -642,7 +648,7 @@ describe Gitlab::Shell do it 'passes the correct params to the gitaly service' do expect(repository.gitaly_repository_client).to receive(:fetch_remote) - .with(remote_name, ssh_auth: ssh_auth, forced: true, no_tags: true, timeout: timeout) + .with(remote_name, ssh_auth: ssh_auth, forced: true, no_tags: true, prune: true, timeout: timeout) subject end |