Merge branch 'bug/500_tag_name' into 'master'

Fix 500 on tags page

Fix bug when grit parse tags wrong and raise exception on tags page.
Fixes #993

Gemfile

@@ -29,7 +29,7 @@ gem 'omniauth-github'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem "gitlab_git", "~> 5.0.0"
+gem "gitlab_git", git: 'https://gitlab.com/gitlab-org/gitlab_git.git', ref: '39bc4f043414f1e49f802ed633fa20e6400bfa49'
 
 # Ruby/Rack Git Smart-HTTP Server Handler
 gem 'gitlab-grack', '~> 2.0.0.pre', require: 'grack'

Gemfile.lock

@@ -5,6 +5,17 @@ GIT
   specs:
     github-markup (0.7.6)
 
+GIT
+  remote: https://gitlab.com/gitlab-org/gitlab_git.git
+  revision: 39bc4f043414f1e49f802ed633fa20e6400bfa49
+  ref: 39bc4f043414f1e49f802ed633fa20e6400bfa49
+  specs:
+    gitlab_git (5.1.0.pre)
+      activesupport (~> 4.0.0)
+      gitlab-grit (~> 2.6.1)
+      gitlab-linguist (~> 3.0.0)
+      rugged (~> 0.19.0)
+
 GEM
   remote: https://rubygems.org/
   specs:
@@ -176,10 +187,6 @@ GEM
       charlock_holmes (~> 0.6.6)
       escape_utils (~> 0.2.4)
       mime-types (~> 1.19)
-    gitlab_git (5.0.0)
-      activesupport (~> 4.0.0)
-      gitlab-grit (~> 2.6.1)
-      gitlab-linguist (~> 3.0.0)
     gitlab_meta (6.0)
     gitlab_omniauth-ldap (1.0.4)
       net-ldap (~> 0.3.1)
@@ -417,6 +424,7 @@ GEM
     ruby-hmac (0.4.0)
     ruby-progressbar (1.2.0)
     rubyntlm (0.1.1)
+    rugged (0.19.0)
     safe_yaml (0.9.7)
     sanitize (2.0.6)
       nokogiri (>= 1.4.4)
@@ -574,7 +582,7 @@ DEPENDENCIES
   gitlab-gollum-lib (~> 1.1.0)
   gitlab-grack (~> 2.0.0.pre)
   gitlab-linguist (~> 3.0.0)
-  gitlab_git (~> 5.0.0)
+  gitlab_git!
   gitlab_meta (= 6.0)
   gitlab_omniauth-ldap (= 1.0.4)
   gon (~> 5.0.0)

app/controllers/projects/tags_controller.rb

@@ -8,7 +8,7 @@ class Projects::TagsController < Projects::ApplicationController
   before_filter :authorize_admin_project!, only: [:destroy]
 
   def index
-    @tags = Kaminari.paginate_array(@repository.tags).page(params[:page]).per(30)
+    @tags = Kaminari.paginate_array(@repository.tags.reverse).page(params[:page]).per(30)
   end
 
   def create

app/helpers/gitlab_markdown_helper.rb

@@ -166,13 +166,13 @@ module GitlabMarkdownHelper
 
   def file_exists?(path)
     return false if path.nil? || path.empty?
-    return @repository.blob_at(current_ref, path).present? || Tree.new(@repository, current_ref, path).entries.any?
+    return @repository.blob_at(current_ref, path).present? || @repository.tree(:head, path).entries.any?
   end
 
   # Check if the path is pointing to a directory(tree) or a file(blob)
   # eg. doc/api is directory and doc/README.md is file
   def local_path(path)
-    return "tree" if Tree.new(@repository, current_ref, path).entries.any?
+    return "tree" if @repository.tree(:head, path).entries.any?
     return "raw" if @repository.blob_at(current_ref, path).image?
     return "blob"
   end

app/models/commit.rb

@@ -16,29 +16,31 @@ class Commit
   DIFF_HARD_LIMIT_FILES = 500
   DIFF_HARD_LIMIT_LINES = 10000
 
-  def self.decorate(commits)
-    commits.map { |c| self.new(c) }
-  end
+  class << self
+    def decorate(commits)
+      commits.map { |c| self.new(c) }
+    end
 
