From 2570be84b2bd4801ebcef1d3aad89462195cc396 Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Sat, 10 May 2014 16:39:45 +1000 Subject: [PATCH 1/3] Fix smart-pull hanging when remote repository is up to date. Running `git fetch ` in git 1.8+ outputs nothing to STDOUT or STDERR if the remote is up-to-date. By changing the fetch command to use --verbose, we ensure something is always output when git fetch runs, even if the remote is up-to-date. This will prevent the IO.pipe from hanging when executing the git fetch command. --- lib/git-smart/git_repo.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/git-smart/git_repo.rb b/lib/git-smart/git_repo.rb index 7df60c8..2c2da97 100644 --- a/lib/git-smart/git_repo.rb +++ b/lib/git-smart/git_repo.rb @@ -60,7 +60,7 @@ def tracking_branch end def fetch!(remote) - git!('fetch', remote) + git!('fetch', remote, '--verbose') end def merge_base(ref_a, ref_b) From 876694c1d69f199e3f4140b51dfba5be74cc9ab1 Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Sat, 10 May 2014 18:19:54 +1000 Subject: [PATCH 2/3] Refactor to avoid use of CHILD_STATUS global variable ($?). --- lib/git-smart/git_repo.rb | 17 +++++++++-------- lib/git-smart/safe_shell.rb | 29 ++++++++++++++++------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/git-smart/git_repo.rb b/lib/git-smart/git_repo.rb index 2c2da97..f64fe8e 100644 --- a/lib/git-smart/git_repo.rb +++ b/lib/git-smart/git_repo.rb @@ -68,8 +68,8 @@ def merge_base(ref_a, ref_b) end def exists?(ref) - git('rev-parse', ref) - $?.success? + output, successful = exec_git('rev-parse', ref) + successful end def rev_list(ref_a, ref_b) @@ -137,15 +137,15 @@ def merge_no_ff!(target) # helper methods, left public in case other commands want to use them directly def git(*args) - output = exec_git(*args) - $?.success? ? output : '' + output, successful = exec_git(*args) + successful ? output : '' end def git!(*args) puts "Executing: #{['git', *args].join(" ")}" - output = exec_git(*args) + output, successful = exec_git(*args) to_display = output.split("\n").map { |l| " #{l}" }.join("\n") - $?.success? ? puts(to_display) : raise(GitSmart::UnexpectedOutput.new(to_display)) + successful ? puts(to_display) : raise(GitSmart::UnexpectedOutput.new(to_display)) output end @@ -164,9 +164,10 @@ def config(name) private def exec_git(*args) - return if @dir.empty? + return '', false if @dir.empty? Dir.chdir(@dir) { - SafeShell.execute('git', *args) + output, exit_status = SafeShell.execute('git', *args) + return output, exit_status.success? } end end diff --git a/lib/git-smart/safe_shell.rb b/lib/git-smart/safe_shell.rb index cd6d314..73adc47 100644 --- a/lib/git-smart/safe_shell.rb +++ b/lib/git-smart/safe_shell.rb @@ -1,21 +1,24 @@ +require 'open3' + module SafeShell def self.execute(command, *args) - read_end, write_end = IO.pipe - pid = fork do - read_end.close - STDOUT.reopen(write_end) - STDERR.reopen(write_end) - exec(command, *args) + cmd = [].push(command).push(args).join(' ') + result = "" + exit_status = nil + + Open3.popen2e (cmd) do |stdin, out, wait_thr| + while line = out.gets + result += line + end + + exit_status = wait_thr.value end - write_end.close - output = read_end.read - Process.waitpid(pid) - read_end.close - output + + return result, exit_status end def self.execute?(*args) - execute(*args) - $?.success? + output, exit_status = execute(*args) + exit_status.success? end end From 14a1e9536afd4298c8a11b5b0a7136a3a9cac902 Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Sat, 10 May 2014 18:50:50 +1000 Subject: [PATCH 3/3] Actually, just wait on the git process to return before reading. --- lib/git-smart/git_repo.rb | 2 +- lib/git-smart/safe_shell.rb | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/git-smart/git_repo.rb b/lib/git-smart/git_repo.rb index f64fe8e..ec2f2db 100644 --- a/lib/git-smart/git_repo.rb +++ b/lib/git-smart/git_repo.rb @@ -60,7 +60,7 @@ def tracking_branch end def fetch!(remote) - git!('fetch', remote, '--verbose') + git!('fetch', remote) end def merge_base(ref_a, ref_b) diff --git a/lib/git-smart/safe_shell.rb b/lib/git-smart/safe_shell.rb index 73adc47..df168a1 100644 --- a/lib/git-smart/safe_shell.rb +++ b/lib/git-smart/safe_shell.rb @@ -7,11 +7,8 @@ def self.execute(command, *args) exit_status = nil Open3.popen2e (cmd) do |stdin, out, wait_thr| - while line = out.gets - result += line - end - exit_status = wait_thr.value + result = out.read end return result, exit_status