diff options
45 files changed, 220 insertions, 560 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d8c4e965190..d4f7615c80e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -445,7 +445,6 @@ Style/Dir: # Cop supports --auto-correct. Style/EachWithObject: Exclude: - - 'config/initializers/gollum.rb' - 'lib/expand_variables.rb' - 'lib/gitlab/ci/ansi2html.rb' - 'lib/gitlab/ee_compat_check.rb' @@ -80,11 +80,9 @@ gem 'gitlab_omniauth-ldap', '~> 2.0.4', require: 'omniauth-ldap' gem 'net-ldap' # Git Wiki -# Required manually in config/initializers/gollum.rb to control load order +# Only used to compute wiki page slugs gem 'gitlab-gollum-lib', '~> 4.2', require: false -gem 'gitlab-gollum-rugged_adapter', '~> 0.4.4', require: false - # Language detection gem 'github-linguist', '~> 5.3.3', require: 'linguist' @@ -134,6 +132,7 @@ gem 'seed-fu', '~> 2.3.7' gem 'html-pipeline', '~> 2.8' gem 'deckar01-task_list', '2.0.0' gem 'gitlab-markup', '~> 1.6.4' +gem 'github-markup', '~> 1.7.0', require: 'github/markup' gem 'redcarpet', '~> 3.4' gem 'commonmarker', '~> 0.17' gem 'RedCloth', '~> 4.3.2' diff --git a/Gemfile.lock b/Gemfile.lock index 69a1c899cc5..30c666932d1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -295,9 +295,6 @@ GEM rouge (~> 3.1) sanitize (~> 4.6.4) stringex (~> 2.6) - gitlab-gollum-rugged_adapter (0.4.4.1) - mime-types (>= 1.15) - rugged (~> 0.25) gitlab-grit (2.8.2) charlock_holmes (~> 0.6) diff-lcs (~> 1.1) @@ -1030,9 +1027,9 @@ DEPENDENCIES gettext_i18n_rails_js (~> 1.3) gitaly-proto (~> 0.118.1) github-linguist (~> 5.3.3) + github-markup (~> 1.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) - gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.4) gitlab-styles (~> 2.4) gitlab_omniauth-ldap (~> 2.0.4) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 30605e460e6..f465a77d0c6 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -298,9 +298,6 @@ GEM rouge (~> 3.1) sanitize (~> 4.6.4) stringex (~> 2.6) - gitlab-gollum-rugged_adapter (0.4.4.1) - mime-types (>= 1.15) - rugged (~> 0.25) gitlab-grit (2.8.2) charlock_holmes (~> 0.6) diff-lcs (~> 1.1) @@ -1039,9 +1036,9 @@ DEPENDENCIES gettext_i18n_rails_js (~> 1.3) gitaly-proto (~> 0.118.1) github-linguist (~> 5.3.3) + github-markup (~> 1.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) - gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.4) gitlab-styles (~> 2.4) gitlab_omniauth-ldap (~> 2.0.4) diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 102907a8bd3..42fd213d03b 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -58,7 +58,7 @@ class WikiPage attr_reader :page # The attributes Hash used for storing and validating - # new Page values before writing to the Gollum repository. + # new Page values before writing to the raw repository. attr_accessor :attributes def hook_attrs @@ -111,10 +111,7 @@ class WikiPage # The processed/formatted content of this page. def formatted_content - # Assuming @page exists, nil formatted_data means we didn't load it - # before hand (i.e. page was fetched by Gitaly), so we fetch it separately. - # If the page was fetched by Gollum, formatted_data would've been a String. - @attributes[:formatted_content] ||= @page&.formatted_data || @wiki.page_formatted_data(@page) + @attributes[:formatted_content] ||= @wiki.page_formatted_data(@page) end # The markup format for the page. diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 4d8d35bf6cf..eccf82ab8dc 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -3,7 +3,6 @@ # that we can stub it for testing, as it is only called when metrics are # enabled. # -# rubocop:disable Metrics/AbcSize def instrument_classes(instrumentation) instrumentation.instrument_instance_methods(Gitlab::Shell) @@ -48,16 +47,6 @@ def instrument_classes(instrumentation) instrumentation.instrument_methods(Premailer::Adapter::Nokogiri) instrumentation.instrument_instance_methods(Premailer::Adapter::Nokogiri) - [ - :Blame, :Branch, :BranchCollection, :Blob, :Commit, :Diff, :Repository, - :Tag, :TagCollection, :Tree - ].each do |name| - const = Rugged.const_get(name) - - instrumentation.instrument_methods(const) - instrumentation.instrument_instance_methods(const) - end - instrumentation.instrument_methods(Banzai::Renderer) instrumentation.instrument_methods(Banzai::Querying) @@ -101,7 +90,6 @@ def instrument_classes(instrumentation) # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/30224#note_32306159 instrumentation.instrument_instance_method(MergeRequestDiff, :load_commits) end -# rubocop:enable Metrics/AbcSize # With prometheus enabled by default this breaks all specs # that stubs methods using `any_instance_of` for the models reloaded here. diff --git a/config/initializers/gollum.rb b/config/initializers/gollum.rb deleted file mode 100644 index ea9cc151a57..00000000000 --- a/config/initializers/gollum.rb +++ /dev/null @@ -1,28 +0,0 @@ -# WARNING changes in this file must be manually propagated to gitaly-ruby. -# -# https://gitlab.com/gitlab-org/gitaly/blob/master/ruby/lib/gitlab/gollum.rb - -module Gollum - GIT_ADAPTER = "rugged".freeze -end -require "gollum-lib" - -module Gollum - class Page - def text_data(encoding = nil) - data = if raw_data.respond_to?(:encoding) - raw_data.force_encoding(encoding || Encoding::UTF_8) - else - raw_data - end - - Gitlab::EncodingHelper.encode!(data) - end - end -end - -Rails.application.configure do - config.after_initialize do - Gollum::Page.per_page = Kaminari.config.default_per_page - end -end diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index f4784c19359..32beafad307 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -1,11 +1,7 @@ # GitLab Developers Guide to Working with Gitaly [Gitaly](https://gitlab.com/gitlab-org/gitaly) is a high-level Git RPC service used by GitLab CE/EE, -Workhorse and GitLab-Shell. All Rugged operations in GitLab CE/EE are currently being phased out to -be replaced by Gitaly API calls. - -Visit the [Gitaly Migration Board](https://gitlab.com/gitlab-org/gitaly/boards/331341) for current -status of the migration. +Workhorse and GitLab-Shell. ## Developing new Git features @@ -52,57 +48,6 @@ comfortable writing Go code. There is documentation for this approach in [the Gitaly repo](https://gitlab.com/gitlab-org/gitaly/blob/master/doc/ruby_endpoint.md). -## Modifying existing Git features - -If you modify existing Git features in `lib/gitlab/git` you need to make -sure the changes also work in Gitaly. Because we are still in the -migration process there are a number of subtle pitfalls. Features that -have been migrated have dual implementations (Gitaly and local). The -Gitaly implementation may or may not use a vendored (and therefore -possibly outdated) copy of the local implementation in `lib/gitlab/git`. - -To avoid unexpected problems and conflicts, all changes to -`lib/gitlab/git` need to be approved by a member of the Gitaly team. - -For the time being, while the Gitaly migration is still in progress, -there should be no Enterprise Edition-only Git code in -`lib/gitlab/git`. Also no mixins. - -## Feature Flags - -Gitaly makes heavy use of [feature flags](feature_flags.md). - -Each Rugged-to-Gitaly migration goes through a [series of phases](https://gitlab.com/gitlab-org/gitaly/blob/master/doc/MIGRATION_PROCESS.md): - -* **Opt-In**: by default the Rugged implementation is used. - * Production instances can choose to enable the Gitaly endpoint by enabling the feature flag. - * For testing purposes, you may wish to enable all feature flags by default. This can be done by exporting the following - environment variable: `GITALY_FEATURE_DEFAULT_ON=1`. - * On developer instances (ie, when `Rails.env.development?` is true), the Gitaly endpoint - is enabled by default, but can be _disabled_ using feature flags. -* **Opt-Out**: by default, the Gitaly endpoint is used, but the feature can be explicitly disabled using the feature flag. -* **Mandatory**: The migration is complete and cannot be disabled. The old codepath is removed. - -### Enabling and Disabling Feature - -In the Rails console, type: - -```ruby -Feature.enable(:gitaly_feature_name) -Feature.disable(:gitaly_feature_name) -``` - -Where `gitaly_feature_name` is the name of the Gitaly feature. This can be determined by finding the appropriate -`gitaly_migrate` code block, for example: - -```ruby -gitaly_migrate(:tag_names) do -... -end -``` - -Since Gitaly features are always prefixed with `gitaly_`, the name of the feature flag in this case would be `gitaly_tag_names`. - ## Gitaly-Related Test Failures If your test-suite is failing with Gitaly issues, as a first step, try running: diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md index a14c0752366..7761f65d78a 100644 --- a/doc/development/instrumentation.md +++ b/doc/development/instrumentation.md @@ -69,7 +69,7 @@ The easiest way to check if a method has been instrumented is to check its source location. For example: ```ruby -method = Rugged::TagCollection.instance_method(:[]) +method = Banzai::Renderer.method(:render) method.source_location ``` @@ -82,7 +82,7 @@ method (along with its source location), this is easier than running the above Ruby code. In case of the above snippet you'd run the following: ``` -$ Rugged::TagCollection#[] +$ Banzai::Renderer.render ``` This will print out something along the lines of: diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 5b264868af0..74cdabfed9d 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -53,9 +53,6 @@ module Gitlab # Already a commit? return commit_id if commit_id.is_a?(Gitlab::Git::Commit) - # A rugged reference? - commit_id = Gitlab::Git::Ref.dereference_object(commit_id) - # Some weird thing? return nil unless commit_id.is_a?(String) @@ -127,8 +124,6 @@ module Gitlab # :topo, or any combination of them (in an array). Commit ordering types # are documented here: # http://www.rubydoc.info/github/libgit2/rugged/Rugged#SORT_NONE-constant) - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/326 def find_all(repo, options = {}) repo.wrapped_gitaly_errors do Gitlab::GitalyClient::CommitService.new(repo).find_all_commits(options) @@ -328,7 +323,6 @@ module Gitlab entry = @repository.gitaly_commit_client.tree_entry(id, path, 1) return unless entry - # To be compatible with the rugged format entry = entry.to_h entry.delete(:data) entry[:name] = File.basename(path) @@ -346,8 +340,8 @@ module Gitlab subject: message_split[0] ? message_split[0].chomp.b : "", body: raw_commit.message.b, parent_ids: raw_commit.parent_ids, - author: gitaly_commit_author_from_rugged(raw_commit.author), - committer: gitaly_commit_author_from_rugged(raw_commit.committer) + author: gitaly_commit_author_from_raw(raw_commit.author), + committer: gitaly_commit_author_from_raw(raw_commit.committer) ) end @@ -381,7 +375,7 @@ module Gitlab SERIALIZE_KEYS end - def gitaly_commit_author_from_rugged(author_or_committer) + def gitaly_commit_author_from_raw(author_or_committer) Gitaly::CommitAuthor.new( name: author_or_committer[:name].b, email: author_or_committer[:email].b, diff --git a/lib/gitlab/git/ref.rb b/lib/gitlab/git/ref.rb index fa71a4e7ea7..31a280155bd 100644 --- a/lib/gitlab/git/ref.rb +++ b/lib/gitlab/git/ref.rb @@ -1,5 +1,3 @@ -# Gitaly note: JV: probably no RPC's here (just one interaction with Rugged). - module Gitlab module Git class Ref @@ -26,13 +24,6 @@ module Gitlab str.gsub(%r{\Arefs/heads/}, '') end - # Gitaly: this method will probably be migrated indirectly via its call sites. - def self.dereference_object(object) - object = object.target while object.is_a?(Rugged::Tag::Annotation) - - object - end - def initialize(repository, name, target, dereferenced_target) @name = Gitlab::Git.ref_name(name) @dereferenced_target = dereferenced_target diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 45d42c7078f..7732049b69b 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -9,14 +9,6 @@ module Gitlab include Gitlab::EncodingHelper include Gitlab::Utils::StrongMemoize - ALLOWED_OBJECT_DIRECTORIES_VARIABLES = %w[ - GIT_OBJECT_DIRECTORY - GIT_ALTERNATE_OBJECT_DIRECTORIES - ].freeze - ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES = %w[ - GIT_OBJECT_DIRECTORY_RELATIVE - GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE - ].freeze SEARCH_CONTEXT_LINES = 3 REV_LIST_COMMIT_LIMIT = 2_000 # In https://gitlab.com/gitlab-org/gitaly/merge_requests/698 @@ -104,15 +96,6 @@ module Gitlab raise Gitlab::Git::CommandError.new(e.message) end - # This method will be removed when Gitaly reaches v1.1. - def rugged - circuit_breaker.perform do - Rugged::Repository.new(path, alternates: alternate_object_directories) - end - rescue Rugged::RepositoryError, Rugged::OSError - raise NoRepository.new('no repository for such path') - end - def circuit_breaker @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(storage) end @@ -638,20 +621,6 @@ module Gitlab end end - AUTOCRLF_VALUES = { - "true" => true, - "false" => false, - "input" => :input - }.freeze - - def autocrlf - AUTOCRLF_VALUES[rugged.config['core.autocrlf']] - end - - def autocrlf=(value) - rugged.config['core.autocrlf'] = AUTOCRLF_VALUES.invert[value] - end - # Returns result like "git ls-files" , recursive and full file path # # Ex. @@ -1024,14 +993,6 @@ module Gitlab found_module && found_module['url'] end - def alternate_object_directories - relative_object_directories.map { |d| File.join(path, d) } - end - - def relative_object_directories - Gitlab::Git::HookEnv.all(gl_repository).values_at(*ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES).flatten.compact - end - # Returns true if the given ref name exists # # Ref names must start with `refs/`. diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index d2dc4f2e688..072019dfb0a 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -1,9 +1,16 @@ +# We only need Gollum::Page so let's not load all of gollum-lib. +require 'gollum-lib/pagination' +require 'gollum-lib/wiki' +require 'gollum-lib/page' + module Gitlab module Git class Wiki DuplicatePageError = Class.new(StandardError) OperationError = Class.new(StandardError) + DEFAULT_PAGINATION = Kaminari.config.default_per_page + CommitDetails = Struct.new(:user_id, :username, :name, :email, :message) do def to_h { user_id: user_id, username: username, name: name, email: email, message: message } @@ -74,7 +81,7 @@ module Gitlab # Gitaly uses gollum-lib to get the versions. Gollum defaults to 20 # per page, but also fetches 20 if `limit` or `per_page` < 20. # Slicing returns an array with the expected number of items. - slice_bound = options[:limit] || options[:per_page] || Gollum::Page.per_page + slice_bound = options[:limit] || options[:per_page] || DEFAULT_PAGINATION versions[0..slice_bound] end @@ -104,26 +111,6 @@ module Gitlab private - def new_page(gollum_page) - Gitlab::Git::WikiPage.new(gollum_page, new_version(gollum_page, gollum_page.version.id)) - end - - def new_version(gollum_page, commit_id) - Gitlab::Git::WikiPageVersion.new(version(commit_id), gollum_page&.format) - end - - def version(commit_id) - commit_find_proc = -> { Gitlab::Git::Commit.find(@repository, commit_id) } - - Gitlab::SafeRequestStore.fetch([:wiki_version_commit, commit_id]) { commit_find_proc.call } - end - - def assert_type!(object, klass) - unless object.is_a?(klass) - raise ArgumentError, "expected a #{klass}, got #{object.inspect}" - end - end - def gitaly_wiki_client @gitaly_wiki_client ||= Gitlab::GitalyClient::WikiService.new(@repository) end diff --git a/lib/gitlab/git/wiki_file.rb b/lib/gitlab/git/wiki_file.rb index 84335aca4bc..64313bb04e8 100644 --- a/lib/gitlab/git/wiki_file.rb +++ b/lib/gitlab/git/wiki_file.rb @@ -3,17 +3,12 @@ module Gitlab class WikiFile attr_reader :mime_type, :raw_data, :name, :path - # This class is meant to be serializable so that it can be constructed - # by Gitaly and sent over the network to GitLab. - # - # Because Gollum::File is not serializable we must get all the data from - # 'gollum_file' during initialization, and NOT store it in an instance - # variable. - def initialize(gollum_file) - @mime_type = gollum_file.mime_type - @raw_data = gollum_file.raw_data - @name = gollum_file.name - @path = gollum_file.path + # This class wraps Gitlab::GitalyClient::WikiFile + def initialize(gitaly_file) + @mime_type = gitaly_file.mime_type + @raw_data = gitaly_file.raw_data + @name = gitaly_file.name + @path = gitaly_file.path end end end diff --git a/lib/gitlab/git/wiki_page.rb b/lib/gitlab/git/wiki_page.rb index 669ae11a423..c4087c9ebdc 100644 --- a/lib/gitlab/git/wiki_page.rb +++ b/lib/gitlab/git/wiki_page.rb @@ -3,25 +3,15 @@ module Gitlab class WikiPage attr_reader :url_path, :title, :format, :path, :version, :raw_data, :name, :text_data, :historical, :formatted_data - # This class is meant to be serializable so that it can be constructed - # by Gitaly and sent over the network to GitLab. - # - # Because Gollum::Page is not serializable we must get all the data from - # 'gollum_page' during initialization, and NOT store it in an instance - # variable. - # - # Note that 'version' is a WikiPageVersion instance which it itself - # serializable. That means it's OK to store 'version' in an instance - # variable. - def initialize(gollum_page, version) - @url_path = gollum_page.url_path - @title = gollum_page.title - @format = gollum_page.format - @path = gollum_page.path - @raw_data = gollum_page.raw_data - @name = gollum_page.name - @historical = gollum_page.historical? - @formatted_data = gollum_page.formatted_data if gollum_page.is_a?(Gollum::Page) + # This class abstracts away Gitlab::GitalyClient::WikiPage + def initialize(gitaly_page, version) + @url_path = gitaly_page.url_path + @title = gitaly_page.title + @format = gitaly_page.format + @path = gitaly_page.path + @raw_data = gitaly_page.raw_data + @name = gitaly_page.name + @historical = gitaly_page.historical? @version = version end diff --git a/lib/gitlab/git/wiki_page_version.rb b/lib/gitlab/git/wiki_page_version.rb index 55f1afedcab..d5e7e70fd31 100644 --- a/lib/gitlab/git/wiki_page_version.rb +++ b/lib/gitlab/git/wiki_page_version.rb @@ -3,11 +3,6 @@ module Gitlab class WikiPageVersion attr_reader :commit, :format - # This class is meant to be serializable so that it can be constructed - # by Gitaly and sent over the network to GitLab. - # - # Both 'commit' (a Gitlab::Git::Commit) and 'format' (a string) are - # serializable. def initialize(commit, format) @commit = commit @format = format diff --git a/lib/gitlab/gitaly_client/wiki_service.rb b/lib/gitlab/gitaly_client/wiki_service.rb index 75be7d1f5a0..7c2c228ad01 100644 --- a/lib/gitlab/gitaly_client/wiki_service.rb +++ b/lib/gitlab/gitaly_client/wiki_service.rb @@ -110,7 +110,7 @@ module Gitlab repository: @gitaly_repo, page_path: encode_binary(page_path), page: options[:page] || 1, - per_page: options[:per_page] || Gollum::Page.per_page + per_page: options[:per_page] || Gitlab::Git::Wiki::DEFAULT_PAGINATION ) stream = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_page_versions, request, timeout: GitalyClient.medium_timeout) diff --git a/scripts/lint-rugged b/scripts/lint-rugged index d0c2c544c47..22e3e1f1505 100755 --- a/scripts/lint-rugged +++ b/scripts/lint-rugged @@ -1,21 +1,9 @@ #!/usr/bin/env ruby ALLOWED = [ - # Can be fixed 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 + # Needed to handle repositories that are not in any storage 'lib/gitlab/bare_repository_import/repository.rb', - # Needs to be migrated, https://gitlab.com/gitlab-org/gitaly/issues/954 - 'lib/tasks/gitlab/cleanup.rake', - - # The only place where Rugged code is still allowed in production - 'lib/gitlab/git/', - # Needed to avoid using the git binary to validate a branch name 'lib/gitlab/git_ref_validator.rb' ].freeze diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index cac8a5068ec..3b37ede8579 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -264,9 +264,9 @@ describe 'GitLab Markdown', :aggregate_failures do @project_wiki = @feat.project_wiki @project_wiki_page = @feat.project_wiki_page - file = Gollum::File.new(@project_wiki.wiki) - expect(file).to receive(:path).and_return('images/example.jpg') - expect(@project_wiki).to receive(:find_file).with('images/example.jpg').and_return(file) + path = 'images/example.jpg' + gitaly_wiki_file = Gitlab::GitalyClient::WikiFile.new(path: path) + expect(@project_wiki).to receive(:find_file).with(path).and_return(Gitlab::Git::WikiFile.new(gitaly_wiki_file)) allow(@project_wiki).to receive(:wiki_base_path) { '/namespace1/gitlabhq/wikis' } @html = markdown(@feat.raw_markdown, { pipeline: :wiki, project_wiki: @project_wiki, page_slug: @project_wiki_page.slug }) diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 747406efc8b..9a4ce426e69 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -83,12 +83,13 @@ describe 'User views a wiki page' do end it 'shows a file stored in a page' do - gollum_file_double = double('Gollum::File', - mime_type: 'image/jpeg', - name: 'images/image.jpg', - path: 'images/image.jpg', - raw_data: '') - wiki_file = Gitlab::Git::WikiFile.new(gollum_file_double) + raw_file = Gitlab::GitalyClient::WikiFile.new( + mime_type: 'image/jpeg', + name: 'images/image.jpg', + path: 'images/image.jpg', + raw_data: '' + ) + wiki_file = Gitlab::Git::WikiFile.new(raw_file) allow(wiki_file).to receive(:mime_type).and_return('image/jpeg') allow_any_instance_of(ProjectWiki).to receive(:find_file).with('image.jpg', nil).and_return(wiki_file) diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index 0735ebd6dcb..5dce3fcbcb6 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :migration, schema: 20171114162227 do + include GitHelpers + let(:merge_request_diffs) { table(:merge_request_diffs) } let(:merge_requests) { table(:merge_requests) } @@ -9,11 +11,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m let(:merge_request) { merge_requests.create!(iid: 1, target_project_id: project.id, source_project_id: project.id, target_branch: 'feature', source_branch: 'master').becomes(MergeRequest) } let(:merge_request_diff) { MergeRequest.find(merge_request.id).create_merge_request_diff } let(:updated_merge_request_diff) { MergeRequestDiff.find(merge_request_diff.id) } - let(:rugged) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged - end - end + let(:rugged) { rugged_repo(project.repository) } before do allow_any_instance_of(MergeRequestDiff) diff --git a/spec/lib/gitlab/conflict/file_spec.rb b/spec/lib/gitlab/conflict/file_spec.rb index 9095ffbfd52..1bd077ddbdf 100644 --- a/spec/lib/gitlab/conflict/file_spec.rb +++ b/spec/lib/gitlab/conflict/file_spec.rb @@ -1,9 +1,11 @@ require 'spec_helper' describe Gitlab::Conflict::File do + include GitHelpers + let(:project) { create(:project, :repository) } let(:repository) { project.repository } - let(:rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged } } + let(:rugged) { rugged_repo(repository) } let(:their_commit) { rugged.branches['conflict-start'].target } let(:our_commit) { rugged.branches['conflict-resolvable'].target } let(:merge_request) { create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) } diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index ea49502ae2e..b243f0dacae 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -4,6 +4,9 @@ require "spec_helper" describe Gitlab::Git::Blob, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:rugged) do + Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) + end describe 'initialize' do let(:blob) { Gitlab::Git::Blob.new(name: 'test') } @@ -139,9 +142,7 @@ describe Gitlab::Git::Blob, :seed_helper do it 'limits the size of a large file' do blob_size = Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE + 1 buffer = Array.new(blob_size, 0) - rugged_blob = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Rugged::Blob.from_buffer(repository.rugged, buffer.join('')) - end + rugged_blob = Rugged::Blob.from_buffer(rugged, buffer.join('')) blob = Gitlab::Git::Blob.raw(repository, rugged_blob) expect(blob.size).to eq(blob_size) @@ -156,9 +157,7 @@ describe Gitlab::Git::Blob, :seed_helper do context 'when sha references a tree' do it 'returns nil' do - tree = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.rev_parse('master^{tree}') - end + tree = rugged.rev_parse('master^{tree}') blob = Gitlab::Git::Blob.raw(repository, tree.oid) @@ -262,11 +261,7 @@ describe Gitlab::Git::Blob, :seed_helper do end describe '.batch_lfs_pointers' do - let(:tree_object) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.rev_parse('master^{tree}') - end - end + let(:tree_object) { rugged.rev_parse('master^{tree}') } let(:non_lfs_blob) do Gitlab::Git::Blob.find( diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 79ccbb79966..0df282d0ae3 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -3,9 +3,7 @@ require "spec_helper" describe Gitlab::Git::Branch, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:rugged) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged - end + Rugged::Repository.new(File.join(TestEnv.repos_path, repository.relative_path)) end subject { repository.branches } @@ -74,9 +72,7 @@ describe Gitlab::Git::Branch, :seed_helper do Gitlab::Git.committer_hash(email: user.email, name: user.name) end let(:params) do - parents = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - [repository.rugged.head.target] - end + parents = [rugged.head.target] tree = parents.first.tree { diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 2718a3c5e49..9ef27081f98 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -1,19 +1,17 @@ require "spec_helper" describe Gitlab::Git::Commit, :seed_helper do + include GitHelpers + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } - let(:rugged_commit) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.lookup(SeedRepo::Commit::ID) - end + let(:rugged_repo) do + Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end + let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } + let(:rugged_commit) { rugged_repo.lookup(SeedRepo::Commit::ID) } + describe "Commit info" do before do - repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - end - @committer = { email: 'mike@smith.com', name: "Mike Smith", @@ -26,12 +24,12 @@ describe Gitlab::Git::Commit, :seed_helper do time: Time.now } - @parents = [repo.head.target] + @parents = [rugged_repo.head.target] @gitlab_parents = @parents.map { |c| described_class.find(repository, c.oid) } @tree = @parents.first.tree sha = Rugged::Commit.create( - repo, + rugged_repo, author: @author, committer: @committer, tree: @tree, @@ -40,7 +38,7 @@ describe Gitlab::Git::Commit, :seed_helper do update_ref: "HEAD" ) - @raw_commit = repo.lookup(sha) + @raw_commit = rugged_repo.lookup(sha) @commit = described_class.find(repository, sha) end @@ -61,10 +59,7 @@ describe Gitlab::Git::Commit, :seed_helper do after do # Erase the new commit so other tests get the original repo - repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - end - repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) + rugged_repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end end @@ -120,9 +115,7 @@ describe Gitlab::Git::Commit, :seed_helper do describe '.find' do it "should return first head commit if without params" do expect(described_class.last(repository).id).to eq( - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.head.target.oid - end + rugged_repo.head.target.oid ) end diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 27d803e0117..8a4415506c4 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -64,9 +64,22 @@ EOT end end - context 'using a Rugged::Patch' do + context 'using a GitalyClient::Diff' do + let(:gitaly_diff) do + Gitlab::GitalyClient::Diff.new( + to_path: ".gitmodules", + from_path: ".gitmodules", + old_mode: 0100644, + new_mode: 0100644, + from_id: '357406f3075a57708d0163752905cc1576fceacc', + to_id: '8e5177d718c561d36efde08bad36b43687ee6bf0', + patch: raw_patch + ) + end + let(:diff) { described_class.new(gitaly_diff) } + context 'with a small diff' do - let(:diff) { described_class.new(gitaly_diff) } + let(:raw_patch) { @raw_diff_hash[:diff] } it 'initializes the diff' do expect(diff.to_hash).to eq(@raw_diff_hash) @@ -78,16 +91,17 @@ EOT end context 'using a diff that is too large' do - it 'prunes the diff' do - gitaly_diff.too_large = true - diff = described_class.new(gitaly_diff) + let(:raw_patch) { 'a' * 204800 } + it 'prunes the diff' do expect(diff.diff).to be_empty expect(diff).to be_too_large end end context 'using a collapsable diff that is too large' do + let(:raw_patch) { 'a' * 204800 } + it 'prunes the diff as a large diff instead of as a collapsed diff' do gitaly_diff.too_large = true diff = described_class.new(gitaly_diff, expanded: false) @@ -97,43 +111,6 @@ EOT expect(diff).not_to be_collapsed end end - end - - context 'using a GitalyClient::Diff' do - let(:diff) do - described_class.new( - Gitlab::GitalyClient::Diff.new( - to_path: ".gitmodules", - from_path: ".gitmodules", - old_mode: 0100644, - new_mode: 0100644, - from_id: '357406f3075a57708d0163752905cc1576fceacc', - to_id: '8e5177d718c561d36efde08bad36b43687ee6bf0', - patch: raw_patch - ) - ) - end - - context 'with a small diff' do - let(:raw_patch) { @raw_diff_hash[:diff] } - - it 'initializes the diff' do - expect(diff.to_hash).to eq(@raw_diff_hash) - end - - it 'does not prune the diff' do - expect(diff).not_to be_too_large - end - end - - context 'using a diff that is too large' do - let(:raw_patch) { 'a' * 204800 } - - it 'prunes the diff' do - expect(diff.diff).to be_empty - expect(diff).to be_too_large - end - end context 'when the patch passed is not UTF-8-encoded' do let(:raw_patch) { @raw_diff_hash[:diff].encode(Encoding::ASCII_8BIT) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index d02536a2fb4..51eb997a325 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -19,7 +19,10 @@ describe Gitlab::Git::Repository, :seed_helper do end end + let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository_path) { File.join(TestEnv.repos_path, repository.relative_path) } + let(:repository_rugged) { Rugged::Repository.new(repository_path) } let(:storage_path) { TestEnv.repos_path } let(:user) { build(:user) } @@ -71,7 +74,6 @@ describe Gitlab::Git::Repository, :seed_helper do describe "Respond to" do subject { repository } - it { is_expected.to respond_to(:rugged) } it { is_expected.to respond_to(:root_ref) } it { is_expected.to respond_to(:tags) } end @@ -91,57 +93,6 @@ describe Gitlab::Git::Repository, :seed_helper do end end - describe "#rugged" do - describe 'when storage is broken', :broken_storage do - it 'raises a storage exception when storage is not available' do - broken_repo = described_class.new('broken', 'a/path.git', '') - - expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Storage::Inaccessible) - end - end - - it 'raises a no repository exception when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git', '') - - expect do - Gitlab::GitalyClient::StorageSettings.allow_disk_access { broken_repo.rugged } - end.to raise_error(Gitlab::Git::Repository::NoRepository) - end - - describe 'alternates keyword argument' do - context 'with no Git env stored' do - before do - allow(Gitlab::Git::HookEnv).to receive(:all).and_return({}) - end - - it "is passed an empty array" do - expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: []) - - repository_rugged - end - end - - context 'with absolute and relative Git object dir envvars stored' do - before do - allow(Gitlab::Git::HookEnv).to receive(:all).and_return({ - 'GIT_OBJECT_DIRECTORY_RELATIVE' => './objects/foo', - 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['./objects/bar', './objects/baz'], - 'GIT_OBJECT_DIRECTORY' => 'ignored', - 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => %w[ignored ignored], - 'GIT_OTHER' => 'another_env' - }) - end - - it "is passed the relative object dir envvars after being converted to absolute ones" do - alternates = %w[foo bar baz].map { |d| File.join(repository_path, './objects', d) } - expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: alternates) - - repository_rugged - end - end - end - end - describe '#branch_names' do subject { repository.branch_names } @@ -284,7 +235,6 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#submodule_url_for' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:ref) { 'master' } def submodule_url(path) @@ -336,7 +286,7 @@ describe Gitlab::Git::Repository, :seed_helper do it { expect(repository.has_local_branches?).to eq(true) } context 'mutable' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:repository) { mutable_repository } after do ensure_seeds @@ -369,7 +319,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe "#delete_branch" do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:repository) { mutable_repository } after do ensure_seeds @@ -393,7 +343,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe "#create_branch" do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:repository) { mutable_repository } after do ensure_seeds @@ -418,39 +368,33 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#delete_refs' do - let(:repo) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - def repo_rugged - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repo.rugged - end - end + let(:repository) { mutable_repository } after do ensure_seeds end it 'deletes the ref' do - repo.delete_refs('refs/heads/feature') + repository.delete_refs('refs/heads/feature') - expect(repo_rugged.references['refs/heads/feature']).to be_nil + expect(repository_rugged.references['refs/heads/feature']).to be_nil end it 'deletes all refs' do refs = %w[refs/heads/wip refs/tags/v1.1.0] - repo.delete_refs(*refs) + repository.delete_refs(*refs) refs.each do |ref| - expect(repo_rugged.references[ref]).to be_nil + expect(repository_rugged.references[ref]).to be_nil end end it 'does not fail when deleting an empty list of refs' do - expect { repo.delete_refs(*[]) }.not_to raise_error + expect { repository.delete_refs(*[]) }.not_to raise_error end it 'raises an error if it failed' do - expect { repo.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) + expect { repository.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) end end @@ -528,9 +472,7 @@ describe Gitlab::Git::Repository, :seed_helper do end def new_repository_path - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - new_repository.path - end + File.join(TestEnv.repos_path, new_repository.relative_path) end end @@ -577,18 +519,16 @@ describe Gitlab::Git::Repository, :seed_helper do Gitlab::Git::Commit.find(repository, @rename_commit_id) end - before(:context) do + before do # Add new commits so that there's a renamed file in the commit history - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - @commit_with_old_name_id = new_commit_edit_old_file(repo).oid - @rename_commit_id = new_commit_move_file(repo).oid - @commit_with_new_name_id = new_commit_edit_new_file(repo).oid + @commit_with_old_name_id = new_commit_edit_old_file(repository_rugged).oid + @rename_commit_id = new_commit_move_file(repository_rugged).oid + @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged).oid end - after(:context) do + after do # Erase our commits so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) + repository_rugged.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end context "where 'follow' == true" do @@ -1010,12 +950,10 @@ describe Gitlab::Git::Repository, :seed_helper do subject { repository.branches } context 'with local and remote branches' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } before do - create_remote_branch(repository, 'joe', 'remote_branch', 'master') + create_remote_branch('joe', 'remote_branch', 'master') repository.create_branch('local_branch', 'master') end @@ -1038,12 +976,10 @@ describe Gitlab::Git::Repository, :seed_helper do end context 'with local and remote branches' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } before do - create_remote_branch(repository, 'joe', 'remote_branch', 'master') + create_remote_branch('joe', 'remote_branch', 'master') repository.create_branch('local_branch', 'master') end @@ -1303,24 +1239,24 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#local_branches' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + let(:repository) { mutable_repository } + + before do + create_remote_branch('joe', 'remote_branch', 'master') + repository.create_branch('local_branch', 'master') end - after(:all) do + after do ensure_seeds end it 'returns the local branches' do - create_remote_branch(@repo, 'joe', 'remote_branch', 'master') - @repo.create_branch('local_branch', 'master') - - expect(@repo.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) - expect(@repo.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) + expect(repository.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) + expect(repository.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) end it 'returns a Branch with UTF-8 fields' do - branches = @repo.local_branches.to_a + branches = repository.local_branches.to_a expect(branches.size).to be > 0 branches.each do |branch| expect(branch.name).to be_utf8 @@ -1331,11 +1267,11 @@ describe Gitlab::Git::Repository, :seed_helper do it 'gets the branches from GitalyClient' do expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(:local_branches) .and_return([]) - @repo.local_branches + repository.local_branches end it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :local_branches do - subject { @repo.local_branches } + subject { repository.local_branches } end end @@ -1391,8 +1327,7 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#fetch_source_branch!' do let(:local_ref) { 'refs/merge-requests/1/head' } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - let(:source_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:source_repository) { mutable_repository } after do ensure_seeds @@ -1401,7 +1336,8 @@ describe Gitlab::Git::Repository, :seed_helper do context 'when the branch exists' do context 'when the commit does not exist locally' do let(:source_branch) { 'new-branch-for-fetch-source-branch' } - let(:source_rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { source_repository.rugged } } + let(:source_path) { File.join(TestEnv.repos_path, source_repository.relative_path) } + let(:source_rugged) { Rugged::Repository.new(source_path) } let(:new_oid) { new_commit_edit_old_file(source_rugged).oid } before do @@ -1513,8 +1449,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#set_config' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - let(:rugged) { repository_rugged } + let(:repository) { mutable_repository } let(:entries) do { 'test.foo1' => 'bla bla', @@ -1526,19 +1461,18 @@ describe Gitlab::Git::Repository, :seed_helper do it 'can set config settings' do expect(repository.set_config(entries)).to be_nil - expect(rugged.config['test.foo1']).to eq('bla bla') - expect(rugged.config['test.foo2']).to eq('1234') - expect(rugged.config['test.foo3']).to eq('true') + expect(repository_rugged.config['test.foo1']).to eq('bla bla') + expect(repository_rugged.config['test.foo2']).to eq('1234') + expect(repository_rugged.config['test.foo3']).to eq('true') end after do - entries.keys.each { |k| rugged.config.delete(k) } + entries.keys.each { |k| repository_rugged.config.delete(k) } end end describe '#delete_config' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - let(:rugged) { repository_rugged } + let(:repository) { mutable_repository } let(:entries) do { 'test.foo1' => 'bla bla', @@ -1549,21 +1483,19 @@ describe Gitlab::Git::Repository, :seed_helper do it 'can delete config settings' do entries.each do |key, value| - rugged.config[key] = value + repository_rugged.config[key] = value end expect(repository.delete_config(*%w[does.not.exist test.foo1 test.foo2])).to be_nil - config_keys = rugged.config.each_key.to_a + config_keys = repository_rugged.config.each_key.to_a expect(config_keys).not_to include('test.foo1') expect(config_keys).not_to include('test.foo2') end end describe '#merge' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } let(:source_sha) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' } let(:target_branch) { 'test-merge-target-branch' } @@ -1602,9 +1534,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#ff_merge' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' } let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:target_branch) { 'test-ff-target-branch' } @@ -1667,9 +1597,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#delete_all_refs_except' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } before do repository.write_ref("refs/delete/a", "0b4bc9a49b562e85de7cc9e834518ea6828729b9") @@ -1693,12 +1621,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe 'remotes' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end - let(:rugged) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged } - end + let(:repository) { mutable_repository } let(:remote_name) { 'my-remote' } let(:url) { 'http://my-repo.git' } @@ -1711,26 +1634,26 @@ describe Gitlab::Git::Repository, :seed_helper do it 'added the remote' do begin - rugged.remotes.delete(remote_name) + repository_rugged.remotes.delete(remote_name) rescue Rugged::ConfigError end repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) - expect(rugged.remotes[remote_name]).not_to be_nil - expect(rugged.config["remote.#{remote_name}.mirror"]).to eq('true') - expect(rugged.config["remote.#{remote_name}.prune"]).to eq('true') - expect(rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) + expect(repository_rugged.remotes[remote_name]).not_to be_nil + expect(repository_rugged.config["remote.#{remote_name}.mirror"]).to eq('true') + expect(repository_rugged.config["remote.#{remote_name}.prune"]).to eq('true') + expect(repository_rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) end end describe '#remove_remote' do it 'removes the remote' do - rugged.remotes.create(remote_name, url) + repository_rugged.remotes.create(remote_name, url) repository.remove_remote(remote_name) - expect(rugged.remotes[remote_name]).to be_nil + expect(repository_rugged.remotes[remote_name]).to be_nil end end end @@ -1901,13 +1824,11 @@ describe Gitlab::Git::Repository, :seed_helper do end context 'when the diff contains a rename' do - let(:repo) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged } - let(:end_sha) { new_commit_move_file(repo).oid } + let(:end_sha) { new_commit_move_file(repository_rugged).oid } after do # Erase our commits so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - repo.references.update('refs/heads/master', SeedRepo::LastCommit::ID) + repository_rugged.references.update('refs/heads/master', SeedRepo::LastCommit::ID) end it 'does not include the renamed file in the sparse checkout' do @@ -1954,10 +1875,9 @@ describe Gitlab::Git::Repository, :seed_helper do end end - def create_remote_branch(repository, remote_name, branch_name, source_branch_name) + def create_remote_branch(remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } - rugged = repository_rugged - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) + repository_rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) end # Build the options hash that's passed to Rugged::Commit#create @@ -2035,16 +1955,4 @@ describe Gitlab::Git::Repository, :seed_helper do line.split("\t").last end end - - def repository_rugged - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged - end - end - - def repository_path - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.path - end - end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index dbd64c4bec0..e7da5565c26 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::GitAccess do include TermsHelper + include GitHelpers let(:user) { create(:user) } @@ -736,21 +737,19 @@ describe Gitlab::GitAccess do def merge_into_protected_branch @protected_branch_merge_commit ||= begin - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.add_branch(user, unprotected_branch, 'feature') - rugged = project.repository.rugged - target_branch = rugged.rev_parse('feature') - source_branch = project.repository.create_file( - user, - 'filename', - 'This is the file content', - message: 'This is a good commit message', - branch_name: unprotected_branch) - author = { email: "email@example.com", time: Time.now, name: "Example Git User" } - - merge_index = rugged.merge_commits(target_branch, source_branch) - Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) - end + project.repository.add_branch(user, unprotected_branch, 'feature') + rugged = rugged_repo(project.repository) + target_branch = rugged.rev_parse('feature') + source_branch = project.repository.create_file( + user, + 'filename', + 'This is the file content', + message: 'This is a good commit message', + branch_name: unprotected_branch) + author = { email: "email@example.com", time: Time.now, name: "Example Git User" } + + merge_index = rugged.merge_commits(target_branch, source_branch) + Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) end end diff --git a/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb b/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb index 5f67fe6b952..d82c9c28da0 100644 --- a/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb @@ -39,6 +39,10 @@ describe Gitlab::GitalyClient::WikiService do expect(wiki_page.title).to eq('My Page') expect(wiki_page.raw_data).to eq('ab') expect(wiki_page_version.format).to eq('markdown') + + expect(wiki_page.title).to be_utf8 + expect(wiki_page.path).to be_utf8 + expect(wiki_page.name).to be_utf8 end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 22dc81acfda..8913644a3ce 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Namespace do include ProjectForksHelper + include GitHelpers let!(:namespace) { create(:namespace) } let(:gitlab_shell) { Gitlab::Shell.new } @@ -361,9 +362,7 @@ describe Namespace do end def project_rugged(project) - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged - end + rugged_repo(project.repository) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d15ba89efb5..8b71919544e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Project do include ProjectForksHelper + include GitHelpers describe 'associations' do it { is_expected.to belong_to(:group) } @@ -4039,8 +4040,6 @@ describe Project do end def rugged_config - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged.config - end + rugged_repo(project.repository).config end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 269d5deca20..3d316fb3c5b 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe RemoteMirror do + include GitHelpers + describe 'URL validation' do context 'with a valid URL' do it 'should be valid' do @@ -74,9 +76,7 @@ describe RemoteMirror do mirror.update_attribute(:url, 'http://foo:baz@test.com') - config = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repo.raw_repository.rugged.config - end + config = rugged_repo(repo).config expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com') end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 784d17e271e..77e549d9528 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' describe Repository do include RepoHelpers + include GitHelpers + TestBlob = Struct.new(:path) let(:project) { create(:project, :repository) } @@ -137,9 +139,7 @@ describe Repository do options = { message: 'test tag message\n', tagger: { name: 'John Smith', email: 'john@gmail.com' } } - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', options) - end + rugged_repo(repository).tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', options) double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) @@ -151,9 +151,7 @@ describe Repository do it { is_expected.to eq(['v1.1.0', 'v1.0.0', annotated_tag_name]) } after do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.tags.delete(annotated_tag_name) - end + rugged_repo(repository).tags.delete(annotated_tag_name) end end end @@ -1678,10 +1676,7 @@ describe Repository do it 'returns the number of branches' do expect(repository.branch_count).to be_an(Integer) - # NOTE: Until rugged goes away, make sure rugged and gitaly are in sync - rugged_count = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.raw_repository.rugged.branches.count - end + rugged_count = rugged_repo(repository).branches.count expect(repository.branch_count).to eq(rugged_count) end @@ -1691,10 +1686,7 @@ describe Repository do it 'returns the number of tags' do expect(repository.tag_count).to be_an(Integer) - # NOTE: Until rugged goes away, make sure rugged and gitaly are in sync - rugged_count = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.raw_repository.rugged.tags.count - end + rugged_count = rugged_repo(repository).tags.count expect(repository.tag_count).to eq(rugged_count) end @@ -2184,9 +2176,7 @@ describe Repository do end def create_remote_branch(remote_name, branch_name, target) - rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged - end + rugged = rugged_repo(repository) rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index 63850939be1..b4732ec0cd5 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -77,7 +77,7 @@ describe WikiPage do end describe "#initialize" do - context "when initialized with an existing gollum page" do + context "when initialized with an existing page" do before do create_page("test page", "test content") @page = wiki.wiki.page(title: "test page") diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index 92159e1e372..2699f6e7bcd 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe GitTagPushService do include RepoHelpers + include GitHelpers let(:user) { create(:user) } let(:project) { create(:project, :repository) } @@ -118,9 +119,7 @@ describe GitTagPushService do before do # Create the lightweight tag - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.raw_repository.rugged.tags.create(tag_name, newrev) - end + rugged_repo(project.repository).tags.create(tag_name, newrev) # Clear tag list cache project.repository.expire_tags_cache diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index 8ab09412f55..53bce15735c 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe MergeRequests::SquashService do + include GitHelpers + let(:service) { described_class.new(project, user, {}) } let(:user) { project.owner } let(:project) { create(:project, :repository) } @@ -63,9 +65,7 @@ describe MergeRequests::SquashService do end it 'has the same diff as the merge request, but a different SHA' do - rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged - end + rugged = rugged_repo(project.repository) mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha) squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha) diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb index cd52bc88f4c..4dd6c6dab86 100644 --- a/spec/services/projects/after_import_service_spec.rb +++ b/spec/services/projects/after_import_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::AfterImportService do + include GitHelpers + subject { described_class.new(project) } let(:project) { create(:project, :repository) } @@ -53,7 +55,7 @@ describe Projects::AfterImportService do end def rugged - Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged } + rugged_repo(repository) end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index bb3f1501f0e..a80c8a7fe51 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::CreateService, '#execute' do + include GitHelpers + let(:gitlab_shell) { Gitlab::Shell.new } let(:user) { create :user } let(:opts) do @@ -295,9 +297,7 @@ describe Projects::CreateService, '#execute' do it 'writes project full path to .git/config' do project = create_project(user, opts) - rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged - end + rugged = rugged_repo(project.repository) expect(rugged.config['gitlab.fullpath']).to eq project.full_path end diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 5f67c325223..0e82194e9ee 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::HashedStorage::MigrateRepositoryService do + include GitHelpers + let(:gitlab_shell) { Gitlab::Shell.new } let(:project) { create(:project, :legacy_storage, :repository, :wiki_repo) } let(:legacy_storage) { Storage::LegacyProject.new(project) } @@ -38,9 +40,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do it 'writes project full path to .git/config' do service.execute - rugged_config = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged.config['gitlab.fullpath'] - end + rugged_config = rugged_repo(project.repository).config['gitlab.fullpath'] expect(rugged_config).to eq project.full_path end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 92c5ac7354a..1411723fb9e 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::TransferService do + include GitHelpers + let(:gitlab_shell) { Gitlab::Shell.new } let(:user) { create(:user) } let(:group) { create(:group) } @@ -291,8 +293,6 @@ describe Projects::TransferService do end def rugged_config - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged.config - end + rugged_repo(project.repository).config end end diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index e0fceae88de..83035788a56 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -1,4 +1,6 @@ module CycleAnalyticsHelpers + include GitHelpers + def create_commit_referencing_issue(issue, branch_name: generate(:branch)) project.repository.add_branch(user, branch_name, 'master') create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) @@ -9,7 +11,7 @@ module CycleAnalyticsHelpers oldrev = repository.commit(branch_name)&.sha || Gitlab::Git::BLANK_SHA if Timecop.frozen? && Gitlab::GitalyClient.feature_enabled?(:operation_user_commit_files) - mock_gitaly_multi_action_dates(repository.raw, commit_time) + mock_gitaly_multi_action_dates(repository, commit_time) end commit_shas = Array.new(count) do |index| @@ -118,18 +120,15 @@ module CycleAnalyticsHelpers protected: false) end - def mock_gitaly_multi_action_dates(raw_repository, commit_time) - allow(raw_repository).to receive(:multi_action).and_wrap_original do |m, *args| + def mock_gitaly_multi_action_dates(repository, commit_time) + allow(repository.raw).to receive(:multi_action).and_wrap_original do |m, *args| new_date = commit_time || Time.now branch_update = m.call(*args) if branch_update.newrev _, opts = args - commit = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - rugged = raw_repository.rugged - rugged.rev_parse(branch_update.newrev) - end + commit = rugged_repo(repository).rev_parse(branch_update.newrev) branch_update.newrev = commit.amend( update_ref: "#{Gitlab::Git::BRANCH_REF_PREFIX}#{opts[:branch_name]}", diff --git a/spec/support/helpers/git_helpers.rb b/spec/support/helpers/git_helpers.rb index fc92bc38561..99a7c39852e 100644 --- a/spec/support/helpers/git_helpers.rb +++ b/spec/support/helpers/git_helpers.rb @@ -1,6 +1,12 @@ # frozen_string_literal: true module GitHelpers + def rugged_repo(repository) + path = File.join(TestEnv.repos_path, repository.disk_path + '.git') + + Rugged::Repository.new(path) + end + def project_hook_exists?(project) Gitlab::GitalyClient::StorageSettings.allow_disk_access do project_path = project.repository.raw_repository.path diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 30e67e67e0e..a159f24f876 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -3,6 +3,8 @@ require 'fileutils' require 'spec_helper' describe GitGarbageCollectWorker do + include GitHelpers + let(:project) { create(:project, :repository) } let(:shell) { Gitlab::Shell.new } let!(:lease_uuid) { SecureRandom.uuid } @@ -197,9 +199,7 @@ describe GitGarbageCollectWorker do # Create a new commit on a random new branch def create_objects(project) - rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged - end + rugged = rugged_repo(project.repository) old_commit = rugged.branches.first.target new_commit_sha = Rugged::Commit.create( rugged, diff --git a/spec/workers/repository_remove_remote_worker_spec.rb b/spec/workers/repository_remove_remote_worker_spec.rb index a653f6f926c..6ddb653d142 100644 --- a/spec/workers/repository_remove_remote_worker_spec.rb +++ b/spec/workers/repository_remove_remote_worker_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper' describe RepositoryRemoveRemoteWorker do include ExclusiveLeaseHelpers + include GitHelpers describe '#perform' do let!(:project) { create(:project, :repository) } @@ -50,9 +51,7 @@ describe RepositoryRemoveRemoteWorker do end def create_remote_branch(remote_name, branch_name, target) - rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.rugged - end + rugged = rugged_repo(project.repository) rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) end diff --git a/vendor/licenses.csv b/vendor/licenses.csv index b36aabb9b3d..6adcb28f736 100644 --- a/vendor/licenses.csv +++ b/vendor/licenses.csv @@ -438,7 +438,6 @@ github-linguist,5.3.3,MIT github-markup,1.7.0,MIT gitlab-flowdock-git-hook,1.0.1,MIT gitlab-gollum-lib,4.2.7.5,MIT -gitlab-gollum-rugged_adapter,0.4.4.1,MIT gitlab-grit,2.8.2,MIT gitlab-markup,1.6.4,MIT gitlab_omniauth-ldap,2.0.4,MIT |