Encode formula revision in installation prefix

In order to allow kegs built with the same version but differing formula
revisions to coexist, we must encode the revision as part of the keg's
name. This is necessary to actually perform an upgrade, as we cannot
upgrade a keg in-place, and temporarily moving it pending the result of
the upgrade is error-prone and potentially slow.

To accomplish this, we introduce a new Formula#pkg_version method that
concatenates the active_spec version with the formula revision. An
exception is made for a formula that has no revision: the tag is
omitted. This preserves compatibility with existing installations.

5 files changed, 107 insertions, 4 deletions

diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index 6ad5eae3a..07fcc5a67 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -10,6 +10,7 @@ require 'build_options'
 require 'formulary'
 require 'software_spec'
 require 'install_renamed'
+require 'pkg_version'
 
 class Formula
   include FileUtils
@@ -18,7 +19,7 @@ class Formula
 
   attr_reader :name, :path, :homepage, :build
   attr_reader :stable, :bottle, :devel, :head, :active_spec
-  attr_reader :revision
+  attr_reader :pkg_version, :revision
 
   # The current working directory during builds and tests.
   # Will only be non-nil inside #stage and #test.
@@ -55,6 +56,7 @@ class Formula
     @active_spec = determine_active_spec
     validate_attributes :url, :name, :version
     @build = determine_build_options
+    @pkg_version = PkgVersion.new(version, revision)
 
     @pin = FormulaPin.new(self)
 
@@ -152,7 +154,7 @@ class Formula
     Keg.new(installed_prefix).version
   end
 
-  def prefix(v=version)
+  def prefix(v=pkg_version)
     Pathname.new("#{HOMEBREW_CELLAR}/#{name}/#{v}")
   end
   def rack; prefix.parent end
diff --git a/Library/Homebrew/keg.rb b/Library/Homebrew/keg.rb
index 42e960ad9..f46c23cfc 100644
--- a/Library/Homebrew/keg.rb
+++ b/Library/Homebrew/keg.rb
@@ -101,8 +101,8 @@ class Keg < Pathname
   end
 
   def version
-    require 'version'
-    Version.new(basename.to_s)
+    require 'pkg_version'
+    PkgVersion.parse(basename.to_s)
   end
 
   def basename
diff --git a/Library/Homebrew/pkg_version.rb b/Library/Homebrew/pkg_version.rb
new file mode 100644
index 000000000..3d81df261
--- /dev/null
+++ b/Library/Homebrew/pkg_version.rb
@@ -0,0 +1,35 @@
+require 'version'
+
+class PkgVersion < Version
+  attr_reader :version, :revision
+
+  RX = /\A(.+?)(?:_(\d+))?\z/
+
+  def self.parse(path)
+    _, version, revision = *path.match(RX)
+    new(version, revision)
+  end
+
+  def initialize(version, revision)
+    super(version)
+
+    if head?
+      @revision = 0
+    else
+      @revision = revision.to_i
+    end
+  end
+
+  def to_s
+    if revision > 0
+      "#{version}_#{revision}"
+    else
+      version
+    end
+  end
+  alias_method :to_str, :to_s
+
+  def <=>(other)
+    super.nonzero? || revision <=> other.revision
+  end
+end
diff --git a/Library/Homebrew/test/test_formula.rb b/Library/Homebrew/test/test_formula.rb
index dd4f9a3e3..9c168e1e6 100644
--- a/Library/Homebrew/test/test_formula.rb
+++ b/Library/Homebrew/test/test_formula.rb
@@ -24,6 +24,11 @@ class FormulaTests < Test::Unit::TestCase
     assert_kind_of Pathname, f.prefix
   end
 
+  def test_revised_prefix
+    f = Class.new(TestBall) { revision 1 }.new
+    assert_equal HOMEBREW_CELLAR/f.name/'0.1_1', f.prefix
+  end
+
   def test_installed?
     f = TestBall.new
     f.stubs(:installed_prefix).returns(stub(:directory? => false))
@@ -233,4 +238,29 @@ class FormulaTests < Test::Unit::TestCase
       assert_equal 'foo', f.class.send(spec).deps.first.name
     end
   end
+
+  def test_simple_version
+    assert_equal PkgVersion.parse('1.0'), formula { url 'foo-1.0.bar' }.pkg_version
+  end
+
+  def test_version_with_revision
+    f = formula do
+      url 'foo-1.0.bar'
+      revision 1
+    end
+
+    assert_equal PkgVersion.parse('1.0_1'), f.pkg_version
+  end
+
+  def test_head_ignores_revisions
+    ARGV.stubs(:build_head?).returns(true)
+
+    f = formula do
+      url 'foo-1.0.bar'
+      revision 1
+      head 'foo'
+    end
+
+    assert_equal PkgVersion.parse('HEAD'), f.pkg_version
+  end
 end
diff --git a/Library/Homebrew/test/test_pkg_version.rb b/Library/Homebrew/test/test_pkg_version.rb
new file mode 100644
index 000000000..d78007dcf
--- /dev/null
+++ b/Library/Homebrew/test/test_pkg_version.rb
@@ -0,0 +1,36 @@
+require 'testing_env'
+require 'pkg_version'
+
+class PkgVersionTests < Test::Unit::TestCase
+  def v(version)
+    PkgVersion.parse(version)
+  end
+
+  def test_parse
+    assert_equal PkgVersion.new("1.0", 1), PkgVersion.parse("1.0_1")
+    assert_equal PkgVersion.new("1.0", 1), PkgVersion.parse("1.0_1")
+    assert_equal PkgVersion.new("1.0", 0), PkgVersion.parse("1.0")
+    assert_equal PkgVersion.new("1.0", 0), PkgVersion.parse("1.0_0")
+    assert_equal PkgVersion.new("2.1.4", 0), PkgVersion.parse("2.1.4_0")
+    assert_equal PkgVersion.new("2.1.4_1", 0), PkgVersion.parse("2.1.4_1_0")
+    assert_equal PkgVersion.new("1.0.1e", 1), PkgVersion.parse("1.0.1e_1")
+  end
+
+  def test_comparison
+    assert_operator v("1.0_0"), :==, v("1.0")
+    assert_operator v("1.0_1"), :==, v("1.0_1")
+    assert_operator v("1.1"), :>, v("1.0_1")
+    assert_operator v("1.0_0"), :==, v("1.0")
+    assert_operator v("1.0_1"), :<, v("2.0_1")
+    assert_operator v("HEAD"), :>, v("1.0")
+    assert_operator v("1.0"), :<, v("HEAD")
+  end
+
+  def test_to_s
+    assert_equal "1.0", PkgVersion.new("1.0", 0).to_s
+    assert_equal "1.0_1", PkgVersion.new("1.0", 1).to_s
+    assert_equal "1.0", PkgVersion.new("1.0", 0).to_s
+    assert_equal "1.0", PkgVersion.new("1.0", 0).to_s
+    assert_equal "HEAD", PkgVersion.new("HEAD", 1).to_s
+  end
+end