Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Improve tap directory structure #16121

Closed
jacknagel opened this Issue · 7 comments

5 participants

@jacknagel
Owner

Homebrew assumes that all Ruby files in taps are formulae, but this prevents taps from splitting shared code out into separate files.

Now, homebrew-php has a directory of shared requirements, but these are getting loaded like they are formulae, and code that iterates over formulae is broken (#16053, #16108).

So we need to allow for a directory of non-formulae code.

As a bonus, it would be nice if files in this directory were loaded automatically.

@mxcl
Collaborator

Indeed. I discussed this with @josegonzalez before, he was keen for auto-loading files in a special directory.

Alternatively we can try to parse Formula before symlinking them into Formula. If they don't parse to a formula, then don't symlink.

@mxcl
Collaborator

Actually what we discussed was adding a directory from the tap to the Ruby module path when tap formula are evaluated.

Prolly auto-loading is a little too opinionated. Flexibility is nicer for development.

@Sharpie
Collaborator

Prolly auto-loading is a little too opinionated. Flexibility is nicer for development.

Agreed. One can easily do something like:

require `homebrew-<whatever>/<module>`

In formulae that need tap-specific tools.

@josegonzalez

For the keg_only? issue, I ended up adding a keg_only? method to all my requirements after people complained that brew doctor was exploding.

For the brew leaves issue, I have a check in the initialize of my base classes - abstract-php and abstract-php-extension - that raises an exception if someone tries to install them. I specifically whitelist certain brew commands, although I should just blacklist instead perhaps...

For the record, I've been using the Formula directory for placing formula, and I think new taps have been more or less following my lead.

I'd like to move the abstract classes out, but then calling require on them is super-annoying since we don't get a HOMEBREW_TAPS constant. In that case, I I would do something like:

require HOMEBREW_TAPS . 'josegonzalez-php/abstract/php.rb'
require HOMEBREW_TAPS . 'josegonzalez-php/abstract/php-extension.rb'
require HOMEBREW_TAPS . 'josegonzalez-php/requirements/composer.rb'
require HOMEBREW_TAPS . 'josegonzalez-php/requirements/homebrew-php.rb'
# etc. The naming might be different - perhaps with modules or something - but you get the idea.

So I would have multiple directories that would be autoloaded. I don't think autoloading is right, but giving us a way to load files from a specific tap would be stellar.

/me goes back to being sick

@mistydemeo
Owner

Prolly auto-loading is a little too opinionated. Flexibility is nicer for development.

I don't like autoloading. I can see conflicts just waiting to happen when the user has stuff from taps which don't talk to each other.

However, providing places to share code seems like a good idea to me. Right now there's no proper way for a tap to do this kind of stuff, aside from hacking around core conventions.

@jacknagel
Owner

I can understand the hesitation to automatically load stuff, but explicitly loading things doesn't really prevent collisions from happening. If formula A from tap X loads a file with a class "Foo" and formula B from tap Y loads a file with a class "Foo", and Homebrew loads both of these formulae at the same time, the collision will happen whether there is explicit requires in the formulae or it is done implicitly. Any operation that loads all formulae at once will suffer from this. It's really just about removing boilerplate.

Anyway, I really only meant that a small number of files that follow a specific convention would be loaded automatically, such as a "Library/requirements.rb" file that defines shared Requirement subclasses, like we do in core.

@jacknagel jacknagel closed this
@jacknagel jacknagel reopened this
@jacknagel jacknagel removed the bug label
@jacknagel
Owner

I think the issues with loading non-formula by accident are fixed. I would like to make it easier to require files from inside a tap, but I will open a separate issue for this.

@jacknagel jacknagel closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.