From 1e5f16722fc3dc8e4250309bc4fc5c579461454b Mon Sep 17 00:00:00 2001 From: Toshi MARUYAMA Date: Sat, 11 Feb 2012 05:42:06 +0000 Subject: [PATCH] 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 | 67 +++++++++++++++++++------------- test/unit/repository_git_test.rb | 66 +++++++++++++++++-------------- 2 files changed, 75 insertions(+), 58 deletions(-) diff --git a/app/models/repository/git.rb b/app/models/repository/git.rb index c5606f240..207a224ec 100644 --- a/app/models/repository/git.rb +++ b/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. - # Branch "last_scmid" is for this requirement. + # Redmine needs to save "heads". # # 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) - end - - def save_revisions(h, scm_brs) - scm_brs.each do |br1| - br = br1.to_s - from_scmid = nil - from_scmid = h["branches"][br]["last_scmid"] if h["branches"][br] - h["branches"][br] ||= {} - begin - scm.revisions('', from_scmid, br, {:reverse => true}) do |rev| - db_rev = find_changeset_by_name(rev.revision) - transaction do - if db_rev.nil? - db_saved_rev = save_revision(rev) - parents = {} - parents[db_saved_rev] = rev.parents unless rev.parents.nil? - parents.each do |ch, chparents| - ch.parents = chparents.collect{|rp| find_changeset_by_name(rp)}.compact - end + + save_revisions(prev_db_heads, repo_heads) + end + + def save_revisions(prev_db_heads, repo_heads) + h = {} + opts = {} + opts[:reverse] = true + opts[:excludes] = prev_db_heads + opts[:includes] = repo_heads + begin + scm.revisions('', nil, nil, opts) do |rev| + db_rev = find_changeset_by_name(rev.scmid) + transaction do + if db_rev.nil? + db_saved_rev = save_revision(rev) + parents = {} + parents[db_saved_rev] = rev.parents unless rev.parents.nil? + parents.each do |ch, chparents| + ch.parents = chparents.collect{|rp| find_changeset_by_name(rp)}.compact 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 diff --git a/test/unit/repository_git_test.rb b/test/unit/repository_git_test.rb index 03e38237a..81cbe2501 100644 --- a/test/unit/repository_git_test.rb +++ b/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"] - assert_equal "1ca7f5ed374f3cb31a93ae5215c2e25cc6ec5127", - extra_info_db["latin-1-path-encoding"]["last_scmid"] - assert_equal "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c", - extra_info_db["master"]["last_scmid"] + extra_info_heads = @repository.extra_info["heads"].dup + assert_equal 5, extra_info_heads.size + extra_info_heads.delete_if { |x| x == "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c" } + assert_equal 4, extra_info_heads.size del_revs = [ "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c", @@ -114,20 +113,19 @@ class RepositoryGitTest < ActiveSupport::TestCase end @project.reload cs1 = @repository.changesets - assert_equal 22, cs1.count - h = @repository.extra_info.dup - h["branches"]["master"]["last_scmid"] = - "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8" + assert_equal NUM_REV - 6, cs1.count + extra_info_heads << "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_equal "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8", - extra_info_db_1["master"]["last_scmid"] - + assert @repository.extra_info["heads"].index("4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8") @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 - assert_equal "83ca5fd546063a3c7dc2e568ba3355661a9e2b2c", - @repository.extra_info["branches"]["master"]["last_scmid"] + extra_info_heads = @repository.extra_info["heads"].dup + assert_equal 5, extra_info_heads.size + 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 - h["branches"]["master"]["last_scmid"] = "abcd1234efgh" + extra_info_heads << "abcd1234efgh" + h = {} + h["heads"] = extra_info_heads @repository.merge_extra_info(h) @repository.save @project.reload - assert_equal "abcd1234efgh", - @repository.extra_info["branches"]["master"]["last_scmid"] + h1 = @repository.extra_info["heads"].dup + 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"] = - "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8" + + extra_info_heads << "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_equal "4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8", - extra_info_db_1["master"]["last_scmid"] - + assert @repository.extra_info["heads"].index("4a07fe31bffcf2888791f3e6cbc9c4545cefe3e8") @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 -- 2.39.5