Merge branch '7-10-rc3' into '7-10-stable'

Fixes for 7.10.0.rc3

content

- [ ] Update gitlab_git to ignore invalid lines in .gitmodules.
- [ ] Add a task that checks repository integrity with `git fsck`.
- [ ] Decrease memory use and increase performance of Google Code importer.
- [ ] Fix username period migration to preserve uniqueness of names and paths.

See merge request !1783

15 files changed, 144 insertions, 85 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 0487cc64745..5de75d368e2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,16 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+v 7.11.0 (unreleased)
+  - Fix clone URL field and X11 Primary selection (Dmitry Medvinsky)
+  - Ignore invalid lines in .gitmodules
+  -
+  -
+  -
+  -
+  -
+  -
+  -
+
 v 7.10.0 (unreleased)
   - Allow users to be invited by email to join a group or project.
   - Don't crash when project repository doesn't exist.
@@ -15,6 +26,7 @@ v 7.10.0 (unreleased)
   - Don't leak existence of group or project via search.
   - Fix bug where Wiki pages that included a '/' were no longer accessible (Stan Hu)
   - Fix bug where error messages from Dropzone would not be displayed on the issues page (Stan Hu)
+  - Add a rake task to check repository integrity with `git fsck`
   - Add ability to configure Reply-To address in gitlab.yml (Stan Hu)
   - Move current user to the top of the list in assignee/author filters (Stan Hu)
   - Fix broken side-by-side diff view on merge request page (Stan Hu)
diff --git a/Gemfile b/Gemfile
index 465e90e71a3..3f262655117 100644
--- a/Gemfile
+++ b/Gemfile
@@ -39,7 +39,7 @@ gem "browser"
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem "gitlab_git", '~> 7.1.6'
+gem "gitlab_git", '~> 7.1.9'
 
 # Ruby/Rack Git Smart-HTTP Server Handler
 gem 'gitlab-grack', '~> 2.0.0.rc2', require: 'grack'
diff --git a/Gemfile.lock b/Gemfile.lock
index dc00acf6292..bfe626521e7 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -212,7 +212,7 @@ GEM
       mime-types (~> 1.19)
     gitlab_emoji (0.1.0)
       gemojione (~> 2.0)
-    gitlab_git (7.1.6)
+    gitlab_git (7.1.9)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.6)
       gitlab-linguist (~> 3.0)
@@ -703,7 +703,7 @@ DEPENDENCIES
   gitlab-grack (~> 2.0.0.rc2)
   gitlab-linguist (~> 3.0.1)
   gitlab_emoji (~> 0.1)
-  gitlab_git (~> 7.1.6)
+  gitlab_git (~> 7.1.9)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (= 1.2.1)
   gollum-lib (~> 4.0.2)
diff --git a/app/controllers/import/google_code_controller.rb b/app/controllers/import/google_code_controller.rb
index fb4ef987367..73c912e285b 100644
--- a/app/controllers/import/google_code_controller.rb
+++ b/app/controllers/import/google_code_controller.rb
@@ -54,6 +54,11 @@ class Import::GoogleCodeController < Import::BaseController
       render "new_user_map" and return
     end
 
+    # This is the default, so let's not save it into the database.
+    user_map.reject! do |key, value|
+      value == Gitlab::GoogleCodeImport::Client.mask_email(key)
+    end
+
     session[:google_code_user_map] = user_map
 
     flash[:notice] = "The user map has been saved. Continue by selecting the projects you want to import."
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index a0d79d7e5c0..e1de114375e 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -59,6 +59,7 @@ class Namespace < ActiveRecord::Base
     end
 
     def clean_path(path)
+      path = path.dup
       path.gsub!(/@.*\z/,             "")
       path.gsub!(/\.git\z/,           "")
       path.gsub!(/\A-+/,              "")
diff --git a/app/models/project.rb b/app/models/project.rb
index c4f50e02ddd..64ee2c2212b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -27,7 +27,6 @@
 #  import_type            :string(255)
 #  import_source          :string(255)
 #  avatar                 :string(255)
-#  import_data            :text
 #
 
 require 'carrierwave/orm/activerecord'
