Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consult a "bounce list" of formulae to skip #486

Merged
merged 8 commits into from Jun 12, 2019

Conversation

jasonkarns
Copy link
Contributor

@jasonkarns jasonkarns commented May 1, 2019

Rationale

I frequently encounter projects (either from clients or open source, etc) where a Brewfile is used to bootstrap system-level project dependencies. This is great and I love homebrew-bundle for this being possible!

However, just as frequently, these brewfiles include packages that I don't want to install. Sometimes the packages are optional, sometimes the packages are only necessary in particular environments and configurations, still other times there are actual conflicts with other formula. A particular example of the latter is docker via cask, vs docker and docker-machine as regular formula. (Perhaps the project needs the cask GUI docker, but my system already has the CLI variants; or vice versa. Of course, they can both be installed, but then the formula conflict over which one can be symlinked.)

Proposal

So my proposed solution is a blocklist wherein brew-bundle will happily skip any formula that is listed on the blocklist. Enter Bouncer.

Bouncer is a small patch to brew-bundle that will consult ~/.config/homebrew/bounce.yml [1] whenever installing/listing/checking a formula. If the formula is present in bounce.yml, the formula in question will not be installed (but report successfully such that bootstrap scripts will continue on); it will report "okay" to brew-bundle-check, and will be skipped by brew-bundle-dump. (In verbose mode, all three commands report the formula as "bounced".)

The yaml file uses a string-keyed hash of the formula type, each entry being an array of formulae to refuse service.

---
brew:
  - docker
  - docker-machine
cask:
mac_app_store:
tap:

[1] (ideally, this would respect XDG_CONFIG_HOME, however, homebrew unsets those env vars while running so they cannot be respected.)

@jasonkarns jasonkarns marked this pull request as ready for review May 1, 2019 20:17
@jasonkarns
Copy link
Contributor Author

I have been running my local machine off this branch for nearly a year. It was initially written for just personal use, so there are no tests. If there is interest in accepting this patch, I can add proper tests for the feature.

@cpruitt
Copy link

cpruitt commented May 1, 2019

Nice! I'd very much second the praise of homebrew-bundle as well as the difficulties described in working with a client provided Brewfile. I find myself frequently in a situation in which an optional development dependency can not be installed because of access/permissions issues, or in a case where conflicts arise. With the only workaround being to edit the Brewfile every time, there is a constant possibility that the temporary changes will be forgotten and committed.

I, for one, would very much welcome a bouncer to keep the peace when things get a little unruly. 😁

@colindean colindean requested a review from MikeMcQuaid May 2, 2019 03:52
@colindean
Copy link
Member

I've never had a conflict occur in the time I've been sticking Brewfiles into repos I control but I can see how that could happen.

I'd like to read more about what problems you've encountered with some examples. Bonus points if those examples are linkable open source projects that have conflicting Brewfiles.

I want to better understand the impact before considering the implementation details.

@MikeMcQuaid
Copy link
Member

I frequently encounter projects (either from clients or open source, etc) where a Brewfile is used to bootstrap system-level project dependencies.

I have no idea what sort of projects you might be referring to 😉

Sometimes the packages are optional, sometimes the packages are only necessary in particular environments and configurations, still other times there are actual conflicts with other formula.

I have also felt this pain 👍

So my proposed solution is a blocklist wherein brew-bundle will happily skip any formula that is listed on the blocklist. Enter Bouncer.

Love the solution but not huge on the name. I think the "bounce" terminology would be best replaced with "skip".

(In verbose mode, all three commands report the formula as "bounced".)

I would like to see them always reported in non-verbose mode and brew bundle check too (although they needn't change the exit code) to ease debugging for users who may have added these and then forgotten and are confused as to why project bootstrap is failing for them.

The yaml file uses a string-keyed hash of the formula type, each entry being an array of formulae to refuse service.

I'm not huge on a YAML, .config/ config file for this (Homebrew doesn't use YAML for anything currently). A JSON file in ~/.homebrew-bundle-skips or even consider environment variables like HOMEBREW_BUNDLE_BREW_SKIPS="docker,docker-machine" or similar. Thoughts?

If there is interest in accepting this patch, I can add proper tests for the feature.

There is definitely interest on this. It may be easier if we hash out the implementation before you write the tests, though.

Great work @jasonkarns, I really look forward to having this merged!

CC @mistydemeo who may also find this interesting and relevant to our recent discussions.

@colindean
Copy link
Member

I like the idea of environment variables for this. That's consistent with other Homebrew configuration and configures just as well within bashrc, etc.

@jasonkarns
Copy link
Contributor Author

Love the solution but not huge on the name. I think the "bounce" terminology would be best replaced with "skip".

👍 the bar bouncer metaphor was stretching the homebrew/beer metaphor a bit far

I would like to see them always reported in non-verbose mode and brew bundle check too (although they needn't change the exit code) to ease debugging for users who may have added these and then forgotten and are confused as to why project bootstrap is failing for them.

Agree on the concern and noisy-by-default for check. I'm hesitant to drift too far from that for the common brew-bundle install use case, but I'll try it out and see if I have any actual concerns. Plus side is that it's unlikely there will ever be that many skipped formulae. Downside is that these bootstrap scripts likely run very frequently for many users. And many of the bootstrap scripts I typically encounter are not already as verbose as... one particular client. ;) Which means verbose "skipped" output could hold a disproportionate amount of the total brew-bundle output.

I'm not huge on a YAML, .config/ config file for this (Homebrew doesn't use YAML for anything currently). A JSON file in ~/.homebrew-bundle-skips or even consider environment variables like HOMEBREW_BUNDLE_BREW_SKIPS="docker,docker-machine" or similar. Thoughts?