-  # Calculate number of lines to render for diffs
-  def self.diff_line_count(diffs)
-    diffs.reduce(0){|sum, d| sum + d.diff.lines.count}
-  end
+    # Calculate number of lines to render for diffs
+    def diff_line_count(diffs)
+      diffs.reduce(0){|sum, d| sum + d.diff.lines.count}
+    end
 
-  def self.diff_suppress?(diffs, line_count = nil)
-    # optimize - check file count first
-    return true if diffs.size > DIFF_SAFE_FILES
+    def diff_suppress?(diffs, line_count = nil)
+      # optimize - check file count first
+      return true if diffs.size > DIFF_SAFE_FILES
 
-    line_count ||= Commit::diff_line_count(diffs)
-    line_count > DIFF_SAFE_LINES
-  end
+      line_count ||= Commit::diff_line_count(diffs)
+      line_count > DIFF_SAFE_LINES
+    end
 
-  def self.diff_force_suppress?(diffs, line_count = nil)
-    # optimize - check file count first
-    return true if diffs.size > DIFF_HARD_LIMIT_FILES
+    def diff_force_suppress?(diffs, line_count = nil)
+      # optimize - check file count first
+      return true if diffs.size > DIFF_HARD_LIMIT_FILES
 
-    line_count ||= Commit::diff_line_count(diffs)
-    line_count > DIFF_HARD_LIMIT_LINES
+      line_count ||= Commit::diff_line_count(diffs)
+      line_count > DIFF_HARD_LIMIT_LINES
+    end
   end
 
   attr_accessor :raw

app/models/repository.rb

@@ -57,7 +57,7 @@ class Repository
 
   def recent_branches(limit = 20)
     branches.sort do |a, b|
-      b.commit.committed_date <=> a.commit.committed_date
+      commit(b.target).committed_date <=> commit(a.target).committed_date
     end[0..limit]
   end
 
@@ -163,7 +163,19 @@ class Repository
 
   def readme
     Rails.cache.fetch(cache_key(:readme)) do
-      Tree.new(self, self.root_ref).readme
+      tree(:head).readme
     end
   end
+
+  def head_commit
+    commit(self.root_ref)
+  end
+
+  def tree(sha = :head, path = nil)
+    if sha == :head
+      sha = head_commit.sha
+    end
+
+    Tree.new(self, sha, path)
+  end
 end

app/models/tree.rb

@@ -23,4 +23,8 @@ class Tree
   def submodules
     @entries.select(&:submodule?)
   end
+
+  def sorted_entries
+    trees + blobs + submodules
+  end
 end

app/views/projects/branches/_branch.html.haml

-- commit = Commit.new(Gitlab::Git::Commit.new(branch.commit))
+- commit = @repository.commit(branch.target)
 %li
   %h4
     = link_to project_commits_path(@project, branch.name) do
@@ -19,10 +19,14 @@
         = link_to project_branch_path(@project, branch.name), class: 'btn grouped btn-small remove-row', method: :delete, data: { confirm: 'Removed branch cannot be restored. Are you sure?'}, remote: true do
           %i.icon-trash
 
-  %p
-    = link_to project_commit_path(@project, commit.id), class: 'commit_short_id' do
-      = commit.short_id
-    = image_tag avatar_icon(commit.author_email), class: "avatar s16", alt: ''
-    %span.light
-      = gfm escape_once(truncate(commit.title, length: 40))
-    #{time_ago_with_tooltip(commit.committed_date)}
+  - if commit
+    %p
+      = link_to project_commit_path(@project, commit.id), class: 'commit_short_id' do
+        = commit.short_id
+      = image_tag avatar_icon(commit.author_email), class: "avatar s16", alt: ''
+      %span.light
+        = gfm escape_once(truncate(commit.title, length: 40))
+      #{time_ago_with_tooltip(commit.committed_date)}
+  - else
+    %p
+      Cant find HEAD commit for this branch

app/views/projects/tags/_tag.html.haml