@@ -51,8 +50,6 @@ class Project < ActiveRecord::Base
   default_value_for :wall_enabled, false
   default_value_for :snippets_enabled, gitlab_config_features.snippets
 
-  serialize :import_data, JSON
-
   # set last_activity_at to the same as created_at
   after_create :set_last_activity_at
   def set_last_activity_at
@@ -117,6 +114,8 @@ class Project < ActiveRecord::Base
   has_many :users_star_projects, dependent: :destroy
   has_many :starrers, through: :users_star_projects, source: :user
 
+  has_one :import_data, dependent: :destroy, class_name: "ProjectImportData"
+
   delegate :name, to: :owner, allow_nil: true, prefix: true
   delegate :members, to: :team, prefix: true
 
@@ -267,8 +266,7 @@ class Project < ActiveRecord::Base
   end
 
   def clear_import_data
-    self.import_data = nil
-    self.save
+    self.import_data.destroy if self.import_data
   end
 
   def import?
diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb
new file mode 100644
index 00000000000..6a8a8a56eb5
--- /dev/null
+++ b/app/models/project_import_data.rb
@@ -0,0 +1,19 @@
+# == Schema Information
+#
+# Table name: project_import_datas
+#
+#  id           :integer          not null, primary key
+#  project_id   :integer
+#  data         :text
+#
+
+require 'carrierwave/orm/activerecord'
+require 'file_size_validator'
+
+class ProjectImportData < ActiveRecord::Base
+  belongs_to :project
+  
+  serialize :data, JSON
+
+  validates :project, presence: true
+end
diff --git a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb
index 7ce53c2a0d6..dc38b0eceb7 100644
--- a/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb
+++ b/db/migrate/20150324133047_remove_periods_at_ends_of_usernames.rb
@@ -3,21 +3,21 @@ class RemovePeriodsAtEndsOfUsernames < ActiveRecord::Migration
 
   class Namespace < ActiveRecord::Base
     class << self
-      def by_path(path)
-        where('lower(path) = :value', value: path.downcase).first
+      def find_by_path_or_name(path)
+        find_by("lower(path) = :path OR lower(name) = :path", path: path.downcase)
       end
 
       def clean_path(path)
         path = path.dup
         path.gsub!(/@.*\z/,             "")
         path.gsub!(/\.git\z/,           "")
-        path.gsub!(/\A-/,               "")
-        path.gsub!(/.\z/,               "")
+        path.gsub!(/\A-+/,              "")
+        path.gsub!(/\.+\z/,             "")
         path.gsub!(/[^a-zA-Z0-9_\-\.]/, "")
 
         counter = 0
         base = path
-        while Namespace.by_path(path).present?
+        while Namespace.find_by_path_or_name(path)
           counter += 1
           path = "#{base}#{counter}"
         end
diff --git a/db/migrate/20150417121913_create_project_import_data.rb b/db/migrate/20150417121913_create_project_import_data.rb
new file mode 100644
index 00000000000..c78f5fde85e
--- /dev/null
+++ b/db/migrate/20150417121913_create_project_import_data.rb
@@ -0,0 +1,8 @@
+class CreateProjectImportData < ActiveRecord::Migration
+  def change
+    create_table :project_import_data do |t|
+      t.references :project
+      t.text :data
+    end
+  end
+end
diff --git a/db/migrate/20150417122318_remove_import_data_from_project.rb b/db/migrate/20150417122318_remove_import_data_from_project.rb
new file mode 100644
index 00000000000..c275b49d228
--- /dev/null
+++ b/db/migrate/20150417122318_remove_import_data_from_project.rb
@@ -0,0 +1,5 @@
+class RemoveImportDataFromProject < ActiveRecord::Migration
+  def change
+    remove_column :projects, :import_data
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 0b91f9c467e..1aee37b2e61 100644
--- a/db/schema.rb
+++ b/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: 20150413192223) do
+ActiveRecord::Schema.define(version: 20150417122318) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -323,6 +323,11 @@ ActiveRecord::Schema.define(version: 20150413192223) do
   add_index "oauth_applications", ["owner_id", "owner_type"], name: "index_oauth_applications_on_owner_id_and_owner_type", using: :btree
   add_index "oauth_applications", ["uid"], name: "index_oauth_applications_on_uid", unique: true, using: :btree
 
