Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Revamp formula attribute validation #18579

Closed
jacknagel opened this issue Mar 19, 2013 · 2 comments
Closed

Revamp formula attribute validation #18579

jacknagel opened this issue Mar 19, 2013 · 2 comments

Comments

@jacknagel
Copy link
Contributor

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.

@jacknagel
Copy link
Contributor Author

Working on this here: https://github.com/jacknagel/homebrew/compare/validation

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.

@jacknagel
Copy link
Contributor Author

Proposal in #19122.

@Homebrew Homebrew 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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant