summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/ci/pipeline.rb2
-rw-r--r--app/models/deployment.rb9
-rw-r--r--app/workers/repository_check/single_repository_worker.rb5
-rw-r--r--config/initializers/rugged_use_gitlab_git_attributes.rb28
-rw-r--r--lib/gitlab/bare_repository_import/repository.rb2
-rw-r--r--lib/gitlab/git/repository.rb2
-rw-r--r--lib/gitlab/gitaly_client/commit_service.rb20
-rw-r--r--lib/gitlab/github_import/importer/pull_requests_importer.rb5
-rw-r--r--lib/tasks/gitlab/cleanup.rake2
-rwxr-xr-xscripts/lint-rugged36
-rwxr-xr-xscripts/static-analysis3
-rw-r--r--spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb6
-rw-r--r--spec/models/repository_spec.rb28
13 files changed, 75 insertions, 73 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index d7153d7b816..f84bf132854 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -524,7 +524,7 @@ module Ci
return unless sha
project.repository.gitlab_ci_yml_for(sha, ci_yaml_file_path)
- rescue GRPC::NotFound, Rugged::ReferenceError, GRPC::Internal
+ rescue GRPC::NotFound, GRPC::Internal
nil
end
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 7bcded5b5e1..3aed071dd49 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -45,14 +45,7 @@ class Deployment < ActiveRecord::Base
def includes_commit?(commit)
return false unless commit
- # Before 8.10, deployments didn't have keep-around refs. Any deployment
- # created before then could have a `sha` referring to a commit that no
- # longer exists in the repository, so just ignore those.
- begin
- project.repository.ancestor?(commit.id, sha)
- rescue Rugged::OdbError
- false
- end
+ project.repository.ancestor?(commit.id, sha)
end
def update_merge_request_metrics!
diff --git a/app/workers/repository_check/single_repository_worker.rb b/app/workers/repository_check/single_repository_worker.rb
index 4e3c691e8da..116bc185b38 100644
--- a/app/workers/repository_check/single_repository_worker.rb
+++ b/app/workers/repository_check/single_repository_worker.rb
@@ -20,10 +20,7 @@ module RepositoryCheck
# Historically some projects never had their wiki repos initialized;
# this happens on project creation now. Let's initialize an empty repo
# if it is not already there.
- begin
- project.create_wiki
- rescue Rugged::RepositoryError
- end
+ project.create_wiki
git_fsck(project.wiki.repository)
else
diff --git a/config/initializers/rugged_use_gitlab_git_attributes.rb b/config/initializers/rugged_use_gitlab_git_attributes.rb
deleted file mode 100644
index c0d45caec42..00000000000
--- a/config/initializers/rugged_use_gitlab_git_attributes.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-# We don't want to ever call Rugged::Repository#fetch_attributes, because it has
-# a lot of I/O overhead:
-# <https://gitlab.com/gitlab-org/gitlab_git/commit/340e111e040ae847b614d35b4d3173ec48329015>
-#
-# While we don't do this from within the GitLab source itself, the Linguist gem
-# has a dependency on Rugged and uses the gitattributes file when calculating
-# repository-wide language statistics:
-# <https://github.com/github/linguist/blob/v4.7.0/lib/linguist/lazy_blob.rb#L33-L36>
-#
-# The options passed by Linguist are those assumed by Gitlab::Git::InfoAttributes
-# anyway, and there is no great efficiency gain from just fetching the listed
-# attributes with our implementation, so we ignore the additional arguments.
-#
-module Rugged
- class Repository
- module UseGitlabGitAttributes
- def fetch_attributes(name, *)
- attributes.attributes(name)
- end
-
- def attributes
- @attributes ||= Gitlab::Git::InfoAttributes.new(path)
- end
- end
-
- prepend UseGitlabGitAttributes
- end
-end
diff --git a/lib/gitlab/bare_repository_import/repository.rb b/lib/gitlab/bare_repository_import/repository.rb
index c0c666dfb7b..fe267248275 100644
--- a/lib/gitlab/bare_repository_import/repository.rb
+++ b/lib/gitlab/bare_repository_import/repository.rb
@@ -1,3 +1,5 @@
+# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/953
+#
module Gitlab
module BareRepositoryImport
class Repository
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index d6436c8c8a6..5333766c338 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -563,6 +563,8 @@ module Gitlab
return false if ancestor_id.nil? || descendant_id.nil?
merge_base_commit(ancestor_id, descendant_id) == ancestor_id
+ rescue Rugged::OdbError
+ false
end
# Returns true is +from+ is direct ancestor to +to+, otherwise false
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 33a8d3e5612..cadc7149301 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -38,19 +38,27 @@ module Gitlab
from_id = case from
when NilClass
EMPTY_TREE_ID
- when Rugged::Commit
- from.oid
else
- from
+ if from.respond_to?(:oid)
+ # This is meant to match a Rugged::Commit. This should be impossible in
+ # the future.
+ from.oid
+ else
+ from
+ end
end
to_id = case to
when NilClass
EMPTY_TREE_ID
- when Rugged::Commit
- to.oid
else
- to
+ if to.respond_to?(:oid)
+ # This is meant to match a Rugged::Commit. This should be impossible in
+ # the future.
+ to.oid
+ else
+ to
+ end
end
request_params = diff_between_commits_request_params(from_id, to_id, options)
diff --git a/lib/gitlab/github_import/importer/pull_requests_importer.rb b/lib/gitlab/github_import/importer/pull_requests_importer.rb
index 5437e32e9f1..e70361c163b 100644
--- a/lib/gitlab/github_import/importer/pull_requests_importer.rb
+++ b/lib/gitlab/github_import/importer/pull_requests_importer.rb
@@ -57,10 +57,7 @@ module Gitlab
end
def commit_exists?(sha)
- project.repository.lookup(sha)
- true
- rescue Rugged::Error
- false
+ project.repository.commit(sha).present?
end
def collection_method
diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake
index 04d56509ac6..ab601b0d66b 100644
--- a/lib/tasks/gitlab/cleanup.rake
+++ b/lib/tasks/gitlab/cleanup.rake
@@ -1,3 +1,5 @@
+# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/954
+#
namespace :gitlab do
namespace :cleanup do
HASHED_REPOSITORY_NAME = '@hashed'.freeze
diff --git a/scripts/lint-rugged b/scripts/lint-rugged
new file mode 100755
index 00000000000..3f8fcb558e3
--- /dev/null
+++ b/scripts/lint-rugged
@@ -0,0 +1,36 @@
+#!/usr/bin/env ruby
+
+ALLOWED = [
+ # Can be deleted (?) once rugged is no longer used in production. Doesn't make Rugged calls.
+ 'config/initializers/8_metrics.rb',
+
+ # Can be deleted once wiki's are fully (mandatory) migrated
+ 'config/initializers/gollum.rb',
+
+ # Needs to be migrated, https://gitlab.com/gitlab-org/gitaly/issues/953
+ 'lib/gitlab/bare_repository_import/repository.rb',
+
+ # Needs to be migrated, https://gitlab.com/gitlab-org/gitaly/issues/954
+ 'lib/tasks/gitlab/cleanup.rake',
+
+ # https://gitlab.com/gitlab-org/gitaly/issues/961
+ 'app/models/repository.rb',
+
+ # The only place where Rugged code is still allowed in production
+ 'lib/gitlab/git/'
+].freeze
+
+rugged_lines = IO.popen(%w[git grep -i -n rugged -- app config lib], &:read).lines
+rugged_lines = rugged_lines.reject { |l| l.start_with?(*ALLOWED) }
+rugged_lines = rugged_lines.reject do |line|
+ code, _comment = line.split('# ', 2)
+ code !~ /rugged/i
+end
+
+exit if rugged_lines.empty?
+
+puts "Using Rugged is only allowed in test and #{ALLOWED}\n\n"
+
+puts rugged_lines
+
+exit(false)
diff --git a/scripts/static-analysis b/scripts/static-analysis
index 9690b42c788..96d08287ded 100755
--- a/scripts/static-analysis
+++ b/scripts/static-analysis
@@ -13,7 +13,8 @@ tasks = [
%w[bundle exec rake gettext:lint],
%w[bundle exec rake lint:static_verification],
%w[scripts/lint-changelog-yaml],
- %w[scripts/lint-conflicts.sh]
+ %w[scripts/lint-conflicts.sh],
+ %w[scripts/lint-rugged]
]
failed_tasks = tasks.reduce({}) do |failures, task|
diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb
index d72572cd510..44695acbe7d 100644
--- a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb
@@ -244,7 +244,7 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do
it 'returns true when a commit exists' do
expect(project.repository)
- .to receive(:lookup)
+ .to receive(:commit)
.with('123')
.and_return(double(:commit))
@@ -253,9 +253,9 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do
it 'returns false when a commit does not exist' do
expect(project.repository)
- .to receive(:lookup)
+ .to receive(:commit)
.with('123')
- .and_raise(Rugged::OdbError)
+ .and_return(nil)
expect(importer.commit_exists?('123')).to eq(false)
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 8f406253f39..7c61c6b7299 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -2356,7 +2356,7 @@ describe Repository do
let(:commit) { repository.commit }
let(:ancestor) { commit.parents.first }
- context 'with Gitaly enabled' do
+ shared_examples '#ancestor?' do
it 'it is an ancestor' do
expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true)
end
@@ -2370,27 +2370,19 @@ describe Repository do
expect(repository.ancestor?(ancestor.id, nil)).to eq(false)
expect(repository.ancestor?(nil, nil)).to eq(false)
end
- end
-
- context 'with Gitaly disabled' do
- before do
- allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(false)
- allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(false)
- end
- it 'it is an ancestor' do
- expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true)
+ it 'returns false for invalid commit IDs' do
+ expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false)
+ expect(repository.ancestor?( Gitlab::Git::BLANK_SHA, commit.id)).to eq(false)
end
+ end
- it 'it is not an ancestor' do
- expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false)
- end
+ context 'with Gitaly enabled' do
+ it_behaves_like('#ancestor?')
+ end
- it 'returns false on nil-values' do
- expect(repository.ancestor?(nil, commit.id)).to eq(false)
- expect(repository.ancestor?(ancestor.id, nil)).to eq(false)
- expect(repository.ancestor?(nil, nil)).to eq(false)
- end
+ context 'with Gitaly disabled', :skip_gitaly_mock do
+ it_behaves_like('#ancestor?')
end
end
View Lorry Controller Status