Revamp formula attribute validation #18579

jacknagel opened this Issue Mar 19, 2013 · 2 comments

1 participant


The validations done by the formula initializer are kind of a mess, and there are some inconsistencies that we should address.

  1. For some reason, we still stick the version of @active_spec into the formula's @version and validate it, even though this detail was moved to SoftwareSpec and Formula#version delegates to @active_spec.

  2. The validation in (1) is only done if @active_spec.version is non-nil, which is kind of weird, but seems to be required to support GithubGistFormula, which has special logic to detect the version from the URL. (Maybe we can just get rid of this?)

  3. @stable is set to nil if it is missing a URL, which I added but it would be caught by the validation of @active_spec.url later in the initializer anyway. This was added to allow things with only "head, version, checksum" to work as head-only formulae, but it could probably just be removed in favor of raising an error.

  4. The bottle stuff is probably unavoidable, since (like other specs) it is initialized at the class level, but information from an instance is required to fully specify the bottle. I've sometimes thought about bottle being an attribute of the stable spec, but that doesn't really solve this problem and the one time I tried to implement it, it made things worse.

I would like to remove usage of validate_variable, since it is really just an artifact, and instead validate that the SoftwareSpec objects are fully specified, raising if there is not a valid spec that can be used.

A lot of the pain here is due to having to construct domain objects at load-time instead of runtime. A definite argument in favor of a block-style template instead of classes for Homebrew 2 I guess.


Working on this here:

Still a WIP.

One thing I noticed is that the only code that calls #url, #mirrors, or #specs on Formula instances is test code. These days we pass @active_spec to the downloader, so I'm thinking about just removing the getters for those attributes.

Another thing that has been bothering me is that we use the term "specs" for two different things, (a) instances of SoftwareSpec and (b) the hash of downloader options passed to the url DSL method. I also find it a little confusing as I tend to associate "specs" with "tests". I think we should rename one (or both!) of them, but naming is hard.


Proposal in #19122.

@jacknagel jacknagel closed this Apr 11, 2013
@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.