Homebrew / homebrew-bundle

📦 Bundler for non-Ruby dependencies from Homebrew, Homebrew Cask and the Mac App Store.
MIT License
5.36k stars 288 forks source link

Remove tap warning for tap-prefixed formulae #803

Closed jasonkarns closed 3 years ago

jasonkarns commented 4 years ago

Some behavior of brew-bundle surprised me...

Given a Brewfile that contains a tap-prefixed formulae, say brew "nejckorasa/tap/dckr", the output of brew-bundle generates a warning about tap being necessary:

Warning: 'nejckorasa/tap/dckr' formula is unreadable: No available formula with the name "nejckorasa/tap/dckr" 
Please tap it and then try again: brew tap nejckorasa/tap
Installing nejckorasa/tap/dckr
Homebrew Bundle complete! 1 Brewfile dependencies now installed.

The warning seems unnecessary for two reasons:

  1. installing the formula directly will automatically tap as necessary (without the warning):
    $ brew install nejckorasa/tap/dckr
    ==> Tapping nejckorasa/tap
    Cloning into '/usr/local/Homebrew/Library/Taps/nejckorasa/homebrew-tap'...
    remote: Enumerating objects: 93, done.
    remote: Total 93 (delta 0), reused 0 (delta 0), pack-reused 93
    Unpacking objects: 100% (93/93), 18.58 KiB | 279.00 KiB/s, done.
    Tapped 2 formulae (120 files, 59KB).
    ==> Installing dckr from nejckorasa/tap
    ==> Downloading https://github.com/nejckorasa/mac-docker-go/archive/v1.1.2.tar.gz
    Already downloaded: /usr/local/cache/Homebrew/downloads/47a5d32dc932fc4ef42244d0293476000b788a203905d4ca167559ef8e91474a--mac-docker-go-1.1.2.tar.gz
    🍺  /usr/local/Cellar/dckr/1.1.2: 4 files, 6.8KB, built in 3 seconds
  2. brew-bundle itself still proceeds to tap and install the formula without requiring an explicit tap

So the tap warning seems unnecessary for brew-bundle; and is inconsistent with direct brew-install (which doesn't warn about tap at all).

To answer the (perhaps obvious) question: why don't I just add the tap line to my Brewfile to prevent the warning? I've personally encountered a number of projects that have tap lines in their Brewfiles for formulae which have long-since been removed from their Brewfile. Especially for Brewfiles that are more than a few lines, a project will add the tap (presumably to prevent the warning?) along with the formula for which the tap is necessary. Then at some point, they stop using the formula and remove it from the Brewfile, but fail to remove the corresponding tap. So to avoid this, I personally prefer to omit the tap entirely, since it isn't functionally necessary. (As long as brew-bundle continues to tap automatically. Perhaps that is planned to change?)

MikeMcQuaid commented 4 years ago

This makes sense as an enhancement. I agree brew bundle should just tap automatically without complaining if a formula is tap-prefixed. The formula is unreadable is because we're trying to infer information about the formula before it is installed. I see two options:

  1. hide this error if the formula is from a tap and the tap isn't installed
  2. explicitly tap the tap before trying to install the formula

I think 1) would probably be an easier PR but 2) is probably preferable in terms of e.g. gracefully handling network failures and reading the formula as expected.

Would you be able to give a PR a go for this, @jasonkarns?

Thanks!

MikeMcQuaid commented 3 years ago

2. brew-bundle itself still proceeds to tap and install the formula without requiring an explicit tap

This was tweaked in https://github.com/Homebrew/homebrew-bundle/pull/982 so that it'll at least not try to install these formulae if the tap fails.

Given that and the lack of other requests: passing on this, sorry!