From 8850fb9d671635178c2ef307dcf8df083a7285d6 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 10 Sep 2018 16:12:49 -0300 Subject: Synchronize the default branch when updating a remote mirror --- app/models/project.rb | 6 + app/models/repository.rb | 1 + .../projects/update_remote_mirror_service.rb | 5 +- spec/models/project_spec.rb | 34 ++++++ .../projects/update_remote_mirror_service_spec.rb | 127 +++++++++++---------- 5 files changed, 113 insertions(+), 60 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 45cf527d7c6..8928bffd36c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2063,6 +2063,12 @@ class Project < ActiveRecord::Base auto_cancel_pending_pipelines == 'enabled' end + # Update the default branch querying the remote to determine its HEAD + def update_root_ref(remote_name) + root_ref = repository.find_remote_root_ref(remote_name) + change_head(root_ref) if root_ref.present? && root_ref != default_branch + end + private def rename_or_migrate_repository! diff --git a/app/models/repository.rb b/app/models/repository.rb index 929d28b9d88..e98021af818 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -24,6 +24,7 @@ class Repository delegate :ref_name_for_sha, to: :raw_repository delegate :bundle_to_disk, to: :raw_repository + delegate :find_remote_root_ref, to: :raw_repository CreateTreeError = Class.new(StandardError) diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index 591b38b8151..85b9eb02803 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -5,13 +5,14 @@ module Projects attr_reader :errors def execute(remote_mirror) - @errors = [] - return success unless remote_mirror.enabled? + errors = [] + begin remote_mirror.ensure_remote! repository.fetch_remote(remote_mirror.remote_name, no_tags: true) + project.update_root_ref(remote_mirror.remote_name) opts = {} if remote_mirror.only_protected_branches? diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cb844cd2102..dfe2de71a76 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3983,6 +3983,40 @@ describe Project do end end + describe '#update_root_ref' do + let(:project) { create(:project, :repository) } + + it 'updates the default branch when HEAD has changed' do + stub_find_remote_root_ref(project, ref: 'feature') + + expect { project.update_root_ref('origin') } + .to change { project.default_branch } + .from('master') + .to('feature') + end + + it 'does not update the default branch when HEAD does not change' do + stub_find_remote_root_ref(project, ref: 'master') + + expect { project.update_root_ref('origin') } + .not_to change { project.default_branch } + end + + it 'does not update the default branch when HEAD does not exist' do + stub_find_remote_root_ref(project, ref: 'foo') + + expect { project.update_root_ref('origin') } + .not_to change { project.default_branch } + end + + def stub_find_remote_root_ref(project, ref:) + allow(project.repository) + .to receive(:find_remote_root_ref) + .with('origin') + .and_return(ref) + end + end + def rugged_config Gitlab::GitalyClient::StorageSettings.allow_disk_access do project.repository.rugged.config diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index 96e8a80b334..56a36432462 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -1,107 +1,118 @@ require 'spec_helper' describe Projects::UpdateRemoteMirrorService do - set(:project) { create(:project, :repository) } - let(:owner) { project.owner } + let(:project) { create(:project, :repository) } let(:remote_project) { create(:forked_project_with_submodules) } - let(:repository) { project.repository } - let(:raw_repository) { repository.raw } let(:remote_mirror) { project.remote_mirrors.create!(url: remote_project.http_url_to_repo, enabled: true, only_protected_branches: false) } + let(:remote_name) { remote_mirror.remote_name } - subject { described_class.new(project, project.creator) } + subject(:service) { described_class.new(project, project.creator) } describe "#execute" do before do - repository.add_branch(owner, 'existing-branch', 'master') + project.repository.add_branch(project.owner, 'existing-branch', 'master') allow(remote_mirror).to receive(:update_repository).and_return(true) end + it "ensures the remote exists" do + stub_fetch_remote(project, remote_name: remote_name) + stub_find_remote_root_ref(project, remote_name: remote_name) + + expect(remote_mirror).to receive(:ensure_remote!) + + service.execute(remote_mirror) + end + it "fetches the remote repository" do - expect(remote_mirror).to receive(:ensure_remote!).and_call_original - expect(repository).to receive(:fetch_remote).with(remote_mirror.remote_name, no_tags: true) do - sync_remote(repository, remote_mirror.remote_name, local_branch_names) - end + stub_find_remote_root_ref(project, remote_name: remote_name) + + expect(project.repository) + .to receive(:fetch_remote) + .with(remote_mirror.remote_name, no_tags: true) + + service.execute(remote_mirror) + end + + it "updates the default branch when HEAD has changed" do + stub_fetch_remote(project, remote_name: remote_name) + stub_find_remote_root_ref(project, remote_name: remote_name, ref: "existing-branch") - subject.execute(remote_mirror) + expect { service.execute(remote_mirror) } + .to change { project.default_branch } + .from("master") + .to("existing-branch") end - it "succeeds" do - allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.remote_name, local_branch_names) } + it "does not update the default branch when HEAD does not change" do + stub_fetch_remote(project, remote_name: remote_name) + stub_find_remote_root_ref(project, remote_name: remote_name, ref: "master") + + expect { service.execute(remote_mirror) }.not_to change { project.default_branch } + end - result = subject.execute(remote_mirror) + it "returns success when updated succeeds" do + stub_fetch_remote(project, remote_name: remote_name) + stub_find_remote_root_ref(project, remote_name: remote_name) + + result = service.execute(remote_mirror) expect(result[:status]).to eq(:success) end context 'when syncing all branches' do it "push all the branches the first time" do - allow(repository).to receive(:fetch_remote) + stub_fetch_remote(project, remote_name: remote_name) + stub_find_remote_root_ref(project, remote_name: remote_name) expect(remote_mirror).to receive(:update_repository).with({}) - subject.execute(remote_mirror) + service.execute(remote_mirror) end end context 'when only syncing protected branches' do - let(:unprotected_branch_name) { 'existing-branch' } - let(:protected_branch_name) do - project.repository.branch_names.find { |n| n != unprotected_branch_name } - end - let!(:protected_branch) do - create(:protected_branch, project: project, name: protected_branch_name) - end - - before do - project.reload + it "sync updated protected branches" do + stub_fetch_remote(project, remote_name: remote_name) + stub_find_remote_root_ref(project, remote_name: remote_name) + protected_branch = create_protected_branch(project) remote_mirror.only_protected_branches = true - end - it "sync updated protected branches" do - allow(repository).to receive(:fetch_remote) - expect(remote_mirror).to receive(:update_repository).with(only_branches_matching: [protected_branch_name]) + expect(remote_mirror) + .to receive(:update_repository) + .with(only_branches_matching: [protected_branch.name]) - subject.execute(remote_mirror) + service.execute(remote_mirror) end - end - end - def sync_remote(repository, remote_name, local_branch_names) - local_branch_names.each do |branch| - commit = repository.commit(branch) - repository.write_ref("refs/remotes/#{remote_name}/#{branch}", commit.id) if commit + def create_protected_branch(project) + branch_name = project.repository.branch_names.find { |n| n != 'existing-branch' } + create(:protected_branch, project: project, name: branch_name) + end end end - def update_remote_branch(repository, remote_name, branch) - masterrev = repository.commit('master').id - - repository.write_ref("refs/remotes/#{remote_name}/#{branch}", masterrev, force: true) - repository.expire_branches_cache + def stub_find_remote_root_ref(project, ref: 'master', remote_name:) + allow(project.repository) + .to receive(:find_remote_root_ref) + .with(remote_name) + .and_return(ref) end - def update_branch(repository, branch) - masterrev = repository.commit('master').id - - repository.write_ref("refs/heads/#{branch}", masterrev, force: true) - repository.expire_branches_cache + def stub_fetch_remote(project, remote_name:) + allow(project.repository) + .to receive(:fetch_remote) + .with(remote_name, no_tags: true) { fetch_remote(project.repository, remote_name) } end - def generate_tags(repository, *tag_names) - tag_names.each_with_object([]) do |name, tags| - tag = repository.find_tag(name) - target = tag.try(:target) - target_commit = tag.try(:dereferenced_target) - tags << Gitlab::Git::Tag.new(repository.raw_repository, { - name: name, - target: target, - target_commit: target_commit - }) + def fetch_remote(repository, remote_name) + local_branch_names(repository).each do |branch| + commit = repository.commit(branch) + repository.write_ref("refs/remotes/#{remote_name}/#{branch}", commit.id) if commit end end - def local_branch_names + def local_branch_names(repository) branch_names = repository.branches.map(&:name) # we want the protected branch to be pushed first branch_names.unshift(branch_names.delete('master')) -- cgit v1.2.1 From dd4877fb6f43eab0932624436eab89039023d4f4 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 10 Sep 2018 16:14:57 -0300 Subject: Add CHANGELOG --- ...-synchronize-the-default-branch-when-updating-a-remote-mirror.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/da-synchronize-the-default-branch-when-updating-a-remote-mirror.yml diff --git a/changelogs/unreleased/da-synchronize-the-default-branch-when-updating-a-remote-mirror.yml b/changelogs/unreleased/da-synchronize-the-default-branch-when-updating-a-remote-mirror.yml new file mode 100644 index 00000000000..723aa3eee8a --- /dev/null +++ b/changelogs/unreleased/da-synchronize-the-default-branch-when-updating-a-remote-mirror.yml @@ -0,0 +1,5 @@ +--- +title: Synchronize the default branch when updating a remote mirror +merge_request: 21653 +author: +type: fixed -- cgit v1.2.1