+  create_table "project_import_data", force: true do |t|
+    t.integer "project_id"
+    t.text    "data"
+  end
+
   create_table "projects", force: true do |t|
     t.string   "name"
     t.string   "path"
@@ -348,7 +353,6 @@ ActiveRecord::Schema.define(version: 20150413192223) do
     t.integer  "star_count",             default: 0,        null: false
     t.string   "import_type"
     t.string   "import_source"
-    t.text     "import_data"
   end
 
   add_index "projects", ["created_at", "id"], name: "index_projects_on_created_at_and_id", using: :btree
diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb
index 777472cf3c5..b5e82563ff1 100644
--- a/lib/gitlab/google_code_import/importer.rb
+++ b/lib/gitlab/google_code_import/importer.rb
@@ -5,7 +5,10 @@ module Gitlab
 
       def initialize(project)
         @project = project
-        @repo = GoogleCodeImport::Repository.new(project.import_data["repo"])
+
+        import_data = project.import_data.try(:data)
+        repo_data = import_data["repo"] if import_data
+        @repo = GoogleCodeImport::Repository.new(repo_data)
 
         @closed_statuses = []
         @known_labels = Set.new
@@ -27,9 +30,10 @@ module Gitlab
 
       def user_map
         @user_map ||= begin
-          user_map = Hash.new { |hash, user| hash[user] = Client.mask_email(user) }
+          user_map = Hash.new { |hash, user| Client.mask_email(user) }
 
-          stored_user_map = project.import_data["user_map"]
+          import_data = project.import_data.try(:data)
+          stored_user_map = import_data["user_map"] if import_data
           user_map.update(stored_user_map) if stored_user_map
 
           user_map
@@ -58,24 +62,7 @@ module Gitlab
       def import_issues
         return unless repo.issues
 
-        last_id = 0
-
-        deleted_issues = []
-
-        repo.issues.each do |raw_issue|
-          while raw_issue["id"] > last_id + 1
-            last_id += 1
-
-            issue = project.issues.create!(
-              title:        "Deleted issue",
-              description:  "*This issue has been deleted*",
-              author_id:    project.creator_id,
-              state:        "closed"
-            )
-            deleted_issues << issue
-          end
-          last_id = raw_issue["id"]
-
+        while raw_issue = repo.issues.shift
           author  = user_map[raw_issue["author"]["name"]]
           date    = DateTime.parse(raw_issue["published"]).to_formatted_s(:long)
 
@@ -112,7 +99,8 @@ module Gitlab
             end
           end
 
