Merge remote-tracking branch 'upstream/master' into qa-secret-variables-scenario

* upstream/master:
  fix bug in webpack_helper in which force_same_domain argument was ignored breaking local rspec tests
  Fix rubocop offenses introduced in !16623
  fix documentation about node version
  Ensure the  job also run for tags
  Open links in a new tab on the user page
  Improve empty project overview
  Look for rugged with static analysis
  Cache rubocop cache for CI
  Update gitlab-styles and re-enable the RSpec/SingleLineHook cop again
  Make Gitlab::Git::Repository#run_git private
  Move user page spinach tests to RSpec
  Handle special characters on API request of issuable templates
  Update secret_values to support dynamic elements within parent
  Disable throwOnError in KaTeX to reveal user where is the problem
  Migrate restoring repo from bundle to Gitaly

.gitlab-ci.yml

@@ -388,7 +388,6 @@ spinach-mysql 2 3: *spinach-metadata-mysql
 
 # Static analysis jobs
 .ruby-static-analysis: &ruby-static-analysis
-  <<: *pull-cache
   variables:
     SIMPLECOV: "false"
     SETUP_DB: "false"
@@ -409,6 +408,12 @@ static-analysis:
   stage: test
   script:
     - scripts/static-analysis
+  cache:
+    key: "ruby-2.3.6-with-yarn-and-rubocop"
+    paths:
+      - vendor/ruby
+      - .yarn-cache/
+      - tmp/rubocop_cache
 
 # Documentation checks:
 # - Check validity of relative links
@@ -717,8 +722,6 @@ pages:
 cache gems:
   <<: *dedicated-runner
   <<: *pull-cache
-  only:
-    - tags
   variables:
     SETUP_DB: "false"
   script:
@@ -729,6 +732,7 @@ cache gems:
   only:
     - master@gitlab-org/gitlab-ce
     - master@gitlab-org/gitlab-ee
+    - tags
 
 gitlab_git_test:
   <<: *dedicated-runner

.rubocop.yml

@@ -17,6 +17,7 @@ AllCops:
     - 'bin/**/*'
     - 'generator_templates/**/*'
     - 'builds/**/*'
+  CacheRootDirectory: tmp
 
 # Gitlab ###################################################################
 

.rubocop_todo.yml

@@ -342,10 +342,6 @@ RSpec/SharedContext:
   Exclude:
     - 'spec/features/admin/admin_groups_spec.rb'
 
-# Offense count: 90
-RSpec/SingleLineHook:
-  Enabled: false
-
 # Offense count: 5
 RSpec/VoidExpect:
   Exclude:

GITALY_SERVER_VERSION

-0.73.0
+0.74.0

Gemfile

@@ -406,7 +406,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.76.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.78.0', require: 'gitaly'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 

Gemfile.lock

