Merge branch '257835-remove-pull-mirror-branch-prefix' into 'master'

Remove the `pull_mirror_branch_prefix` feature flag

See merge request gitlab-org/gitlab!49340

ee/app/models/ee/project.rb

@@ -23,6 +23,7 @@ module EE
       include ProjectSecurityScannersInformation
 
       ignore_columns :mirror_last_update_at, :mirror_last_successful_update_at, remove_after: '2019-12-15', remove_with: '12.6'
+      ignore_columns :pull_mirror_branch_prefix, remove_after: '2021-02-22', remove_with: '14.0'
 
       before_save :set_override_pull_mirror_available, unless: -> { ::Gitlab::CurrentSettings.mirror_available }
       before_save :set_next_execution_timestamp_to_now, if: ->(project) { project.mirror? && project.mirror_changed? && project.import_state }
@@ -206,9 +207,6 @@ module EE
 
       validates :approvals_before_merge, numericality: true, allow_blank: true
 
-      validates :pull_mirror_branch_prefix, length: { maximum: 50 }
-      validate :check_pull_mirror_branch_prefix
-
       with_options if: :mirror? do
         validates :import_url, presence: true
         validates :mirror_user, presence: true
@@ -780,15 +778,6 @@ module EE
       end
     end
 
-    def check_pull_mirror_branch_prefix
-      return if pull_mirror_branch_prefix.blank?
-      return unless pull_mirror_branch_prefix_changed?
-
-      unless ::Gitlab::GitRefValidator.validate("#{pull_mirror_branch_prefix}master")
-        errors.add(:pull_mirror_branch_prefix, _('Invalid Git ref'))
-      end
-    end
-
     def user_defined_rules
       strong_memoize(:user_defined_rules) do
         # Loading the relation in order to memoize it loaded

ee/app/models/ee/repository.rb

@@ -15,7 +15,6 @@ module EE
       include Elastic::RepositoriesSearch
 
       delegate :checksum, :find_remote_root_ref, to: :raw_repository
-      delegate :pull_mirror_branch_prefix, to: :project
     end
 
     # Transiently sets a configuration variable
@@ -34,19 +33,6 @@ module EE
       expire_content_cache
     end
 
-    def upstream_branch_name(branch_name)
-      return branch_name unless ::Feature.enabled?(:pull_mirror_branch_prefix, project)
-      return branch_name unless pull_mirror_branch_prefix
-
-      # when pull_mirror_branch_prefix is set, a branch not starting with it
-      # is a local branch that doesn't tracking upstream
-      if branch_name.start_with?(pull_mirror_branch_prefix)
-        branch_name.delete_prefix(pull_mirror_branch_prefix)
-      else
-        nil
-      end
-    end
-
     def fetch_upstream(url, forced: false)
       add_remote(MIRROR_REMOTE, url)
       fetch_remote(MIRROR_REMOTE, ssh_auth: project&.import_data, forced: forced)
@@ -57,10 +43,7 @@ module EE
     end
 
     def diverged_from_upstream?(branch_name)
-      upstream_branch = upstream_branch_name(branch_name)
-      return false unless upstream_branch
-
-      diverged?(branch_name, MIRROR_REMOTE, upstream_branch_name: upstream_branch) do |branch_commit, upstream_commit|
+      diverged?(branch_name, MIRROR_REMOTE) do |branch_commit, upstream_commit|
         !raw_repository.ancestor?(branch_commit.id, upstream_commit.id)
       end
     end
@@ -72,10 +55,7 @@ module EE
     end
 
     def up_to_date_with_upstream?(branch_name)
-      upstream_branch = upstream_branch_name(branch_name)
-      return false unless upstream_branch
-
-      diverged?(branch_name, MIRROR_REMOTE, upstream_branch_name: upstream_branch) do |branch_commit, upstream_commit|
+      diverged?(branch_name, MIRROR_REMOTE) do |branch_commit, upstream_commit|
         ancestor?(branch_commit.id, upstream_commit.id)
       end
     end
@@ -111,9 +91,9 @@ module EE
 
     private
 
-    def diverged?(branch_name, remote_ref, upstream_branch_name: branch_name)
+    def diverged?(branch_name, remote_ref)
       branch_commit = commit("refs/heads/#{branch_name}")
