Commit e9fad3e5 authored by Stan Hu's avatar Stan Hu

Make --prune a configurable parameter in fetching a git remote

By default, --prune is added to the command-line of a `git fetch` operation,
but for repositories with many references this can take a long time to run. We
shouldn't need to run --prune the first time we fetch a new repository.
parent 2e87923d
...@@ -866,20 +866,20 @@ class Repository ...@@ -866,20 +866,20 @@ class Repository
raw_repository.ancestor?(ancestor_id, descendant_id) raw_repository.ancestor?(ancestor_id, descendant_id)
end 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 unless remote_name
remote_name = "tmp-#{SecureRandom.hex}" remote_name = "tmp-#{SecureRandom.hex}"
tmp_remote_name = true tmp_remote_name = true
end end
add_remote(remote_name, url, mirror_refmap: refmap) add_remote(remote_name, url, mirror_refmap: refmap)
fetch_remote(remote_name, forced: forced) fetch_remote(remote_name, forced: forced, prune: prune)
ensure ensure
remove_remote(remote_name) if tmp_remote_name remove_remote(remote_name) if tmp_remote_name
end end
def fetch_remote(remote, forced: false, ssh_auth: nil, no_tags: false) 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) gitlab_shell.fetch_remote(raw_repository, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, prune: prune)
end end
def fetch_source_branch!(source_repository, source_branch, local_ref) def fetch_source_branch!(source_repository, source_branch, local_ref)
......
---
title: Make --prune a configurable parameter in fetching a git remote
merge_request:
author:
type: performance
...@@ -63,11 +63,12 @@ module Gitlab ...@@ -63,11 +63,12 @@ module Gitlab
end end
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' tags_option = tags ? '--tags' : '--no-tags'
logger.info "Fetching remote #{name} for repository #{repository_absolute_path}." 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 << '--force' if force
cmd << tags_option cmd << tags_option
......
...@@ -45,10 +45,10 @@ module Gitlab ...@@ -45,10 +45,10 @@ module Gitlab
GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request) GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request)
end 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( request = Gitaly::FetchRemoteRequest.new(
repository: @gitaly_repo, remote: remote, force: forced, 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? if ssh_auth&.ssh_import?
......
...@@ -125,13 +125,13 @@ module Gitlab ...@@ -125,13 +125,13 @@ module Gitlab
# Ex. # Ex.
# fetch_remote(my_repo, "upstream") # 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| gitaly_migrate(:fetch_remote) do |is_enabled|
if 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 else
storage_path = Gitlab.config.repositories.storages[repository.storage]["path"] 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 end
end end
...@@ -428,8 +428,8 @@ module Gitlab ...@@ -428,8 +428,8 @@ module Gitlab
) )
end end
def local_fetch_remote(storage_path, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false) 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 } vars = { force: forced, tags: !no_tags, prune: prune }
if ssh_auth&.ssh_import? if ssh_auth&.ssh_import?
if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present?
......
...@@ -61,10 +61,11 @@ describe Gitlab::Git::GitlabProjects do ...@@ -61,10 +61,11 @@ describe Gitlab::Git::GitlabProjects do
let(:remote_name) { 'remote-name' } let(:remote_name) { 'remote-name' }
let(:branch_name) { 'master' } let(:branch_name) { 'master' }
let(:force) { false } let(:force) { false }
let(:prune) { true }
let(:tags) { 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(: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) } subject { gl_projects.fetch_remote(remote_name, 600, args) }
...@@ -97,7 +98,7 @@ describe Gitlab::Git::GitlabProjects do ...@@ -97,7 +98,7 @@ describe Gitlab::Git::GitlabProjects do
context 'with --force' do context 'with --force' do
let(:force) { true } 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 it 'executes the command with forced option' do
stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) stub_spawn(cmd, 600, tmp_repo_path, {}, success: true)
...@@ -108,7 +109,18 @@ describe Gitlab::Git::GitlabProjects do ...@@ -108,7 +109,18 @@ describe Gitlab::Git::GitlabProjects do
context 'with --no-tags' do context 'with --no-tags' do
let(:tags) { false } 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 it 'executes the command' do
stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) stub_spawn(cmd, 600, tmp_repo_path, {}, success: true)
......
...@@ -85,6 +85,20 @@ describe Gitlab::GitalyClient::RepositoryService do ...@@ -85,6 +85,20 @@ describe Gitlab::GitalyClient::RepositoryService do
end end
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 describe '#rebase_in_progress?' do
let(:rebase_id) { 1 } let(:rebase_id) { 1 }
......
...@@ -508,8 +508,8 @@ describe Gitlab::Shell do ...@@ -508,8 +508,8 @@ describe Gitlab::Shell do
end end
shared_examples 'fetch_remote' do |gitaly_on| shared_examples 'fetch_remote' do |gitaly_on|
def fetch_remote(ssh_auth = nil) def fetch_remote(ssh_auth = nil, prune = true)
gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', ssh_auth: ssh_auth) gitlab_shell.fetch_remote(repository.raw_repository, 'remote-name', ssh_auth: ssh_auth, prune: prune)
end end
def expect_gitlab_projects(fail = false, options = {}) def expect_gitlab_projects(fail = false, options = {})
...@@ -555,27 +555,33 @@ describe Gitlab::Shell do ...@@ -555,27 +555,33 @@ describe Gitlab::Shell do
end end
it 'returns true when the command succeeds' do 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 expect(fetch_remote).to be_truthy
end 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 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) expect { fetch_remote }.to raise_error(Gitlab::Shell::Error)
end end
it 'allows forced and no_tags to be changed' do 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 expect(result).to be_truthy
end end
context 'SSH auth' do context 'SSH auth' do
it 'passes the SSH key if specified' 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') ssh_auth = build_ssh_auth(ssh_key_auth?: true, ssh_private_key: 'foo')
...@@ -583,7 +589,7 @@ describe Gitlab::Shell do ...@@ -583,7 +589,7 @@ describe Gitlab::Shell do
end end
it 'does not pass an empty SSH key' do 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: '') ssh_auth = build_ssh_auth(ssh_key_auth: true, ssh_private_key: '')
...@@ -591,7 +597,7 @@ describe Gitlab::Shell do ...@@ -591,7 +597,7 @@ describe Gitlab::Shell do
end end
it 'does not pass the key unless SSH key auth is to be used' do 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') ssh_auth = build_ssh_auth(ssh_key_auth: false, ssh_private_key: 'foo')
...@@ -599,7 +605,7 @@ describe Gitlab::Shell do ...@@ -599,7 +605,7 @@ describe Gitlab::Shell do
end end
it 'passes the known_hosts data if specified' do 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') ssh_auth = build_ssh_auth(ssh_known_hosts: 'foo')
...@@ -607,7 +613,7 @@ describe Gitlab::Shell do ...@@ -607,7 +613,7 @@ describe Gitlab::Shell do
end end
it 'does not pass empty known_hosts data' do 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: '') ssh_auth = build_ssh_auth(ssh_known_hosts: '')
...@@ -615,7 +621,7 @@ describe Gitlab::Shell do ...@@ -615,7 +621,7 @@ describe Gitlab::Shell do
end end
it 'does not pass known_hosts data unless SSH is to be used' do 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') ssh_auth = build_ssh_auth(ssh_import?: false, ssh_known_hosts: 'foo')
...@@ -642,7 +648,7 @@ describe Gitlab::Shell do ...@@ -642,7 +648,7 @@ describe Gitlab::Shell do
it 'passes the correct params to the gitaly service' do it 'passes the correct params to the gitaly service' do
expect(repository.gitaly_repository_client).to receive(:fetch_remote) 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 subject
end end
......
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