Merge branch 'gitaly-commit-languages' into 'master'

Add option to use Gitaly CommitLanguages RPC

See merge request !13084

.gitlab-ci.yml

@@ -96,6 +96,7 @@ stages:
     - export KNAPSACK_GENERATE_REPORT=true
     - export CACHE_CLASSES=true
     - cp ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} ${KNAPSACK_REPORT_PATH}
+    - scripts/gitaly-test-spawn
     - knapsack rspec "--color --format documentation"
   artifacts:
     expire_in: 31d
@@ -221,6 +222,7 @@ setup-test-env:
     - bundle exec rake gettext:po_to_json
     - bundle exec rake gitlab:assets:compile
     - bundle exec ruby -Ispec -e 'require "spec_helper" ; TestEnv.init'
+    - scripts/gitaly-test-build # Do not use 'bundle exec' here
   artifacts:
     expire_in: 7d
     paths:
@@ -486,6 +488,7 @@ karma:
     BABEL_ENV: "coverage"
     CHROME_LOG_FILE: "chrome_debug.log"
   script:
+    - scripts/gitaly-test-spawn
     - bundle exec rake gettext:po_to_json
     - bundle exec rake karma
   coverage: '/^Statements *: (\d+\.\d+%)/'

GITALY_SERVER_VERSION

-0.25.0
+0.26.0

app/controllers/projects/graphs_controller.rb

@@ -43,23 +43,7 @@ class Projects::GraphsController < Projects::ApplicationController
   end
   def get_languages
-    @languages = Linguist::Repository.new(@repository.rugged, @repository.rugged.head.target_id).languages
-    total = @languages.map(&:last).sum
-
-    @languages = @languages.map do |language|
-      name, share = language
-      color = Linguist::Language[name].color || "##{Digest::SHA256.hexdigest(name)[0...6]}"
-      {
-        value: (share.to_f * 100 / total).round(2),
-        label: name,
-        color: color,
-        highlight: color
-      }
-    end
-
-    @languages.sort! do |x, y|
-      y[:value] <=> x[:value]
-    end
+    @languages = @project.repository.languages
   end
   def fetch_graph

lib/gitlab/git/repository.rb

@@ -636,6 +636,33 @@ module Gitlab
         @attributes.attributes(path)
       end
+      def languages(ref = nil)
+        Gitlab::GitalyClient.migrate(:commit_languages) do |is_enabled|
+          if is_enabled
+            gitaly_commit_client.languages(ref)
+          else
+            ref ||= rugged.head.target_id
+            languages = Linguist::Repository.new(rugged, ref).languages
+            total = languages.map(&:last).sum
+
+            languages = languages.map do |language|
+              name, share = language
+              color = Linguist::Language[name].color || "##{Digest::SHA256.hexdigest(name)[0...6]}"
+              {
+                value: (share.to_f * 100 / total).round(2),
+                label: name,
+                color: color,
+                highlight: color
+              }
+            end
+
+            languages.sort do |x, y|
+              y[:value] <=> x[:value]
+            end
+          end
+        end
+      end
+
       def gitaly_repository
         Gitlab::GitalyClient::Util.repository(@storage, @relative_path)
       end

lib/gitlab/gitaly_client/commit_service.rb

@@ -118,6 +118,13 @@ module Gitlab
         consume_commits_response(response)
       end
+      def languages(ref = nil)
+        request = Gitaly::CommitLanguagesRequest.new(repository: @gitaly_repo, revision: ref || '')
+        response = GitalyClient.call(@repository.storage, :commit_service, :commit_languages, request)
+
+        response.languages.map { |l| { value: l.share.round(2), label: l.name, color: l.color, highlight: l.color } }
+      end
+
       private
       def commit_diff_request_params(commit, options = {})

lib/tasks/gitlab/gitaly.rake

@@ -19,7 +19,10 @@ namespace :gitlab do
       Dir.chdir(args.dir) do
         create_gitaly_configuration
-        Bundler.with_original_env { run_command!([command]) }
+        # In CI we run scripts/gitaly-test-build instead of this command
+        unless ENV['CI'].present?
+          Bundler.with_original_env { run_command!(%w[/usr/bin/env -u BUNDLE_GEMFILE] + [command]) }
+        end
       end
     end
@@ -30,7 +33,9 @@ namespace :gitlab do
       puts "# Gitaly storage configuration generated from #{Gitlab.config.source} on #{Time.current.to_s(:long)}"
       puts "# This is in TOML format suitable for use in Gitaly's config.toml file."
-      puts gitaly_configuration_toml
+      # Exclude gitaly-ruby configuration because that depends on the gitaly
+      # installation directory.
+      puts gitaly_configuration_toml(gitaly_ruby: false)
     end
     private
@@ -41,7 +46,7 @@ namespace :gitlab do
     # only generate a configuration for the most common and simplest case: when
     # we have exactly one Gitaly process and we are sure it is running locally
     # because it uses a Unix socket.
-    def gitaly_configuration_toml
+    def gitaly_configuration_toml(gitaly_ruby: true)
       storages = []
       address = nil
@@ -60,6 +65,7 @@ namespace :gitlab do
       end
       config = { socket_path: address.sub(%r{\Aunix:}, ''), storage: storages }
       config[:auth] = { token: 'secret' } if Rails.env.test?
+      config[:'gitaly-ruby'] = { dir: File.join(Dir.pwd, 'ruby') } if gitaly_ruby
       TOML.dump(config)
     end

scripts/gitaly-test-build