-      upstream_commit = commit("refs/remotes/#{remote_ref}/#{upstream_branch_name}")
+      upstream_commit = commit("refs/remotes/#{remote_ref}/#{branch_name}")
 
       if branch_commit && upstream_commit
         yield branch_commit, upstream_commit

ee/app/services/ee/projects/update_service.rb

@@ -14,7 +14,6 @@ module EE
         mirror_trigger_builds
         only_mirror_protected_branches
         mirror_overwrites_diverged_branches
-        pull_mirror_branch_prefix
         import_data_attributes
       ].freeze
 

ee/app/services/projects/update_mirror_service.rb

@@ -66,7 +66,7 @@ module Projects
       errors = []
 
       repository.upstream_branches.each do |upstream_branch|
-        name = target_branch_name(upstream_branch.name)
+        name = upstream_branch.name
 
         next if skip_branch?(name)
 
@@ -188,11 +188,5 @@ module Projects
     def log_error(error_message)
       service_logger.error(base_payload.merge(error_message: error_message))
     end
-
-    def target_branch_name(upstream_branch_name)
-      return upstream_branch_name unless Feature.enabled?(:pull_mirror_branch_prefix, project)
-
-      "#{project.pull_mirror_branch_prefix}#{upstream_branch_name}"
-    end
   end
 end

ee/app/views/projects/mirrors/_mirror_repos_form.html.haml

@@ -33,13 +33,6 @@
     .help-block
       = _('You will be the author of all events in the activity feed that are the result of an update, like new branches being created or new commits being pushed to existing branches.')
 
-  - if Feature.enabled?(:pull_mirror_branch_prefix, @project)
-    .form-group
-      = f.label :pull_mirror_branch_prefix, _('Branch prefix'), class: 'label-light'
-      = f.text_field :pull_mirror_branch_prefix, class: 'form-control', placeholder: 'Example: upstream/'
-      .help-block
-        = _("Mirrored branches will have this prefix. If you enabled 'Only mirror protected branches' you need to include this prefix on protected branches in this project or nothing will be mirrored.")
-
   .form-check.gl-mb-3
     = f.check_box :mirror_overwrites_diverged_branches, class: 'form-check-input', checked: false
     = f.label :mirror_overwrites_diverged_branches, _('Overwrite diverged branches'), class: 'form-check-label'

ee/config/feature_flags/development/pull_mirror_branch_prefix.yml

----
-name: pull_mirror_branch_prefix
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/17368
-rollout_issue_url: 
-milestone: '12.4'
-type: development
-group: group::source code
-default_enabled: false

ee/spec/features/projects/settings/ee/repository_mirrors_settings_spec.rb

@@ -26,7 +26,6 @@ RSpec.describe 'Project settings > [EE] repository' do
         expect(page).to have_no_selector('#mirror_user_id_select', visible: false)
         expect(page).to have_no_selector('#project_mirror_overwrites_diverged_branches')
         expect(page).to have_no_selector('#project_mirror_trigger_builds')
-        expect(page).to have_no_selector('#project_pull_mirror_branch_prefix')
       end
     end
   end
@@ -46,21 +45,6 @@ RSpec.describe 'Project settings > [EE] repository' do
         expect(page).to have_selector('#mirror_user_id_select', visible: false)
         expect(page).to have_selector('#project_mirror_overwrites_diverged_branches')
         expect(page).to have_selector('#project_mirror_trigger_builds')
-        expect(page).to have_selector('#project_pull_mirror_branch_prefix')
-      end
-    end
-
-    context 'pull_mirror_branch_prefix feature flag disabled' do
-      before do
-        stub_feature_flags(pull_mirror_branch_prefix: false)
-      end
-
-      it 'shows pull mirror settings', :js do
-        visit project_settings_repository_path(project)
-
-        page.within('.project-mirror-settings') do
-          expect(page).to have_no_selector('#project_pull_mirror_branch_prefix')
-        end
       end
     end
 

ee/spec/models/project_spec.rb

@@ -339,16 +339,6 @@ RSpec.describe Project do
         project2.update(mirror: true, import_url: generate(:url), mirror_user: project.creator)
       end.to change { ProjectImportState.where(project: project2).count }.from(0).to(1)
     end
