scm: git: call "git log" only once instead of per branch in fetching revisions (#8857)

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8839 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/models/repository/git.rb

@@ -107,8 +107,7 @@ class Repository::Git < Repository
   # However, Git does not have a sequential commit numbering.
   #
   # In order to fetch only new adding revisions,
-  # Redmine needs to parse revisions per branch.
+  # Redmine needs to save "heads".
-  # Branch "last_scmid" is for this requirement.
   #
   # In Git and Mercurial, revisions are not in date order.
   # Redmine Mercurial fixed issues.
@@ -131,9 +130,17 @@ class Repository::Git < Repository
   def fetch_changesets
     scm_brs = branches
     return if scm_brs.nil? || scm_brs.empty?
+
     h1 = extra_info || {}
     h  = h1.dup
-    h["branches"]       ||= {}
+    repo_heads = scm_brs.map{ |br| br.scmid }
+    h["heads"] ||= []
+    prev_db_heads = h["heads"].dup
+    if prev_db_heads.empty?
+      prev_db_heads += heads_from_branches_hash
+    end
+    return if prev_db_heads.sort == repo_heads.sort
+
     h["db_consistent"]  ||= {}
     if changesets.count == 0
       h["db_consistent"]["ordering"] = 1
@@ -144,35 +151,39 @@ class Repository::Git < Repository
       merge_extra_info(h)
       self.save
     end
-    save_revisions(h, scm_brs)
+
+    save_revisions(prev_db_heads, repo_heads)
   end
 
-  def save_revisions(h, scm_brs)
+  def save_revisions(prev_db_heads, repo_heads)
-    scm_brs.each do |br1|
+    h = {}
-      br = br1.to_s
+    opts = {}
-      from_scmid = nil
+    opts[:reverse]  = true
-      from_scmid = h["branches"][br]["last_scmid"] if h["branches"][br]
+    opts[:excludes] = prev_db_heads
-      h["branches"][br] ||= {}
+    opts[:includes] = repo_heads
-      begin
+    begin
-        scm.revisions('', from_scmid, br, {:reverse => true}) do |rev|
+      scm.revisions('', nil, nil, opts) do |rev|
-          db_rev = find_changeset_by_name(rev.revision)
+        db_rev = find_changeset_by_name(rev.scmid)
-          transaction do
+        transaction do
-            if db_rev.nil?
+          if db_rev.nil?
-              db_saved_rev = save_revision(rev)
+            db_saved_rev = save_revision(rev)
-              parents = {}
+            parents = {}
-              parents[db_saved_rev] = rev.parents unless rev.parents.nil?
+            parents[db_saved_rev] = rev.parents unless rev.parents.nil?
-              parents.each do |ch, chparents|
+            parents.each do |ch, chparents|
-                ch.parents = chparents.collect{|rp| find_changeset_by_name(rp)}.compact
+              ch.parents = chparents.collect{|rp| find_changeset_by_name(rp)}.compact
-              end
             end
-            h["branches"][br]["last_scmid"] = rev.scmid
-            merge_extra_info(h)
-            self.save
           end
+          h["heads"] = prev_db_heads.dup
+          h["heads"] << rev.scmid
+          merge_extra_info(h)
+          self.save
         end
-      rescue Redmine::Scm::Adapters::CommandFailed => e
-        logger.error("save revisions error: #{e.message}")
       end
+      h["heads"] = repo_heads.dup
+      merge_extra_info(h)
+      self.save
+    rescue Redmine::Scm::Adapters::CommandFailed => e
+      logger.error("save revisions error: #{e.message}")
     end
   end
   private :save_revisions

test/unit/repository_git_test.rb

@@ -87,7 +87,7 @@ class RepositoryGitTest < ActiveSupport::TestCase
       assert_equal "README", change.path
       assert_equal "A", change.action
 
-      assert_equal 5, @repository.extra_info["branches"].size
+      assert_equal 5, @repository.extra_info["heads"].size
     end
 
     def test_fetch_changesets_incremental
@@ -95,11 +95,10 @@ class RepositoryGitTest < ActiveSupport::TestCase
       @repository.fetch_changesets
       @project.reload
       assert_equal NUM_REV, @repository.changesets.count
-      extra_info_db = @repository.extra_info["branches"]
+      extra_info_heads = @repository.extra_info["heads"].dup
-      assert_equal "1ca7f5ed374f3cb31a93ae5215c2e25cc6ec5127",
+      assert_equal 5, extra_info_heads.size
-                    extra_info_db["latin-1-path-encoding"]["last_scmid"]
+      extra_info_heads.delete_if { |x| x == "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c" }
-      assert_equal "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c",
+      assert_equal 4, extra_info_heads.size
-                    extra_info_db["master"]["last_scmid"]
 
       del_revs = [
           "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c",
@@ -114,20 +113,19 @@ class RepositoryGitTest < ActiveSupport::TestCase
       end
       @project.reload
       cs1 = @repository.changesets
-      assert_equal 22, cs1.count
+      assert_equal NUM_REV - 6, cs1.count
-      h = @repository.extra_info.dup
+      extra_info_heads << "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8"
-      h["branches"]["master"]["last_scmid"] =
+      h = {}
-            "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8"
+      h["heads"] = extra_info_heads
       @repository.merge_extra_info(h)
       @repository.save
       @project.reload
-      extra_info_db_1 = @repository.extra_info["branches"]
+      assert @repository.extra_info["heads"].index("4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8")
-      assert_equal "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8",
-                    extra_info_db_1["master"]["last_scmid"]
-
       @repository.fetch_changesets
       @project.reload
       assert_equal NUM_REV, @repository.changesets.count
+      assert_equal 5, @repository.extra_info["heads"].size
+      assert @repository.extra_info["heads"].index("83ca5fd546063a3c7dc2e568ba3355661a9e2b2c")
     end
 
     def test_fetch_changesets_history_editing
@@ -135,9 +133,10 @@ class RepositoryGitTest < ActiveSupport::TestCase
       @repository.fetch_changesets
       @project.reload
       assert_equal NUM_REV, @repository.changesets.count
-      assert_equal 5, @repository.extra_info["branches"].size
+      extra_info_heads = @repository.extra_info["heads"].dup
-      assert_equal "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c",
+      assert_equal 5, extra_info_heads.size
-                     @repository.extra_info["branches"]["master"]["last_scmid"]
+      extra_info_heads.delete_if { |x| x == "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c" }
+      assert_equal 4, extra_info_heads.size
 
       del_revs = [
           "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c",
@@ -162,17 +161,21 @@ class RepositoryGitTest < ActiveSupport::TestCase
       @project.reload
       assert_equal NUM_REV - 5, @repository.changesets.count
 
-      h = @repository.extra_info.dup
+      extra_info_heads << "abcd1234efgh"
-      h["branches"]["master"]["last_scmid"] = "abcd1234efgh"
+      h = {}
+      h["heads"] = extra_info_heads
       @repository.merge_extra_info(h)
       @repository.save
       @project.reload
-      assert_equal "abcd1234efgh",
+      h1 = @repository.extra_info["heads"].dup
-                    @repository.extra_info["branches"]["master"]["last_scmid"]
+      assert h1.index("abcd1234efgh")
+      assert_equal 5, h1.size
 
       @repository.fetch_changesets
       @project.reload
       assert_equal NUM_REV - 5, @repository.changesets.count
+      h2 = @repository.extra_info["heads"].dup
+      assert_equal h1, h2
     end
 
     def test_parents
@@ -216,6 +219,8 @@ class RepositoryGitTest < ActiveSupport::TestCase
       @project.reload
       assert_equal 0, @repository.extra_info["db_consistent"]["ordering"]
 
+      extra_info_heads = @repository.extra_info["heads"].dup
+      extra_info_heads.delete_if { |x| x == "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c" }
       del_revs = [
           "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c",
           "ed5bb786bbda2dee66a2d50faf51429dbc043a7b",
@@ -231,18 +236,19 @@ class RepositoryGitTest < ActiveSupport::TestCase
       cs1 = @repository.changesets
       assert_equal NUM_REV - 6, cs1.count
       assert_equal 0, @repository.extra_info["db_consistent"]["ordering"]
-      h = @repository.extra_info.dup
+
-      h["branches"]["master"]["last_scmid"] =
+      extra_info_heads << "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8"
-            "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8"
+      h = {}
+      h["heads"] = extra_info_heads
       @repository.merge_extra_info(h)
       @repository.save
       @project.reload
-      extra_info_db_1 = @repository.extra_info["branches"]
+      assert @repository.extra_info["heads"].index("4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8")
-      assert_equal "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8",
-                    extra_info_db_1["master"]["last_scmid"]
-
       @repository.fetch_changesets
+      @project.reload
       assert_equal NUM_REV, @repository.changesets.count
+      assert_equal 5, @repository.extra_info["heads"].size
+
       assert_equal 0, @repository.extra_info["db_consistent"]["ordering"]
     end
 
@@ -481,7 +487,7 @@ class RepositoryGitTest < ActiveSupport::TestCase
       @repository.fetch_changesets
       @project.reload
       assert_equal NUM_REV, @repository.changesets.count
-      %w|95488a44bc25f7d1f97d775a31359539ff333a63 95488a44b|.each do |r1|
+      %w|7234cb2750b63f47bff735edc50a1c0a433c2518 7234cb275|.each do |r1|
         changeset = @repository.find_changeset_by_name(r1)
         assert_nil changeset.previous
       end
@@ -505,7 +511,7 @@ class RepositoryGitTest < ActiveSupport::TestCase
       @repository.fetch_changesets
       @project.reload
       assert_equal NUM_REV, @repository.changesets.count
-      %w|67e7792ce20ccae2e4bb73eed09bb397819c8834 67e7792ce20cca|.each do |r1|
+      %w|2a682156a3b6e77a8bf9cd4590e8db757f3c6c78 2a682156a3b6e77a|.each do |r1|
         changeset = @repository.find_changeset_by_name(r1)
         assert_nil changeset.next
       end