+#!/usr/bin/env ruby
+
+# 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.
+
+abort 'gitaly build failed' unless system('make', chdir: 'tmp/tests/gitaly')

scripts/gitaly-test-spawn

+#!/usr/bin/env ruby
+
+gitaly_dir = 'tmp/tests/gitaly'
+args = %W[#{gitaly_dir}/gitaly #{gitaly_dir}/config.toml]
+
+# Print the PID of the spawned process
+puts spawn(*args, [:out, :err] => 'log/gitaly-test.log')

spec/controllers/projects/graphs_controller_spec.rb

@@ -24,37 +24,4 @@ describe Projects::GraphsController do
       expect(response).to redirect_to action: :charts
     end
   end
-
-  describe 'GET charts' do
-    let(:linguist_repository) do
-      double(languages: {
-               'Ruby'         => 1000,
-               'CoffeeScript' => 350,
-               'NSIS'         => 15
-             })
-    end
-
-    let(:expected_values) do
-      nsis_color = "##{Digest::SHA256.hexdigest('NSIS')[0...6]}"
-      [
-        # colors from Linguist:
-        { label: "Ruby",         color: "#701516",  highlight: "#701516" },
-        { label: "CoffeeScript", color: "#244776",  highlight: "#244776" },
-        # colors from SHA256 fallback:
-        { label: "NSIS",         color: nsis_color, highlight: nsis_color }
-      ]
-    end
-
-    before do
-      allow(Linguist::Repository).to receive(:new).and_return(linguist_repository)
-    end
-
-    it 'sets the correct colour according to language' do
-      get(:charts, namespace_id: project.namespace, project_id: project, id: 'master')
-
-      expected_values.each do |val|
-        expect(assigns(:languages)).to include(a_hash_including(val))
-      end
-    end
-  end
 end

spec/lib/gitlab/git/repository_spec.rb

@@ -1127,6 +1127,45 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
+  describe '#languages' do
+    shared_examples 'languages' do
+      it 'returns exactly the expected results' do
+        languages = repository.languages('4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6')
+        expected_languages = [
+          { value: 66.63, label: "Ruby", color: "#701516", highlight: "#701516" },
+          { value: 22.96, label: "JavaScript", color: "#f1e05a", highlight: "#f1e05a" },
+          { value: 7.9, label: "HTML", color: "#e44b23", highlight: "#e44b23" },
+          { value: 2.51, label: "CoffeeScript", color: "#244776", highlight: "#244776" }
+        ]
+
+        expect(languages.size).to eq(expected_languages.size)
+
+        expected_languages.size.times do |i|
+          a = expected_languages[i]
+          b = languages[i]
+
+          expect(a.keys.sort).to eq(b.keys.sort)
+          expect(a[:value]).to be_within(0.1).of(b[:value])
+
+          non_float_keys = a.keys - [:value]
+          expect(a.values_at(*non_float_keys)).to eq(b.values_at(*non_float_keys))
+        end
+      end
+
+      it "uses the repository's HEAD when no ref is passed" do
+        lang = repository.languages.first
+
+        expect(lang[:label]).to eq('Ruby')
+      end
+    end
+
+    it_behaves_like 'languages'
+
+    context 'with rugged', skip_gitaly_mock: true do
+      it_behaves_like 'languages'
+    end
+  end
+
   def create_remote_branch(repository, remote_name, branch_name, source_branch_name)
     source_branch = repository.branches.find { |branch| branch.name == source_branch_name }
     rugged = repository.rugged

spec/support/test_env.rb

@@ -144,10 +144,13 @@ module TestEnv
   end
   def start_gitaly(gitaly_dir)
-    gitaly_exec = File.join(gitaly_dir, 'gitaly')
-    gitaly_config = File.join(gitaly_dir, 'config.toml')
-    log_file = Rails.root.join('log/gitaly-test.log').to_s
-    @gitaly_pid = Bundler.with_original_env { spawn(gitaly_exec, gitaly_config, [:out, :err] => log_file) }
+    if ENV['CI'].present?
+      # Gitaly has been spawned outside this process already
+      return
+    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 }
   end
   def stop_gitaly

spec/tasks/gitlab/gitaly_rake_spec.rb

@@ -41,6 +41,16 @@ describe 'gitlab:gitaly namespace rake task' do
     end
     describe 'gmake/make' do
+      let(:command_preamble) { %w[/usr/bin/env -u BUNDLE_GEMFILE] }
+
+      before(:all) do
+        @old_env_ci = ENV.delete('CI')
+      end
+
+      after(:all) do
+        ENV['CI'] = @old_env_ci if @old_env_ci
+      end
+
       before do
         FileUtils.mkdir_p(clone_path)
         expect(Dir).to receive(:chdir).with(clone_path).and_call_original
@@ -49,12 +59,12 @@ describe 'gitlab:gitaly namespace rake task' do
       context 'gmake is available' do
         before do
           expect_any_instance_of(Object).to receive(:checkout_or_clone_version)
-          allow_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true)
+          allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true)
         end
         it 'calls gmake in the gitaly directory' do
           expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0])
-          expect_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true)
+          expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true)
           run_rake_task('gitlab:gitaly:install', clone_path)
         end
@@ -63,12 +73,12 @@ describe 'gitlab:gitaly namespace rake task' do
       context 'gmake is not available' do
         before do
           expect_any_instance_of(Object).to receive(:checkout_or_clone_version)
-          allow_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true)
+          allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true)
         end
         it 'calls make in the gitaly directory' do
           expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42])
-          expect_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true)
+          expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true)
           run_rake_task('gitlab:gitaly:install', clone_path)
         end