Totally on board with JSON and the idea of an environment variable. One implementation concern for the env var, though... since the idea is for this to support skipping all of the same types of things that you can do in a Brewfile, that means taps, casks, mas; we'll need a way for the env var to distinguish between them. I don't think we want an env var per type. Would it be too gross for the HOMEBREW_BUNDLE_BREW_SKIPS value to literally be JSON?

HOMEBREW_BUNDLE_BREW_SKIPS='{"brew":["docker", "docker-machine"], "mas": ["Keynote"]}'

Other alternatives?

A JSON file in ~/.homebrew-bundle-skips

I have a strong anti-reaction to this file not being XDG compliant (or even XDG "friendly" which is how I describe config files that are hardcoded to use the XDG default locations, without respecting the XDG env vars). Are there any other ongoing efforts for homebrew to have a general purpose configuration mechanism other than env vars?

@colindean
Copy link
Member

Would it be too gross for the HOMEBREW_BUNDLE_BREW_SKIPS value to literally be JSON?

This would not be ideal but it's manageable through heredocs.

@MikeMcQuaid
Copy link
Member

Which means verbose "skipped" output could hold a disproportionate amount of the total brew-bundle output.

I'm thinking they just display SKIPPED: package instead of Using: package or Installing: package which should be the same level of output unless I'm missing something. I don't propose non-default brew bundle check have any output set.

Would it be too gross for the HOMEBREW_BUNDLE_BREW_SKIPS value to literally be JSON?

I was thinking HOMEBREW_BUNDLE_BREW_SKIPS, HOMEBREW_BUNDLE_MAS_SKIPS, etc.

Are there any other ongoing efforts for homebrew to have a general purpose configuration mechanism other than env vars?

Nope. It's something that could be considered, though. I'd suggest dodging it for the first Homebrew/bundle PR though to cut scope.

I'd struggle to see arguments against a key/value file that allows setting environment variables. It would likely need to be in a format easily readable by Bash to allow sourceing.

respecting the XDG env vars

I believe Homebrew already respects these variables when they are set. Any configuration file could set these.

@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented May 23, 2019

@jasonkarns any updates on this?

jasonkarns and others added 8 commits June 12, 2019 12:20
Tired of projects installing apps on your system you don't want?
Unable to avoid using their Brewfiles due to their setup scripts?

List the formulae (taps, brews, casks, or mas) that are NOT ON THE LIST.
They will be summarily bounced from the bar when they attempt to enter
(works for bundle-{install,check,list} subcommands).

The bouncer's list should be a yaml file located at:

`~/.config/homebrew/bounce.yml` *

The yaml file should define a string-keyed hash of the formula type,
each entry being an array for formulae to refuse service.

```yml
---
brew:
  - docker
  - docker-machine
cask:
mac_app_store:
tap:
```

* (ideally, this would respect `XDG_CONFIG_HOME`, however, homebrew unsets those env vars while running so they cannot be respected.)
This makes it consistent with the DSL.
@MikeMcQuaid
Copy link
Member

Thanks for the initial PR and your work on this @jasonkarns!

The version I ended up with has the following notable changes:

  • it's called a "skipper" instead of "bouncer" internally
  • it uses mas rather than mac_app_store as the type (to be more consistent with Brewfiles)
  • it uses space-separated environment variables instead of YAML for configuration:
    • HOMEBREW_BUNDLE_BREW_SKIP (e.g. export HOMEBREW_BUNDLE_BREW_SKIP="rakudo-star mkcert nss")
    • HOMEBREW_BUNDLE_CASK_SKIP
    • HOMEBREW_BUNDLE_MAS_SKIP
    • HOMEBREW_BUNDLE_TAP_SKIP

@MikeMcQuaid
Copy link
Member

Are there any other ongoing efforts for homebrew to have a general purpose configuration mechanism other than env vars?

Nope. It's something that could be considered, though. I'd suggest dodging it for the first Homebrew/bundle PR though to cut scope.

I'd struggle to see arguments against a key/value file that allows setting environment variables. It would likely need to be in a format easily readable by Bash to allow sourceing.

@jasonkarns to expand on this: it would need to be a change in Library/Homebrew/brew.sh that read a series of e.g. HOMEBREW_FOO=bar entries from a file e.g. ~/.homebrew (or XDG equivalents) in Bash without arbritrarily eval or sourceing the file and set (only HOMEBREW_* environment variables to their values after the =. We'd welcome a PR for this (or even a well written feature request issue).

@jasonkarns
Copy link
Contributor Author

@MikeMcQuaid thanks for getting this in after I went MIA!

regarding the general purpose configuration file: I'm lukewarm on the feature, honestly. I mainly asked so that this PR could leverage or at least be consistent if such a feature were in the works. There's isn't a huge need for file based configuration, other than the hypothetical problem of hitting the limit of env vars. (which I know exists but do not know what the limit is)

Indeed, the env-var approach adds some extra versatility by essentially supporting per-project skip lists.

@jasonkarns jasonkarns deleted the bouncer branch June 17, 2019 21:52
@MikeMcQuaid
Copy link
Member

@jasonkarns Cool, glad you're happy with how this ended up!

@colindean
Copy link
Member

Thanks again for this work, @jasonkarns. I'm setting this up now on a machine that inspired #456 and it's working great.

HOMEBREW_BUNDLE_MAS_SKIP="Trello" brew bundle --file=Brewfile.lusankya --no-upgrade

@lock lock bot added the outdated label Jan 1, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants