From 23827949d849970a0d94ad12aa154ddd7ca8b3b9 Mon Sep 17 00:00:00 2001 From: James Fargher Date: Wed, 10 May 2023 10:09:18 +1200 Subject: [PATCH 1/4] Stop relying on only branch being the default in branches feature spec --- spec/features/projects/branches_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb index e1f1a63565c1d9..b8598035b6c82b 100644 --- a/spec/features/projects/branches_spec.rb +++ b/spec/features/projects/branches_spec.rb @@ -275,11 +275,11 @@ let_it_be(:project) { create(:project, :public, :empty_repo) } before do - sha = create_file(branch_name: "branch") + sha = create_file(branch_name: "master") create(:ci_pipeline, project: project, user: user, - ref: "branch", + ref: "master", sha: sha, status: :success, created_at: 5.months.ago) -- GitLab From 407b40c11d412fa50553c47471212763fb0e7d4b Mon Sep 17 00:00:00 2001 From: James Fargher Date: Wed, 10 May 2023 10:15:18 +1200 Subject: [PATCH 2/4] Use actual default branch in CI editor feature spec --- spec/features/projects/ci/editor_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/projects/ci/editor_spec.rb b/spec/features/projects/ci/editor_spec.rb index 20c1ef1b21f45c..7cfe00792da9bb 100644 --- a/spec/features/projects/ci/editor_spec.rb +++ b/spec/features/projects/ci/editor_spec.rb @@ -8,7 +8,7 @@ let(:project) { create(:project_empty_repo, :public) } let(:user) { create(:user) } - let(:default_branch) { 'main' } + let(:default_branch) { 'master' } let(:other_branch) { 'test' } before do -- GitLab From 9c97e08aec1b46aab7836a1d4bb7990194ac6bb3 Mon Sep 17 00:00:00 2001 From: James Fargher Date: Tue, 2 May 2023 09:44:43 +1200 Subject: [PATCH 3/4] Ensure the default branch is always set One of the complications to retrieving the default branch from gitaly is that gitaly uses a heuristic instead of solely relying on the repository HEAD. This heuristic will return nothing on empty repositories. Gitaly are looking to remove this heuristic and simplify the code such that it always returns HEAD. Doing this means that the RPC FindDefaultBranchName will always return a branch name. Here we are trying to minimise the differences between these two implementations by ensuring HEAD is set correctly even on empty repositories. --- app/models/repository.rb | 2 ++ app/models/snippet.rb | 2 +- app/models/wiki.rb | 2 +- lib/gitlab/git/repository.rb | 4 ++++ spec/factories/projects.rb | 14 ++++---------- .../backfill_snippet_repositories_spec.rb | 2 +- spec/lib/gitlab/git/repository_spec.rb | 14 ++++++++++++++ spec/models/snippet_spec.rb | 4 ++-- 8 files changed, 29 insertions(+), 15 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index e942157993b8dc..ebc470cd8c825e 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -552,6 +552,8 @@ def blobs_at(items, blob_size_limit: Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE) end def root_ref + return if empty? + raw_repository&.root_ref end cache_method_asymmetrically :root_ref diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 181130d45bdae9..2b94a12f55c14d 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -334,7 +334,7 @@ def repository_storage def create_repository return if repository_exists? && snippet_repository - repository.create_if_not_exists(default_branch) + repository.create_if_not_exists(default_branch_from_preferences) track_snippet_repository(repository.storage) end diff --git a/app/models/wiki.rb b/app/models/wiki.rb index 39d22ea0e07594..f67d955703c0f0 100644 --- a/app/models/wiki.rb +++ b/app/models/wiki.rb @@ -170,7 +170,7 @@ def path end def create_wiki_repository - repository.create_if_not_exists(default_branch) + repository.create_if_not_exists(default_branch_from_preferences) raise CouldNotCreateWikiError unless repository_exists? rescue StandardError => e diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 80d0fd17568fb4..9e6a1ba4c200a8 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -92,6 +92,8 @@ def path # Default branch in the repository def root_ref + return if empty? # TODO: Is this a good idea? + gitaly_ref_client.default_branch_name rescue GRPC::NotFound => e raise NoRepository, e.message @@ -104,6 +106,8 @@ def exists? end def create_repository(default_branch = nil) + default_branch ||= Gitlab::DefaultBranch.value(object: self) + wrapped_gitaly_errors do gitaly_repository_client.create_repository(default_branch) rescue GRPC::AlreadyExists => e diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 856f0f6cd055bb..55bd00648ad3ed 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -231,7 +231,7 @@ end after :create do |project, evaluator| - raise "Failed to create repository!" unless project.repository.exists? || project.create_repository + raise "Failed to create repository!" unless project.repository.exists? || project.create_repository(default_branch: 'master') evaluator.files.each do |filename, content| project.repository.create_file( @@ -239,23 +239,17 @@ filename, content, message: "Automatically created file #{filename}", - branch_name: project.default_branch || 'master' + branch_name: 'master' ) end end end - # A basic repository with a single file 'test.txt'. It also has the HEAD as the default branch. + # A basic repository with a single file 'test.txt' in the default branch master trait :small_repo do custom_repo files { { 'test.txt' => 'test' } } - - after(:create) do |project| - Sidekiq::Worker.skipping_transaction_check do - raise "Failed to assign the repository head!" unless project.change_head(project.default_branch_or_main) - end - end end # Test repository - https://gitlab.com/gitlab-org/gitlab-test @@ -340,7 +334,7 @@ trait :empty_repo do after(:create) do |project| - raise "Failed to create repository!" unless project.create_repository + raise "Failed to create repository!" unless project.create_repository(default_branch: 'master') end end diff --git a/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb b/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb index 4a50d08b2aad5d..b51c046e0f6115 100644 --- a/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb @@ -49,7 +49,7 @@ allow(snippet_with_repo).to receive(:disk_path).and_return(disk_path(snippet_with_repo)) raw_repository(snippet_with_repo).create_from_bundle(TestEnv.factory_repo_bundle_path) - raw_repository(snippet_with_empty_repo).create_repository + raw_repository(snippet_with_empty_repo).create_repository('main') end after do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 06904849ef51d2..4b5fe9f1452159 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -54,6 +54,20 @@ it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RepositoryService, :create_repository do subject { repository.create_repository } end + + it 'uses the system default branch' do + allow(repository.gitaly_repository_client) + .to receive(:create_repository).with('master') + + repository.create_repository + end + + it 'uses the passed default branch' do + allow(repository.gitaly_repository_client) + .to receive(:create_repository).with('pineapple') + + repository.create_repository('pineapple') + end end describe '#branch_names' do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 6a5456fce3fd93..6315f3e51cfeb5 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -709,7 +709,7 @@ def to_json(params = {}) end it 'sets the default branch' do - expect(snippet).to receive(:default_branch).and_return('default-branch-1') + expect(snippet).to receive(:default_branch_from_preferences).and_return('default-branch-1') expect(subject).to be_truthy snippet.repository.create_file(snippet.author, 'file', 'content', message: 'initial commit', branch_name: 'default-branch-1') @@ -728,7 +728,7 @@ def to_json(params = {}) expect(snippet).to receive(:repository_storage).and_return('picked') expect(snippet).to receive(:repository_exists?).and_return(false) expect(snippet.repository).to receive(:create_if_not_exists) - allow(snippet).to receive(:default_branch).and_return('picked') + allow(snippet).to receive(:default_branch_from_preferences).and_return('picked') subject -- GitLab From 3add6a09fd3bce22c1dde2074e01c9791e9fbc58 Mon Sep 17 00:00:00 2001 From: James Fargher Date: Tue, 2 May 2023 11:55:54 +1200 Subject: [PATCH 4/4] Temporarily switch gitaly to add a feature --- GITALY_SERVER_VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 3cc5d640565cb2..268778047be1a2 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -91b69d050acf344c09a9238f24a75c4938001113 +3b00b12130fcaf0a6d52cdad2676520e14d2dd51 -- GitLab