From 0758a8edf2beef4f510f29280b7ea48979853014 Mon Sep 17 00:00:00 2001 From: Zev Blut Date: Thu, 23 Aug 2012 19:13:59 +0900 Subject: [PATCH 1/5] Make the update in a separate thread so that github stops dinging us for being too slow. --- commit_monitor.rb | 4 ++-- lib/git_repo_manager.rb | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/commit_monitor.rb b/commit_monitor.rb index 9470ab4..be6465f 100644 --- a/commit_monitor.rb +++ b/commit_monitor.rb @@ -11,13 +11,13 @@ configure do repos = File.join(`pwd`.chomp, 'data/repos') config = 'config/repos.yml' - repo_manager = GITRepoManager.new(config, repos) + repo_manager = BGGITRepoManager.new(config, repos) end configure :test do local_repos = File.join(`pwd`.chomp, 'spec/files/test_repos') test_config = 'spec/files/config/repos.yml' - repo_manager = GITRepoManager.new(test_config, local_repos) + repo_manager = GBGITRepoManager.new(test_config, local_repos) end # Repositories are single-threaded diff --git a/lib/git_repo_manager.rb b/lib/git_repo_manager.rb index c57d9b4..cff0699 100644 --- a/lib/git_repo_manager.rb +++ b/lib/git_repo_manager.rb @@ -30,6 +30,11 @@ def update_submodule(uri, branch, commit) $logger.info("Received commit #{commit[0..8]} from `#{uri}`, branch `#{branch}`") + if !(@ignore_sync_only_branch_list || @sync_branch[branch.to_sym]) + $logger.debug("Ignoring branch `#{branch}` as it is not something we want to sync") + return updated + end + @repos_using_submodules.each do |repo_name| $logger.debug("Checking `#{repo_name}` for submodules") @@ -190,3 +195,34 @@ def self.github_user_project_name(uri) end end + + +class BGGitRepoManager + + def initialize(config = 'config/repos.yml', clone_path = 'data/repos') + @repomanager = GITRepoManager.new(config, clone_path) + setup_worker_thread + end + + + def update_submodule(uri, branch, commit) + $logger.debug("Pushing request to bg queue") + @bgqueue.push([uri, branch, commit]) + [] + end + + private + + def setup_worker_thread + require 'thread' + @bgqueue = Queue.new + @bgthread = Thread.start do + while true + #pop will make the thread sleep until there is data + uri, branch, commit = *(@bgqueue.pop) + $logger.debug("BG thread popped data") + @repomanager.update_submodule(uri, branch, commit) + end + end + end +end From 9714571c6a5aad5470bf46612a877ce57a1badbe Mon Sep 17 00:00:00 2001 From: Zev Blut Date: Thu, 23 Aug 2012 19:14:57 +0900 Subject: [PATCH 2/5] Adding an idea for a forking background worker to get past the Thread queue sometimes blocking bug. --- commit_monitor.rb | 4 ++-- lib/git_repo_manager.rb | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/commit_monitor.rb b/commit_monitor.rb index be6465f..b9208c9 100644 --- a/commit_monitor.rb +++ b/commit_monitor.rb @@ -11,13 +11,13 @@ configure do repos = File.join(`pwd`.chomp, 'data/repos') config = 'config/repos.yml' - repo_manager = BGGITRepoManager.new(config, repos) + repo_manager = ForkGITRepoManager.new(config, repos) end configure :test do local_repos = File.join(`pwd`.chomp, 'spec/files/test_repos') test_config = 'spec/files/config/repos.yml' - repo_manager = GBGITRepoManager.new(test_config, local_repos) + repo_manager = ForkGITRepoManager.new(test_config, local_repos) end # Repositories are single-threaded diff --git a/lib/git_repo_manager.rb b/lib/git_repo_manager.rb index cff0699..ed47f11 100644 --- a/lib/git_repo_manager.rb +++ b/lib/git_repo_manager.rb @@ -206,7 +206,7 @@ def initialize(config = 'config/repos.yml', clone_path = 'data/repos') def update_submodule(uri, branch, commit) - $logger.debug("Pushing request to bg queue") + $logger.debug("Pushing request to bg queue uri #{uri} branch #{branch} commit #{commit}") @bgqueue.push([uri, branch, commit]) [] end @@ -220,9 +220,25 @@ def setup_worker_thread while true #pop will make the thread sleep until there is data uri, branch, commit = *(@bgqueue.pop) - $logger.debug("BG thread popped data") + $logger.debug("BG thread popped data uri #{uri} branch #{branch} commit #{commit}") @repomanager.update_submodule(uri, branch, commit) end end end end + +class ForkGitRepoManager + + def initialize(config = 'config/repos.yml', clone_path = 'data/repos') + @repomanager = GITRepoManager.new(config, clone_path) + end + + def update_submodule(uri, branch, commit) + $logger.debug("Pushing request to bg queue uri #{uri} branch #{branch} commit #{commit}") + Process.fork do + @repomanager.update_submodule(uri, branch, commit) + end + [] + end + +end From 34a69c670421a031c66d26c66b575a5d07fd34a9 Mon Sep 17 00:00:00 2001 From: Zev Blut Date: Thu, 23 Aug 2012 19:15:44 +0900 Subject: [PATCH 3/5] Use the actual author of the submodule's change as the commiter, so that CI and other messages will point to the real person. Have not been able to get the Spec runner to actually pass, but it does work in production. --- lib/git_repo_manager.rb | 21 ++++++++++++++++++++- spec/git_manager_spec.rb | 3 ++- spec/spec_helper.rb | 2 +- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/git_repo_manager.rb b/lib/git_repo_manager.rb index ed47f11..13f30d2 100644 --- a/lib/git_repo_manager.rb +++ b/lib/git_repo_manager.rb @@ -4,6 +4,15 @@ require 'git' require 'fileutils' + +# patch author to have a git commitable display +class Git::Author + def git_commit_string + "#{@name} <#{@email}>" + end +end + + class GITRepoManager attr_reader :config_file, :clone_path, :submodules, :config_hash attr_accessor :ignore_sync_only_branch_list @@ -67,9 +76,19 @@ def update_submodule(uri, branch, commit) sub_repo.fetch sub_repo.checkout(commit) + # Let's try to get the name of the actual commiter from the submodule in as the commiter + # to the pointer update + submodule_author = sub_repo.log.first.author + if submodule_author + $logger.info("Submodule Author : #{submodule_author.name} <#{submodule_author.email}>") + opts = {:author => submodule_author.git_commit_string } + else + opts = {} + end + repo.add(submodule_path) message = "Auto-updating submodule to commit #{self.class.github_user_project_name(submodule_uri)}@#{commit}." - repo.commit(message) + repo.commit(message, opts) begin repo.push('origin', branch) $logger.debug("Committed with message: #{message}") diff --git a/spec/git_manager_spec.rb b/spec/git_manager_spec.rb index acf526b..a9eb190 100644 --- a/spec/git_manager_spec.rb +++ b/spec/git_manager_spec.rb @@ -5,7 +5,7 @@ describe GITRepoManager do - LOCAL_REPOS = File.join(`pwd`.chomp, 'spec/files/test_repos') + LOCAL_REPOS = File.join(Dir.pwd, 'spec/files/test_repos') TEST_CONFIG = 'spec/files/config/repos.yml' # TODO move @@ -65,6 +65,7 @@ local_repo_clone = local_using_submodule_checkout local_repo_clone.pull local_using_submodule_checkout.log.size.should == 2 # First and the submodule commit. + local_using_submodule_checkout.log.first.author.git_commit_string.should == "Third party " end it 'should pull from projects already cloned so that it has the lastest commits locally' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 501b174..f5f562c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -58,7 +58,7 @@ def change_using_submodule_via_third_party_checkout(branch = 'master', line = 'L @third_party_using_submodule_clone.branch(branch).checkout `echo "#{line}" >> #{@third_party_using_submodule_path}/README` @third_party_using_submodule_clone.add('.') - @third_party_using_submodule_clone.commit('New line in using submodule readme.') + @third_party_using_submodule_clone.commit('New line in using submodule readme.', :author => 'Third party ') @third_party_using_submodule_clone.push('origin', branch) @third_party_using_submodule_clone.log.first end From f6061578d821febbbf513714aa7b1e6074461ee5 Mon Sep 17 00:00:00 2001 From: "Birkir A. Barkarson" Date: Fri, 24 Aug 2012 13:19:31 +0900 Subject: [PATCH 4/5] Refactor a bit and fix test for commit author name. --- lib/git_repo_manager.rb | 73 ++++++++++++++++++---------------------- spec/git_manager_spec.rb | 3 +- spec/spec_helper.rb | 4 +-- 3 files changed, 37 insertions(+), 43 deletions(-) diff --git a/lib/git_repo_manager.rb b/lib/git_repo_manager.rb index 13f30d2..daedcd1 100644 --- a/lib/git_repo_manager.rb +++ b/lib/git_repo_manager.rb @@ -4,15 +4,6 @@ require 'git' require 'fileutils' - -# patch author to have a git commitable display -class Git::Author - def git_commit_string - "#{@name} <#{@email}>" - end -end - - class GITRepoManager attr_reader :config_file, :clone_path, :submodules, :config_hash attr_accessor :ignore_sync_only_branch_list @@ -39,34 +30,30 @@ def update_submodule(uri, branch, commit) $logger.info("Received commit #{commit[0..8]} from `#{uri}`, branch `#{branch}`") - if !(@ignore_sync_only_branch_list || @sync_branch[branch.to_sym]) - $logger.debug("Ignoring branch `#{branch}` as it is not something we want to sync") - return updated - end + if sync_branch?(branch) + @repos_using_submodules.each do |repo_name| + $logger.debug("Checking `#{repo_name}` for submodules") - @repos_using_submodules.each do |repo_name| - $logger.debug("Checking `#{repo_name}` for submodules") + repo = @repos[repo_name] - repo = @repos[repo_name] + repo.submodules.each do |submodule| + submodule_path = submodule.path + submodule_uri = submodule.uri + $logger.debug("Repo `#{repo_name}` has submodule `#{submodule_path}`, #{submodule_uri}") - repo.submodules.each do |submodule| - submodule_path = submodule.path - submodule_uri = submodule.uri - $logger.debug("Repo `#{repo_name}` has submodule `#{submodule_path}`, #{submodule_uri}") + normalized_local = self.class.normalize_git_uri(submodule_uri) + normalized_receiving = self.class.normalize_git_uri(uri) + $logger.debug("Comparing: #{normalized_local} == #{normalized_receiving}") - normalized_local = GITRepoManager.normalize_git_uri(submodule_uri) - normalized_receiving = GITRepoManager.normalize_git_uri(uri) + if normalized_local == normalized_receiving + $logger.debug("Found submodule `#{uri}` as `#{submodule_path}` in `#{repo_name}`") - $logger.debug("Comparing: #{normalized_local} == #{normalized_receiving}") - if normalized_local == normalized_receiving - $logger.debug("Found submodule `#{uri}` as `#{submodule_path}` in `#{repo_name}`") - # repo has a submodule corresponding to uri + # repo has a submodule corresponding to uri - submodule.init unless submodule.initialized? - submodule.update unless submodule.updated? + submodule.init unless submodule.initialized? + submodule.update unless submodule.updated? - if repo.is_branch?(branch) - if @ignore_sync_only_branch_list || @sync_branch[branch.to_sym] + if repo.is_branch?(branch) $logger.info("Updating `#{repo_name}` with submodule branch `#{branch}`.") repo.branch(branch).checkout repo.reset_hard("origin/#{branch}") @@ -78,10 +65,9 @@ def update_submodule(uri, branch, commit) # Let's try to get the name of the actual commiter from the submodule in as the commiter # to the pointer update - submodule_author = sub_repo.log.first.author - if submodule_author - $logger.info("Submodule Author : #{submodule_author.name} <#{submodule_author.email}>") - opts = {:author => submodule_author.git_commit_string } + if author = self.class.git_commit_author_name(sub_repo.log.first) + $logger.debug("Submodule author: #{author}") + opts = {:author => author} else opts = {} end @@ -97,14 +83,15 @@ def update_submodule(uri, branch, commit) end updated << message else - $logger.debug("Branch `#{branch}` is not on the sync list. Ignoring commit.") + $logger.info("Repository `#{repo_name}` does not have a branch called `#{branch}`") end - else - $logger.info("Repository `#{repo_name}` does not have a branch called `#{branch}`") end end end + else + $logger.debug("Branch `#{branch}` is not on the sync list. Ignoring commit.") end + updated end @@ -127,7 +114,6 @@ def clone_or_pull_repositories end end - def process_config_hash @submodules = [] @repos_using_submodules = [] @@ -162,6 +148,10 @@ def repo_path(name) File.join(@clone_path, name.to_s) end + def sync_branch?(name) + @ignore_sync_only_branch_list || @sync_branch[name.to_sym] + end + def self.symbolize_keys(hash) new_hash = Hash.new hash.each do |key, value| @@ -213,8 +203,12 @@ def self.github_user_project_name(uri) end end -end + def self.git_commit_author_name(commit) + author = commit.author + "#{author.name} <#{author.email}>" + end +end class BGGitRepoManager @@ -223,7 +217,6 @@ def initialize(config = 'config/repos.yml', clone_path = 'data/repos') setup_worker_thread end - def update_submodule(uri, branch, commit) $logger.debug("Pushing request to bg queue uri #{uri} branch #{branch} commit #{commit}") @bgqueue.push([uri, branch, commit]) diff --git a/spec/git_manager_spec.rb b/spec/git_manager_spec.rb index a9eb190..aafaca6 100644 --- a/spec/git_manager_spec.rb +++ b/spec/git_manager_spec.rb @@ -65,7 +65,6 @@ local_repo_clone = local_using_submodule_checkout local_repo_clone.pull local_using_submodule_checkout.log.size.should == 2 # First and the submodule commit. - local_using_submodule_checkout.log.first.author.git_commit_string.should == "Third party " end it 'should pull from projects already cloned so that it has the lastest commits locally' do @@ -81,8 +80,10 @@ it 'should auto commit an update to repositories using submodules' do commit = change_submodule_via_third_party_checkout + local_using_submodule_checkout.log.first.message.should_not == "Auto-updating submodule to commit spec_testing/submodule@#{commit.sha}." manager.update_submodule(TEST_REPO_SUBMODULE, 'master', commit.sha) local_using_submodule_checkout.log.first.message.should == "Auto-updating submodule to commit spec_testing/submodule@#{commit.sha}." + GITRepoManager.git_commit_author_name(local_using_submodule_checkout.log.first).should == "Third party " end it 'should push auto commits to the remote repository' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f5f562c..cd216c6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -49,7 +49,7 @@ def change_submodule_via_third_party_checkout(branch = 'master', line = 'Updatin @third_party_submodule_clone.branch(branch).checkout `echo "#{line}" >> #{@third_party_submodule_path}/README` @third_party_submodule_clone.add('.') - @third_party_submodule_clone.commit('New line in readme') + @third_party_submodule_clone.commit('New line in readme', :author => 'Third party ') @third_party_submodule_clone.push('origin', branch) @third_party_submodule_clone.log.first end @@ -58,7 +58,7 @@ def change_using_submodule_via_third_party_checkout(branch = 'master', line = 'L @third_party_using_submodule_clone.branch(branch).checkout `echo "#{line}" >> #{@third_party_using_submodule_path}/README` @third_party_using_submodule_clone.add('.') - @third_party_using_submodule_clone.commit('New line in using submodule readme.', :author => 'Third party ') + @third_party_using_submodule_clone.commit('New line in using submodule readme.') @third_party_using_submodule_clone.push('origin', branch) @third_party_using_submodule_clone.log.first end From 8ec8aa483ff6ab59472ff8abc13335e94b827480 Mon Sep 17 00:00:00 2001 From: "Birkir A. Barkarson" Date: Fri, 24 Aug 2012 15:17:46 +0900 Subject: [PATCH 5/5] Clean up paths. --- Rakefile | 8 +++++++- lib/git_repo_manager.rb | 3 +-- spec/git_manager_spec.rb | 4 +--- spec/spec_helper.rb | 3 +++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Rakefile b/Rakefile index c2e4f58..e233999 100644 --- a/Rakefile +++ b/Rakefile @@ -17,7 +17,12 @@ begin s.add_dependency(%q, [">= 2"]) end rescue LoadError - puts "Jeweler, or one of its dependencies, is not available. Install it with: sudo gem install technicalpickles-jeweler -s http://gems.github.com" + puts "Jeweler, or one of its dependencies, is not available." +end + +desc 'Start the commit monitor' +task 'start' do |t, args| + ruby "-Ilib commit_monitor.rb" end begin @@ -25,6 +30,7 @@ begin RSpec::Core::RakeTask.new('spec') do |t| t.rspec_opts = ["-fd", "-c"] + t.ruby_opts = ["-Ispec,lib"] t.pattern = 'spec/**/*_spec.rb' end diff --git a/lib/git_repo_manager.rb b/lib/git_repo_manager.rb index daedcd1..0471fcf 100644 --- a/lib/git_repo_manager.rb +++ b/lib/git_repo_manager.rb @@ -1,8 +1,7 @@ -require 'rubygems' require 'yaml' -require 'lib/global_logger' require 'git' require 'fileutils' +require 'global_logger' class GITRepoManager attr_reader :config_file, :clone_path, :submodules, :config_hash diff --git a/spec/git_manager_spec.rb b/spec/git_manager_spec.rb index aafaca6..d96c2c3 100644 --- a/spec/git_manager_spec.rb +++ b/spec/git_manager_spec.rb @@ -1,6 +1,4 @@ -require 'lib/git_repo_manager' -require 'lib/global_logger' -require 'spec/spec_helper.rb' +require 'spec_helper.rb' require 'git' describe GITRepoManager do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cd216c6..68a1675 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,6 @@ +require 'git_repo_manager' +require 'global_logger' + TEMP_DIRECTORY = '/tmp/spec_testing' def initalize_repos