refactor update command to use git diff instead of parsing pull o…

…utput

This fixes reporting of which formulae changed in git versions where `pull`
output is not compatible.

Signed-off-by: Adam Vandenberg <flangy@gmail.com>

Library/Homebrew/cmd/update.rb

@@ -16,69 +16,71 @@ class RefreshBrew
   INIT_COMMAND     = "git init"
   CHECKOUT_COMMAND = "git checkout -q master"
   UPDATE_COMMAND   = "git pull #{REPOSITORY_URL} master"
-  REVISION_COMMAND = "git log -l -1 --pretty=format:%H 2> /dev/null"
-  GIT_UP_TO_DATE   = "Already up-to-date."
+  REVISION_COMMAND = "git rev-parse HEAD"
+  DIFF_COMMAND     = "git diff-tree -r --name-status -z %s %s"
 
-  formula_regexp   = 'Library/Formula/(.+?)\.rb'
-  ADDED_FORMULA    = %r{^\s+create mode \d+ #{formula_regexp}$}
-  UPDATED_FORMULA  = %r{^\s+#{formula_regexp}\s}
-  DELETED_FORMULA  = %r{^\s+delete mode \d+ #{formula_regexp}$}
-
-  example_regexp   = 'Library/Contributions/examples/([^.\s]+).*'
-  ADDED_EXAMPLE    = %r{^\s+create mode \d+ #{example_regexp}$}
-  UPDATED_EXAMPLE  = %r{^\s+#{example_regexp}}
-  DELETED_EXAMPLE  = %r{^\s+delete mode \d+ #{example_regexp}$}
+  FORMULA_DIR = 'Library/Formula/'
+  EXAMPLE_DIR = 'Library/Contributions/examples/'
 
   attr_reader :added_formulae, :updated_formulae, :deleted_formulae, :installed_formulae
   attr_reader :added_examples, :updated_examples, :deleted_examples
-  attr_reader :initial_revision
+  attr_reader :initial_revision, :current_revision
 
   def initialize
     @added_formulae, @updated_formulae, @deleted_formulae, @installed_formulae = [], [], [], []
     @added_examples, @updated_examples, @deleted_examples = [], [], []
-    @initial_revision = self.current_revision
+    @initial_revision, @current_revision = nil
   end
 
   # Performs an update of the homebrew source. Returns +true+ if a newer
   # version was available, +false+ if already up-to-date.
   def update_from_masterbrew!
-    output = ''
     HOMEBREW_REPOSITORY.cd do
-      if File.directory? '.git'
+      if git_repo?
         safe_system CHECKOUT_COMMAND
+        @initial_revision = read_revision
       else
         safe_system INIT_COMMAND
       end
-      output = execute(UPDATE_COMMAND)
+      execute(UPDATE_COMMAND)
+      @current_revision = read_revision
     end
 
-    output.split("\n").reverse.each do |line|
-      case line
-      when ADDED_FORMULA
-        @added_formulae << $1
-      when DELETED_FORMULA
-        @deleted_formulae << $1
-      when UPDATED_FORMULA
-        @updated_formulae << $1 unless @added_formulae.include?($1) or @deleted_formulae.include?($1)
-      when ADDED_EXAMPLE
-        @added_examples << $1
-      when DELETED_EXAMPLE
-        @deleted_examples << $1
-      when UPDATED_EXAMPLE
-        @updated_examples << $1 unless @added_examples.include?($1) or @deleted_examples.include?($1)
+    if initial_revision && initial_revision != current_revision
+      # hash with status characters for keys:
+      # Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R)
+      @changes_map = Hash.new {|h,k| h[k] = [] }
+
+      changes = HOMEBREW_REPOSITORY.cd do
+        execute(DIFF_COMMAND % [initial_revision, current_revision]).split("\0")
+      end
+
+      while status = changes.shift
+        file = changes.shift
+        @changes_map[status] << file
+      end
+
+      if @changes_map.any?
+        @added_formulae   = changed_items('A', FORMULA_DIR)
+        @deleted_formulae = changed_items('D', FORMULA_DIR)
+        @updated_formulae = changed_items('M', FORMULA_DIR)
+        @added_examples   = changed_items('A', EXAMPLE_DIR)
+        @deleted_examples = changed_items('D', EXAMPLE_DIR)
+        @updated_examples = changed_items('M', EXAMPLE_DIR)
+
+        @installed_formulae = HOMEBREW_CELLAR.children.
+          select{ |pn| pn.directory? }.
+          map{ |pn| pn.basename.to_s }.sort
+
+        return true
       end
     end
-    @added_formulae.sort!
-    @updated_formulae.sort!
-    @deleted_formulae.sort!
-    @added_examples.sort!
-    @updated_examples.sort!
-    @deleted_examples.sort!
-    @installed_formulae = HOMEBREW_CELLAR.children.
-      select{ |pn| pn.directory? }.
-      map{ |pn| pn.basename.to_s }.sort
+    # assume nothing was updated
+    return false
+  end
 
-    output.strip != GIT_UP_TO_DATE
+  def git_repo?
+    File.directory? '.git'
   end
 
   def pending_formulae_changes?
@@ -105,12 +107,6 @@ def deleted_examples?
     !@deleted_examples.empty?
   end
 
-  def current_revision
-    HOMEBREW_REPOSITORY.cd { execute(REVISION_COMMAND).strip }
-  rescue
-    'TAIL'
-  end
-
   def report
     puts "Updated Homebrew from #{initial_revision[0,8]} to #{current_revision[0,8]}."
     ## New Formulae
@@ -151,10 +147,27 @@ def report
 
   private
 
+  def read_revision
+    execute(REVISION_COMMAND).chomp
+  end
+
+  def filter_by_directory(files, dir)
+    files.select { |f| f.index(dir) == 0 }
+  end
+
+  def basenames(files)
+    files.map { |f| File.basename(f, '.rb') }
+  end
+
+  # extracts items by status from @changes_map
+  def changed_items(status, dir)
+    basenames(filter_by_directory(@changes_map[status], dir)).sort
+  end
+
   def execute(cmd)
     out = `#{cmd}`
     if $? && !$?.success?
-      puts out
+      $stderr.puts out
       raise "Failed while executing #{cmd}"
     end
     ohai(cmd, out) if ARGV.verbose?

Library/Homebrew/test/fixtures/updater_fixture.yaml

@@ -1,53 +1,26 @@
-update_git_pull_output_without_formulae_changes: |
-  remote: counting objects: 58, done.
-  remote: Compressing objects: 100% (35/35), done.
-  remote: Total 39 (delta 20), reused 0 (delta 0)
-  Unpacking objects: 100% (39/39), done.
-  From git://github.com/mxcl/homebrew
-   * branch            master -> FETCH_HEAD
-  Updating 14ef7f9..f414bc8
-  Fast forward
-   Library/Homebrew/ARGV+yeast.rb                |   35 ++--
-   Library/Homebrew/beer_events.rb               |  181 +++++++++++++
-   Library/Homebrew/hardware.rb                  |   71 ++++++
-   Library/Homebrew/hw.model.c                   |   17 --
-   README                                        |  337 +++++++++++++------------
-   bin/brew                                      |  137 ++++++++---
-   40 files changed, 1107 insertions(+), 426 deletions(-)
-   create mode 100644 Library/Homebrew/beer_events.rb
-   create mode 100644 Library/Homebrew/hardware.rb
-   delete mode 100644 Library/Homebrew/hw.model.c
-   delete mode 100644 Library/Homebrew/hw.model.rb
-update_git_pull_output_with_formulae_changes: |
-  remote: counting objects: 58, done.
-  remote: Compressing objects: 100% (35/35), done.
-  remote: Total 39 (delta 20), reused 0 (delta 0)
-  Unpacking objects: 100% (39/39), done.
-  From git://github.com/mxcl/homebrew
-   * branch            master -> FETCH_HEAD
-  Updating 14ef7f9..f414bc8
-  Fast forward
-   Library/Contributions/brew_bash_completion.sh |    6 +-
-   Library/Formula/antiword.rb                   |   13 +
-   Library/Formula/bash-completion.rb            |   25 ++
-   Library/Formula/xar.rb                        |   19 ++
-   Library/Formula/yajl.rb                       |    2 +-
-   Library/Homebrew/ARGV+yeast.rb                |   35 ++--
-   Library/Homebrew/beer_events.rb               |  181 +++++++++++++
-   Library/Homebrew/hardware.rb                  |   71 ++++++
-   Library/Homebrew/hw.model.c                   |   17 --
-   Library/Homebrew/pathname+yeast.rb            |   28 ++-
-   Library/Homebrew/unittest.rb                  |  106 ++++++++-
-   Library/Homebrew/utils.rb                     |   36 ++-
-   README                                        |  337 +++++++++++++------------
-   bin/brew                                      |  137 ++++++++---
-   40 files changed, 1107 insertions(+), 426 deletions(-)
-   create mode 100644 Library/Formula/antiword.rb
-   create mode 100644 Library/Formula/bash-completion.rb
-   create mode 100644 Library/Formula/ddrescue.rb
-   create mode 100644 Library/Formula/dict.rb
-   create mode 100644 Library/Formula/lua.rb
-   create mode 100644 Library/Homebrew/beer_events.rb
-   create mode 100644 Library/Homebrew/hardware.rb
-   delete mode 100644 Library/Homebrew/hw.model.c
-   delete mode 100644 Library/Homebrew/hw.model.rb
+update_git_diff_output_without_formulae_changes: |
+  M       Library/Homebrew/ARGV+yeast.rb
+  A       Library/Homebrew/beer_events.rb
+  A       Library/Homebrew/hardware.rb
+  D       Library/Homebrew/hw.model.c
+  M       README
+  M       bin/brew
+update_git_diff_output_with_formulae_changes: |
+  M       Library/Contributions/brew_bash_completion.sh
+  A       Library/Formula/antiword.rb
+  A       Library/Formula/bash-completion.rb
+  M       Library/Formula/xar.rb
+  M       Library/Formula/yajl.rb
+  M       Library/Homebrew/ARGV+yeast.rb
+  M       Library/Homebrew/pathname+yeast.rb
+  M       Library/Homebrew/unittest.rb
+  M       Library/Homebrew/utils.rb
+  M       README
+  M       bin/brew
+  A       Library/Formula/ddrescue.rb
+  A       Library/Formula/dict.rb
+  A       Library/Formula/lua.rb
+  A       Library/Homebrew/beer_events.rb
+  A       Library/Homebrew/hardware.rb
+  D       Library/Homebrew/hw.model.c
+  D       Library/Homebrew/hw.model.rb

Library/Homebrew/test/test_updater.rb

@@ -11,15 +11,23 @@
 require 'cmd/update'
 
 class RefreshBrewMock < RefreshBrew
-  def in_prefix_expect(expect, returns = '')
-    @expect ||= {}
-    @expect[expect] = returns
+  def git_repo?
+    @git_repo
+  end
+  attr_writer :git_repo
+
+  def in_prefix_expect(cmd, output = '')
+    @outputs  ||= Hash.new { |h,k| h[k] = [] }
+    @expected ||= []
+    @expected << cmd
+    @outputs[cmd] << output
   end
 
   def `(cmd)
-    if Dir.pwd == HOMEBREW_PREFIX.to_s and @expect.has_key?(cmd)
-      (@called ||= []) << cmd
-      @expect[cmd]
+    if Dir.pwd == HOMEBREW_PREFIX.to_s and @expected.include?(cmd) and !@outputs[cmd].empty?
+      @called ||= []
+      @called << cmd
+      @outputs[cmd].shift
     else
       raise "#{inspect} Unexpectedly called backticks in pwd `#{HOMEBREW_PREFIX}' and command `#{cmd}'"
     end
@@ -28,7 +36,7 @@ def `(cmd)
   alias safe_system `
 
   def expectations_met?
-    @expect.keys.sort == @called.sort
+    @expected == @called
   end
 
   def inspect
@@ -54,11 +62,30 @@ def self.fixture_data
     @fixture_data
   end
 
+  def test_init_homebrew
+    outside_prefix do
+      updater = RefreshBrewMock.new
+      updater.git_repo = false
+      updater.in_prefix_expect("git init")
+      updater.in_prefix_expect("git pull #{RefreshBrewMock::REPOSITORY_URL} master")
+      updater.in_prefix_expect("git rev-parse HEAD", "1234abcd")
+
+      assert_equal false, updater.update_from_masterbrew!
+      assert updater.expectations_met?
+      assert updater.updated_formulae.empty?
+      assert updater.added_formulae.empty?
+    end
+  end
+
   def test_update_homebrew_without_any_changes
     outside_prefix do
       updater = RefreshBrewMock.new
-      updater.in_prefix_expect(RefreshBrew::INIT_COMMAND)
-      updater.in_prefix_expect(RefreshBrew::UPDATE_COMMAND, "Already up-to-date.\n")
+      updater.git_repo = true
+      updater.in_prefix_expect("git checkout -q master")
+      updater.in_prefix_expect("git rev-parse HEAD", "1234abcd")
+      updater.in_prefix_expect("git pull #{RefreshBrewMock::REPOSITORY_URL} master")
+      updater.in_prefix_expect("git rev-parse HEAD", "3456cdef")
+      updater.in_prefix_expect("git diff-tree -r --name-status -z 1234abcd 3456cdef", "")
 
       assert_equal false, updater.update_from_masterbrew!
       assert updater.expectations_met?
@@ -70,9 +97,14 @@ def test_update_homebrew_without_any_changes
   def test_update_homebrew_without_formulae_changes
     outside_prefix do
       updater = RefreshBrewMock.new
-      updater.in_prefix_expect(RefreshBrew::INIT_COMMAND)
-      output = fixture('update_git_pull_output_without_formulae_changes')
-      updater.in_prefix_expect(RefreshBrew::UPDATE_COMMAND, output)
+      updater.git_repo = true
+      diff_output = fixture('update_git_diff_output_without_formulae_changes')
+
+      updater.in_prefix_expect("git checkout -q master")
+      updater.in_prefix_expect("git rev-parse HEAD", "1234abcd")
+      updater.in_prefix_expect("git pull #{RefreshBrewMock::REPOSITORY_URL} master")
+      updater.in_prefix_expect("git rev-parse HEAD", "3456cdef")
+      updater.in_prefix_expect("git diff-tree -r --name-status -z 1234abcd 3456cdef", diff_output.gsub(/\s+/, "\0"))
 
       assert_equal true, updater.update_from_masterbrew!
       assert !updater.pending_formulae_changes?
@@ -84,22 +116,19 @@ def test_update_homebrew_without_formulae_changes
   def test_update_homebrew_with_formulae_changes
     outside_prefix do
       updater = RefreshBrewMock.new
-      updater.in_prefix_expect(RefreshBrew::INIT_COMMAND)
-      output = fixture('update_git_pull_output_with_formulae_changes')
-      updater.in_prefix_expect(RefreshBrew::UPDATE_COMMAND, output)
+      updater.git_repo = true
+      diff_output = fixture('update_git_diff_output_with_formulae_changes')
+
+      updater.in_prefix_expect("git checkout -q master")
+      updater.in_prefix_expect("git rev-parse HEAD", "1234abcd")
+      updater.in_prefix_expect("git pull #{RefreshBrewMock::REPOSITORY_URL} master")
+      updater.in_prefix_expect("git rev-parse HEAD", "3456cdef")
+      updater.in_prefix_expect("git diff-tree -r --name-status -z 1234abcd 3456cdef", diff_output.gsub(/\s+/, "\0"))
 
       assert_equal true, updater.update_from_masterbrew!
       assert updater.pending_formulae_changes?
       assert_equal %w{ xar yajl }, updater.updated_formulae
       assert_equal %w{ antiword bash-completion ddrescue dict lua }, updater.added_formulae
     end
   end
-
-  def test_updater_returns_current_revision
-    outside_prefix do
-      updater = RefreshBrewMock.new
-      updater.in_prefix_expect(RefreshBrew::REVISION_COMMAND, 'the-revision-hash')
-      assert_equal 'the-revision-hash', updater.current_revision
-    end
-  end
 end