@@ -285,7 +285,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly-proto (0.76.0)
+    gitaly-proto (0.78.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -304,7 +304,7 @@ GEM
       mime-types (>= 1.16)
       posix-spawn (~> 0.3)
     gitlab-markup (1.6.3)
-    gitlab-styles (2.3.0)
+    gitlab-styles (2.3.1)
       rubocop (~> 0.51)
       rubocop-gitlab-security (~> 0.1.0)
       rubocop-rspec (~> 1.19)
@@ -1056,7 +1056,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly-proto (~> 0.76.0)
+  gitaly-proto (~> 0.78.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.6.2)

app/assets/javascripts/api.js

@@ -178,7 +178,7 @@ const Api = {
 
   issueTemplate(namespacePath, projectPath, key, type, callback) {
     const url = Api.buildUrl(Api.issuableTemplatePath)
-      .replace(':key', key)
+      .replace(':key', encodeURIComponent(key))
       .replace(':type', type)
       .replace(':project_path', projectPath)
       .replace(':namespace_path', namespacePath);

app/assets/javascripts/behaviors/secret_values.js

@@ -2,18 +2,19 @@ import { n__ } from '../locale';
 import { convertPermissionToBoolean } from '../lib/utils/common_utils';
 
 export default class SecretValues {
-  constructor(container) {
+  constructor({
+    container,
+    valueSelector = '.js-secret-value',
+    placeholderSelector = '.js-secret-value-placeholder',
+  }) {
     this.container = container;
+    this.valueSelector = valueSelector;
+    this.placeholderSelector = placeholderSelector;
   }
 
   init() {
-    this.values = this.container.querySelectorAll('.js-secret-value');
-    this.placeholders = this.container.querySelectorAll('.js-secret-value-placeholder');
     this.revealButton = this.container.querySelector('.js-secret-value-reveal-button');
 
-    this.revealText = n__('Reveal value', 'Reveal values', this.values.length);
-    this.hideText = n__('Hide value', 'Hide values', this.values.length);
-
     const isRevealed = convertPermissionToBoolean(this.revealButton.dataset.secretRevealStatus);
     this.updateDom(isRevealed);
 
@@ -28,15 +29,17 @@ export default class SecretValues {
   }
 
   updateDom(isRevealed) {
-    this.values.forEach((value) => {
+    const values = this.container.querySelectorAll(this.valueSelector);
+    values.forEach((value) => {
       value.classList.toggle('hide', !isRevealed);
     });
 
-    this.placeholders.forEach((placeholder) => {
+    const placeholders = this.container.querySelectorAll(this.placeholderSelector);
+    placeholders.forEach((placeholder) => {
       placeholder.classList.toggle('hide', isRevealed);
     });
 
-    this.revealButton.textContent = isRevealed ? this.hideText : this.revealText;
+    this.revealButton.textContent = isRevealed ? n__('Hide value', 'Hide values', values.length) : n__('Reveal value', 'Reveal values', values.length);
     this.revealButton.dataset.secretRevealStatus = isRevealed;
   }
 }

app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js

@@ -3,7 +3,9 @@ import SecretValues from '~/behaviors/secret_values';
 export default () => {
   const secretVariableTable = document.querySelector('.js-secret-variable-table');
   if (secretVariableTable) {
-    const secretVariableTableValues = new SecretValues(secretVariableTable);
+    const secretVariableTableValues = new SecretValues({
+      container: secretVariableTable,
+    });
     secretVariableTableValues.init();
   }
 };

app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js

@@ -6,13 +6,17 @@ export default function () {
   initSettingsPanels();
   const runnerToken = document.querySelector('.js-secret-runner-token');
   if (runnerToken) {
-    const runnerTokenSecretValue = new SecretValues(runnerToken);
+    const runnerTokenSecretValue = new SecretValues({
+      container: runnerToken,
+    });
     runnerTokenSecretValue.init();
   }
 
   const secretVariableTable = document.querySelector('.js-secret-variable-table');
   if (secretVariableTable) {
-    const secretVariableTableValues = new SecretValues(secretVariableTable);
+    const secretVariableTableValues = new SecretValues({
+      container: secretVariableTable,
+    });
     secretVariableTableValues.init();
   }
 }

app/assets/javascripts/render_math.js

@@ -18,7 +18,7 @@ function renderWithKaTeX(elements) {
 
     const display = $this.attr('data-math-style') === 'display';
     try {
-      katex.render($this.text(), mathNode.get(0), { displayMode: display });
+      katex.render($this.text(), mathNode.get(0), { displayMode: display, throwOnError: false });
       mathNode.insertAfter($this);
       $this.remove();
     } catch (err) {

app/helpers/webpack_helper.rb

@@ -2,7 +2,7 @@ require 'webpack/rails/manifest'
 
 module WebpackHelper
   def webpack_bundle_tag(bundle, force_same_domain: false)
-    javascript_include_tag(*gitlab_webpack_asset_paths(bundle, force_same_domain: true))
+    javascript_include_tag(*gitlab_webpack_asset_paths(bundle, force_same_domain: force_same_domain))
   end
 
   # override webpack-rails gem helper until changes can make it upstream

app/models/ci/pipeline.rb

@@ -524,7 +524,7 @@ module Ci
       return unless sha
 
       project.repository.gitlab_ci_yml_for(sha, ci_yaml_file_path)
-    rescue GRPC::NotFound, Rugged::ReferenceError, GRPC::Internal
+    rescue GRPC::NotFound, GRPC::Internal
       nil
     end
 

app/models/deployment.rb

@@ -45,14 +45,7 @@ class Deployment < ActiveRecord::Base
   def includes_commit?(commit)
     return false unless commit
 
-    # Before 8.10, deployments didn't have keep-around refs. Any deployment
-    # created before then could have a `sha` referring to a commit that no
-    # longer exists in the repository, so just ignore those.
-    begin
-      project.repository.ancestor?(commit.id, sha)
-    rescue Rugged::OdbError
-      false
-    end
+    project.repository.ancestor?(commit.id, sha)
   end
 
   def update_merge_request_metrics!

app/models/repository.rb

@@ -20,7 +20,7 @@ class Repository
   attr_accessor :full_path, :disk_path, :project, :is_wiki
 
   delegate :ref_name_for_sha, to: :raw_repository
-  delegate :bundle_to_disk, to: :raw_repository
+  delegate :bundle_to_disk, :create_from_bundle, to: :raw_repository
 
   CreateTreeError = Class.new(StandardError)
 
@@ -166,16 +166,10 @@ class Repository
       return []
     end
 
-    raw_repository.gitaly_migrate(:commits_by_message) do |is_enabled|
-      commits =
-        if is_enabled
-          find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
-        else
-          find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
-        end
-
-      CommitCollection.new(project, commits, ref)
+    commits = raw_repository.find_commits_by_message(query, ref, path, limit, offset).map do |c|
+      commit(c)
     end
+    CommitCollection.new(project, commits, ref)
   end
 
   def find_branch(name, fresh_repo: true)
@@ -740,23 +734,6 @@ class Repository
     Commit.order_by(collection: commits, order_by: order_by, sort: sort)
   end
 
-  def refs_contains_sha(ref_type, sha)
-    args = %W(#{ref_type} --contains #{sha})
-    names = run_git(args).first
-
-    if names.respond_to?(:split)
-      names = names.split("\n").map(&:strip)
-
-      names.each do |name|
-        name.slice! '* '
-      end
-
-      names
-    else
-      []
-    end
-  end
-
   def branch_names_contains(sha)
     refs_contains_sha('branch', sha)
   end
@@ -921,25 +898,6 @@ class Repository
     end
   end
 
-  def search_files_by_content(query, ref)
-    return [] if empty? || query.blank?
-
-    offset = 2
-    args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
-
-    run_git(args).first.scrub.split(/^--$/)
-  end
-
-  def search_files_by_name(query, ref)
-    safe_query = Regexp.escape(query.sub(/^\/*/, ""))
-
-    return [] if empty? || safe_query.blank?
-
-    args = %W(ls-tree --full-tree -r #{ref || root_ref} --name-status | #{safe_query})
-
-    run_git(args).first.lines.map(&:strip)
-  end
-
   def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil)
     unless remote_name
       remote_name = "tmp-#{SecureRandom.hex}"
@@ -973,6 +931,18 @@ class Repository
     raw_repository.ls_files(actual_ref)
   end
 
+  def search_files_by_content(query, ref)
+    return [] if empty? || query.blank?
+
+    raw_repository.search_files_by_content(query, ref)
+  end
+
+  def search_files_by_name(query, ref)
+    return [] if empty?
+
+    raw_repository.search_files_by_name(query, ref)
+  end
+
   def copy_gitattributes(ref)
     actual_ref = ref || root_ref
     begin
@@ -1133,25 +1103,4 @@ class Repository
   def rugged_can_be_merged?(their_commit, our_commit)
     !rugged.merge_commits(our_commit, their_commit).conflicts?
   end
-
-  def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
-    ref ||= root_ref
-
-    args = %W(
-      log #{ref} --pretty=%H --skip #{offset}
-      --max-count #{limit} --grep=#{query} --regexp-ignore-case
-    )
-    args = args.concat(%W(-- #{path})) if path.present?
-
-    git_log_results = run_git(args).first.lines
-
-    git_log_results.map { |c| commit(c.chomp) }.compact
-  end
-
-  def find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
-    raw_repository
-      .gitaly_commit_client
-      .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
-      .map { |c| commit(c) }
-  end
 end

app/views/projects/_readme.html.haml

@@ -9,15 +9,15 @@
 
 - else
   .row-content-block.second-block.center
-    %h3.page-title
+    %h4
       This project does not have a README yet
+
     - if can?(current_user, :push_code, @project)
       %p
         A
         %code README
         file contains information about other files in a repository and is commonly
         distributed with computer software, forming part of its documentation.
+        GitLab will render it here instead of this message.
       %p
-        We recommend you to
-        = link_to "add a README", add_special_file_path(@project, file_name: 'README.md')
-        file to the repository and GitLab will render it here instead of this message.
+        = link_to "Add Readme", add_special_file_path(@project, file_name: 'README.md'),  class: 'btn btn-new'

app/views/projects/buttons/_dropdown.html.haml

@@ -6,7 +6,10 @@
     %ul.dropdown-menu.dropdown-menu-align-right.project-home-dropdown
       - can_create_issue = can?(current_user, :create_issue, @project)
       - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
-      - can_create_snippet = can?(current_user, :create_snippet, @project)
+      - can_create_project_snippet = can?(current_user, :create_project_snippet, @project)
+
+      - if can_create_issue || merge_project || can_create_project_snippet
+        %li.dropdown-header= _('This project')
 
       - if can_create_issue
         %li= link_to _('New issue'), new_project_issue_path(@project)
@@ -14,11 +17,11 @@
       - if merge_project
         %li= link_to _('New merge request'), project_new_merge_request_path(merge_project)
 
-      - if can_create_snippet
+      - if can_create_project_snippet
         %li= link_to _('New snippet'), new_project_snippet_path(@project)
 
-      - if can_create_issue || merge_project || can_create_snippet
-        %li.divider
+      - if can?(current_user, :push_code, @project)
+        %li.dropdown-header= _('This repository')
 
       - if can?(current_user, :push_code, @project)
         %li= link_to _('New file'), project_new_blob_path(@project, @project.default_branch || 'master')
@@ -31,5 +34,5 @@
         - continue_params = { to:         project_new_blob_path(@project, @project.default_branch || 'master'),
                               notice:     edit_in_new_fork_notice,
                               notice_now: edit_in_new_fork_notice_now }
-        - fork_path = project_forks_path(@project, namespace_key:  current_user.namespace.id, continue: continue_params)
+        - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params)
         %li= link_to _('New file'), fork_path, method: :post

app/views/projects/empty.html.haml

@@ -6,8 +6,9 @@
 = render "home_panel"
 
 .row-content-block.second-block.center
-  %h3.page-title
+  %h4
     The repository for this project is empty
+
   - if can?(current_user, :push_code, @project)
     %p
       If you already have files you can push them using command line instructions below.
@@ -28,8 +29,8 @@
       %p
         - link = link_to(s_('AutoDevOps|Auto DevOps (Beta)'), project_settings_ci_cd_path(@project, anchor: 'js-general-pipeline-settings'))
         = s_('AutoDevOps|You can activate %{link_to_settings} for this project.').html_safe % { link_to_settings: link }
-      %p
-        = s_('AutoDevOps|It will automatically build, test, and deploy your application based on a predefined CI/CD configuration.')
+      %p= s_('AutoDevOps|It will automatically build, test, and deploy your application based on a predefined CI/CD configuration.')
+      %p= link_to _('New file'), project_new_blob_path(@project, @project.default_branch || 'master'), class: 'btn btn-new'
 
 - if can?(current_user, :push_code, @project)
   %div{ class: container_class }
@@ -79,4 +80,4 @@
 
           - if can? current_user, :remove_project, @project
             .prepend-top-20
-              = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-remove pull-right"
+              = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-inverted btn-remove pull-right"

app/views/users/show.html.haml

@@ -58,15 +58,15 @@
                 = icon('skype')
           - unless @user.linkedin.blank?
             .profile-link-holder.middle-dot-divider
-              = link_to linkedin_url(@user), title: "LinkedIn" do
+              = link_to linkedin_url(@user), title: "LinkedIn", target: '_blank', rel: 'noopener noreferrer nofollow' do
                 = icon('linkedin-square')
           - unless @user.twitter.blank?
             .profile-link-holder.middle-dot-divider
-              = link_to twitter_url(@user), title: "Twitter" do
+              = link_to twitter_url(@user), title: "Twitter", target: '_blank', rel: 'noopener noreferrer nofollow' do
                 = icon('twitter-square')
           - unless @user.website_url.blank?
             .profile-link-holder.middle-dot-divider
-              = link_to @user.short_website_url, @user.full_website_url, class: 'text-link'
+              = link_to @user.short_website_url, @user.full_website_url, class: 'text-link', target: '_blank', rel: 'noopener noreferrer nofollow'
           - unless @user.location.blank?
             .profile-link-holder.middle-dot-divider
               = icon('map-marker')

app/workers/repository_check/single_repository_worker.rb

@@ -20,10 +20,7 @@ module RepositoryCheck
         # Historically some projects never had their wiki repos initialized;
         # this happens on project creation now. Let's initialize an empty repo
         # if it is not already there.
-        begin
-          project.create_wiki
-        rescue Rugged::RepositoryError
-        end
+        project.create_wiki
 
         git_fsck(project.wiki.repository)
       else

changelogs/unreleased/40028-special-characters-on-issuable-templates.yml

+---
+title: Handle special characters on API request of issuable templates
+merge_request: 15323
+author: Takuya Noguchi
+type: fixed

changelogs/unreleased/disable-throwOnError-in-katex.yml

+---
+title: Disable throwOnError in KaTeX to reveal user where is the problem
+merge_request: 16684
+author: Jakub Jirutka
+type: other

changelogs/unreleased/feat-add-section-headers-to-project-repo-buttons.yml

+---
+title: Improve empty project overview
+merge_request: 16617
+author: George Tsiolis
+type: added

changelogs/unreleased/update-node-docs.yml

+---
+title: fix documentation about node version
+merge_request: 16720
+author: Tobias Gurtzick
+type: other

config/initializers/rugged_use_gitlab_git_attributes.rb

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

config/routes/project.rb

@@ -40,7 +40,7 @@ constraints(ProjectUrlConstrainer.new) do
       #
       # Templates
       #
-      get '/templates/:template_type/:key' => 'templates#show', as: :template
+      get '/templates/:template_type/:key' => 'templates#show', as: :template, constraints: { key: /[^\/]+/ }
 
       resource  :avatar, only: [:show, :destroy]
       resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do

doc/update/10.2-to-10.3.md

@@ -54,17 +54,16 @@ sudo gem install bundler --no-ri --no-rdoc
 
 ### 4. Update Node
 
-GitLab now runs [webpack](http://webpack.js.org) to compile frontend assets and
-it has a minimum requirement of node v4.3.0.
+GitLab now runs [webpack](http://webpack.js.org) to compile frontend assets.
+We require a minimum version of node v6.0.0.
 
 You can check which version you are running with `node -v`. If you are running
-a version older than `v4.3.0` you will need to update to a newer version.  You
+a version older than `v6.0.0` you will need to update to a newer version. You
 can find instructions to install from community maintained packages or compile
 from source at the nodejs.org website.
 
 <https://nodejs.org/en/download/>
 
-
 Since 8.17, GitLab requires the use of yarn `>= v0.17.0` to manage
 JavaScript dependencies.
 

doc/update/10.3-to-10.4.md

@@ -56,17 +56,16 @@ sudo gem install bundler --no-ri --no-rdoc
 
 ### 4. Update Node
 
-GitLab now runs [webpack](http://webpack.js.org) to compile frontend assets and
-it has a minimum requirement of node v4.3.0.
+GitLab now runs [webpack](http://webpack.js.org) to compile frontend assets.
+We require a minimum version of node v6.0.0.
 
 You can check which version you are running with `node -v`. If you are running
-a version older than `v4.3.0` you will need to update to a newer version.  You
+a version older than `v6.0.0` you will need to update to a newer version. You
 can find instructions to install from community maintained packages or compile
 from source at the nodejs.org website.
 
 <https://nodejs.org/en/download/>
 
-
 Since 8.17, GitLab requires the use of yarn `>= v0.17.0` to manage
 JavaScript dependencies.
 

features/steps/user.rb

-class Spinach::Features::User < Spinach::FeatureSteps
-  include SharedAuthentication
-  include SharedPaths
-  include SharedUser
-  include SharedProject
-
-  step 'I should see user "John Doe" page' do
-    expect(title).to match(/^\s*John Doe/)
-  end
-
-  step '"John Doe" has contributions' do
-    user = User.find_by(name: 'John Doe')
-    project = contributed_project
-
-    # Issue contribution
-    issue_params = { title: 'Bug in old browser' }
-    Issues::CreateService.new(project, user, issue_params).execute
-
-    # Push code contribution
-    event = create(:push_event, project: project, author: user)
-
-    create(:push_event_payload, event: event, commit_count: 3)
-  end
-
-  step 'I should see contributed projects' do
-    page.within '#contributed' do
-      expect(page).to have_content(@contributed_project.name)
-    end
-  end
-
-  step 'I should see contributions calendar' do
-    expect(page).to have_css('.js-contrib-calendar')
-  end
-
-  def contributed_project
-    @contributed_project ||= create(:project, :public, :empty_repo)
-  end
-end

features/user.feature

-Feature: User
-  Background:
-    Given User "John Doe" exists
-    And "John Doe" owns private project "Enterprise"
-
-  # Signed out
-
-  @javascript
-  Scenario: I visit user "John Doe" page while not signed in when he owns a public project
-    Given "John Doe" owns internal project "Internal"
-    And "John Doe" owns public project "Community"
-    When I visit user "John Doe" page
-    And I click on "Personal projects" tab
-    Then I should see user "John Doe" page
-    And I should not see project "Enterprise"
-    And I should not see project "Internal"
-    And I should see project "Community"
-
-  # Signed in as someone else
-
-  @javascript
-  Scenario: I visit user "John Doe" page while signed in as someone else when he owns a public project
-    Given "John Doe" owns public project "Community"
-    And "John Doe" owns internal project "Internal"
-    And I sign in as a user
-    When I visit user "John Doe" page
-    And I click on "Personal projects" tab
-    Then I should see user "John Doe" page
-    And I should not see project "Enterprise"
-    And I should see project "Internal"
-    And I should see project "Community"
-
-  @javascript
-  Scenario: I visit user "John Doe" page while signed in as someone else when he is not authorized to a public project
-    Given "John Doe" owns internal project "Internal"
-    And I sign in as a user
-    When I visit user "John Doe" page
-    And I click on "Personal projects" tab
-    Then I should see user "John Doe" page
-    And I should not see project "Enterprise"
-    And I should see project "Internal"
-    And I should not see project "Community"
-
-  @javascript
-  Scenario: I visit user "John Doe" page while signed in as someone else when he is not authorized to a project I can see
-    Given I sign in as a user
-    When I visit user "John Doe" page
-    And I click on "Personal projects" tab
-    Then I should see user "John Doe" page
-    And I should not see project "Enterprise"
-    And I should not see project "Internal"
-    And I should not see project "Community"
-
-  # Signed in as the user himself
-
-  @javascript
-  Scenario: I visit user "John Doe" page while signed in as "John Doe" when he has a public project
-    Given "John Doe" owns internal project "Internal"
-    And "John Doe" owns public project "Community"
-    And I sign in as "John Doe"
-    When I visit user "John Doe" page
-    And I click on "Personal projects" tab
-    Then I should see user "John Doe" page
-    And I should see project "Enterprise"
-    And I should see project "Internal"
-    And I should see project "Community"
-
-  @javascript
-  Scenario: I visit user "John Doe" page while signed in as "John Doe" when he has no public project
-    Given I sign in as "John Doe"
-    When I visit user "John Doe" page
-    And I click on "Personal projects" tab
-    Then I should see user "John Doe" page
-    And I should see project "Enterprise"
-    And I should not see project "Internal"
-    And I should not see project "Community"
-
-  @javascript
-  Scenario: "John Doe" contribution profile
-    Given I sign in as a user
-    And "John Doe" has contributions
-    When I visit user "John Doe" page
-    And I click on "Contributed projects" tab
-    Then I should see user "John Doe" page
-    And I should see contributed projects
-    And I should see contributions calendar

lib/gitlab/bare_repository_import/repository.rb

+# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/953
+#
 module Gitlab
   module BareRepositoryImport
     class Repository

lib/gitlab/git/repository.rb

@@ -563,6 +563,8 @@ module Gitlab
         return false if ancestor_id.nil? || descendant_id.nil?
 
         merge_base_commit(ancestor_id, descendant_id) == ancestor_id
+      rescue Rugged::OdbError
+        false
       end
 
       # Returns true is +from+ is direct ancestor to +to+, otherwise false
@@ -1125,23 +1127,6 @@ module Gitlab
         target_ref
       end
 
-      # Refactoring aid; allows us to copy code from app/models/repository.rb
-      def run_git(args, chdir: path, env: {}, nice: false, &block)
-        cmd = [Gitlab.config.git.bin_path, *args]
-        cmd.unshift("nice") if nice
-        circuit_breaker.perform do
-          popen(cmd, chdir, env, &block)
-        end
-      end
-
-      def run_git!(args, chdir: path, env: {}, nice: false, &block)
-        output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block)
-
-        raise GitError, output unless status.zero?
-
-        output
-      end
-
       # Refactoring aid; allows us to copy code from app/models/repository.rb
       def run_git_with_timeout(args, timeout, env: {})
         circuit_breaker.perform do
@@ -1203,6 +1188,19 @@ module Gitlab
         end
       end
 
+      def create_from_bundle(bundle_path)
+        gitaly_migrate(:create_repo_from_bundle) do |is_enabled|
+          if is_enabled
+            gitaly_repository_client.create_from_bundle(bundle_path)
+          else
+            run_git!(%W(clone --bare -- #{bundle_path} #{path}), chdir: nil)
+            self.class.create_hooks(path, File.expand_path(Gitlab.config.gitlab_shell.hooks_path))
+          end
+        end
+
+        true
+      end
+
       def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:)
         gitaly_migrate(:rebase) do |is_enabled|
           if is_enabled
@@ -1365,6 +1363,52 @@ module Gitlab
         raise CommandError.new(e)
       end
 
+      def refs_contains_sha(ref_type, sha)
+        args = %W(#{ref_type} --contains #{sha})
+        names = run_git(args).first
+
+        if names.respond_to?(:split)
+          names = names.split("\n").map(&:strip)
+
+          names.each do |name|
+            name.slice! '* '
+          end
+
+          names
+        else
+          []
+        end
+      end
+
+      def search_files_by_content(query, ref)
+        return [] if empty? || query.blank?
+
+        offset = 2
+        args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
+
+        run_git(args).first.scrub.split(/^--$/)
+      end
+
+      def search_files_by_name(query, ref)
+        safe_query = Regexp.escape(query.sub(/^\/*/, ""))
+
+        return [] if empty? || safe_query.blank?
+
+        args = %W(ls-tree --full-tree -r #{ref || root_ref} --name-status | #{safe_query})
+
+        run_git(args).first.lines.map(&:strip)
+      end
+
+      def find_commits_by_message(query, ref, path, limit, offset)
+        gitaly_migrate(:commits_by_message) do |is_enabled|
+          if is_enabled
+            find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
+          else
+            find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
+          end
+        end
+      end
+
       private
 
       def shell_write_ref(ref_path, ref, old_ref)
@@ -1386,6 +1430,22 @@ module Gitlab
         Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}"
       end
 
+      def run_git(args, chdir: path, env: {}, nice: false, &block)
+        cmd = [Gitlab.config.git.bin_path, *args]
+        cmd.unshift("nice") if nice
+        circuit_breaker.perform do
+          popen(cmd, chdir, env, &block)
+        end
+      end
+
+      def run_git!(args, chdir: path, env: {}, nice: false, &block)
+        output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block)
+
+        raise GitError, output unless status.zero?
+
+        output
+      end
+
       def fresh_worktree?(path)
         File.exist?(path) && !clean_stuck_worktree(path)
       end
@@ -2161,6 +2221,26 @@ module Gitlab
       def gitlab_projects_error
         raise CommandError, @gitlab_projects.output
       end
+
+      def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
+        ref ||= root_ref
+
+        args = %W(
+          log #{ref} --pretty=%H --skip #{offset}
+          --max-count #{limit} --grep=#{query} --regexp-ignore-case
+        )
+        args = args.concat(%W(-- #{path})) if path.present?
+
+        git_log_results = run_git(args).first.lines
+
+        git_log_results.map { |c| commit(c.chomp) }.compact
+      end
+
+      def find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
+        gitaly_commit_client
+          .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
+          .map { |c| commit(c) }
+      end
     end
   end
 end

lib/gitlab/gitaly_client/commit_service.rb

@@ -38,19 +38,27 @@ module Gitlab
         from_id = case from
                   when NilClass
                     EMPTY_TREE_ID
-                  when Rugged::Commit
-                    from.oid
                   else
-                    from
+                    if from.respond_to?(:oid)
+                      # This is meant to match a Rugged::Commit. This should be impossible in
+                      # the future.
+                      from.oid
+                    else
+                      from
+                    end
                   end
 
         to_id = case to
                 when NilClass
                   EMPTY_TREE_ID
-                when Rugged::Commit
-                  to.oid
                 else
-                  to
+                  if to.respond_to?(:oid)
+                    # This is meant to match a Rugged::Commit. This should be impossible in
+                    # the future.
+                    to.oid
+                  else
+                    to
+                  end
                 end
 
         request_params = diff_between_commits_request_params(from_id, to_id, options)

lib/gitlab/gitaly_client/repository_service.rb

@@ -3,6 +3,8 @@ module Gitlab
     class RepositoryService
       include Gitlab::EncodingHelper
 
+      MAX_MSG_SIZE = 128.kilobytes.freeze
+
       def initialize(repository)
         @repository = repository
         @gitaly_repo = repository.gitaly_repository
@@ -178,6 +180,29 @@ module Gitlab
           end
         end
       end
+
+      def create_from_bundle(bundle_path)
+        request = Gitaly::CreateRepositoryFromBundleRequest.new(repository: @gitaly_repo)
+        enum = Enumerator.new do |y|
+          File.open(bundle_path, 'rb') do |f|
+            while data = f.read(MAX_MSG_SIZE)
+              request.data = data
+
+              y.yield request
+
+              request = Gitaly::CreateRepositoryFromBundleRequest.new
+            end
+          end
+        end
+
+        GitalyClient.call(
+          @storage,
+          :repository_service,
+          :create_repository_from_bundle,
+          enum,
+          timeout: GitalyClient.default_timeout
+        )
+      end
     end
   end
 end

lib/gitlab/github_import/importer/pull_requests_importer.rb

@@ -57,10 +57,7 @@ module Gitlab
         end
 
         def commit_exists?(sha)
-          project.repository.lookup(sha)
-          true
-        rescue Rugged::Error
-          false
+          project.repository.commit(sha).present?
         end
 
         def collection_method

lib/gitlab/import_export/command_line_util.rb

@@ -11,11 +11,6 @@ module Gitlab
         untar_with_options(archive: archive, dir: dir, options: 'zxf')
       end
 
-      def git_clone_bundle(repo_path:, bundle_path:)
-        execute(%W(#{git_bin_path} clone --bare -- #{bundle_path} #{repo_path}))
-        Gitlab::Git::Repository.create_hooks(repo_path, File.expand_path(Gitlab.config.gitlab_shell.hooks_path))
-      end
-
       def mkdir_p(path)
         FileUtils.mkdir_p(path, mode: DEFAULT_MODE)
         FileUtils.chmod(DEFAULT_MODE, path)

lib/gitlab/import_export/repo_restorer.rb

@@ -13,7 +13,7 @@ module Gitlab
       def restore
         return true unless File.exist?(@path_to_bundle)
 
-        git_clone_bundle(repo_path: @project.repository.path_to_repo, bundle_path: @path_to_bundle)
+        @project.repository.create_from_bundle(@path_to_bundle)
       rescue => e
         @shared.error(e)
         false

lib/tasks/gitlab/cleanup.rake

+# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/954
+#
 namespace :gitlab do
   namespace :cleanup do
     HASHED_REPOSITORY_NAME = '@hashed'.freeze

scripts/lint-rugged

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

scripts/static-analysis

@@ -13,7 +13,8 @@ tasks = [
   %w[bundle exec rake gettext:lint],
   %w[bundle exec rake lint:static_verification],
   %w[scripts/lint-changelog-yaml],
-  %w[scripts/lint-conflicts.sh]
+  %w[scripts/lint-conflicts.sh],
+  %w[scripts/lint-rugged]
 ]
 
 failed_tasks = tasks.reduce({}) do |failures, task|

spec/features/user_page_spec.rb

+require 'spec_helper'
+
+describe 'User page', :js do
+  let!(:user) { create :user }
+  let!(:private_project) do
+    create :project, :private, name: 'private', namespace: user.namespace do |project|
+      project.add_master(user)
+    end
+  end
+
+  let!(:internal_project) do
+    create :project, :internal, name: 'internal', namespace: user.namespace do |project|
+      project.add_master(user)
+    end
+  end
+
+  let!(:public_project) do
+    create :project, :public, name: 'public', namespace: user.namespace do |project|
+      project.add_master(user)
+    end
+  end
+
+  def click_nav_link(name)
+    page.within '.nav-links' do
+      click_link name
+    end
+  end
+
+  context 'when not signed in' do
+    it 'renders user public project' do
+      visit user_path(user)
+      click_nav_link('Personal projects')
+
+      expect(page).to have_css('.tab-content #projects.active')
+      expect(title).to start_with(user.name)
+
+      expect(page).to have_content(public_project.name)
+      expect(page).not_to have_content(private_project.name)
+      expect(page).not_to have_content(internal_project.name)
+    end
+  end
+
+  context 'when signed in as another user' do
+    let(:another_user) { create :user }
+
+    before do
+      sign_in(another_user)
+    end
+
+    it 'renders user public and internal projects' do
+      visit user_path(user)
+      click_nav_link('Personal projects')
+
+      expect(title).to start_with(user.name)
+
+      expect(page).not_to have_content(private_project.name)
+      expect(page).to have_content(public_project.name)
+      expect(page).to have_content(internal_project.name)
+    end
+  end
+
+  context 'when signed in as user' do
+    before do
+      sign_in(user)
+    end
+
+    describe 'personal projects' do
+      it 'renders all user projects' do
+        visit user_path(user)
+        click_nav_link('Personal projects')
+
+        expect(title).to start_with(user.name)
+
+        expect(page).to have_content(private_project.name)
+        expect(page).to have_content(public_project.name)
+        expect(page).to have_content(internal_project.name)
+      end
+    end
+
+    describe 'contributed projects' do
+      context 'when user has contributions' do
+        let(:contributed_project) do
+          create :project, :public, :empty_repo
+        end
+
+        before do
+          Issues::CreateService.new(contributed_project, user, { title: 'Bug in old browser' }).execute
+          event = create(:push_event, project: contributed_project, author: user)
+          create(:push_event_payload, event: event, commit_count: 3)
+        end
+
+        it 'renders contributed project' do
+          visit user_path(user)
+
+          expect(title).to start_with(user.name)
+          expect(page).to have_css('.js-contrib-calendar')
+
+          click_nav_link('Contributed projects')
+
+          page.within '#contributed' do
+            expect(page).to have_content(contributed_project.name)
+          end
+        end
+      end
+    end
+  end
+end

spec/javascripts/api_spec.js

@@ -262,9 +262,9 @@ describe('Api', () => {
     it('fetches an issue template', (done) => {
       const namespace = 'some namespace';
       const project = 'some project';
-      const templateKey = 'template key';
+      const templateKey = ' template #%?.key ';
       const templateType = 'template type';
-      const expectedUrl = `${dummyUrlRoot}/${namespace}/${project}/templates/${templateType}/${templateKey}`;
+      const expectedUrl = `${dummyUrlRoot}/${namespace}/${project}/templates/${templateType}/${encodeURIComponent(templateKey)}`;
       spyOn(jQuery, 'ajax').and.callFake((request) => {
         expect(request.url).toEqual(expectedUrl);
         return sendDummyResponse();

spec/javascripts/behaviors/secret_values_spec.js

 import SecretValues from '~/behaviors/secret_values';
 
-function generateFixtureMarkup(secrets, isRevealed) {
+function generateValueMarkup(
+  secret,
+  valueClass = 'js-secret-value',
+  placeholderClass = 'js-secret-value-placeholder',
+) {
+  return `
+    <div class="${placeholderClass}">
+      ***
+    </div>
+    <div class="hide ${valueClass}">
+      ${secret}
+    </div>
+  `;
+}
+
+function generateFixtureMarkup(secrets, isRevealed, valueClass, placeholderClass) {
   return `
   <div class="js-secret-container">
-    ${secrets.map(secret => `
-      <div class="js-secret-value-placeholder">
-        ***
-      </div>
-      <div class="hide js-secret-value">
-        ${secret}
-      </div>
-    `).join('')}
+    ${secrets.map(secret => generateValueMarkup(secret, valueClass, placeholderClass)).join('')}
     <button
       class="js-secret-value-reveal-button"
       data-secret-reveal-status="${isRevealed}"
@@ -21,11 +29,25 @@ function generateFixtureMarkup(secrets, isRevealed) {
   `;
 }
 
-function setupSecretFixture(secrets, isRevealed) {
+function setupSecretFixture(
+  secrets,
+  isRevealed,
+  valueClass = 'js-secret-value',
+  placeholderClass = 'js-secret-value-placeholder',
+) {
   const wrapper = document.createElement('div');
-  wrapper.innerHTML = generateFixtureMarkup(secrets, isRevealed);
-
-  const secretValues = new SecretValues(wrapper.querySelector('.js-secret-container'));
+  wrapper.innerHTML = generateFixtureMarkup(
+    secrets,
+    isRevealed,
+    valueClass,
+    placeholderClass,
+  );
+
+  const secretValues = new SecretValues({
+    container: wrapper.querySelector('.js-secret-container'),
+    valueSelector: `.${valueClass}`,
+    placeholderSelector: `.${placeholderClass}`,
+  });
   secretValues.init();
 
   return wrapper;
@@ -49,7 +71,7 @@ describe('setupSecretValues', () => {
       expect(revealButton.textContent).toEqual('Hide value');
     });
 
-    it('should value hidden initially', () => {
+    it('should have value hidden initially', () => {
       const wrapper = setupSecretFixture(secrets, false);
       const values = wrapper.querySelectorAll('.js-secret-value');
       const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder');
@@ -143,4 +165,64 @@ describe('setupSecretValues', () => {
       });
     });
   });
+
+  describe('with dynamic secrets', () => {
+    const secrets = ['mysecret123', 'happygoat456', 'tanuki789'];
+
+    it('should toggle values and placeholders', () => {
+      const wrapper = setupSecretFixture(secrets, false);
+      // Insert the new dynamic row
+      wrapper.querySelector('.js-secret-container').insertAdjacentHTML('afterbegin', generateValueMarkup('foobarbazdynamic'));
+
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+      const values = wrapper.querySelectorAll('.js-secret-value');
+      const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder');
+
+      revealButton.click();
+
+      expect(values.length).toEqual(4);
+      values.forEach((value) => {
+        expect(value.classList.contains('hide')).toEqual(false);
+      });
+      expect(placeholders.length).toEqual(4);
+      placeholders.forEach((placeholder) => {
+        expect(placeholder.classList.contains('hide')).toEqual(true);
+      });
+
+      revealButton.click();
+
+      expect(values.length).toEqual(4);
+      values.forEach((value) => {
+        expect(value.classList.contains('hide')).toEqual(true);
+      });
+      expect(placeholders.length).toEqual(4);
+      placeholders.forEach((placeholder) => {
+        expect(placeholder.classList.contains('hide')).toEqual(false);
+      });
+    });
+  });
+
+  describe('selector options', () => {
+    const secrets = ['mysecret123'];
+
+    it('should respect `valueSelector` and `placeholderSelector` options', () => {
+      const valueClass = 'js-some-custom-placeholder-selector';
+      const placeholderClass = 'js-some-custom-value-selector';
+
+      const wrapper = setupSecretFixture(secrets, false, valueClass, placeholderClass);
+      const values = wrapper.querySelectorAll(`.${valueClass}`);
+      const placeholders = wrapper.querySelectorAll(`.${placeholderClass}`);
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+
+      expect(values.length).toEqual(1);
+      expect(placeholders.length).toEqual(1);
+
+      revealButton.click();
+
+      expect(values.length).toEqual(1);
+      expect(values[0].classList.contains('hide')).toEqual(false);
+      expect(placeholders.length).toEqual(1);
+      expect(placeholders[0].classList.contains('hide')).toEqual(true);
+    });
+  });
 });

spec/lib/gitlab/git/repository_spec.rb

@@ -1992,6 +1992,47 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
+  describe '#create_from_bundle' do
+    shared_examples 'creating repo from bundle' do
+      let(:bundle_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") }
+      let(:project) { create(:project) }
+      let(:imported_repo) { project.repository.raw }
+
+      before do
+        expect(repository.bundle_to_disk(bundle_path)).to be true
+      end
+
+      after do
+        FileUtils.rm_rf(bundle_path)
+      end
+
+      it 'creates a repo from a bundle file' do
+        expect(imported_repo).not_to exist
+
+        result = imported_repo.create_from_bundle(bundle_path)
+
+        expect(result).to be true
+        expect(imported_repo).to exist
+        expect { imported_repo.fsck }.not_to raise_exception
+      end
+
+      it 'creates a symlink to the global hooks dir' do
+        imported_repo.create_from_bundle(bundle_path)
+        hooks_path = File.join(imported_repo.path, 'hooks')
+
+        expect(File.readlink(hooks_path)).to eq(Gitlab.config.gitlab_shell.hooks_path)
+      end
+    end
+
+    context 'when Gitaly create_repo_from_bundle feature is enabled' do
+      it_behaves_like 'creating repo from bundle'
+    end
+
+    context 'when Gitaly create_repo_from_bundle feature is disabled', :disable_gitaly do
+      it_behaves_like 'creating repo from bundle'
+    end
+  end
+
   context 'gitlab_projects commands' do
     let(:gitlab_projects) { repository.gitlab_projects }
     let(:timeout) { Gitlab.config.gitlab_shell.git_timeout }

spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb

@@ -244,7 +244,7 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do
 
     it 'returns true when a commit exists' do
       expect(project.repository)
-        .to receive(:lookup)
+        .to receive(:commit)
         .with('123')
         .and_return(double(:commit))
 
@@ -253,9 +253,9 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do
 
     it 'returns false when a commit does not exist' do
       expect(project.repository)
-        .to receive(:lookup)
+        .to receive(:commit)
         .with('123')
-        .and_raise(Rugged::OdbError)
+        .and_return(nil)
 
       expect(importer.commit_exists?('123')).to eq(false)
     end

spec/models/repository_spec.rb

@@ -2356,7 +2356,7 @@ describe Repository do
     let(:commit) { repository.commit }
     let(:ancestor) { commit.parents.first }
 
-    context 'with Gitaly enabled' do
+    shared_examples '#ancestor?' do
       it 'it is an ancestor' do
         expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true)
       end
@@ -2370,27 +2370,19 @@ describe Repository do
         expect(repository.ancestor?(ancestor.id, nil)).to eq(false)
         expect(repository.ancestor?(nil, nil)).to eq(false)
       end
-    end
-
-    context 'with Gitaly disabled' do
-      before do
-        allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(false)
-        allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(false)
-      end
 
-      it 'it is an ancestor' do
-        expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true)
+      it 'returns false for invalid commit IDs' do
+        expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false)
+        expect(repository.ancestor?( Gitlab::Git::BLANK_SHA, commit.id)).to eq(false)
       end
+    end
 
-      it 'it is not an ancestor' do
-        expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false)
-      end
+    context 'with Gitaly enabled' do
+      it_behaves_like('#ancestor?')
+    end
 
-      it 'returns false on nil-values' do
-        expect(repository.ancestor?(nil, commit.id)).to eq(false)
-        expect(repository.ancestor?(ancestor.id, nil)).to eq(false)
-        expect(repository.ancestor?(nil, nil)).to eq(false)
-      end
+    context 'with Gitaly disabled', :skip_gitaly_mock do
+      it_behaves_like('#ancestor?')
     end
   end