Remove Gitlab::Git::Repository#rugged and Gollum code

Cleanup code, and refactor tests that still use Rugged. After this, there should
be no Rugged code that access the instance's repositories on non-test
environments. There is still some rugged code for other tasks like the
repository import task, but since it doesn't access any repository storage path
it can stay.

.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'

Gemfile

@@ -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'

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)

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)

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.

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.

config/initializers/gollum.rb

-# 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

doc/development/diffs.md

@@ -2,13 +2,10 @@
 Currently we rely on different sources to present diffs, these include:
-- Rugged gem
 - Gitaly service
 - Database (through `merge_request_diff_files`)
 - Redis (cached highlighted diffs)
-We're constantly moving Rugged calls to Gitaly and the progress can be followed through [Gitaly repo](https://gitlab.com/gitlab-org/gitaly).
-
 ## Architecture overview
 ### Merge request diffs

doc/development/gitaly.md

 # 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:

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:

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,

lib/gitlab/git/ref.rb

-# 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

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/`.

lib/gitlab/git/wiki.rb

+# 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

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

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

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

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)

scripts/lint-rugged

 #!/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

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 })