Eagerly initialize formula specs

Declarations of dependencies, options, and resources in the DSL only
apply to specs that have already been initialized. For example, given
this snippet:

  url ...
  sha1 ...

  depends_on 'foo'

  devel do
    url ...
    sha1 ...
  end

The dependency 'foo' will be recorded for the stable spec, but not the
devel spec, since it was not initialized prior to the call to
depends_on.

While it is considered best practice to declare all specs (stable,
devel, head, and bottle) prior to other declarations, there is nothing
that enforces this ordering, so when it happens it can be confusing and
hard to debug.

To prevent this, we can initialize all specs up front. This comes with
a performance penalty for commands that load all formulae into memory,
but that is probably outweighed by what we gain in correctness.

Fixes Homebrew/homebrew#23425.
This commit is contained in:
Jack Nagel 2013-10-22 13:07:08 -05:00
parent 98b28f5ac3
commit 63e1c71c50
2 changed files with 36 additions and 23 deletions

View File

@ -56,7 +56,6 @@ class Formula
def set_spec(name)
spec = self.class.send(name)
return if spec.nil?
if block_given? && yield(spec) || !spec.url.nil?
spec.owner = self
instance_variable_set("@#{name}", spec)
@ -656,69 +655,57 @@ class Formula
attr_rw :plist_startup, :plist_manual
def specs
@specs ||= []
end
def create_spec(klass)
spec = klass.new
specs << spec
spec
@specs ||= [stable, devel, head, bottle].freeze
end
def url val, specs={}
@stable ||= create_spec(SoftwareSpec)
@stable.url(val, specs)
stable.url(val, specs)
end
def version val=nil
@stable ||= create_spec(SoftwareSpec)
@stable.version(val)
stable.version(val)
end
def mirror val
@stable ||= create_spec(SoftwareSpec)
@stable.mirror(val)
stable.mirror(val)
end
Checksum::TYPES.each do |cksum|
class_eval <<-EOS, __FILE__, __LINE__ + 1
def #{cksum}(val)
@stable ||= create_spec(SoftwareSpec)
@stable.#{cksum}(val)
stable.#{cksum}(val)
end
EOS
end
def build
@stable ||= create_spec(SoftwareSpec)
@stable.build
stable.build
end
def stable &block
@stable ||= SoftwareSpec.new
return @stable unless block_given?
@stable ||= create_spec(SoftwareSpec)
@stable.instance_eval(&block)
end
def bottle *, &block
@bottle ||= Bottle.new
return @bottle unless block_given?
@bottle ||= create_spec(Bottle)
@bottle.instance_eval(&block)
@bottle.version = @stable.version
end
def devel &block
@devel ||= SoftwareSpec.new
return @devel unless block_given?
@devel ||= create_spec(SoftwareSpec)
@devel.instance_eval(&block)
end
def head val=nil, specs={}, &block
@head ||= HeadSoftwareSpec.new
if block_given?
@head ||= create_spec(HeadSoftwareSpec)
@head.instance_eval(&block)
elsif val
@head ||= create_spec(HeadSoftwareSpec)
@head.url(val, specs)
else
@head

View File

@ -212,4 +212,30 @@ class FormulaTests < Test::Unit::TestCase
ensure
path.unlink
end
def test_class_specs_are_always_initialized
f = formula { url 'foo-1.0' }
%w{stable devel head bottle}.each do |spec|
assert_kind_of SoftwareSpec, f.class.send(spec)
end
end
def test_incomplete_instance_specs_are_not_accessible
f = formula { url 'foo-1.0' }
%w{devel head bottle}.each { |spec| assert_nil f.send(spec) }
end
def test_honors_attributes_declared_before_specs
f = formula do
url 'foo-1.0'
depends_on 'foo'
devel { url 'foo-1.1' }
end
%w{stable devel head bottle}.each do |spec|
assert_equal 'foo', f.class.send(spec).deps.first.name
end
end
end