Automatic merge of gitlab-org/gitlab master

app/models/repository.rb

@@ -1125,7 +1125,11 @@ class Repository
       copy_gitattributes(branch)
       after_change_head
     else
+      # For example, `Wiki` does not have `errors` because it is not an `ActiveModel`
+      if container.respond_to?(:errors)
         container.errors.add(:base, _("Could not change HEAD: branch '%{branch}' does not exist") % { branch: branch })
+      end
+
       false
     end
   end

db/post_migrate/20210823193234_remove_allow_editing_commit_messages_from_project_settings.rb

-# frozen_string_literal: true
-
-class RemoveAllowEditingCommitMessagesFromProjectSettings < ActiveRecord::Migration[6.1]
-  include Gitlab::Database::MigrationHelpers
-
-  def up
-    with_lock_retries do
-      remove_column :project_settings, :allow_editing_commit_messages
-    end
-  end
-
-  def down
-    with_lock_retries do
-      add_column :project_settings, :allow_editing_commit_messages, :boolean, default: false, null: false
-    end
-  end
-end

db/schema_migrations/20210823193234

-b85ef326056bb152d527e34b49caa3c40ee8685c3b14654992246c6adf082f8c
\ No newline at end of file

db/structure.sql

@@ -17343,6 +17343,7 @@ CREATE TABLE project_settings (
     squash_option smallint DEFAULT 3,
     has_confluence boolean DEFAULT false NOT NULL,
     has_vulnerabilities boolean DEFAULT false NOT NULL,
+    allow_editing_commit_messages boolean DEFAULT false NOT NULL,
     prevent_merge_without_jira_issue boolean DEFAULT false NOT NULL,
     cve_id_request_enabled boolean DEFAULT true NOT NULL,
     mr_default_target_self boolean DEFAULT false NOT NULL,

doc/ci/yaml/index.md

@@ -1595,7 +1595,9 @@ production:
   can refer to jobs in the same stage as the job you are configuring. This feature is
   enabled on GitLab.com and ready for production use. On self-managed [GitLab 14.2 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/30632)
   this feature is available by default.
-- In GitLab 14.0 and older, you can only refer to jobs in earlier stages.
+- In GitLab 14.0 and older, you can only refer to jobs in earlier stages. Stages must be
+  explicitly defined for all jobs that use the `needs:` keyword, or are referenced
+  in a job's `needs:` section.
 - In GitLab 13.9 and older, if `needs:` refers to a job that might not be added to
   a pipeline because of `only`, `except`, or `rules`, the pipeline might fail to create.
 - The maximum number of jobs that a single job can need in the `needs:` array is limited:
@@ -1609,8 +1611,6 @@ production:
 - `needs:` is similar to `dependencies:` in that it must use jobs from prior stages,
   meaning it's impossible to create circular dependencies. Depending on jobs in the
   current stage is not possible either, but [an issue exists](https://gitlab.com/gitlab-org/gitlab/-/issues/30632).
-- Stages must be explicitly defined for all jobs
-  that have the keyword `needs:` or are referred to by one.
 
 ##### Changing the `needs:` job limit **(FREE SELF)**
 

ee/app/services/geo/framework_repository_sync_service.rb

@@ -37,7 +37,6 @@ module Geo
     def sync_repository
       start_registry_sync!
       fetch_repository
-      update_root_ref
       mark_sync_as_successful
     rescue Gitlab::Git::Repository::NoRepository => e
       log_info('Marking the repository for a forced re-download')
@@ -83,6 +82,8 @@ module Geo
         fetch_geo_mirror(repository)
         @new_repository = true
       end
+
+      update_root_ref
     end
 
     def redownload_repository

ee/app/services/geo/repository_base_sync_service.rb

@@ -63,6 +63,8 @@ module Geo
         fetch_geo_mirror(repository)
         @new_repository = true
       end
+
+      update_root_ref
     end
 
     def redownload?
@@ -270,5 +272,13 @@ module Geo
       checksum = project.repository_state.public_send("#{type}_verification_checksum") # rubocop:disable GitlabSecurity/PublicSend
       checksum && checksum != Gitlab::Git::Repository::EMPTY_REPOSITORY_CHECKSUM
     end
+
+    def update_root_ref
+      authorization = ::Gitlab::Geo::RepoSyncRequest.new(
+        scope: repository.full_path
+      ).authorization
+
+      repository.update_root_ref(remote_url, authorization)
+    end
   end
 end

ee/app/services/geo/repository_sync_service.rb

@@ -9,7 +9,6 @@ module Geo
     def sync_repository
       start_registry_sync!
       fetch_repository
-      update_root_ref
       mark_sync_as_successful
     rescue Gitlab::Git::Repository::NoRepository => e
       log_info('Setting force_to_redownload flag')
@@ -49,14 +48,6 @@ module Geo
       project.ensure_repository
     end
 
-    def update_root_ref
-      authorization = ::Gitlab::Geo::RepoSyncRequest.new(
-        scope: repository.full_path
-      ).authorization
-
-      repository.update_root_ref(remote_url, authorization)
-    end
-
     def execute_housekeeping
       Geo::ProjectHousekeepingService.new(project, new_repository: new_repository?).execute
     end

ee/spec/services/geo/design_repository_sync_service_spec.rb

@@ -35,12 +35,11 @@ RSpec.describe Geo::DesignRepositorySyncService do
       stub_exclusive_lease(lease_key, lease_uuid)
       stub_exclusive_lease("geo_project_housekeeping:#{project.id}")
 
-      allow_any_instance_of(Repository).to receive(:fetch_as_mirror)
-        .and_return(true)
+      allow(repository).to receive(:fetch_as_mirror).and_return(true)
 
-      allow_any_instance_of(Repository)
+      allow(repository)
         .to receive(:find_remote_root_ref)
-        .with(url_to_repo)
+        .with(url_to_repo, anything)
         .and_return('master')
 
       allow_any_instance_of(Geo::ProjectHousekeepingService).to receive(:execute)

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -34,7 +34,7 @@ RSpec.describe Geo::RepositorySyncService, :geo do
       allow_any_instance_of(Repository).to receive(:fetch_as_mirror)
         .and_return(true)
 
-      allow_any_instance_of(Repository)
+      allow(repository)
         .to receive(:find_remote_root_ref)
         .with(url_to_repo, anything)
         .and_return('master')

ee/spec/services/geo/wiki_sync_service_spec.rb

@@ -45,6 +45,8 @@ RSpec.describe Geo::WikiSyncService, :geo do
     end
 
     it 'voids the failure message when it succeeds after an error' do
+      allow(repository).to receive(:update_root_ref)
+
       registry = create(:geo_project_registry, project: project, last_wiki_sync_failure: 'error')
 
       expect { subject.execute }.to change { registry.reload.last_wiki_sync_failure }.to(nil)
@@ -126,6 +128,8 @@ RSpec.describe Geo::WikiSyncService, :geo do
     end
 
     it 'marks primary_wiki_checksummed as true when wiki has been verified on primary' do
+      allow(repository).to receive(:update_root_ref)
+
       create(:repository_state, :wiki_verified, project: project)
       registry = create(:geo_project_registry, project: project, primary_wiki_checksummed: false)
 
@@ -133,6 +137,8 @@ RSpec.describe Geo::WikiSyncService, :geo do
     end
 
     it 'marks primary_wiki_checksummed as false when wiki has not been verified on primary' do
+      allow(repository).to receive(:update_root_ref)
+
       create(:repository_state, :wiki_failed, project: project)
       registry = create(:geo_project_registry, project: project, primary_wiki_checksummed: true)
 
@@ -166,6 +172,8 @@ RSpec.describe Geo::WikiSyncService, :geo do
         end
 
         it 'sets last_wiki_successful_sync_at' do
+          allow(repository).to receive(:update_root_ref)
+
           subject.execute
 
           expect(registry.last_wiki_successful_sync_at).not_to be_nil
@@ -190,7 +198,9 @@ RSpec.describe Geo::WikiSyncService, :geo do
         end
 
         it 'logs success with timings' do
+          allow(repository).to receive(:update_root_ref)
           allow(Gitlab::Geo::Logger).to receive(:info).and_call_original
+
           expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including(:message, :update_delay_s, :download_time_s)).and_call_original
 
           subject.execute
@@ -232,6 +242,7 @@ RSpec.describe Geo::WikiSyncService, :geo do
             force_to_redownload_wiki: true
           )
 
+          allow(project.wiki.repository).to receive(:update_root_ref)
           expect(project.wiki.repository).to receive(:expire_exists_cache).exactly(3).times.and_call_original
           expect(subject).not_to receive(:fail_registry_sync!)
 

ee/spec/support/shared_examples/services/base_sync_service_shared_examples.rb

@@ -65,6 +65,7 @@ RSpec.shared_examples 'geo base sync fetch' do
 
     before do
       allow(subject).to receive(:fetch_geo_mirror).and_return(true)
+      allow(repository).to receive(:update_root_ref)
     end
 
     it 'cleans up temporary repository' do
@@ -79,6 +80,12 @@ RSpec.shared_examples 'geo base sync fetch' do
       fetch_repository
     end
 
+    it 'syncs the HEAD ref' do
+      expect(repository).to receive(:update_root_ref)
+
+      fetch_repository
+    end
+
     context 'repository does not exist' do
       before do
         allow_any_instance_of(Repository).to receive(:exists?) { false }