-          issue = project.issues.create!(
+          issue = Issue.create!(
+            project_id:   project.id,
             title:        raw_issue["title"],
             description:  body,
             author_id:    project.creator_id,
@@ -121,39 +109,46 @@ module Gitlab
           )
           issue.add_labels_by_names(labels)
 
+          if issue.iid != raw_issue["id"]
+            issue.update_attribute(:iid, raw_issue["id"])
+          end
+
           import_issue_comments(issue, comments)
         end
-
-        deleted_issues.each(&:destroy!)
       end
 
       def import_issue_comments(issue, comments)
-        comments.each do |raw_comment|
-          next if raw_comment.has_key?("deletedBy")
-
-          content     = format_content(raw_comment["content"])
-          updates     = format_updates(raw_comment["updates"])
-          attachments = format_attachments(issue.iid, raw_comment["id"], raw_comment["attachments"])
-
-          next if content.blank? && updates.blank? && attachments.blank?
-
-          author  = user_map[raw_comment["author"]["name"]]
-          date    = DateTime.parse(raw_comment["published"]).to_formatted_s(:long)
-
-          body = format_issue_comment_body(
-            raw_comment["id"],
-            author,
-            date,
-            content,
-            updates,
-            attachments
-          )
+        Note.transaction do
+          while raw_comment = comments.shift
+            next if raw_comment.has_key?("deletedBy")
+
+            content     = format_content(raw_comment["content"])
+            updates     = format_updates(raw_comment["updates"])
+            attachments = format_attachments(issue.iid, raw_comment["id"], raw_comment["attachments"])
+
+            next if content.blank? && updates.blank? && attachments.blank?
+
+            author  = user_map[raw_comment["author"]["name"]]
+            date    = DateTime.parse(raw_comment["published"]).to_formatted_s(:long)
+
+            body = format_issue_comment_body(
+              raw_comment["id"],
+              author,
+              date,
+              content,
+              updates,
+              attachments
+            )
 
-          issue.notes.create!(
-            project_id: project.id,
-            author_id:  project.creator_id,
-            note:       body
-          )
+            # Needs to match order of `comment_columns` below.
+            Note.create!(
+              project_id:     project.id,
+              noteable_type:  "Issue",
+              noteable_id:    issue.id,
+              author_id:      project.creator_id,
+              note:           body
+            )
+          end
         end
       end
 
@@ -232,7 +227,7 @@ module Gitlab
 
       def create_label(name)
         color = nice_label_color(name)
-        project.labels.create!(name: name, color: color)
+        Label.create!(project_id: project.id, name: name, color: color)
       end
 
       def format_content(raw_content)
diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb
index 7ac4387d79d..0cfeaf9d61c 100644
--- a/lib/gitlab/google_code_import/project_creator.rb
+++ b/lib/gitlab/google_code_import/project_creator.rb
@@ -11,12 +11,7 @@ module Gitlab
       end
 
       def execute
-        import_data = {
-          "repo"      => repo.raw_data,
-          "user_map"  => user_map
-        }
-
-        @project = Project.new(
+        project = ::Projects::CreateService.new(current_user,
           name: repo.name,
           path: repo.name,
           description: repo.summary,
@@ -25,21 +20,17 @@ module Gitlab
           visibility_level: Gitlab::VisibilityLevel::PUBLIC,
           import_type: "google_code",
           import_source: repo.name,
-          import_url: repo.import_url,
-          import_data: import_data
-        )
+          import_url: repo.import_url
+        ).execute
 
-        if @project.save!
-          @project.reload
-
-          if @project.import_failed?
-            @project.import_retry
-          else
-            @project.import_start
-          end
-        end
+        import_data = project.create_import_data(
+          data: {
+            "repo"      => repo.raw_data,
+            "user_map"  => user_map
+          }
+        )
 
-        @project
+        project
       end
     end
   end
diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake
index d791b7155f9..04a2eb12db0 100644
--- a/lib/tasks/gitlab/check.rake
+++ b/lib/tasks/gitlab/check.rake
@@ -687,6 +687,23 @@ namespace :gitlab do
     end
   end
 
+  namespace :repo do
+    desc "GITLAB | Check the integrity of the repositories managed by GitLab"
+    task check: :environment do
+      namespace_dirs = Dir.glob(
+        File.join(Gitlab.config.gitlab_shell.repos_path, '*')
+      )
+
+      namespace_dirs.each do |namespace_dir|
+        repo_dirs = Dir.glob(File.join(namespace_dir, '*'))
+        repo_dirs.each do |dir|
+          puts "\nChecking repo at #{dir}"
+          system(*%w(git fsck), chdir: dir)
+        end
+      end
+    end
+  end
+
   # Helper methods
   ##########################
 
diff --git a/spec/lib/gitlab/google_code_import/importer_spec.rb b/spec/lib/gitlab/google_code_import/importer_spec.rb
index 107ba49962a..1c4503ae0ef 100644
--- a/spec/lib/gitlab/google_code_import/importer_spec.rb
+++ b/spec/lib/gitlab/google_code_import/importer_spec.rb
@@ -12,9 +12,13 @@ describe Gitlab::GoogleCodeImport::Importer do
       }
     } 
   }
-  let(:project) { create(:project, import_data: import_data) }
+  let(:project) { create(:project) }
   subject       { described_class.new(project) }
 
+  before do
+    project.create_import_data(data: import_data)
+  end
+
   describe "#execute" do
 
     it "imports status labels" do