diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb
index d57f5ddc3b4a8b42833738bbadcb5d71bc700407..b5299c0384aa2667873ad88de9d77673a86dd540 100644
--- a/Library/Homebrew/exceptions.rb
+++ b/Library/Homebrew/exceptions.rb
@@ -22,6 +22,17 @@ class NoSuchKegError < RuntimeError
end
end
+class FormulaValidationError < StandardError
+ attr_reader :attr
+
+ def initialize(attr, value)
+ @attr = attr
+ msg = "invalid attribute: #{attr}"
+ msg << " (#{value.inspect})" unless value.empty?
+ super msg
+ end
+end
+
class FormulaUnavailableError < RuntimeError
attr_reader :name
attr_accessor :dependent
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index a6305eaaff32285cc60ebcde83f1ecc74f88e311..eddacf08ddea999a8b467c25297ce5a78af5daa4 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -24,44 +24,31 @@ class Formula
# Homebrew determines the name
def initialize name='__UNKNOWN__', path=nil
- set_instance_variable :homepage
- set_instance_variable :stable
- set_instance_variable :bottle
- set_instance_variable :devel
- set_instance_variable :head
-
@name = name
- validate_variable :name
-
- # If a checksum or version was set in the DSL, but no stable URL
- # was defined, make @stable nil and save callers some trouble
- @stable = nil if @stable and @stable.url.nil?
-
- # Ensure the bottle URL is set. If it does not have a checksum,
- # then a bottle is not available for the current platform.
- if @bottle and not (@bottle.checksum.nil? or @bottle.checksum.empty?)
- @bottle.url ||= bottle_url(self)
- if @bottle.cat_without_underscores
- @bottle.url.gsub!(MacOS.cat.to_s, MacOS.cat_without_underscores.to_s)
+ # If we got an explicit path, use that, else determine from the name
+ @path = path.nil? ? self.class.path(name) : Pathname.new(path)
+ @homepage = self.class.homepage
+
+ set_spec :stable
+ set_spec :devel
+ set_spec :head
+ set_spec :bottle do |bottle|
+ # Ensure the bottle URL is set. If it does not have a checksum,
+ # then a bottle is not available for the current platform.
+ # TODO: push this down into Bottle; we can pass the formula instance
+ # into a validation method on the bottle instance.
+ unless bottle.checksum.nil? || bottle.checksum.empty?
+ @bottle = bottle
+ bottle.url ||= bottle_url(self)
+ if bottle.cat_without_underscores
+ bottle.url.gsub!(MacOS.cat.to_s,
+ MacOS.cat_without_underscores.to_s)
+ end
end
- else
- @bottle = nil
end
- @active_spec = if @head and ARGV.build_head? then @head # --HEAD
- elsif @devel and ARGV.build_devel? then @devel # --devel
- elsif @bottle and install_bottle?(self) then @bottle # bottle available
- elsif @stable.nil? and @head then @head # head-only
- else @stable # default
- end
-
- @version = @active_spec.version
- validate_variable :version if @version
-
- raise "No url provided for formula #{name}" if @active_spec.url.nil?
-
- # If we got an explicit path, use that, else determine from the name
- @path = path.nil? ? self.class.path(name) : Pathname.new(path)
+ @active_spec = determine_active_spec
+ validate_attributes :url, :name, :version
@downloader = download_strategy.new(name, @active_spec)
# Combine DSL `option` and `def options`
@@ -73,10 +60,36 @@ class Formula
@pin = FormulaPin.new(self)
end
- def url; @active_spec.url; end
- def version; @active_spec.version; end
- def specs; @active_spec.specs; end
- def mirrors; @active_spec.mirrors; end
+ def set_spec(name)
+ spec = self.class.send(name)
+ return if spec.nil?
+ if block_given? && yield(spec) || !spec.url.nil?
+ instance_variable_set("@#{name}", spec)
+ end
+ end
+
+ def determine_active_spec
+ case
+ when @head && ARGV.build_head? then @head # --HEAD
+ when @devel && ARGV.build_devel? then @devel # --devel
+ when @bottle && install_bottle?(self) then @bottle # bottle available
+ when @stable.nil? && @head then @head # head-only
+ else @stable
+ end
+ end
+
+ def validate_attributes(*attrs)
+ attrs.each do |attr|
+ if (value = send(attr).to_s).empty? || value =~ /\s/
+ raise FormulaValidationError.new(attr, value)
+ end
+ end
+ end
+
+ def url; active_spec.url; end
+ def version; active_spec.version; end
+ def specs; active_spec.specs; end
+ def mirrors; active_spec.mirrors; end
# if the dir is there, but it's empty we consider it not installed
def installed?
@@ -100,21 +113,21 @@ class Formula
end
def linked_keg
- HOMEBREW_REPOSITORY/'Library/LinkedKegs'/@name
+ HOMEBREW_REPOSITORY/'Library/LinkedKegs'/name
end
def installed_prefix
- devel_prefix = unless @devel.nil?
- HOMEBREW_CELLAR/@name/@devel.version
+ devel_prefix = unless devel.nil?
+ HOMEBREW_CELLAR/name/devel.version
end
- head_prefix = unless @head.nil?
- HOMEBREW_CELLAR/@name/@head.version
+ head_prefix = unless head.nil?
+ HOMEBREW_CELLAR/name/head.version
end
- if @active_spec == @head || @head and head_prefix.directory?
+ if active_spec == head || head and head_prefix.directory?
head_prefix
- elsif @active_spec == @devel || @devel and devel_prefix.directory?
+ elsif active_spec == devel || devel and devel_prefix.directory?
devel_prefix
else
prefix
@@ -127,9 +140,7 @@ class Formula
end
def prefix
- validate_variable :name
- validate_variable :version
- HOMEBREW_CELLAR/@name/@version
+ HOMEBREW_CELLAR/name/version
end
def rack; prefix.parent end
@@ -174,11 +185,11 @@ class Formula
def opt_prefix; HOMEBREW_PREFIX/:opt/name end
def download_strategy
- @active_spec.download_strategy
+ active_spec.download_strategy
end
def cached_download
- @downloader.cached_location
+ downloader.cached_location
end
# Can be overridden to selectively disable bottles from formulae.
@@ -237,8 +248,7 @@ class Formula
# yields self with current working directory set to the uncompressed tarball
def brew
- validate_variable :name
- validate_variable :version
+ validate_attributes :name, :version
stage do
begin
@@ -599,12 +609,12 @@ class Formula
def fetch
# Ensure the cache exists
HOMEBREW_CACHE.mkpath
- return @downloader.fetch, @downloader
+ return downloader.fetch, downloader
end
# For FormulaInstaller.
def verify_download_integrity fn
- @active_spec.verify_download_integrity(fn)
+ active_spec.verify_download_integrity(fn)
end
def test
@@ -655,17 +665,6 @@ class Formula
end
end
- def validate_variable name
- v = instance_variable_get("@#{name}")
- raise "Invalid @#{name}" if v.to_s.empty? or v.to_s =~ /\s/
- end
-
- def set_instance_variable(type)
- return if instance_variable_defined? "@#{type}"
- class_value = self.class.send(type)
- instance_variable_set("@#{type}", class_value) if class_value
- end
-
def self.method_added method
case method
when :brew
diff --git a/Library/Homebrew/formula_specialties.rb b/Library/Homebrew/formula_specialties.rb
index 4e63440a6f6e26c79b0ed780d4cc65872a6019a5..daf9dd6d013d45afea7ac5e390884c679110f412 100644
--- a/Library/Homebrew/formula_specialties.rb
+++ b/Library/Homebrew/formula_specialties.rb
@@ -10,10 +10,9 @@ end
# See flac.rb for an example
class GithubGistFormula < ScriptFileFormula
def initialize name='__UNKNOWN__', path=nil
- super name, path
- @stable.version(File.basename(File.dirname(url))[0,6])
- @version = @active_spec.version
- validate_variable :version
+ url = self.class.stable.url
+ self.class.stable.version(File.basename(File.dirname(url))[0,6])
+ super
end
end
diff --git a/Library/Homebrew/test/test_formula.rb b/Library/Homebrew/test/test_formula.rb
index 30ef41589d9beed1703567e4a1ef4e7b41379413..434dd3ac12d4ab6a1d627eca7d83c662fa3afb04 100644
--- a/Library/Homebrew/test/test_formula.rb
+++ b/Library/Homebrew/test/test_formula.rb
@@ -1,10 +1,6 @@
require 'testing_env'
require 'test/testball'
-class MostlyAbstractFormula < Formula
- url ''
-end
-
class FormulaTests < Test::Unit::TestCase
include VersionAssertions
@@ -58,19 +54,6 @@ class FormulaTests < Test::Unit::TestCase
assert_equal 'FooBar', Formula.class_s('foo_bar')
end
- def test_cant_override_brew
- assert_raises(RuntimeError) do
- Class.new(Formula) { def brew; end }
- end
- end
-
- def test_abstract_formula
- f=MostlyAbstractFormula.new
- assert_equal '__UNKNOWN__', f.name
- assert_raises(RuntimeError) { f.prefix }
- shutup { assert_raises(RuntimeError) { f.brew } }
- end
-
def test_mirror_support
f = Class.new(Formula) do
url "file:///#{TEST_FOLDER}/bad_url/testball-0.1.tbz"
@@ -286,7 +269,7 @@ class FormulaTests < Test::Unit::TestCase
f << %{
require 'formula'
class #{Formula.class_s(foobar)} < Formula
- url ''
+ url 'foo-1.0'
def initialize(*args)
@homepage = 'http://example.com/'
super
diff --git a/Library/Homebrew/test/test_formula_validation.rb b/Library/Homebrew/test/test_formula_validation.rb
new file mode 100644
index 0000000000000000000000000000000000000000..be4c38c641289975930de67e0f30a98b26440b02
--- /dev/null
+++ b/Library/Homebrew/test/test_formula_validation.rb
@@ -0,0 +1,69 @@
+require 'testing_env'
+require 'formula'
+
+class FormulaValidationTests < Test::Unit::TestCase
+ def formula(*args, &block)
+ Class.new(Formula, &block).new(*args)
+ end
+
+ def assert_invalid(attr, &block)
+ e = assert_raises(FormulaValidationError, &block)
+ assert_equal attr, e.attr
+ end
+
+ def test_cant_override_brew
+ assert_raises(RuntimeError) do
+ Class.new(Formula) { def brew; end }
+ end
+ end
+
+ def test_validates_name
+ assert_invalid :name do
+ formula "name with spaces" do
+ url "foo"
+ version "1.0"
+ end
+ end
+ end
+
+ def test_validates_url
+ assert_invalid :url do
+ formula do
+ url ""
+ version "1"
+ end
+ end
+ end
+
+ def test_validates_version
+ assert_invalid :version do
+ formula do
+ url "foo"
+ version "version with spaces"
+ end
+ end
+
+ assert_invalid :version do
+ formula do
+ url "foo"
+ version ""
+ end
+ end
+ end
+
+ def test_validates_when_initialize_overridden
+ assert_invalid :name do
+ formula do
+ def initialize; end
+ end.brew {}
+ end
+ end
+
+ def test_head_only_valid
+ assert_nothing_raised do
+ formula do
+ head "foo"
+ end
+ end
+ end
+end
diff --git a/Library/Homebrew/test/test_versions.rb b/Library/Homebrew/test/test_versions.rb
index d503f4acec23eaa3035a3cd66ff3b7cd17ff7e07..bf943637ec90f0b9c5a8667c24fe5c559b9324c0 100644
--- a/Library/Homebrew/test/test_versions.rb
+++ b/Library/Homebrew/test/test_versions.rb
@@ -50,14 +50,6 @@ class VersionParsingTests < Test::Unit::TestCase
assert_version_nil 'foo'
end
- def test_bad_version
- assert_raises(RuntimeError) do
- Class.new(TestBall) do
- version "versions can't have spaces"
- end.new
- end
- end
-
def test_version_all_dots
assert_version_detected '1.14', 'http://example.com/foo.bar.la.1.14.zip'
end