Merge branch '10-8-stable-patch-3' into '10-8-stable'

.gitignore

@@ -64,6 +64,7 @@ eslint-report.html
 /tags
 /tmp/*
 /vendor/bundle/*
+/vendor/gitaly-ruby
 /builds*
 /shared/*
 /.gitlab_workhorse_secret

.gitlab-ci.yml

@@ -6,10 +6,11 @@ image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.7-golang-1.9-git
     - gitlab-org
 
 .default-cache: &default-cache
-  key: "ruby-2.3.7-with-yarn"
+  key: "ruby-2.3.7-debian-stretch-with-yarn"
   paths:
     - vendor/ruby
     - .yarn-cache/
+    - vendor/gitaly-ruby
 
 .push-cache: &push-cache
   cache:
@@ -438,6 +439,7 @@ setup-test-env:
     paths:
       - tmp/tests
       - config/secrets.yml
+      - vendor/gitaly-ruby
 
 rspec-pg 0 28: *rspec-metadata-pg
 rspec-pg 1 28: *rspec-metadata-pg
@@ -575,7 +577,7 @@ static-analysis:
   script:
     - scripts/static-analysis
   cache:
-    key: "ruby-2.3.7-with-yarn-and-rubocop"
+    key: "ruby-2.3.7-debian-stretch-with-yarn-and-rubocop"
     paths:
       - vendor/ruby
       - .yarn-cache/

GITALY_SERVER_VERSION

-0.100.0
+0.100.1

GITLAB_WORKHORSE_VERSION

-4.2.0
+4.2.1

Gemfile.lock

@@ -811,7 +811,7 @@ GEM
     rubyzip (1.2.1)
     rufus-scheduler (3.4.0)
       et-orbi (~> 1.0)
-    rugged (0.27.0)
+    rugged (0.27.1)
     safe_yaml (1.0.4)
     sanitize (2.1.0)
       nokogiri (>= 1.4.4)
@@ -940,7 +940,7 @@ GEM
     unf (0.1.4)
       unf_ext
     unf_ext (0.0.7.5)
-    unicode-display_width (1.3.0)
+    unicode-display_width (1.3.2)
     unicorn (5.1.0)
       kgio (~> 2.6)
       raindrops (~> 0.7)

app/assets/javascripts/compare_autocomplete.js

@@ -54,7 +54,7 @@ export default function initCompareAutocomplete(limitTo = null, clickHandler = (
             .attr('href', '#')
             .addClass(ref === selected ? 'is-active' : '')
             .text(ref)
-            .attr('data-ref', escape(ref));
+            .attr('data-ref', ref);
           return $('<li />').append(link);
         }
       },

app/assets/javascripts/issuable_form.js

@@ -30,7 +30,7 @@ export default class IssuableForm {
     }
 
     this.initAutosave();
-    this.form.on('submit:success', this.handleSubmit);
+    this.form.on('submit', this.handleSubmit);
     this.form.on('click', '.btn-cancel', this.resetAutosave);
     this.initWip();
 

app/models/ci/build.rb

@@ -617,7 +617,7 @@ module Ci
         variables.append(key: 'GITLAB_FEATURES', value: project.licensed_features.join(','))
         variables.append(key: 'CI_SERVER_NAME', value: 'GitLab')
         variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION)
-        variables.append(key: 'CI_SERVER_REVISION', value: Gitlab::REVISION)
+        variables.append(key: 'CI_SERVER_REVISION', value: Gitlab.revision)
         variables.append(key: 'CI_JOB_NAME', value: name)
         variables.append(key: 'CI_JOB_STAGE', value: stage)
         variables.append(key: 'CI_COMMIT_SHA', value: sha)

app/views/admin/dashboard/index.html.haml

@@ -126,7 +126,7 @@
               GitLab
               %span.pull-right
                 = Gitlab::VERSION
-                = "(#{Gitlab::REVISION})"
+                = "(#{Gitlab.revision})"
             %p
               GitLab Shell
               %span.pull-right

app/views/help/index.html.haml

@@ -8,7 +8,7 @@
   Community Edition
   - if user_signed_in?
     %span= Gitlab::VERSION
-    %small= link_to Gitlab::REVISION, Gitlab::COM_URL + namespace_project_commits_path('gitlab-org', 'gitlab-ce', Gitlab::REVISION)
+    %small= link_to Gitlab.revision, Gitlab::COM_URL + namespace_project_commits_path('gitlab-org', 'gitlab-ce', Gitlab.revision)
     = version_status_badge
   %hr
 

changelogs/unreleased/46600-fix-gitlab-revision-when-not-in-git-repo.yml

+---
+title: Replace Gitlab::REVISION with Gitlab.revision and handle installations without
+  a .git directory
+merge_request: 19125
+author:
+type: fixed

changelogs/unreleased/ensure-remote-mirror-columns-in-ce.yml

+---
+title: Fix remote mirror database inconsistencies when upgrading from EE to CE
+merge_request: 19196
+author:
+type: fixed

changelogs/unreleased/memoize-database-version.yml

+---
+title: Memoize Gitlab::Database.version
+merge_request:
+author:
+type: performance

changelogs/unreleased/sh-fix-submit-not-clearing-local-storage.yml

+---
+title: Fix local storage not being cleared after creating a new issue
+merge_request:
+author:
+type: fixed

changelogs/unreleased/winh-new-merge-request-encoding.yml

+---
+title: Fix encoding of branch names on compare and new merge request page
+merge_request: 19143
+author:
+type: fixed

config/initializers/console_message.rb

@@ -3,7 +3,7 @@ if defined?(Rails::Console)
   # note that this will not print out when using `spring`
   justify = 15
   puts "-------------------------------------------------------------------------------------"
-  puts " Gitlab:".ljust(justify) + "#{Gitlab::VERSION} (#{Gitlab::REVISION})"
+  puts " Gitlab:".ljust(justify) + "#{Gitlab::VERSION} (#{Gitlab.revision})"
   puts " Gitlab Shell:".ljust(justify) + Gitlab::Shell.new.version
   puts " #{Gitlab::Database.adapter_name}:".ljust(justify) + Gitlab::Database.version
   puts "-------------------------------------------------------------------------------------"

config/initializers/sentry.rb

@@ -13,7 +13,7 @@ def configure_sentry
   if sentry_enabled
     Raven.configure do |config|
       config.dsn = Gitlab::CurrentSettings.current_application_settings.sentry_dsn
-      config.release = Gitlab::REVISION
+      config.release = Gitlab.revision
 
       # Sanitize fields based on those sanitized from Rails.
       config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s)

db/migrate/20180529093006_ensure_remote_mirror_columns.rb

+class EnsureRemoteMirrorColumns < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column :remote_mirrors, :last_update_started_at, :datetime unless column_exists?(:remote_mirrors, :last_update_started_at)
+    add_column :remote_mirrors, :remote_name, :string unless column_exists?(:remote_mirrors, :remote_name)
+
+    unless column_exists?(:remote_mirrors, :only_protected_branches)
+      add_column_with_default(:remote_mirrors,
+                              :only_protected_branches,
+                              :boolean,
+                              default: false,
+                              allow_null: false)
+    end
+  end
+
+  def down
+    # db/migrate/20180503131624_create_remote_mirrors.rb will remove the table
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180508102840) do
+ActiveRecord::Schema.define(version: 20180529093006) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

doc/user/snippets.md

@@ -27,3 +27,36 @@ Personal snippets are not related to any project and can be created completely i
 You can download the raw content of a snippet.
 
 By default snippets will be downloaded with Linux-style line endings (`LF`). If you want to preserve the original line endings you need to add a parameter `line_ending=raw` (eg. `https://gitlab.com/snippets/SNIPPET_ID/raw?line_ending=raw`). In case a snippet was created using the GitLab web interface the original line ending is Windows-like (`CRLF`).
+
+## Embedded Snippets
+
+> Introduced in GitLab 10.8.
+
+Public snippets can not only be shared, but also embedded on any website. This
+allows to reuse a GitLab snippet in multiple places and any change to the source
+is automatically reflected in the embedded snippet.
+
+To embed a snippet, first make sure that:
+
+- The project is public (if it's a project snippet)
+- The snippet is public
+- In **Project > Settings > Permissions**, the snippets permissions are
+  set to **Everyone with access**
+
+Once the above conditions are met, the "Embed" section will appear in your snippet
+where you can simply click on the "Copy to clipboard" button. This copies a one-line
+script that you can add to any website or blog post.
+
+Here's how an example code looks like:
+
+```html
+<script src="https://gitlab.com/namespace/project/snippets/SNIPPET_ID.js"></script>
+```
+
+Here's how an embedded snippet looks like:
+
+<script src="https://gitlab.com/gitlab-org/gitlab-ce/snippets/1717978.js"></script>
+
+Embedded snippets are displayed with a header that shows the file name if defined,
+the snippet size, a link to GitLab, and the actual snippet content. Actions in
+the header allow users to see the snippet in raw format and download it.

lib/api/internal.rb

@@ -113,7 +113,7 @@ module API
         {
           api_version: API.version,
           gitlab_version: Gitlab::VERSION,
-          gitlab_rev: Gitlab::REVISION,
+          gitlab_rev: Gitlab.revision,
           redis: redis_ping
         }
       end

lib/api/version.rb

@@ -6,7 +6,7 @@ module API
       detail 'This feature was introduced in GitLab 8.13.'
     end
     get '/version' do
-      { version: Gitlab::VERSION, revision: Gitlab::REVISION }
+      { version: Gitlab::VERSION, revision: Gitlab.revision }
     end
   end
 end

lib/gitlab.rb

@@ -9,11 +9,26 @@ module Gitlab
     Settings
   end
 
+  def self.revision
+    @_revision ||= begin
+      if File.exist?(root.join("REVISION"))
+        File.read(root.join("REVISION")).strip.freeze
+      else
+        result = Gitlab::Popen.popen_with_detail(%W[#{config.git.bin_path} log --pretty=format:%h -n 1])
+
+        if result.status.success?
+          result.stdout.chomp.freeze
+        else
+          "Unknown".freeze
+        end
+      end
+    end
+  end
+
   COM_URL = 'https://gitlab.com'.freeze
   APP_DIRS_PATTERN = %r{^/?(app|config|ee|lib|spec|\(\w*\))}
   SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z}
   VERSION = File.read(root.join("VERSION")).strip.freeze
-  REVISION = Gitlab::Popen.popen(%W(#{config.git.bin_path} log --pretty=format:%h -n 1)).first.chomp.freeze
 
   def self.com?
     # Check `gl_subdomain?` as well to keep parity with gitlab.com

lib/gitlab/database.rb

@@ -43,7 +43,7 @@ module Gitlab
     end
 
     def self.version
-      database_version.match(/\A(?:PostgreSQL |)([^\s]+).*\z/)[1]
+      @version ||= database_version.match(/\A(?:PostgreSQL |)([^\s]+).*\z/)[1]
     end
 
     def self.join_lateral_supported?

lib/gitlab/gon_helper.rb

@@ -15,7 +15,7 @@ module Gitlab
       gon.user_color_scheme      = Gitlab::ColorSchemes.for_user(current_user).css_class
       gon.sentry_dsn             = Gitlab::CurrentSettings.clientside_sentry_dsn if Gitlab::CurrentSettings.clientside_sentry_enabled
       gon.gitlab_url             = Gitlab.config.gitlab.url
-      gon.revision               = Gitlab::REVISION
+      gon.revision               = Gitlab.revision
       gon.gitlab_logo            = ActionController::Base.helpers.asset_path('gitlab_logo.png')
       gon.sprite_icons           = IconsHelper.sprite_icon_path
       gon.sprite_file_icons      = IconsHelper.sprite_file_icons_path

lib/tasks/gitlab/info.rake

@@ -47,7 +47,7 @@ namespace :gitlab do
       puts ""
       puts "GitLab information".color(:yellow)
       puts "Version:\t#{Gitlab::VERSION}"
-      puts "Revision:\t#{Gitlab::REVISION}"
+      puts "Revision:\t#{Gitlab.revision}"
       puts "Directory:\t#{Rails.root}"
       puts "DB Adapter:\t#{database_adapter}"
       puts "URL:\t\t#{Gitlab.config.gitlab.url}"

scripts/gitaly-test-build

@@ -2,28 +2,29 @@
 
 require 'fileutils'
 
+require_relative 'gitaly_test'
+
 # This script assumes tmp/tests/gitaly already contains the correct
 # Gitaly version. We just have to compile it and run its 'bundle
-# install'. We have this separate script for that because weird things
-# were happening in CI when we have a 'bundle exec' process that later
-# called 'bundle install' using a different Gemfile, as happens with
-# gitlab-ce and gitaly.
+# install'. We have this separate script for that to avoid bundle
+# poisoning in CI. This script should only be run in CI.
+class GitalyTestBuild
+  include GitalyTest
 
-tmp_tests_gitaly_dir = File.expand_path('../tmp/tests/gitaly', __dir__)
+  def run
+    abort 'gitaly build failed' unless system(env, 'make', chdir: tmp_tests_gitaly_dir)
 
-# Use the top-level bundle vendor folder so that we don't reinstall gems twice
-bundle_vendor_path = File.expand_path('../vendor', __dir__)
+    check_gitaly_config!
 
-env = {
-  # This ensure the `clean` config set in `scripts/prepare_build.sh` isn't taken into account
-  'BUNDLE_IGNORE_CONFIG' => 'true',
-  'BUNDLE_GEMFILE' => File.join(tmp_tests_gitaly_dir, 'ruby', 'Gemfile'),
-  'BUNDLE_FLAGS' => "--jobs=4 --path=#{bundle_vendor_path} --retry=3"
-}
+    # Starting gitaly further validates its configuration
+    pid = start_gitaly
+    Process.kill('TERM', pid)
 
-abort 'gitaly build failed' unless system(env, 'make', chdir: tmp_tests_gitaly_dir)
+    # Make the 'gitaly' executable look newer than 'GITALY_SERVER_VERSION'.
+    # Without this a gitaly executable created in the setup-test-env job
+    # will look stale compared to GITALY_SERVER_VERSION.
+    FileUtils.touch(File.join(tmp_tests_gitaly_dir, 'gitaly'), mtime: Time.now + (1 << 24))
+  end
+end
 
-# Make the 'gitaly' executable look newer than 'GITALY_SERVER_VERSION'.
-# Without this a gitaly executable created in the setup-test-env job
-# will look stale compared to GITALY_SERVER_VERSION.
-FileUtils.touch(File.join(tmp_tests_gitaly_dir, 'gitaly'), mtime: Time.now + (1 << 24))
+GitalyTestBuild.new.run

scripts/gitaly-test-spawn

 #!/usr/bin/env ruby
 
-gitaly_dir = 'tmp/tests/gitaly'
-env = { 'HOME' => File.expand_path('tmp/tests'),
-        'GEM_PATH' => Gem.path.join(':') }
-args = %W[#{gitaly_dir}/gitaly #{gitaly_dir}/config.toml]
+# This script is used both in CI and in local development 'rspec' runs.
 
-# Print the PID of the spawned process
-puts spawn(env, *args, [:out, :err] => 'log/gitaly-test.log')
+require_relative 'gitaly_test'
+
+class GitalyTestSpawn
+  include GitalyTest
+
+  def run
+    check_gitaly_config!
+
+    # # Uncomment line below to see all gitaly logs merged into CI trace
+    # spawn('sleep 1; tail -f log/gitaly-test.log')
+
+    pid = start_gitaly
+
+    # In local development this pid file is used by rspec.
+    IO.write(File.expand_path('../tmp/tests/gitaly.pid', __dir__), pid)
+  end
+end
+
+GitalyTestSpawn.new.run

scripts/gitaly_test.rb

+# This file contains environment settings for gitaly when it's running
+# as part of the gitlab-ce/ee test suite.
+#
+# Please be careful when modifying this file. Your changes must work
+# both for local development rspec runs, and in CI.
+
+require 'socket'
+
+module GitalyTest
+  def tmp_tests_gitaly_dir
+    File.expand_path('../tmp/tests/gitaly', __dir__)
+  end
+
+  def gemfile
+    File.join(tmp_tests_gitaly_dir, 'ruby', 'Gemfile')
+  end
+
+  def env
+    env_hash = {
+      'HOME' => File.expand_path('tmp/tests'),
+      'GEM_PATH' => Gem.path.join(':'),
+      'BUNDLE_APP_CONFIG' => File.join(File.dirname(gemfile), '.bundle/config'),
+      'BUNDLE_FLAGS' => "--jobs=4 --retry=3",
+      'BUNDLE_INSTALL_FLAGS' => nil,
+      'BUNDLE_GEMFILE' => gemfile,
+      'RUBYOPT' => nil
+    }
+
+    if ENV['CI']
+      bundle_path = File.expand_path('../vendor/gitaly-ruby', __dir__)
+      env_hash['BUNDLE_FLAGS'] << " --path=#{bundle_path}"
+    end
+
+    env_hash
+  end
+
+  def config_path
+    File.join(tmp_tests_gitaly_dir, 'config.toml')
+  end
+
+  def start_gitaly
+    args = %W[#{tmp_tests_gitaly_dir}/gitaly #{config_path}]
+    pid = spawn(env, *args, [:out, :err] => 'log/gitaly-test.log')
+
+    begin
+      try_connect!
+    rescue
+      Process.kill('TERM', pid)
+      raise
+    end
+
+    pid
+  end
+
+  def check_gitaly_config!
+    puts 'Checking gitaly-ruby bundle...'
+    abort 'bundle check failed' unless system(env, 'bundle', 'check', chdir: File.dirname(gemfile))
+  end
+
+  def read_socket_path
+    # This code needs to work in an environment where we cannot use bundler,
+    # so we cannot easily use the toml-rb gem. This ad-hoc parser should be
+    # good enough.
+    config_text = IO.read(config_path)
+
+    config_text.lines.each do |line|
+      match_data = line.match(/^\s*socket_path\s*=\s*"([^"]*)"$/)
+
+      return match_data[1] if match_data
+    end
+
+    raise "failed to find socket_path in #{config_path}"
+  end
+
+  def try_connect!
+    print "Trying to connect to gitaly: "
+    timeout = 20
+    delay = 0.1
+    socket = read_socket_path
+
+    Integer(timeout / delay).times do
+      begin
+        UNIXSocket.new(socket)
+        puts ' OK'
+
+        return
+      rescue Errno::ENOENT, Errno::ECONNREFUSED
+        print '.'
+        sleep delay
+      end
+    end
+
+    puts ' FAILED'
+
+    raise "could not connect to #{socket}"
+  end
+end

spec/features/issues_spec.rb

@@ -591,6 +591,20 @@ describe 'Issues' do
         end
       end
 
+      it 'clears local storage after creating a new issue', :js do
+        2.times.each do
+          visit new_project_issue_path(project)
+          wait_for_requests
+
+          expect(page).to have_field('Title', with: '')
+
+          fill_in 'issue_title', with: 'bug 345'
+          fill_in 'issue_description', with: 'bug description'
+
+          click_button 'Submit issue'
+        end
+      end
+
       context 'dropzone upload file', :js do
         before do
           visit new_project_issue_path(project)

spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb

@@ -4,6 +4,12 @@ describe 'Merge request > User selects branches for new MR', :js do
   let(:project) { create(:project, :public, :repository) }
   let(:user) { project.creator }
 
+  def select_source_branch(branch_name)
+    find('.js-source-branch', match: :first).click
+    find('.js-source-branch-dropdown .dropdown-input-field').native.send_keys branch_name
+    find('.js-source-branch-dropdown .dropdown-content a', text: branch_name, match: :first).click
+  end
+
   before do
     project.add_master(user)
     sign_in(user)
@@ -43,8 +49,7 @@ describe 'Merge request > User selects branches for new MR', :js do
   it 'generates a diff for an orphaned branch' do
     visit project_new_merge_request_path(project)
 
-    find('.js-source-branch', match: :first).click
-    find('.js-source-branch-dropdown .dropdown-content a', text: 'orphaned-branch', match: :first).click
+    select_source_branch('orphaned-branch')
 
     click_button "Compare branches"
     click_link "Changes"
@@ -169,4 +174,31 @@ describe 'Merge request > User selects branches for new MR', :js do
       end
     end
   end
+
+  context 'with special characters in branch names' do
+    it 'escapes quotes in branch names' do
+      special_branch_name = '"with-quotes"'
+      CreateBranchService.new(project, user)
+        .execute(special_branch_name, 'add-pdf-file')
+
+      visit project_new_merge_request_path(project)
+      select_source_branch(special_branch_name)
+
+      source_branch_input = find('[name="merge_request[source_branch]"]', visible: false)
+      expect(source_branch_input.value).to eq special_branch_name
+    end
+
+    it 'does not escape unicode in branch names' do
+      special_branch_name = 'ʕ•ᴥ•ʔ'
+      CreateBranchService.new(project, user)
+        .execute(special_branch_name, 'add-pdf-file')
+
+      visit project_new_merge_request_path(project)
+      select_source_branch(special_branch_name)
+
+      click_button "Compare branches"
+
+      expect(page).to have_button("Submit merge request")
+    end
+  end
 end

spec/features/users/terms_spec.rb

@@ -62,7 +62,8 @@ describe 'Users > Terms' do
       expect(current_path).to eq(project_issues_path(project))
     end
 
-    it 'redirects back to the page the user was trying to save' do
+    # Disabled until https://gitlab.com/gitlab-org/gitlab-ce/issues/37162 is solved properly
+    xit 'redirects back to the page the user was trying to save' do
       visit new_project_issue_path(project)
 
       fill_in :issue_title, with: 'Hello world, a new issue'

spec/lib/gitlab/database_spec.rb

@@ -32,6 +32,12 @@ describe Gitlab::Database do
   end
 
   describe '.version' do
+    around do |example|
+      described_class.instance_variable_set(:@version, nil)
+      example.run
+      described_class.instance_variable_set(:@version, nil)
+    end
+
     context "on mysql" do
       it "extracts the version number" do
         allow(described_class).to receive(:database_version)
@@ -49,6 +55,14 @@ describe Gitlab::Database do
         expect(described_class.version).to eq '9.4.4'
       end
     end
+
+    it 'memoizes the result' do
+      count = ActiveRecord::QueryRecorder
+        .new { 2.times { described_class.version } }
+        .count
+
+      expect(count).to eq(1)
+    end
   end
 
   describe '.join_lateral_supported?' do

spec/lib/gitlab_spec.rb

@@ -8,6 +8,66 @@ describe Gitlab do
       expect(described_class.root).to eq(Pathname.new(File.expand_path('../..', __dir__)))
     end
   end
+  describe '.revision' do
+    let(:cmd) { %W[#{described_class.config.git.bin_path} log --pretty=format:%h -n 1] }
+
+    around do |example|
+      described_class.instance_variable_set(:@_revision, nil)
+      example.run
+      described_class.instance_variable_set(:@_revision, nil)
+    end
+
+    context 'when a REVISION file exists' do
+      before do
+        expect(File).to receive(:exist?)
+          .with(described_class.root.join('REVISION'))
+          .and_return(true)
+      end
+
+      it 'returns the actual Git revision' do
+        expect(File).to receive(:read)
+          .with(described_class.root.join('REVISION'))
+          .and_return("abc123\n")
+
+        expect(described_class.revision).to eq('abc123')
+      end
+
+      it 'memoizes the revision' do
+        expect(File).to receive(:read)
+          .once
+          .with(described_class.root.join('REVISION'))
+          .and_return("abc123\n")
+
+        2.times { described_class.revision }
+      end
+    end
+
+    context 'when no REVISION file exist' do
+      context 'when the Git command succeeds' do
+        before do
+          expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with(cmd)
+          .and_return(Gitlab::Popen::Result.new(cmd, 'abc123', '', double(success?: true)))
+        end
+
+        it 'returns the actual Git revision' do
+          expect(described_class.revision).to eq('abc123')
+        end
+      end
+
+      context 'when the Git command fails' do
+        before do
+          expect(Gitlab::Popen).to receive(:popen_with_detail)
+          .with(cmd)
+          .and_return(Gitlab::Popen::Result.new(cmd, '', 'fatal: Not a git repository', double('Process::Status', success?: false)))
+        end
+
+        it 'returns "Unknown"' do
+          expect(described_class.revision).to eq('Unknown')
+        end
+      end
+    end
+  end
 
   describe '.com?' do
     it 'is true when on GitLab.com' do

spec/models/ci/build_spec.rb

@@ -1497,7 +1497,7 @@ describe Ci::Build do
         { key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true },
         { key: 'CI_SERVER_NAME', value: 'GitLab', public: true },
         { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true },
-        { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true },
+        { key: 'CI_SERVER_REVISION', value: Gitlab.revision, public: true },
         { key: 'CI_JOB_NAME', value: 'test', public: true },
         { key: 'CI_JOB_STAGE', value: 'test', public: true },
         { key: 'CI_COMMIT_SHA', value: build.sha, public: true },

spec/requests/api/version_spec.rb

@@ -18,7 +18,7 @@ describe API::Version do
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['version']).to eq(Gitlab::VERSION)
-        expect(json_response['revision']).to eq(Gitlab::REVISION)
+        expect(json_response['revision']).to eq(Gitlab.revision)
       end
     end
   end

spec/support/helpers/test_env.rb

@@ -159,7 +159,11 @@ module TestEnv
     end
 
     spawn_script = Rails.root.join('scripts/gitaly-test-spawn').to_s
-    @gitaly_pid = Bundler.with_original_env { IO.popen([spawn_script], &:read).to_i }
+    Bundler.with_original_env do
+      raise "gitaly spawn failed" unless system(spawn_script)
+    end
+    @gitaly_pid = Integer(File.read('tmp/tests/gitaly.pid'))
+
     Kernel.at_exit { stop_gitaly }
 
     wait_gitaly

spec/views/admin/dashboard/index.html.haml_spec.rb

@@ -22,6 +22,6 @@ describe 'admin/dashboard/index.html.haml' do
   it "includes revision of GitLab" do
     render
 
-    expect(rendered).to have_content "#{Gitlab::VERSION} (#{Gitlab::REVISION})"
+    expect(rendered).to have_content "#{Gitlab::VERSION} (#{Gitlab.revision})"
   end
 end

spec/views/help/index.html.haml_spec.rb

@@ -39,7 +39,7 @@ describe 'help/index' do
 
   def stub_version(version, revision)
     stub_const('Gitlab::VERSION', version)
-    stub_const('Gitlab::REVISION', revision)
+    allow(Gitlab).to receive(:revision).and_return(revision)
   end
 
   def stub_helpers