-
-    describe 'pull_mirror_branch_prefix' do
-      it { is_expected.to validate_length_of(:pull_mirror_branch_prefix).is_at_most(50) }
-
-      it 'rejects invalid git refs' do
-        project = build(:project, pull_mirror_branch_prefix: 'an invalid prefix..')
-
-        expect(project).not_to be_valid
-      end
-    end
   end
 
   describe 'setting up a mirror' do

ee/spec/models/repository_spec.rb

@@ -27,7 +27,6 @@ RSpec.describe Repository do
 
     it { is_expected.to delegate_method(:checksum).to(:raw_repository) }
     it { is_expected.to delegate_method(:find_remote_root_ref).to(:raw_repository) }
-    it { is_expected.to delegate_method(:pull_mirror_branch_prefix).to(:project) }
   end
 
   describe '#after_sync' do
@@ -219,63 +218,6 @@ RSpec.describe Repository do
     end
   end
 
-  describe '#upstream_branch_name' do
-    let(:pull_mirror_branch_prefix) { 'upstream/' }
-    let(:branch_name) { 'upstream/master' }
-
-    subject { repository.upstream_branch_name(branch_name) }
-
-    before do
-      project.update(pull_mirror_branch_prefix: pull_mirror_branch_prefix)
-    end
-
-    it { is_expected.to eq('master') }
-
-    context 'when the branch is local (not mirrored)' do
-      let(:branch_name) { 'a-local-branch' }
-
-      it { is_expected.to be_nil }
-    end
-
-    context 'when pull_mirror_branch_prefix is nil' do
-      let(:pull_mirror_branch_prefix) { nil }
-
-      it { is_expected.to eq(branch_name) }
-    end
-
-    context 'when pull_mirror_branch_prefix is empty' do
-      let(:pull_mirror_branch_prefix) { '' }
-
-      it { is_expected.to eq(branch_name) }
-    end
-
-    context 'when pull_mirror_branch_prefix feature flag is disabled' do
-      before do
-        stub_feature_flags(pull_mirror_branch_prefix: false)
-      end
-
-      it { is_expected.to eq(branch_name) }
-
-      context 'when the branch is local (not mirrored)' do
-        let(:branch_name) { 'a-local-branch' }
-
-        it { is_expected.to eq(branch_name) }
-      end
-
-      context 'when pull_mirror_branch_prefix is nil' do
-        let(:pull_mirror_branch_prefix) { nil }
-
-        it { is_expected.to eq(branch_name) }
-      end
-
-      context 'when pull_mirror_branch_prefix is empty' do
-        let(:pull_mirror_branch_prefix) { '' }
-
-        it { is_expected.to eq(branch_name) }
-      end
-    end
-  end
-
   describe '#lfs_enabled?' do
     subject { repository.lfs_enabled? }
 

ee/spec/services/projects/update_mirror_service_spec.rb

@@ -202,9 +202,8 @@ RSpec.describe Projects::UpdateMirrorService do
     end
 
     context 'updating branches' do
-      shared_examples 'a working pull mirror' do |branch_prefix|
       context 'when the mirror has a repository' do
-          let(:master) { "#{branch_prefix}master"}
+        let(:master) { "master"}
 
         before do
           stub_fetch_mirror(project)
@@ -213,19 +212,19 @@ RSpec.describe Projects::UpdateMirrorService do
         it 'creates new branches' do
           service.execute
 
-            expect(project.repository.branch_names).to include("#{branch_prefix}new-branch")
+          expect(project.repository.branch_names).to include("new-branch")
         end
 
         it 'updates existing branches' do
           service.execute
 
-            expect(project.repository.find_branch("#{branch_prefix}existing-branch").dereferenced_target)
+          expect(project.repository.find_branch("existing-branch").dereferenced_target)
             .to eq(project.repository.find_branch(master).dereferenced_target)
         end
 
         context 'when mirror only protected branches option is set' do
-            let(:new_protected_branch_name) { "#{branch_prefix}new-branch" }
-            let(:protected_branch_name) { "#{branch_prefix}existing-branch" }
+          let(:new_protected_branch_name) { "new-branch" }
+          let(:protected_branch_name) { "existing-branch" }
 
           before do
             project.update!(only_mirror_protected_branches: true)
@@ -265,7 +264,7 @@ RSpec.describe Projects::UpdateMirrorService do
         end
 
         context 'with diverged branches' do
-            let(:diverged_branch) { "#{branch_prefix}markdown"}
+          let(:diverged_branch) { "markdown"}
 
           context 'when mirror_overwrites_diverged_branches is true' do
             it 'update diverged branches' do
@@ -315,43 +314,6 @@ RSpec.describe Projects::UpdateMirrorService do
           expect(repository.branch_names).not_to include('master')
         end
       end
-      end
-
-      context 'when pull_mirror_branch_prefix is set' do
-        let(:pull_mirror_branch_prefix) { 'upstream/' }
-
-        before do
-          project.update!(pull_mirror_branch_prefix: pull_mirror_branch_prefix)
-        end
-
-        it "doesn't create unprefixed branches" do
-          stub_fetch_mirror(project)
-
-          service.execute
-
-          expect(project.repository.branch_names).not_to include('new-branch')
-        end
-
-        it_behaves_like 'a working pull mirror', 'upstream/'
-
-        context 'when pull_mirror_branch_prefix feature flag is disabled' do
-          before do
-            stub_feature_flags(pull_mirror_branch_prefix: false)
-          end
-
-          it_behaves_like 'a working pull mirror'
-
-          it "doesn't create prefixed branches" do
-            stub_fetch_mirror(project)
-
-            service.execute
-
-            expect(project.repository.branch_names).not_to include("#{pull_mirror_branch_prefix}new-branch")
-          end
-        end
-      end
-
-      it_behaves_like 'a working pull mirror'
 
       def create_file(repository)
         repository.create_file(
@@ -467,42 +429,16 @@ RSpec.describe Projects::UpdateMirrorService do
     end
   end
 
-  def rewrite_refs_as_pull_mirror(project)
-    return unless project.pull_mirror_branch_prefix
-    return unless Feature.enabled?(:pull_mirror_branch_prefix)
-
-    repository = project.repository
-    old_branches = repository.branches.each_with_object({}) do |branch, branches|
-      branches[branch.name] = branch.dereferenced_target.id
-    end
-
-    rugged = rugged_repo(repository)
-    old_branches.each do |name, target|
-      mirrored_branch_ref = "refs/heads/#{project.pull_mirror_branch_prefix}#{name}"
-      rugged.references.create(mirrored_branch_ref, target)
-      rugged.head = mirrored_branch_ref if name == 'master'
-      rugged.branches.delete(name)
-    end
-
-    repository.expire_branches_cache
-    repository.branches
-  end
-
   def stub_fetch_mirror(project, repository: project.repository)
-    branch_prefix = project.pull_mirror_branch_prefix
-    branch_prefix = '' unless Feature.enabled?(:pull_mirror_branch_prefix)
-
-    rewrite_refs_as_pull_mirror(project)
-
-    allow(project).to receive(:fetch_mirror) { fetch_mirror(repository, branch_prefix: branch_prefix) }
+    allow(project).to receive(:fetch_mirror) { fetch_mirror(repository) }
   end
 
-  def fetch_mirror(repository, branch_prefix: '')
+  def fetch_mirror(repository)
     rugged = rugged_repo(repository)
-    masterrev = repository.find_branch("#{branch_prefix}master").dereferenced_target.id
+    masterrev = repository.find_branch("master").dereferenced_target.id
 
     parentrev = repository.commit(masterrev).parent_id
-    rugged.references.create("refs/heads/#{branch_prefix}existing-branch", parentrev)
+    rugged.references.create("refs/heads/existing-branch", parentrev)
 
     repository.expire_branches_cache
     repository.branches

locale/gitlab.pot

@@ -4609,9 +4609,6 @@ msgstr ""
 msgid "Branch not loaded - %{branchId}"
 msgstr ""
 
-msgid "Branch prefix"
-msgstr ""
-
 msgid "BranchSwitcherPlaceholder|Search branches"
 msgstr ""
 
@@ -14943,9 +14940,6 @@ msgstr ""
 msgid "Introducing Your DevOps Report"
 msgstr ""
 
-msgid "Invalid Git ref"
-msgstr ""
-
 msgid "Invalid Insights config file detected"
 msgstr ""
 
@@ -17876,9 +17870,6 @@ msgstr ""
 msgid "Mirror user"
 msgstr ""
 
-msgid "Mirrored branches will have this prefix. If you enabled 'Only mirror protected branches' you need to include this prefix on protected branches in this project or nothing will be mirrored."
-msgstr ""
-
 msgid "Mirrored repositories"
 msgstr ""