Look for rugged with static analysis
This commit is contained in:
parent
1f5af51b47
commit
6d6f7536bd
13 changed files with 76 additions and 74 deletions
|
@ -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
|
||||
|
||||
|
|
|
@ -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!
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
|
@ -1,3 +1,5 @@
|
|||
# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/953
|
||||
#
|
||||
module Gitlab
|
||||
module BareRepositoryImport
|
||||
class Repository
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
36
scripts/lint-rugged
Executable file
36
scripts/lint-rugged
Executable file
|
@ -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)
|
|
@ -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|
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
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
|
||||
|
||||
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
|
||||
context 'with Gitaly enabled' do
|
||||
it_behaves_like('#ancestor?')
|
||||
end
|
||||
|
||||
it 'it is an ancestor' do
|
||||
expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true)
|
||||
end
|
||||
|
||||
it 'it is not an ancestor' do
|
||||
expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false)
|
||||
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
|
||||
|
||||
|
|
Loading…
Reference in a new issue