Remove the `pull_mirror_branch_prefix` feature flag

This feature flag gates functionality that was added, but never used,
to limit mirroring of branches to those with a certain prefix.

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 }
@@ -205,9 +206,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
@@ -779,15 +777,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

@@ -338,16 +338,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,157 +202,119 @@ 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"}
+      context 'when the mirror has a repository' do
+        let(:master) { "master"}
+
+        before do
+          stub_fetch_mirror(project)
+        end
+
+        it 'creates new branches' do
+          service.execute
+
+          expect(project.repository.branch_names).to include("new-branch")
+        end
+
+        it 'updates existing branches' do
+          service.execute
+
+          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) { "new-branch" }
+          let(:protected_branch_name) { "existing-branch" }
 
           before do
-            stub_fetch_mirror(project)
+            project.update!(only_mirror_protected_branches: true)
           end
 
-          it 'creates new branches' do
+          it 'creates a new protected branch' do
+            create(:protected_branch, project: project, name: new_protected_branch_name)
+            project.reload
+
             service.execute
 
-            expect(project.repository.branch_names).to include("#{branch_prefix}new-branch")
+            expect(project.repository.branch_names).to include(new_protected_branch_name)
           end
 
-          it 'updates existing branches' do
+          it 'does not create an unprotected branch' do
             service.execute
 
-            expect(project.repository.find_branch("#{branch_prefix}existing-branch").dereferenced_target)
-              .to eq(project.repository.find_branch(master).dereferenced_target)
+            expect(project.repository.branch_names).not_to include(new_protected_branch_name)
           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" }
+          it 'updates existing protected branches' do
+            create(:protected_branch, project: project, name: protected_branch_name)
+            project.reload
 
-            before do
-              project.update!(only_mirror_protected_branches: true)
-            end
-
-            it 'creates a new protected branch' do
-              create(:protected_branch, project: project, name: new_protected_branch_name)
-              project.reload
+            service.execute
 
-              service.execute
+            expect(project.repository.find_branch(protected_branch_name).dereferenced_target)
+              .to eq(project.repository.find_branch(master).dereferenced_target)
+          end
 
-              expect(project.repository.branch_names).to include(new_protected_branch_name)
-            end
+          it 'does not update unprotected branches' do
+            service.execute
 
-            it 'does not create an unprotected branch' do
-              service.execute
+            expect(project.repository.find_branch(protected_branch_name).dereferenced_target)
+              .not_to eq(project.repository.find_branch(master).dereferenced_target)
+          end
+        end
 
-              expect(project.repository.branch_names).not_to include(new_protected_branch_name)
-            end
+        context 'with diverged branches' do
+          let(:diverged_branch) { "markdown"}
 
-            it 'updates existing protected branches' do
-              create(:protected_branch, project: project, name: protected_branch_name)
-              project.reload
+          context 'when mirror_overwrites_diverged_branches is true' do
+            it 'update diverged branches' do
+              project.mirror_overwrites_diverged_branches = true
 
               service.execute
 
-              expect(project.repository.find_branch(protected_branch_name).dereferenced_target)
+              expect(project.repository.find_branch(diverged_branch).dereferenced_target)
                 .to eq(project.repository.find_branch(master).dereferenced_target)
             end
+          end
+
+          context 'when mirror_overwrites_diverged_branches is false' do
+            it "doesn't update diverged branches" do
+              project.mirror_overwrites_diverged_branches = false
 
-            it 'does not update unprotected branches' do
               service.execute
 
-              expect(project.repository.find_branch(protected_branch_name).dereferenced_target)
+              expect(project.repository.find_branch(diverged_branch).dereferenced_target)
                 .not_to eq(project.repository.find_branch(master).dereferenced_target)
             end
           end
 
-          context 'with diverged branches' do
-            let(:diverged_branch) { "#{branch_prefix}markdown"}
-
-            context 'when mirror_overwrites_diverged_branches is true' do
-              it 'update diverged branches' do
-                project.mirror_overwrites_diverged_branches = true
-
-                service.execute
-
-                expect(project.repository.find_branch(diverged_branch).dereferenced_target)
-                  .to eq(project.repository.find_branch(master).dereferenced_target)
-              end
-            end
-
-            context 'when mirror_overwrites_diverged_branches is false' do
-              it "doesn't update diverged branches" do
-                project.mirror_overwrites_diverged_branches = false
-
-                service.execute
-
-                expect(project.repository.find_branch(diverged_branch).dereferenced_target)
-                  .not_to eq(project.repository.find_branch(master).dereferenced_target)
-              end
-            end
+          context 'when mirror_overwrites_diverged_branches is nil' do
+            it "doesn't update diverged branches" do
+              project.mirror_overwrites_diverged_branches = nil
 
-            context 'when mirror_overwrites_diverged_branches is nil' do
-              it "doesn't update diverged branches" do
-                project.mirror_overwrites_diverged_branches = nil
-
-                service.execute
+              service.execute
 
-                expect(project.repository.find_branch(diverged_branch).dereferenced_target)
-                  .not_to eq(project.repository.find_branch(master).dereferenced_target)
-              end
+              expect(project.repository.find_branch(diverged_branch).dereferenced_target)
+                .not_to eq(project.repository.find_branch(master).dereferenced_target)
             end
           end
         end
-
-        context 'when project is empty' do
-          it 'does not add a default master branch' do
-            project    = create(:project_empty_repo, :mirror, import_url: Project::UNKNOWN_IMPORT_URL)
-            repository = project.repository
-
-            allow(project).to receive(:fetch_mirror) { create_file(repository) }
-            expect(::Branches::CreateService).not_to receive(:create_master_branch)
-
-            service.execute
-
-            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/' }
+      context 'when project is empty' do
+        it 'does not add a default master branch' do
+          project    = create(:project_empty_repo, :mirror, import_url: Project::UNKNOWN_IMPORT_URL)
+          repository = project.repository
 
-        before do
-          project.update!(pull_mirror_branch_prefix: pull_mirror_branch_prefix)
-        end
-
-        it "doesn't create unprefixed branches" do
-          stub_fetch_mirror(project)
+          allow(project).to receive(:fetch_mirror) { create_file(repository) }
+          expect(::Branches::CreateService).not_to receive(:create_master_branch)
 
           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
+          expect(repository.branch_names).not_to include('master')
         end
       end
 
-      it_behaves_like 'a working pull mirror'
-
       def create_file(repository)
         repository.create_file(
           project.owner,
@@ -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

@@ -4576,9 +4576,6 @@ msgstr ""
 msgid "Branch not loaded - %{branchId}"
 msgstr ""
 
-msgid "Branch prefix"
-msgstr ""
-
 msgid "BranchSwitcherPlaceholder|Search branches"
 msgstr ""
 
@@ -14916,9 +14913,6 @@ msgstr ""
 msgid "Introducing Your DevOps Report"
 msgstr ""
 
-msgid "Invalid Git ref"
-msgstr ""
-
 msgid "Invalid Insights config file detected"
 msgstr ""
 
@@ -17822,9 +17816,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 ""