Brew: Extensible Homebrew DSLs

[maxim-belkin]

Description

I propose to start splitting various independent pieces of various DSLs in Homebrew into smaller files which we can then "require" in the corresponding "main" DSL file.

For example: formula.rb provides a DSL for Formula object. Some features are completely independent of others, such as: skip_clean, link_overwrite, ignore_missing_libraries, ... we can factor these out into files in extend/formula/skip_clean.rb, extend/formula/link_overwrite.rb and extend/formula/ignore_missing_libraries.rb and require those in formula.rb.

Motivation

As I was working on "missing libraries" feature (PR 7847), my changes to formula.rb made it longer than 1400 lines, which causes style check to fail. I guess it's possible to increase this limit somewhere, but I think that proposed approach is better in that it would also enable us to quickly and conditionally turn certain features on and off when desired

Relevance

• Small (yet not tiny) files are easier to work with.
• Richer DSL control (enable/disable features if/when necessary).

Alternatives

Keep using current files and increase the number-of-lines limit for all files in audit checks.

[maxim-belkin]

Those who agree with this proposal: what location do you think makes sense to use?

• extend/<dsl-name>/<feature>...
• dsl/<dsl-name>/<feature>
• extend/dsl/<dsl-name>/<feature>...
  
  
  
  • other ?
  
  

Are there any obvious drawbacks?

[Rylan12]

Is just using Library/Homebrew/formula/ as the directory a bad idea?

This way, if anything is going to need these features, the line to use is require "formula/...". To me, that makes sense because it's clear that what's being required is a "subset" of formula.

Putting things in Library/Homebrew/extend/formula would also make sense to me, as saying require "extend/formula/..." still is clear about what's being required.

I don't know if there's a convention to follow here, but that's my gut feeling.

[jonchang]

I think putting stuff in a formula/ subdirectory makes sense if there are separate classes within Formula (like we do with Utils and such). Otherwise we should put it in extend/ (as it is extending the main Formula class).

[maxim-belkin]

I agree that it seems logical to use extend folder for extending DSLs. I wonder if we should wait for more votes on that or if we can proceed with those changes. For example, I could use this approach in #7847.

[MikeMcQuaid]

Formula is a massive class but I do think it’s better to keep definitions of a class in a single file. I wouldn’t worry too much about that lint.

• Small (yet not tiny) files are easier to work with.

I agree with this when multiple classes are used. I disagree when the definition of a class is split across multiple files.

[maxim-belkin]

I agree with this when multiple classes are used. I disagree when the definition of a class is split across multiple files.

What is your concern exactly? From what I see online, people do this in Ruby quite frequently and I think formula.rb with its 2.7K lines is the best candidate for this approach. Also, I'm sure Homebrew maintainers aren't "Quiché Eaters" but even syntax highlighting breaks in that file, and we all know how important it is :) Joking aside, I think it'll be a very useful change in the long-run: users won't notice a thing and developers will enjoy its modularity. Adding new features and fixing bugs will be a breeze :)

[MikeMcQuaid]

Also, I'm sure Homebrew maintainers aren't "Quiché Eaters" but even syntax highlighting breaks in that file, and we all know how important it is :)

Works for me.

What is your concern exactly?
Joking aside, I think it'll be a very useful change in the long-run: users won't notice a thing and developers will enjoy its modularity. Adding new features and fixing bugs will be a breeze :)

I have found the opposite to be the case. I'd ideally like to see Homebrew have each class be defined in a file with the equivalent name. If Formula#installed? is defined in formula/some_module.rb: how would I know to find the definition there?

This isn't a speculative problem, either, I have (regularly) found this a problem with other Ruby projects. I think smaller classes are better but I don't think a long file is inherently problematic if it's due to being a class that's actually got a lot of public methods.

[maxim-belkin]

If Formula#installed? is defined in formula/some_module.rb: how would I know to find the definition there?

Well, right now one has to search in a file to find that method. If we don't do anything special, one would have to grep for it instead. But... we could add follow-up comments to all require lines like so:

require "something"
# Defines instance methods:
#   - skip_clean

require "something_else"
# Defines class methods:
#   - sad_panda?

We would then be able to provide in-depth comments in the "required" files.

I'm not sure if this is something you want to hear more opinions about or not. If not -- I'm OK with closing this issue.

[MikeMcQuaid]

But... we could add follow-up comments to all require lines like so:

These will become out of date.

I'm not sure if this is something you want to hear more opinions about or not. If not -- I'm OK with closing this issue.

If you're OK with that: I think that'd be better, sorry :sob:.

[maxim-belkin]

I think that'd be better, sorry

No worries! This is just a suggestion. ✌️

@Rylan12 @Bo98 @SeekingMeaning @jonchang @dawidd6, I'm ready to close this so if you think there is an important argument I missed -- please post it today. Otherwise, I'll close this issue in a few hours.

[Rylan12]

The only thing I would add is that I think we should exclude formula.rb from the 1400 line limit. If we are intentionally having that file be all-inclusive I don't see a reason for the limit.

Actually, I'm not sure what the reasoning behind the limit is in general. I would think it is a way to say "hey, this class is super long so let's try to break it up a little bit" but, based on this thread, it seems like we would prefer not breaking it up even for longer files. Maybe formula.rb is an exception here but I'm not sure I see the benefit of having the 1400 line limit.

[MikeMcQuaid]

The only thing I would add is that I think we should exclude formula.rb from the 1400 line limit. If we are intentionally having that file be all-inclusive I don't see a reason for the limit.

Done: https://github.com/Homebrew/brew/pull/8209

Maybe formula.rb is an exception here but I'm not sure I see the benefit of having the 1400 line limit.

I think formula.rb and formula_installer.rb are exceptions (and maybe diagnostic.rb).

[maxim-belkin]

Good point, Rylan.
Thanks for clarification, Mike.

I think the situation is a bit clearer now, so this issue can be closed.