+- commit = @repository.commit(tag.target)
+%li
+  %h4
+    = link_to project_commits_path(@project, tag.name), class: "" do
+      %i.icon-tag
+      = tag.name
+    .pull-right
+      %small.cdark
+        %i.icon-calendar
+        #{time_ago_with_tooltip(commit.committed_date)}
+  %p.prepend-left-20
+    = link_to commit.short_id(8), project_commit_path(@project, commit), class: "monospace"
+    –
+    = link_to_gfm truncate(commit.title, length: 70), project_commit_path(@project, commit.id), class: "cdark"
+
+    %span.pull-right
+      - if can? current_user, :download_code, @project
+        = render 'projects/repositories/download_archive', ref: tag.name, btn_class: 'grouped btn-group-small'
+      - if can?(current_user, :admin_project, @project)
+        = link_to project_tag_path(@project, tag.name), class: 'btn btn-small remove-row grouped', method: :delete, data: { confirm: 'Removed tag cannot be restored. Are you sure?'}, remote: true do
+          %i.icon-trash
+

app/views/projects/tags/index.html.haml

@@ -13,29 +13,7 @@
 - unless @tags.empty?
   %ul.bordered-list
     - @tags.each do |tag|
-      - commit = Commit.new(Gitlab::Git::Commit.new(tag.commit))
-      %li
-        %h4
-          = link_to project_commits_path(@project, tag.name), class: "" do
-            %i.icon-tag
-            = tag.name
-          %small
-            = truncate(tag.message || '', length: 70)
-          .pull-right
-            %small.cdark
-              %i.icon-calendar
-              #{time_ago_with_tooltip(commit.committed_date)}
-        %p.prepend-left-20
-          = link_to commit.short_id(8), project_commit_path(@project, commit), class: "monospace"
-          –
-          = link_to_gfm truncate(commit.title, length: 70), project_commit_path(@project, commit.id), class: "cdark"
-
-          %span.pull-right
-            - if can? current_user, :download_code, @project
-              = render 'projects/repositories/download_archive', ref: tag.name, btn_class: 'grouped btn-group-small'
-            - if can?(current_user, :admin_project, @project)
-              = link_to project_tag_path(@project, tag.name), class: 'btn btn-small remove-row grouped', method: :delete, data: { confirm: 'Removed tag cannot be restored. Are you sure?'}, remote: true do
-                %i.icon-trash
+      = render 'tag', tag: tag
 
   = paginate @tags, theme: 'gitlab'
 

lib/api/entities.rb

@@ -79,7 +79,16 @@ module API
     end
 
     class RepoObject < Grape::Entity
-      expose :name, :commit
+      expose :name
+
+      expose :commit do |repo_obj, options|
+        if repo_obj.respond_to?(:commit)
+          repo_obj.commit
+        elsif options[:project]
+          options[:project].repository.commit(repo_obj.target)
+        end
+      end
+
       expose :protected do |repo, options|
         if options[:project]
           options[:project].protected_branch? repo.name
@@ -87,6 +96,16 @@ module API
       end
     end
 
+    class RepoTreeObject < Grape::Entity
+      expose :id, :name, :type
+
+      expose :mode do |obj, options|
+        filemode = obj.mode.to_s(8)
+        filemode = "0" + filemode if filemode.length < 6
+        filemode
+      end
+    end
+
     class RepoCommit < Grape::Entity
       expose :id, :short_id, :title, :author_name, :author_email, :created_at
     end

lib/api/repositories.rb

@@ -82,7 +82,7 @@ module API
       # Example Request:
       #   GET /projects/:id/repository/tags
       get ":id/repository/tags" do
-        present user_project.repo.tags.sort_by(&:name).reverse, with: Entities::RepoObject
+        present user_project.repo.tags.sort_by(&:name).reverse, with: Entities::RepoObject, project: user_project
       end
 
       # Get a project repository commits
@@ -141,15 +141,9 @@ module API
         path = params[:path] || nil
 
         commit = user_project.repository.commit(ref)
-        tree = Tree.new(user_project.repository, commit.id, path)
+        tree = user_project.repository.tree(commit.id, path)
 
-        trees = []
-
-        %w(trees blobs submodules).each do |type|
-          trees += tree.send(type).map { |t| {name: t.name, type: type.singularize, mode: t.mode, id: t.id} }
-        end
-
-        trees
+        present tree.sorted_entries, with: Entities::RepoTreeObject
       end
 
       # Get a raw file contents
@@ -233,4 +227,3 @@ module API
     end
   end
 end
-

lib/extracts_path.rb

@@ -117,7 +117,7 @@ module ExtractsPath
   end
 
   def tree
-    @tree ||= Tree.new(@repo, @commit.id, @path)
+    @tree ||= @repo.tree(@commit.id, @path)
   end
 
   private